diff mbox

[U-Boot,1/7] omap_hsmmc: update struct hsmmc to accommodate omap3 from DT

Message ID 1492000720-11743-1-git-send-email-aford173@gmail.com
State Superseded
Delegated to: Tom Rini
Headers show

Commit Message

Adam Ford April 12, 2017, 12:38 p.m. UTC
This patch fixes and issue where DM_MMC adds a 0x100 byte offset to the
base register.  This is necessary for AM33xx, OMAP4+ and newer devices, but
it is not necessary for OMAP34XX boards.

This patch will now only apply the x100 byte offset correction if DM_MMC
is enabled and the device is not OMAP34XX.

Fixes 11e1582506c6 ("omap_hsmmc: update struct hsmmc to accomodate
base address from DT")

Signed-off-by: Adam Ford <aford173@gmail.com>

Comments

Tom Rini April 12, 2017, 7:18 p.m. UTC | #1
On Wed, Apr 12, 2017 at 07:38:40AM -0500, Adam Ford wrote:

> This patch fixes and issue where DM_MMC adds a 0x100 byte offset to the
> base register.  This is necessary for AM33xx, OMAP4+ and newer devices, but
> it is not necessary for OMAP34XX boards.
> 
> This patch will now only apply the x100 byte offset correction if DM_MMC
> is enabled and the device is not OMAP34XX.
> 
> Fixes 11e1582506c6 ("omap_hsmmc: update struct hsmmc to accomodate
> base address from DT")
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> 
> diff --git a/arch/arm/include/asm/omap_mmc.h b/arch/arm/include/asm/omap_mmc.h
> index f2bf645..5b44c40 100644

Reviewed-by: Tom Rini <trini@konsulko.com>
Raghavendra, Vignesh April 13, 2017, 4:24 a.m. UTC | #2
On Wednesday 12 April 2017 06:08 PM, Adam Ford wrote:
> This patch fixes and issue where DM_MMC adds a 0x100 byte offset to the
> base register.  This is necessary for AM33xx, OMAP4+ and newer devices, but
> it is not necessary for OMAP34XX boards.
> 
> This patch will now only apply the x100 byte offset correction if DM_MMC
> is enabled and the device is not OMAP34XX.
> 

Although this is an easy solution, I think its better to handle this w/o
ifdefs and instead using compatible string. See how omap3 base address
is handled in Linux omap_hsmmc driver (drivers/mmc/host/omap_hsmmc.c).

Also, please send rest of the series in reply to Patch 0/7,
git send-email should automatically do that for you.

> Fixes 11e1582506c6 ("omap_hsmmc: update struct hsmmc to accomodate
> base address from DT")
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> 
> diff --git a/arch/arm/include/asm/omap_mmc.h b/arch/arm/include/asm/omap_mmc.h
> index f2bf645..5b44c40 100644
> --- a/arch/arm/include/asm/omap_mmc.h
> +++ b/arch/arm/include/asm/omap_mmc.h
> @@ -26,7 +26,7 @@
>  #define OMAP_MMC_H_
>  
>  struct hsmmc {
> -#ifdef CONFIG_DM_MMC
> +#if defined(CONFIG_DM_MMC) && !defined(CONFIG_OMAP34XX)
>  	unsigned char res0[0x100];
>  #endif
>  	unsigned char res1[0x10];
>
Adam Ford April 14, 2017, 10:33 p.m. UTC | #3
On Wed, Apr 12, 2017 at 11:24 PM, Vignesh R <vigneshr@ti.com> wrote:
>
>
> On Wednesday 12 April 2017 06:08 PM, Adam Ford wrote:
>> This patch fixes and issue where DM_MMC adds a 0x100 byte offset to the
>> base register.  This is necessary for AM33xx, OMAP4+ and newer devices, but
>> it is not necessary for OMAP34XX boards.
>>
>> This patch will now only apply the x100 byte offset correction if DM_MMC
>> is enabled and the device is not OMAP34XX.
>>
>
> Although this is an easy solution, I think its better to handle this w/o
> ifdefs and instead using compatible string. See how omap3 base address
> is handled in Linux omap_hsmmc driver (drivers/mmc/host/omap_hsmmc.c).
>

I have the basic ideal, but I am not sure how extract the
udevice_id->data from the corresponding omap_hsmmc_ids

Like Linux, I was going to create a structure

static const struct omap_mmc_of_data omap4_mmc_of_data = {
.reg_offset = 0x100,
};

and attach it to the necessary .compatible parts.
{
.compatible = "ti,omap4-hsmmc",
.data = &omap4_mmc_of_data
}

When ."compatible" matches, I need to extract the corresponding
->data, but I don't what what the best driver example to follow would
be.

Can you point me to to good example or give me some suggestions on how
to extract it?  This device tree decoding stuff is new to me, but  I
want to be helpful too.

adam

> Also, please send rest of the series in reply to Patch 0/7,
> git send-email should automatically do that for you.

I'll read the docs again on submitting patch series, and try to figure
it out once I address the MMC offset thing.  This is my first patch
series.

thanks for the feedback.

adam
>
>> Fixes 11e1582506c6 ("omap_hsmmc: update struct hsmmc to accomodate
>> base address from DT")
>>
>> Signed-off-by: Adam Ford <aford173@gmail.com>
>>
>> diff --git a/arch/arm/include/asm/omap_mmc.h b/arch/arm/include/asm/omap_mmc.h
>> index f2bf645..5b44c40 100644
>> --- a/arch/arm/include/asm/omap_mmc.h
>> +++ b/arch/arm/include/asm/omap_mmc.h
>> @@ -26,7 +26,7 @@
>>  #define OMAP_MMC_H_
>>
>>  struct hsmmc {
>> -#ifdef CONFIG_DM_MMC
>> +#if defined(CONFIG_DM_MMC) && !defined(CONFIG_OMAP34XX)
>>       unsigned char res0[0x100];
>>  #endif
>>       unsigned char res1[0x10];
>>
>
> --
> Regards
> Vignesh
Raghavendra, Vignesh April 15, 2017, 12:31 p.m. UTC | #4
Hi,

On 4/15/2017 4:03 AM, Adam Ford wrote:
> On Wed, Apr 12, 2017 at 11:24 PM, Vignesh R <vigneshr@ti.com> wrote:
>>
>>
>> On Wednesday 12 April 2017 06:08 PM, Adam Ford wrote:
>>> This patch fixes and issue where DM_MMC adds a 0x100 byte offset to the
>>> base register.  This is necessary for AM33xx, OMAP4+ and newer devices, but
>>> it is not necessary for OMAP34XX boards.
>>>
>>> This patch will now only apply the x100 byte offset correction if DM_MMC
>>> is enabled and the device is not OMAP34XX.
>>>
>>
>> Although this is an easy solution, I think its better to handle this w/o
>> ifdefs and instead using compatible string. See how omap3 base address
>> is handled in Linux omap_hsmmc driver (drivers/mmc/host/omap_hsmmc.c).
>>
> 
> I have the basic ideal, but I am not sure how extract the
> udevice_id->data from the corresponding omap_hsmmc_ids
> 
> Like Linux, I was going to create a structure
> 
> static const struct omap_mmc_of_data omap4_mmc_of_data = {
> .reg_offset = 0x100,
> };
> 
> and attach it to the necessary .compatible parts.
> {
> .compatible = "ti,omap4-hsmmc",
> .data = &omap4_mmc_of_data
> }
> 
> When ."compatible" matches, I need to extract the corresponding
> ->data, but I don't what what the best driver example to follow would
> be.
> 
> Can you point me to to good example or give me some suggestions on how
> to extract it?  This device tree decoding stuff is new to me, but  I
> want to be helpful too.
> 

You could look at drivers/spi/omap3_spi.c in U-Boot tree, that handles
similar problem.

> 
>> Also, please send rest of the series in reply to Patch 0/7,
>> git send-email should automatically do that for you.
> 
> I'll read the docs again on submitting patch series, and try to figure
> it out once I address the MMC offset thing.  This is my first patch
> series.
> 

Thanks for the patch!
Maybe, this link could help: https://kernelnewbies.org/FirstKernelPatch

Regards
Vignesh
diff mbox

Patch

diff --git a/arch/arm/include/asm/omap_mmc.h b/arch/arm/include/asm/omap_mmc.h
index f2bf645..5b44c40 100644
--- a/arch/arm/include/asm/omap_mmc.h
+++ b/arch/arm/include/asm/omap_mmc.h
@@ -26,7 +26,7 @@ 
 #define OMAP_MMC_H_
 
 struct hsmmc {
-#ifdef CONFIG_DM_MMC
+#if defined(CONFIG_DM_MMC) && !defined(CONFIG_OMAP34XX)
 	unsigned char res0[0x100];
 #endif
 	unsigned char res1[0x10];