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