[PATCH v2 3/4] omap4 hsmmc: Register offset handling

[PATCH v2 3/4] omap4 hsmmc: Register offset handling

[kishore kadiyala]

The offset handling implementation of omap4 mmc registers which
was already present can't be reused once hwmod modifications are done
for mmc driver.
Since hwmod data file for OMAP4 is an auto generated the base
address for MMC will remain same as OMAP3 and thus the offset deviation
of registers should be updated in the hsmmc driver.
The omap-mmc platform data is updated with a register offset which
will be updated and used in the driver accordingly.

Signed-off-by: Kishore Kadiyala <kishore.kadiyala@ti.com>
---
 arch/arm/mach-omap2/devices.c         |    8 +++-----
 arch/arm/mach-omap2/hsmmc.c           |    4 ++++
 arch/arm/plat-omap/include/plat/mmc.h |    3 +++
 drivers/mmc/host/omap_hsmmc.c         |    2 +-
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 2dbb265..bb7ec13 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -745,13 +745,13 @@ void __init omap2_init_mmc(struct omap_mmc_platform_data **mmc_data,
 		case 3:
 			if (!cpu_is_omap44xx())
 				return;
-			base = OMAP4_MMC4_BASE + OMAP4_MMC_REG_OFFSET;
+			base = OMAP4_MMC4_BASE;
 			irq = OMAP44XX_IRQ_MMC4;
 			break;
 		case 4:
 			if (!cpu_is_omap44xx())
 				return;
-			base = OMAP4_MMC5_BASE + OMAP4_MMC_REG_OFFSET;
+			base = OMAP4_MMC5_BASE;
 			irq = OMAP44XX_IRQ_MMC5;
 			break;
 		default:
@@ -762,10 +762,8 @@ void __init omap2_init_mmc(struct omap_mmc_platform_data **mmc_data,
 			size = OMAP2420_MMC_SIZE;
 			name = "mmci-omap";
 		} else if (cpu_is_omap44xx()) {
-			if (i < 3) {
-				base += OMAP4_MMC_REG_OFFSET;
+			if (i < 3)
 				irq += OMAP44XX_IRQ_GIC_START;
-			}
 			size = OMAP4_HSMMC_SIZE;
 			name = "mmci-omap-hs";
 		} else {
diff --git a/arch/arm/mach-omap2/hsmmc.c b/arch/arm/mach-omap2/hsmmc.c
index c8f647b..49d76a7 100644
--- a/arch/arm/mach-omap2/hsmmc.c
+++ b/arch/arm/mach-omap2/hsmmc.c
@@ -261,6 +261,10 @@ void __init omap2_hsmmc_init(struct omap2_hsmmc_info *controllers)
 		mmc->slots[0].wires = c->wires;
 		mmc->slots[0].internal_clock = !c->ext_clock;
 		mmc->dma_mask = 0xffffffff;
+		if (cpu_is_omap44xx())
+			mmc->reg_offset = OMAP4_MMC_REG_OFFSET;
+		else
+			mmc->reg_offset = 0;

 		mmc->get_context_loss_count = hsmmc_get_context_loss;

diff --git a/arch/arm/plat-omap/include/plat/mmc.h b/arch/arm/plat-omap/include/plat/mmc.h
index 9b89ec6..4e6ef07 100644
--- a/arch/arm/plat-omap/include/plat/mmc.h
+++ b/arch/arm/plat-omap/include/plat/mmc.h
@@ -71,6 +71,9 @@ struct omap_mmc_platform_data {

 	u64 dma_mask;

+	/* Register offset deviation */
+	u16 reg_offset;
+
 	struct omap_mmc_slot_data {

 		/* 4 wire signaling is optional, and is used for SD/SDIO/HSMMC;
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index a51894d..bfca9ca 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -2034,7 +2034,7 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
 	host->irq	= irq;
 	host->id	= pdev->id;
 	host->slot_id	= 0;
-	host->mapbase	= res->start;
+	host->mapbase	= res->start + pdata->reg_offset;
 	host->base	= ioremap(host->mapbase, SZ_4K);
 	host->power_mode = MMC_POWER_OFF;

-- 
1.7.0.4

[PATCH v2 3/4] omap4 hsmmc: Register offset handling

[Cousson, Benoit]

Hi Kishore

On 9/18/2010 6:34 PM, Kadiyala, Kishore wrote:
> The offset handling implementation of omap4 mmc registers which
> was already present can't be reused once hwmod modifications are done
> for mmc driver.
> Since hwmod data file for OMAP4 is an auto generated the base
> address for MMC will remain same as OMAP3 and thus the offset deviation
> of registers should be updated in the hsmmc driver.

That explanation does not sound very accurate to me.
The fact that the hwmods are auto-generated has nothing to do with your 
offset problem. The real cause is due to the update of the IP to support 
the new PM programming model.
Because of that, and to keep a certain level of compatibility, the 
legacy registers were all shifted by 0x100 and the new one were added 
from offset 0 to 0x10.

Regards,
Benoit

> The omap-mmc platform data is updated with a register offset which
> will be updated and used in the driver accordingly.
>
> Signed-off-by: Kishore Kadiyala<kishore.kadiyala@ti.com>
> ---
>   arch/arm/mach-omap2/devices.c         |    8 +++-----
>   arch/arm/mach-omap2/hsmmc.c           |    4 ++++
>   arch/arm/plat-omap/include/plat/mmc.h |    3 +++
>   drivers/mmc/host/omap_hsmmc.c         |    2 +-
>   4 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> index 2dbb265..bb7ec13 100644
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -745,13 +745,13 @@ void __init omap2_init_mmc(struct omap_mmc_platform_data **mmc_data,
>   		case 3:
>   			if (!cpu_is_omap44xx())
>   				return;
> -			base = OMAP4_MMC4_BASE + OMAP4_MMC_REG_OFFSET;
> +			base = OMAP4_MMC4_BASE;
>   			irq = OMAP44XX_IRQ_MMC4;
>   			break;
>   		case 4:
>   			if (!cpu_is_omap44xx())
>   				return;
> -			base = OMAP4_MMC5_BASE + OMAP4_MMC_REG_OFFSET;
> +			base = OMAP4_MMC5_BASE;
>   			irq = OMAP44XX_IRQ_MMC5;
>   			break;
>   		default:
> @@ -762,10 +762,8 @@ void __init omap2_init_mmc(struct omap_mmc_platform_data **mmc_data,
>   			size = OMAP2420_MMC_SIZE;
>   			name = "mmci-omap";
>   		} else if (cpu_is_omap44xx()) {
> -			if (i<  3) {
> -				base += OMAP4_MMC_REG_OFFSET;
> +			if (i<  3)
>   				irq += OMAP44XX_IRQ_GIC_START;
> -			}
>   			size = OMAP4_HSMMC_SIZE;
>   			name = "mmci-omap-hs";
>   		} else {
> diff --git a/arch/arm/mach-omap2/hsmmc.c b/arch/arm/mach-omap2/hsmmc.c
> index c8f647b..49d76a7 100644
> --- a/arch/arm/mach-omap2/hsmmc.c
> +++ b/arch/arm/mach-omap2/hsmmc.c
> @@ -261,6 +261,10 @@ void __init omap2_hsmmc_init(struct omap2_hsmmc_info *controllers)
>   		mmc->slots[0].wires = c->wires;
>   		mmc->slots[0].internal_clock = !c->ext_clock;
>   		mmc->dma_mask = 0xffffffff;
> +		if (cpu_is_omap44xx())
> +			mmc->reg_offset = OMAP4_MMC_REG_OFFSET;
> +		else
> +			mmc->reg_offset = 0;
>
>   		mmc->get_context_loss_count = hsmmc_get_context_loss;
>
> diff --git a/arch/arm/plat-omap/include/plat/mmc.h b/arch/arm/plat-omap/include/plat/mmc.h
> index 9b89ec6..4e6ef07 100644
> --- a/arch/arm/plat-omap/include/plat/mmc.h
> +++ b/arch/arm/plat-omap/include/plat/mmc.h
> @@ -71,6 +71,9 @@ struct omap_mmc_platform_data {
>
>   	u64 dma_mask;
>
> +	/* Register offset deviation */
> +	u16 reg_offset;
> +
>   	struct omap_mmc_slot_data {
>
>   		/* 4 wire signaling is optional, and is used for SD/SDIO/HSMMC;
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index a51894d..bfca9ca 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -2034,7 +2034,7 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
>   	host->irq	= irq;
>   	host->id	= pdev->id;
>   	host->slot_id	= 0;
> -	host->mapbase	= res->start;
> +	host->mapbase	= res->start + pdata->reg_offset;
>   	host->base	= ioremap(host->mapbase, SZ_4K);
>   	host->power_mode = MMC_POWER_OFF;
>

[PATCH v2 3/4] omap4 hsmmc: Register offset handling

[kishore kadiyala]

Hi Benoit,

On Tue, Sep 21, 2010 at 2:26 AM, Cousson, Benoit <b-cousson@ti.com> wrote:
> Hi Kishore
>
> On 9/18/2010 6:34 PM, Kadiyala, Kishore wrote:
>>
>> The offset handling implementation of omap4 mmc registers which
>> was already present can't be reused once hwmod modifications are done
>> for mmc driver.
>> Since hwmod data file for OMAP4 is an auto generated the base
>> address for MMC will remain same as OMAP3 and thus the offset deviation
>> of registers should be updated in the hsmmc driver.
>
> That explanation does not sound very accurate to me.
> The fact that the hwmods are auto-generated has nothing to do with your
> offset problem.

Agree the change log needs to be updated  as said  but when
coming to implementation this how it is:

With current mainline kernel, offset appending is already implemented in
mach-omap2/devices.c and works fine [here bases are updated with 0x100
and then passed to driver which has same register offset's as OMAP3].

When hwmod modifications are done to the MMC driver, the base address are
picked from omap_hwmod_44xx_data.c and omap_device is build.
Coming to the driver it still has the same old register offset's as OMAP3 and
there should be a way to add 0x100 some where.

The current patch moves the offset appending to the driver file which is updated
in mach-omap2/hsmmc.c which will fit for current implementation and even
hwmod adaptation.

>The real cause is due to the update of the IP to support the
> new PM programming model.
> Because of that, and to keep a certain level of compatibility, the legacy
> registers were all shifted by 0x100 and the new one were added from offset 0
> to 0x10.
>
> Regards,
> Benoit
>

<snip>

Regards,
Kishore

[PATCH v2 3/4] omap4 hsmmc: Register offset handling

[G, Manjunath Kondaiah]

> -----Original Message-----
> From: linux-omap-owner at vger.kernel.org 
> [mailto:linux-omap-owner at vger.kernel.org] On Behalf Of 
> Kadiyala, Kishore
> Sent: Saturday, September 18, 2010 10:04 PM
> To: linux-mmc at vger.kernel.org; linux-omap at vger.kernel.org; 
> linux-arm-kernel at lists.infradead.org
> Cc: khilman at deeprootsystems.com; Cousson, Benoit
> Subject: [PATCH v2 3/4] omap4 hsmmc: Register offset handling
> 
> The offset handling implementation of omap4 mmc registers 
> which was already present can't be reused once hwmod 
> modifications are done for mmc driver.
> Since hwmod data file for OMAP4 is an auto generated the base 
> address for MMC will remain same as OMAP3 and thus the offset 
> deviation of registers should be updated in the hsmmc driver.
> The omap-mmc platform data is updated with a register offset 
> which will be updated and used in the driver accordingly.
> 
> Signed-off-by: Kishore Kadiyala <kishore.kadiyala@ti.com>
> ---
>  arch/arm/mach-omap2/devices.c         |    8 +++-----
>  arch/arm/mach-omap2/hsmmc.c           |    4 ++++
>  arch/arm/plat-omap/include/plat/mmc.h |    3 +++
>  drivers/mmc/host/omap_hsmmc.c         |    2 +-
>  4 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/devices.c 
> b/arch/arm/mach-omap2/devices.c index 2dbb265..bb7ec13 100644
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -745,13 +745,13 @@ void __init omap2_init_mmc(struct 
> omap_mmc_platform_data **mmc_data,
>  		case 3:
>  			if (!cpu_is_omap44xx())
>  				return;
> -			base = OMAP4_MMC4_BASE + OMAP4_MMC_REG_OFFSET;
> +			base = OMAP4_MMC4_BASE;
>  			irq = OMAP44XX_IRQ_MMC4;
>  			break;
>  		case 4:
>  			if (!cpu_is_omap44xx())
>  				return;
> -			base = OMAP4_MMC5_BASE + OMAP4_MMC_REG_OFFSET;
> +			base = OMAP4_MMC5_BASE;
>  			irq = OMAP44XX_IRQ_MMC5;
>  			break;
>  		default:
> @@ -762,10 +762,8 @@ void __init omap2_init_mmc(struct 
> omap_mmc_platform_data **mmc_data,
>  			size = OMAP2420_MMC_SIZE;
>  			name = "mmci-omap";
>  		} else if (cpu_is_omap44xx()) {
> -			if (i < 3) {
> -				base += OMAP4_MMC_REG_OFFSET;
> +			if (i < 3)
>  				irq += OMAP44XX_IRQ_GIC_START;
> -			}
>  			size = OMAP4_HSMMC_SIZE;
>  			name = "mmci-omap-hs";
>  		} else {
> diff --git a/arch/arm/mach-omap2/hsmmc.c 
> b/arch/arm/mach-omap2/hsmmc.c index c8f647b..49d76a7 100644
> --- a/arch/arm/mach-omap2/hsmmc.c
> +++ b/arch/arm/mach-omap2/hsmmc.c
> @@ -261,6 +261,10 @@ void __init omap2_hsmmc_init(struct 
> omap2_hsmmc_info *controllers)
>  		mmc->slots[0].wires = c->wires;
>  		mmc->slots[0].internal_clock = !c->ext_clock;
>  		mmc->dma_mask = 0xffffffff;
> +		if (cpu_is_omap44xx())
> +			mmc->reg_offset = OMAP4_MMC_REG_OFFSET;
> +		else
> +			mmc->reg_offset = 0;

1. Since you mentioned that, this is useful for hwmod, the 
register offsets should use enum and static tables. You can
refer to i2c-omap.c for more details and dma hwmod review 
comments for more details.
https://patchwork.kernel.org/patch/126251/

2. Replace macros with inline functions for low level read/write
functions.

 -Manjunath

[PATCH v2 3/4] omap4 hsmmc: Register offset handling

[kishore kadiyala]

Manju,

On Fri, Oct 1, 2010 at 8:32 PM, G, Manjunath Kondaiah <manjugk@ti.com> wrote:
>
>
>
>> -----Original Message-----
>> From: linux-omap-owner at vger.kernel.org
>> [mailto:linux-omap-owner at vger.kernel.org] On Behalf Of
>> Kadiyala, Kishore
>> Sent: Saturday, September 18, 2010 10:04 PM
>> To: linux-mmc at vger.kernel.org; linux-omap at vger.kernel.org;
>> linux-arm-kernel at lists.infradead.org
>> Cc: khilman at deeprootsystems.com; Cousson, Benoit
>> Subject: [PATCH v2 3/4] omap4 hsmmc: Register offset handling
>>
>> The offset handling implementation of omap4 mmc registers
>> which was already present can't be reused once hwmod
>> modifications are done for mmc driver.
>> Since hwmod data file for OMAP4 is an auto generated the base
>> address for MMC will remain same as OMAP3 and thus the offset
>> deviation of registers should be updated in the hsmmc driver.
>> The omap-mmc platform data is updated with a register offset
>> which will be updated and used in the driver accordingly.
>>
>> Signed-off-by: Kishore Kadiyala <kishore.kadiyala@ti.com>
>> ---
>> ?arch/arm/mach-omap2/devices.c ? ? ? ? | ? ?8 +++-----
>> ?arch/arm/mach-omap2/hsmmc.c ? ? ? ? ? | ? ?4 ++++
>> ?arch/arm/plat-omap/include/plat/mmc.h | ? ?3 +++
>> ?drivers/mmc/host/omap_hsmmc.c ? ? ? ? | ? ?2 +-
>> ?4 files changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/devices.c
>> b/arch/arm/mach-omap2/devices.c index 2dbb265..bb7ec13 100644
>> --- a/arch/arm/mach-omap2/devices.c
>> +++ b/arch/arm/mach-omap2/devices.c
>> @@ -745,13 +745,13 @@ void __init omap2_init_mmc(struct
>> omap_mmc_platform_data **mmc_data,
>> ? ? ? ? ? ? ? case 3:
>> ? ? ? ? ? ? ? ? ? ? ? if (!cpu_is_omap44xx())
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? return;
>> - ? ? ? ? ? ? ? ? ? ? base = OMAP4_MMC4_BASE + OMAP4_MMC_REG_OFFSET;
>> + ? ? ? ? ? ? ? ? ? ? base = OMAP4_MMC4_BASE;
>> ? ? ? ? ? ? ? ? ? ? ? irq = OMAP44XX_IRQ_MMC4;
>> ? ? ? ? ? ? ? ? ? ? ? break;
>> ? ? ? ? ? ? ? case 4:
>> ? ? ? ? ? ? ? ? ? ? ? if (!cpu_is_omap44xx())
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? return;
>> - ? ? ? ? ? ? ? ? ? ? base = OMAP4_MMC5_BASE + OMAP4_MMC_REG_OFFSET;
>> + ? ? ? ? ? ? ? ? ? ? base = OMAP4_MMC5_BASE;
>> ? ? ? ? ? ? ? ? ? ? ? irq = OMAP44XX_IRQ_MMC5;
>> ? ? ? ? ? ? ? ? ? ? ? break;
>> ? ? ? ? ? ? ? default:
>> @@ -762,10 +762,8 @@ void __init omap2_init_mmc(struct
>> omap_mmc_platform_data **mmc_data,
>> ? ? ? ? ? ? ? ? ? ? ? size = OMAP2420_MMC_SIZE;
>> ? ? ? ? ? ? ? ? ? ? ? name = "mmci-omap";
>> ? ? ? ? ? ? ? } else if (cpu_is_omap44xx()) {
>> - ? ? ? ? ? ? ? ? ? ? if (i < 3) {
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? base += OMAP4_MMC_REG_OFFSET;
>> + ? ? ? ? ? ? ? ? ? ? if (i < 3)
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? irq += OMAP44XX_IRQ_GIC_START;
>> - ? ? ? ? ? ? ? ? ? ? }
>> ? ? ? ? ? ? ? ? ? ? ? size = OMAP4_HSMMC_SIZE;
>> ? ? ? ? ? ? ? ? ? ? ? name = "mmci-omap-hs";
>> ? ? ? ? ? ? ? } else {
>> diff --git a/arch/arm/mach-omap2/hsmmc.c
>> b/arch/arm/mach-omap2/hsmmc.c index c8f647b..49d76a7 100644
>> --- a/arch/arm/mach-omap2/hsmmc.c
>> +++ b/arch/arm/mach-omap2/hsmmc.c
>> @@ -261,6 +261,10 @@ void __init omap2_hsmmc_init(struct
>> omap2_hsmmc_info *controllers)
>> ? ? ? ? ? ? ? mmc->slots[0].wires = c->wires;
>> ? ? ? ? ? ? ? mmc->slots[0].internal_clock = !c->ext_clock;
>> ? ? ? ? ? ? ? mmc->dma_mask = 0xffffffff;
>> + ? ? ? ? ? ? if (cpu_is_omap44xx())
>> + ? ? ? ? ? ? ? ? ? ? mmc->reg_offset = OMAP4_MMC_REG_OFFSET;
>> + ? ? ? ? ? ? else
>> + ? ? ? ? ? ? ? ? ? ? mmc->reg_offset = 0;
>
> 1. Since you mentioned that, this is useful for hwmod, the
> register offsets should use enum and static tables. You can
> refer to i2c-omap.c for more details and dma hwmod review
> comments for more details.
> https://patchwork.kernel.org/patch/126251/

I've avoided using static tables for register offset handling as suggested
by Benoit for HSMMC driver.Instead had the offset updated in hsmmc.c
and used in driver.

Could you check the latest version V4
http://www.spinics.net/lists/linux-omap/msg37226.html


>
> 2. Replace macros with inline functions for low level read/write
> functions.

Didn't touch this in the latest V4

Regards,
Kishore
>
> ?-Manjunath
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>