[PATCH v2 1/3] dmaengine: imx-sdma: use platform_device_id to identify sdma version
Uwe Kleine-König
u.kleine-koenig at pengutronix.de
Mon Sep 5 19:16:29 EST 2011
On Fri, Jul 15, 2011 at 05:44:24PM +0800, Shawn Guo wrote:
> It might be not good to use software defined version to identify sdma
> device type, when hardware does not define such version. Instead,
> soc name is stable enough to define the device type.
I still don't get this rational. If you ask me v1, v2, v3 is better, as
it doesn't confuse people that wonder why an i.MX51 machine has a
imx35-sdma device. And if the version number is known to be virtual
(opposed to hardware defined) there is no problem with stability.
> The patch uses platform_device_id rather than version number passed
> by platform data to identify sdma device type/version.
>
> Signed-off-by: Shawn Guo <shawn.guo at linaro.org>
> Cc: Vinod Koul <vinod.koul at intel.com>
> Cc: Sascha Hauer <s.hauer at pengutronix.de>
> Acked-by: Grant Likely <grant.likely at secretlab.ca>
> ---
> arch/arm/mach-imx/clock-imx25.c | 3 +-
> arch/arm/mach-imx/clock-imx31.c | 2 +-
> arch/arm/mach-imx/clock-imx35.c | 2 +-
> arch/arm/mach-imx/mm-imx25.c | 4 +-
> arch/arm/mach-imx/mm-imx31.c | 3 +-
> arch/arm/mach-imx/mm-imx35.c | 3 +-
> arch/arm/mach-mx5/clock-mx51-mx53.c | 6 ++-
> arch/arm/mach-mx5/mm.c | 8 ++--
> arch/arm/plat-mxc/devices/platform-imx-dma.c | 4 +-
> arch/arm/plat-mxc/include/mach/devices-common.h | 2 +-
> arch/arm/plat-mxc/include/mach/dma.h | 3 +-
> arch/arm/plat-mxc/include/mach/sdma.h | 2 -
> drivers/dma/imx-sdma.c | 39 +++++++++++++++++------
> 13 files changed, 50 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm/mach-imx/clock-imx25.c b/arch/arm/mach-imx/clock-imx25.c
> index 17d6d1b..5736e04 100644
> --- a/arch/arm/mach-imx/clock-imx25.c
> +++ b/arch/arm/mach-imx/clock-imx25.c
> @@ -308,7 +308,8 @@ static struct clk_lookup lookups[] = {
> _REGISTER_CLOCK(NULL, "audmux", audmux_clk)
> _REGISTER_CLOCK("flexcan.0", NULL, can1_clk)
> _REGISTER_CLOCK("flexcan.1", NULL, can2_clk)
> - _REGISTER_CLOCK("imx-sdma", NULL, sdma_clk)
> + /* i.mx25 has the i.mx35 type sdma */
> + _REGISTER_CLOCK("imx35-sdma", NULL, sdma_clk)
> };
>
> int __init mx25_clocks_init(void)
> diff --git a/arch/arm/mach-imx/clock-imx31.c b/arch/arm/mach-imx/clock-imx31.c
> index 8d212a9..d973770 100644
> --- a/arch/arm/mach-imx/clock-imx31.c
> +++ b/arch/arm/mach-imx/clock-imx31.c
> @@ -565,7 +565,7 @@ static struct clk_lookup lookups[] = {
> _REGISTER_CLOCK(NULL, "ata", ata_clk)
> _REGISTER_CLOCK(NULL, "rtic", rtic_clk)
> _REGISTER_CLOCK(NULL, "rng", rng_clk)
> - _REGISTER_CLOCK("imx-sdma", NULL, sdma_clk1)
> + _REGISTER_CLOCK("imx31-sdma", NULL, sdma_clk1)
> _REGISTER_CLOCK(NULL, "sdma_ipg", sdma_clk2)
> _REGISTER_CLOCK(NULL, "mstick", mstick1_clk)
> _REGISTER_CLOCK(NULL, "mstick", mstick2_clk)
> diff --git a/arch/arm/mach-imx/clock-imx35.c b/arch/arm/mach-imx/clock-imx35.c
> index 2d0b4c8..749acff 100644
> --- a/arch/arm/mach-imx/clock-imx35.c
> +++ b/arch/arm/mach-imx/clock-imx35.c
> @@ -481,7 +481,7 @@ static struct clk_lookup lookups[] = {
> _REGISTER_CLOCK(NULL, "rtc", rtc_clk)
> _REGISTER_CLOCK(NULL, "rtic", rtic_clk)
> _REGISTER_CLOCK(NULL, "scc", scc_clk)
> - _REGISTER_CLOCK("imx-sdma", NULL, sdma_clk)
> + _REGISTER_CLOCK("imx35-sdma", NULL, sdma_clk)
> _REGISTER_CLOCK(NULL, "spba", spba_clk)
> _REGISTER_CLOCK(NULL, "spdif", spdif_clk)
> _REGISTER_CLOCK("imx-ssi.0", NULL, ssi1_clk)
> diff --git a/arch/arm/mach-imx/mm-imx25.c b/arch/arm/mach-imx/mm-imx25.c
> index 8bf0291..cc4d152 100644
> --- a/arch/arm/mach-imx/mm-imx25.c
> +++ b/arch/arm/mach-imx/mm-imx25.c
> @@ -79,7 +79,6 @@ static struct sdma_script_start_addrs imx25_sdma_script __initdata = {
> };
>
> static struct sdma_platform_data imx25_sdma_pdata __initdata = {
> - .sdma_version = 2,
> .fw_name = "sdma-imx25.bin",
> .script_addrs = &imx25_sdma_script,
> };
> @@ -92,5 +91,6 @@ void __init imx25_soc_init(void)
> mxc_register_gpio("imx31-gpio", 2, MX25_GPIO3_BASE_ADDR, SZ_16K, MX25_INT_GPIO3, 0);
> mxc_register_gpio("imx31-gpio", 3, MX25_GPIO4_BASE_ADDR, SZ_16K, MX25_INT_GPIO4, 0);
>
> - imx_add_imx_sdma(MX25_SDMA_BASE_ADDR, MX25_INT_SDMA, &imx25_sdma_pdata);
> + /* i.mx25 has the i.mx35 type sdma */
> + imx_add_imx_sdma("imx35-sdma", MX25_SDMA_BASE_ADDR, MX25_INT_SDMA, &imx25_sdma_pdata);
> }
> diff --git a/arch/arm/mach-imx/mm-imx31.c b/arch/arm/mach-imx/mm-imx31.c
> index 61bff38..b7c55e7 100644
> --- a/arch/arm/mach-imx/mm-imx31.c
> +++ b/arch/arm/mach-imx/mm-imx31.c
> @@ -69,7 +69,6 @@ static struct sdma_script_start_addrs imx31_to2_sdma_script __initdata = {
> };
>
> static struct sdma_platform_data imx31_sdma_pdata __initdata = {
> - .sdma_version = 1,
> .fw_name = "sdma-imx31-to2.bin",
> .script_addrs = &imx31_to2_sdma_script,
> };
> @@ -88,5 +87,5 @@ void __init imx31_soc_init(void)
> imx31_sdma_pdata.script_addrs = &imx31_to1_sdma_script;
> }
>
> - imx_add_imx_sdma(MX31_SDMA_BASE_ADDR, MX31_INT_SDMA, &imx31_sdma_pdata);
> + imx_add_imx_sdma("imx31-sdma", MX31_SDMA_BASE_ADDR, MX31_INT_SDMA, &imx31_sdma_pdata);
> }
> diff --git a/arch/arm/mach-imx/mm-imx35.c b/arch/arm/mach-imx/mm-imx35.c
> index 98769ae..f49bac7 100644
> --- a/arch/arm/mach-imx/mm-imx35.c
> +++ b/arch/arm/mach-imx/mm-imx35.c
> @@ -86,7 +86,6 @@ static struct sdma_script_start_addrs imx35_to2_sdma_script __initdata = {
> };
>
> static struct sdma_platform_data imx35_sdma_pdata __initdata = {
> - .sdma_version = 2,
> .fw_name = "sdma-imx35-to2.bin",
> .script_addrs = &imx35_to2_sdma_script,
> };
> @@ -106,5 +105,5 @@ void __init imx35_soc_init(void)
> imx35_sdma_pdata.script_addrs = &imx35_to1_sdma_script;
> }
>
> - imx_add_imx_sdma(MX35_SDMA_BASE_ADDR, MX35_INT_SDMA, &imx35_sdma_pdata);
> + imx_add_imx_sdma("imx35-sdma", MX35_SDMA_BASE_ADDR, MX35_INT_SDMA, &imx35_sdma_pdata);
> }
> diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
> index 31d904c..0a2fa8e 100644
> --- a/arch/arm/mach-mx5/clock-mx51-mx53.c
> +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
> @@ -1447,7 +1447,8 @@ static struct clk_lookup mx51_lookups[] = {
> _REGISTER_CLOCK("imx-ssi.0", NULL, ssi1_clk)
> _REGISTER_CLOCK("imx-ssi.1", NULL, ssi2_clk)
> _REGISTER_CLOCK("imx-ssi.2", NULL, ssi3_clk)
> - _REGISTER_CLOCK("imx-sdma", NULL, sdma_clk)
> + /* i.mx51 has the i.mx35 type sdma */
> + _REGISTER_CLOCK("imx35-sdma", NULL, sdma_clk)
> _REGISTER_CLOCK(NULL, "ckih", ckih_clk)
> _REGISTER_CLOCK(NULL, "ckih2", ckih2_clk)
> _REGISTER_CLOCK(NULL, "gpt_32k", gpt_32k_clk)
> @@ -1494,7 +1495,8 @@ static struct clk_lookup mx53_lookups[] = {
> _REGISTER_CLOCK("imx35-cspi.0", NULL, cspi_clk)
> _REGISTER_CLOCK("imx2-wdt.0", NULL, dummy_clk)
> _REGISTER_CLOCK("imx2-wdt.1", NULL, dummy_clk)
> - _REGISTER_CLOCK("imx-sdma", NULL, sdma_clk)
> + /* i.mx53 has the i.mx35 type sdma */
> + _REGISTER_CLOCK("imx35-sdma", NULL, sdma_clk)
> _REGISTER_CLOCK("imx-ssi.0", NULL, ssi1_clk)
> _REGISTER_CLOCK("imx-ssi.1", NULL, ssi2_clk)
> _REGISTER_CLOCK("imx-ssi.2", NULL, ssi3_clk)
> diff --git a/arch/arm/mach-mx5/mm.c b/arch/arm/mach-mx5/mm.c
> index ef8aec9..baea6e5 100644
> --- a/arch/arm/mach-mx5/mm.c
> +++ b/arch/arm/mach-mx5/mm.c
> @@ -115,7 +115,6 @@ static struct sdma_script_start_addrs imx51_sdma_script __initdata = {
> };
>
> static struct sdma_platform_data imx51_sdma_pdata __initdata = {
> - .sdma_version = 2,
> .fw_name = "sdma-imx51.bin",
> .script_addrs = &imx51_sdma_script,
> };
> @@ -135,7 +134,6 @@ static struct sdma_script_start_addrs imx53_sdma_script __initdata = {
> };
>
> static struct sdma_platform_data imx53_sdma_pdata __initdata = {
> - .sdma_version = 2,
> .fw_name = "sdma-imx53.bin",
> .script_addrs = &imx53_sdma_script,
> };
> @@ -148,7 +146,8 @@ void __init imx51_soc_init(void)
> mxc_register_gpio("imx31-gpio", 2, MX51_GPIO3_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO3_LOW, MX51_MXC_INT_GPIO3_HIGH);
> mxc_register_gpio("imx31-gpio", 3, MX51_GPIO4_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO4_LOW, MX51_MXC_INT_GPIO4_HIGH);
>
> - imx_add_imx_sdma(MX51_SDMA_BASE_ADDR, MX51_INT_SDMA, &imx51_sdma_pdata);
> + /* i.mx51 has the i.mx35 type sdma */
> + imx_add_imx_sdma("imx35-sdma", MX51_SDMA_BASE_ADDR, MX51_INT_SDMA, &imx51_sdma_pdata);
> }
>
> void __init imx53_soc_init(void)
> @@ -162,5 +161,6 @@ void __init imx53_soc_init(void)
> mxc_register_gpio("imx31-gpio", 5, MX53_GPIO6_BASE_ADDR, SZ_16K, MX53_INT_GPIO6_LOW, MX53_INT_GPIO6_HIGH);
> mxc_register_gpio("imx31-gpio", 6, MX53_GPIO7_BASE_ADDR, SZ_16K, MX53_INT_GPIO7_LOW, MX53_INT_GPIO7_HIGH);
>
> - imx_add_imx_sdma(MX53_SDMA_BASE_ADDR, MX53_INT_SDMA, &imx53_sdma_pdata);
> + /* i.mx53 has the i.mx35 type sdma */
> + imx_add_imx_sdma("imx35-sdma", MX53_SDMA_BASE_ADDR, MX53_INT_SDMA, &imx53_sdma_pdata);
> }
> diff --git a/arch/arm/plat-mxc/devices/platform-imx-dma.c b/arch/arm/plat-mxc/devices/platform-imx-dma.c
> index 2b0fdb2..7fa7e9c 100644
> --- a/arch/arm/plat-mxc/devices/platform-imx-dma.c
> +++ b/arch/arm/plat-mxc/devices/platform-imx-dma.c
> @@ -14,7 +14,7 @@ struct platform_device __init __maybe_unused *imx_add_imx_dma(void)
> "imx-dma", -1, NULL, 0, NULL, 0);
> }
>
> -struct platform_device __init __maybe_unused *imx_add_imx_sdma(
> +struct platform_device __init __maybe_unused *imx_add_imx_sdma(char *name,
const char *name ?
> resource_size_t iobase, int irq, struct sdma_platform_data *pdata)
> {
> struct resource res[] = {
> @@ -29,6 +29,6 @@ struct platform_device __init __maybe_unused *imx_add_imx_sdma(
> },
> };
>
> - return platform_device_register_resndata(&mxc_ahb_bus, "imx-sdma",
> + return platform_device_register_resndata(&mxc_ahb_bus, name,
> -1, res, ARRAY_SIZE(res), pdata, sizeof(*pdata));
> }
> diff --git a/arch/arm/plat-mxc/include/mach/devices-common.h b/arch/arm/plat-mxc/include/mach/devices-common.h
> index c9e7b66..f5cb8d5 100644
> --- a/arch/arm/plat-mxc/include/mach/devices-common.h
> +++ b/arch/arm/plat-mxc/include/mach/devices-common.h
> @@ -298,5 +298,5 @@ struct platform_device *__init imx_add_spi_imx(
> const struct spi_imx_master *pdata);
>
> struct platform_device *imx_add_imx_dma(void);
> -struct platform_device *imx_add_imx_sdma(
> +struct platform_device *imx_add_imx_sdma(char *name,
> resource_size_t iobase, int irq, struct sdma_platform_data *pdata);
> diff --git a/arch/arm/plat-mxc/include/mach/dma.h b/arch/arm/plat-mxc/include/mach/dma.h
> index ef77515..233d0a5 100644
> --- a/arch/arm/plat-mxc/include/mach/dma.h
> +++ b/arch/arm/plat-mxc/include/mach/dma.h
> @@ -60,7 +60,8 @@ static inline int imx_dma_is_ipu(struct dma_chan *chan)
>
> static inline int imx_dma_is_general_purpose(struct dma_chan *chan)
> {
> - return !strcmp(dev_name(chan->device->dev), "imx-sdma") ||
> + return !strcmp(dev_name(chan->device->dev), "imx31-sdma") ||
> + !strcmp(dev_name(chan->device->dev), "imx35-sdma") ||
> !strcmp(dev_name(chan->device->dev), "imx-dma");
> }
>
> diff --git a/arch/arm/plat-mxc/include/mach/sdma.h b/arch/arm/plat-mxc/include/mach/sdma.h
> index f495c87..3a39428 100644
> --- a/arch/arm/plat-mxc/include/mach/sdma.h
> +++ b/arch/arm/plat-mxc/include/mach/sdma.h
> @@ -48,12 +48,10 @@ struct sdma_script_start_addrs {
> /**
> * struct sdma_platform_data - platform specific data for SDMA engine
> *
> - * @sdma_version The version of this SDMA engine
> * @fw_name The firmware name
> * @script_addrs SDMA scripts addresses in SDMA ROM
> */
> struct sdma_platform_data {
> - int sdma_version;
> char *fw_name;
> struct sdma_script_start_addrs *script_addrs;
> };
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 1ea47db..4a7aa72 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -65,8 +65,8 @@
> #define SDMA_ONCE_RTB 0x060
> #define SDMA_XTRIG_CONF1 0x070
> #define SDMA_XTRIG_CONF2 0x074
> -#define SDMA_CHNENBL0_V2 0x200
> -#define SDMA_CHNENBL0_V1 0x080
> +#define SDMA_CHNENBL0_IMX35 0x200
> +#define SDMA_CHNENBL0_IMX31 0x080
> #define SDMA_CHNPRI_0 0x100
>
> /*
> @@ -299,13 +299,18 @@ struct sdma_firmware_header {
> u32 ram_code_size;
> };
>
> +enum sdma_devtype {
> + IMX31_SDMA, /* runs on i.mx31 */
> + IMX35_SDMA, /* runs on i.mx35 and later */
I don't like "and later". IMHO better list the affected SoCs currently
known.
> +};
> +
> struct sdma_engine {
> struct device *dev;
> struct device_dma_parameters dma_parms;
> struct sdma_channel channel[MAX_DMA_CHANNELS];
> struct sdma_channel_control *channel_control;
> void __iomem *regs;
> - unsigned int version;
> + enum sdma_devtype devtype;
> unsigned int num_events;
> struct sdma_context_data *context;
> dma_addr_t context_phys;
> @@ -314,6 +319,18 @@ struct sdma_engine {
> struct sdma_script_start_addrs *script_addrs;
> };
>
> +static struct platform_device_id sdma_devtypes[] = {
> + {
> + .name = "imx31-sdma",
> + .driver_data = IMX31_SDMA,
> + }, {
> + .name = "imx35-sdma",
> + .driver_data = IMX35_SDMA,
> + }, {
> + /* sentinel */
> + }
> +};
> +
> #define SDMA_H_CONFIG_DSPDMA (1 << 12) /* indicates if the DSPDMA is used */
> #define SDMA_H_CONFIG_RTD_PINS (1 << 11) /* indicates if Real-Time Debug pins are enabled */
> #define SDMA_H_CONFIG_ACR (1 << 4) /* indicates if AHB freq /core freq = 2 or 1 */
> @@ -321,8 +338,8 @@ struct sdma_engine {
>
> static inline u32 chnenbl_ofs(struct sdma_engine *sdma, unsigned int event)
> {
> - u32 chnenbl0 = (sdma->version == 2 ? SDMA_CHNENBL0_V2 : SDMA_CHNENBL0_V1);
> -
> + u32 chnenbl0 = (sdma->devtype == IMX31_SDMA ? SDMA_CHNENBL0_IMX31 :
> + SDMA_CHNENBL0_IMX35);
This is a place that might easily be missed when we get imx-sdma-v3
(hehe, you see, I can talk about this version, you cannot because you
don't know the SoC that will introduce it :-). But this problem isn't
introduced by your patch.
> return chnenbl0 + event * 4;
> }
>
> @@ -1162,15 +1179,16 @@ static int __init sdma_init(struct sdma_engine *sdma)
> int i, ret;
> dma_addr_t ccb_phys;
>
> - switch (sdma->version) {
> - case 1:
> + switch (sdma->devtype) {
> + case IMX31_SDMA:
> sdma->num_events = 32;
> break;
> - case 2:
> + case IMX35_SDMA:
> sdma->num_events = 48;
> break;
> default:
> - dev_err(sdma->dev, "Unknown version %d. aborting\n", sdma->version);
> + dev_err(sdma->dev, "Unknown sdma type %d. aborting\n",
> + sdma->devtype);
> return -ENODEV;
> }
>
> @@ -1284,7 +1302,7 @@ static int __init sdma_probe(struct platform_device *pdev)
> if (!sdma->script_addrs)
> goto err_alloc;
>
> - sdma->version = pdata->sdma_version;
> + sdma->devtype = pdev->id_entry->driver_data;
platform_get_device_id(pdev)->driver_data
>
> dma_cap_set(DMA_SLAVE, sdma->dma_device.cap_mask);
> dma_cap_set(DMA_CYCLIC, sdma->dma_device.cap_mask);
> @@ -1366,6 +1384,7 @@ static struct platform_driver sdma_driver = {
> .driver = {
> .name = "imx-sdma",
> },
> + .id_table = sdma_devtypes,
> .remove = __exit_p(sdma_remove),
> };
>
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
More information about the devicetree-discuss
mailing list