[PATCH] ARM omap hsmmc: Fix adding iobase for OMAP3

[PATCH] ARM omap hsmmc: Fix adding iobase for OMAP3

[Juergen Kilb]

Only the OMAP4 hsmmc module has an offset of 0x100 from its register
base address. With commit 416a991 the offset was added for all SOCs
which use the omap_hsmmc driver.
This has broken the driver for OMAP3/AM33xx platforms.

Signed-off-by: Juergen Kilb <J.Kilb@phytec.de>
---
 drivers/mci/omap_hsmmc.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/mci/omap_hsmmc.c b/drivers/mci/omap_hsmmc.c
index 6471ea6..68fb3e1 100644
--- a/drivers/mci/omap_hsmmc.c
+++ b/drivers/mci/omap_hsmmc.c
@@ -577,8 +577,12 @@ static int omap_mmc_probe(struct device_d *dev)
 	hsmmc->mci.hw_dev = dev;
 
 	hsmmc->iobase = dev_request_mem_region(dev, 0);
-	hsmmc->base = hsmmc->iobase + 0x100;
 
+#if defined(CONFIG_ARCH_OMAP4)
+	hsmmc->base = hsmmc->iobase + 0x100;
+#else
+	hsmmc->base = hsmmc->iobase;
+#endif
 	hsmmc->mci.voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
 
 	hsmmc->mci.f_min = 400000;
-- 
1.7.0.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM omap hsmmc: Fix adding iobase for OMAP3

[Sascha Hauer]

Hi Jürgen,

On Fri, Jan 04, 2013 at 05:13:12PM +0100, Juergen Kilb wrote:
> Only the OMAP4 hsmmc module has an offset of 0x100 from its register
> base address. With commit 416a991 the offset was added for all SOCs
> which use the omap_hsmmc driver.

The offset was present before 416a991 aswell. It seems that the device
convenience functions introduced with bc872dbe79 did it wrong for OMAP3.
When then the boards started using the convenience functions with
5a12f45 it became broken.

> This has broken the driver for OMAP3/AM33xx platforms.
> 
> Signed-off-by: Juergen Kilb <J.Kilb@phytec.de>
> ---
>  drivers/mci/omap_hsmmc.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mci/omap_hsmmc.c b/drivers/mci/omap_hsmmc.c
> index 6471ea6..68fb3e1 100644
> --- a/drivers/mci/omap_hsmmc.c
> +++ b/drivers/mci/omap_hsmmc.c
> @@ -577,8 +577,12 @@ static int omap_mmc_probe(struct device_d *dev)
>  	hsmmc->mci.hw_dev = dev;
>  
>  	hsmmc->iobase = dev_request_mem_region(dev, 0);
> -	hsmmc->base = hsmmc->iobase + 0x100;
>  
> +#if defined(CONFIG_ARCH_OMAP4)
> +	hsmmc->base = hsmmc->iobase + 0x100;
> +#else
> +	hsmmc->base = hsmmc->iobase;
> +#endif
>  	hsmmc->mci.voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
>  
>  	hsmmc->mci.f_min = 400000;

I prefer to fix this properly. Does the following fix it for you?


From 666677b0e954bf70760c17f9ebe56bda4b4ee34a Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Mon, 7 Jan 2013 10:09:22 +0100
Subject: [PATCH] ARM omap / mci: Fix register offsets

Only the OMAP4 has a register offset of 0x100 in the register space. Fix
this by using the device id mechanism. This became broken when the device
register convenience functions were introduced.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-omap/include/mach/devices.h       |    3 --
 arch/arm/mach-omap/include/mach/omap3-devices.h |    9 ++++--
 arch/arm/mach-omap/include/mach/omap4-devices.h |    9 ++++--
 arch/arm/mach-omap/omap_devices.c               |    7 -----
 drivers/mci/omap_hsmmc.c                        |   34 ++++++++++++++++++++++-
 5 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-omap/include/mach/devices.h b/arch/arm/mach-omap/include/mach/devices.h
index 6cbb25e..adae01b 100644
--- a/arch/arm/mach-omap/include/mach/devices.h
+++ b/arch/arm/mach-omap/include/mach/devices.h
@@ -9,9 +9,6 @@ void omap_add_sram0(resource_size_t base, resource_size_t size);
 
 struct device_d *omap_add_uart(int id, unsigned long base);
 
-struct device_d *omap_add_mmc(int id, unsigned long base,
-		struct omap_hsmmc_platform_data *pdata);
-
 struct device_d *omap_add_i2c(int id, unsigned long base, void *pdata);
 
 #endif /* __MACH_OMAP_DEVICES_H */
diff --git a/arch/arm/mach-omap/include/mach/omap3-devices.h b/arch/arm/mach-omap/include/mach/omap3-devices.h
index 2e8e06f..de67ea0 100644
--- a/arch/arm/mach-omap/include/mach/omap3-devices.h
+++ b/arch/arm/mach-omap/include/mach/omap3-devices.h
@@ -59,17 +59,20 @@ static inline struct device_d *omap3_add_uart3(void)
 
 static inline struct device_d *omap3_add_mmc1(struct omap_hsmmc_platform_data *pdata)
 {
-	return omap_add_mmc(0, OMAP3_MMC1_BASE, pdata);
+	return add_generic_device("omap3-hsmmc", 0, NULL,
+			OMAP3_MMC1_BASE, SZ_4K, IORESOURCE_MEM, pdata);
 }
 
 static inline struct device_d *omap3_add_mmc2(struct omap_hsmmc_platform_data *pdata)
 {
-	return omap_add_mmc(1, OMAP3_MMC2_BASE, pdata);
+	return add_generic_device("omap3-hsmmc", 1, NULL,
+			OMAP3_MMC2_BASE, SZ_4K, IORESOURCE_MEM, pdata);
 }
 
 static inline struct device_d *omap3_add_mmc3(struct omap_hsmmc_platform_data *pdata)
 {
-	return omap_add_mmc(2, OMAP3_MMC3_BASE, pdata);
+	return add_generic_device("omap3-hsmmc", 2, NULL,
+			OMAP3_MMC3_BASE, SZ_4K, IORESOURCE_MEM, pdata);
 }
 
 static inline struct device_d *omap3_add_i2c1(void *pdata)
diff --git a/arch/arm/mach-omap/include/mach/omap4-devices.h b/arch/arm/mach-omap/include/mach/omap4-devices.h
index 5aba72d..2f78ab8 100644
--- a/arch/arm/mach-omap/include/mach/omap4-devices.h
+++ b/arch/arm/mach-omap/include/mach/omap4-devices.h
@@ -30,17 +30,20 @@ static inline struct device_d *omap44xx_add_uart3(void)
 
 static inline struct device_d *omap44xx_add_mmc1(struct omap_hsmmc_platform_data *pdata)
 {
-	return omap_add_mmc(0, OMAP44XX_MMC1_BASE, pdata);
+	return add_generic_device("omap4-hsmmc", 0, NULL,
+			OMAP44XX_MMC1_BASE, SZ_4K, IORESOURCE_MEM, pdata);
 }
 
 static inline struct device_d *omap44xx_add_mmc2(struct omap_hsmmc_platform_data *pdata)
 {
-	return omap_add_mmc(1, OMAP44XX_MMC2_BASE, pdata);
+	return add_generic_device("omap4-hsmmc", 1, NULL,
+			OMAP44XX_MMC2_BASE, SZ_4K, IORESOURCE_MEM, pdata);
 }
 
 static inline struct device_d *omap44xx_add_mmc3(struct omap_hsmmc_platform_data *pdata)
 {
-	return omap_add_mmc(2, OMAP44XX_MMC3_BASE, pdata);
+	return add_generic_device("omap4-hsmmc", 2, NULL,
+			OMAP44XX_MMC3_BASE, SZ_4K, IORESOURCE_MEM, pdata);
 }
 
 static inline struct device_d *omap44xx_add_mmc4(struct omap_hsmmc_platform_data *pdata)
diff --git a/arch/arm/mach-omap/omap_devices.c b/arch/arm/mach-omap/omap_devices.c
index e83ac85..056cba5 100644
--- a/arch/arm/mach-omap/omap_devices.c
+++ b/arch/arm/mach-omap/omap_devices.c
@@ -25,13 +25,6 @@ struct device_d *omap_add_uart(int id, unsigned long base)
 			IORESOURCE_MEM_8BIT, &serial_plat);
 }
 
-struct device_d *omap_add_mmc(int id, unsigned long base,
-		struct omap_hsmmc_platform_data *pdata)
-{
-	return add_generic_device("omap-hsmmc", id, NULL,
-			base, SZ_4K, IORESOURCE_MEM, pdata);
-}
-
 struct device_d *omap_add_i2c(int id, unsigned long base, void *pdata)
 {
 	return add_generic_device("i2c-omap", id, NULL, base, SZ_4K,
diff --git a/drivers/mci/omap_hsmmc.c b/drivers/mci/omap_hsmmc.c
index 888cba9..ffcb4ae 100644
--- a/drivers/mci/omap_hsmmc.c
+++ b/drivers/mci/omap_hsmmc.c
@@ -59,6 +59,18 @@ struct hsmmc {
 	unsigned int capa;		/* 0x140 */
 };
 
+struct omap_mmc_driver_data {
+	unsigned long reg_ofs;
+};
+
+static struct omap_mmc_driver_data omap3_data = {
+	.reg_ofs = 0,
+};
+
+static struct omap_mmc_driver_data omap4_data = {
+	.reg_ofs = 0x100,
+};
+
 /*
  * OMAP HS MMC Bit definitions
  */
@@ -566,6 +578,13 @@ static int omap_mmc_probe(struct device_d *dev)
 {
 	struct omap_hsmmc *hsmmc;
 	struct omap_hsmmc_platform_data *pdata;
+	struct omap_mmc_driver_data *drvdata;
+	unsigned long reg_ofs = 0;
+	int ret;
+
+	ret = dev_get_drvdata(dev, (unsigned long *)&drvdata);
+	if (!ret)
+		reg_ofs = drvdata->reg_ofs;
 
 	hsmmc = xzalloc(sizeof(*hsmmc));
 
@@ -577,7 +596,7 @@ static int omap_mmc_probe(struct device_d *dev)
 	hsmmc->mci.hw_dev = dev;
 
 	hsmmc->iobase = dev_request_mem_region(dev, 0);
-	hsmmc->base = hsmmc->iobase + 0x100;
+	hsmmc->base = hsmmc->iobase + reg_ofs;
 
 	hsmmc->mci.voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
 
@@ -594,9 +613,22 @@ static int omap_mmc_probe(struct device_d *dev)
 	return 0;
 }
 
+static struct platform_device_id omap_mmc_ids[] = {
+	{
+		.name = "omap3-hsmmc",
+		.driver_data = (unsigned long)&omap3_data,
+	}, {
+		.name = "omap4-hsmmc",
+		.driver_data = (unsigned long)&omap4_data,
+	}, {
+		/* sentinel */
+	},
+};
+
 static struct driver_d omap_mmc_driver = {
 	.name  = "omap-hsmmc",
 	.probe = omap_mmc_probe,
+	.id_table = omap_mmc_ids,
 };
 
 static int omap_mmc_init_driver(void)
-- 
1.7.10.4

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM omap hsmmc: Fix adding iobase for OMAP3

[Jürgen Kilb]

Hi Sascha,

On 07.01.2013 10:23, Sascha Hauer wrote:
> Hi Jürgen,
>
> On Fri, Jan 04, 2013 at 05:13:12PM +0100, Juergen Kilb wrote:
>> Only the OMAP4 hsmmc module has an offset of 0x100 from its register
>> base address. With commit 416a991 the offset was added for all SOCs
>> which use the omap_hsmmc driver.
> The offset was present before 416a991 aswell. It seems that the device
> convenience functions introduced with bc872dbe79 did it wrong for OMAP3.
> When then the boards started using the convenience functions with
> 5a12f45 it became broken.
OK, I've not dug deeper...
> I prefer to fix this properly. Does the following fix it for you?
Yes, your patch looks better then my simple version.
Your patch fixed it for me.
You can add my "Tested-by: J.Kilb@Phytec.de" line.

thanks,
Jürgen

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM omap hsmmc: Fix adding iobase for OMAP3

[Sascha Hauer]

Hi Jürgen,

I'm a bit confused...

On Fri, Jan 04, 2013 at 05:13:12PM +0100, Juergen Kilb wrote:
> Only the OMAP4 hsmmc module has an offset of 0x100 from its register
> base address. With commit 416a991 the offset was added for all SOCs
> which use the omap_hsmmc driver.
> This has broken the driver for OMAP3/AM33xx platforms.

You state that am33xx is broken,...

> 
> Signed-off-by: Juergen Kilb <J.Kilb@phytec.de>
> ---
>  drivers/mci/omap_hsmmc.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mci/omap_hsmmc.c b/drivers/mci/omap_hsmmc.c
> index 6471ea6..68fb3e1 100644
> --- a/drivers/mci/omap_hsmmc.c
> +++ b/drivers/mci/omap_hsmmc.c
> @@ -577,8 +577,12 @@ static int omap_mmc_probe(struct device_d *dev)
>  	hsmmc->mci.hw_dev = dev;
>  
>  	hsmmc->iobase = dev_request_mem_region(dev, 0);
> -	hsmmc->base = hsmmc->iobase + 0x100;
>  
> +#if defined(CONFIG_ARCH_OMAP4)
> +	hsmmc->base = hsmmc->iobase + 0x100;
> +#else
> +	hsmmc->base = hsmmc->iobase;
> +#endif

and remove the 0x100 offset for am33xx here.

Now I realize am33xx MMC is broken and I need the following patch to
make it work again. Could it be you didn't test it on am33xx or is there
something else wrong here?

Sascha


From b337e4b71ae7b3b2c9668350252ba86f8ecfa764 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Wed, 9 Jan 2013 11:02:50 +0100
Subject: [PATCH] ARM am33xx: the hsmmc is a omap4 type mmc controller

The am33xx hsmmc controller is actually a omap4 type controller which
means that it has a 0x100 offset in the registers.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-omap/include/mach/am33xx-devices.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap/include/mach/am33xx-devices.h b/arch/arm/mach-omap/include/mach/am33xx-devices.h
index 4c83b28..c194303 100644
--- a/arch/arm/mach-omap/include/mach/am33xx-devices.h
+++ b/arch/arm/mach-omap/include/mach/am33xx-devices.h
@@ -26,7 +26,7 @@ static inline struct device_d *am33xx_add_uart2(void)
 
 static inline struct device_d *am33xx_add_mmc0(struct omap_hsmmc_platform_data *pdata)
 {
-	return add_generic_device("omap3-hsmmc", 0, NULL,
+	return add_generic_device("omap4-hsmmc", 0, NULL,
 			AM33XX_MMCHS0_BASE, SZ_4K, IORESOURCE_MEM, pdata);
 }
 
-- 
1.7.10.4

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM omap hsmmc: Fix adding iobase for OMAP3

[Jürgen Kilb]

Hi Sascha,

On 09.01.2013 12:00, Sascha Hauer wrote:
> Hi Jürgen,
>
> I'm a bit confused...
me too ;-)
> On Fri, Jan 04, 2013 at 05:13:12PM +0100, Juergen Kilb wrote:
>> Only the OMAP4 hsmmc module has an offset of 0x100 from its register
>> base address. With commit 416a991 the offset was added for all SOCs
>> which use the omap_hsmmc driver.
>> This has broken the driver for OMAP3/AM33xx platforms.
> You state that am33xx is broken,...
It seems, I've made a mistake and looked in the wrong datasheet (AM35xx).
Sorry!
>
>> Signed-off-by: Juergen Kilb <J.Kilb@phytec.de>
>> ---
>>   drivers/mci/omap_hsmmc.c |    6 +++++-
>>   1 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/mci/omap_hsmmc.c b/drivers/mci/omap_hsmmc.c
>> index 6471ea6..68fb3e1 100644
>> --- a/drivers/mci/omap_hsmmc.c
>> +++ b/drivers/mci/omap_hsmmc.c
>> @@ -577,8 +577,12 @@ static int omap_mmc_probe(struct device_d *dev)
>>   	hsmmc->mci.hw_dev = dev;
>>   
>>   	hsmmc->iobase = dev_request_mem_region(dev, 0);
>> -	hsmmc->base = hsmmc->iobase + 0x100;
>>   
>> +#if defined(CONFIG_ARCH_OMAP4)
>> +	hsmmc->base = hsmmc->iobase + 0x100;
>> +#else
>> +	hsmmc->base = hsmmc->iobase;
>> +#endif
> and remove the 0x100 offset for am33xx here.
>
> Now I realize am33xx MMC is broken and I need the following patch to
> make it work again. Could it be you didn't test it on am33xx or is there
> something else wrong here?
>
Now I've checked the correct datasheet (AM33xx) and I see that
my patch has broken the AM33xx hsmmc support.
I've only tested on an phyCARD-A-L1 (OMAP3530).

> Sascha
>
>
>  From b337e4b71ae7b3b2c9668350252ba86f8ecfa764 Mon Sep 17 00:00:00 2001
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Date: Wed, 9 Jan 2013 11:02:50 +0100
> Subject: [PATCH] ARM am33xx: the hsmmc is a omap4 type mmc controller
>
> The am33xx hsmmc controller is actually a omap4 type controller which
> means that it has a 0x100 offset in the registers.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>   arch/arm/mach-omap/include/mach/am33xx-devices.h |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap/include/mach/am33xx-devices.h b/arch/arm/mach-omap/include/mach/am33xx-devices.h
> index 4c83b28..c194303 100644
> --- a/arch/arm/mach-omap/include/mach/am33xx-devices.h
> +++ b/arch/arm/mach-omap/include/mach/am33xx-devices.h
> @@ -26,7 +26,7 @@ static inline struct device_d *am33xx_add_uart2(void)
>   
>   static inline struct device_d *am33xx_add_mmc0(struct omap_hsmmc_platform_data *pdata)
>   {
> -	return add_generic_device("omap3-hsmmc", 0, NULL,
> +	return add_generic_device("omap4-hsmmc", 0, NULL,
>   			AM33XX_MMCHS0_BASE, SZ_4K, IORESOURCE_MEM, pdata);
>   }
>   
I agree with you, that the AM33XX has an offset ox 0x100 like the OMAP4.

greetings,
Jürgen


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox