diff mbox

[U-Boot,v2,4/6] am33xx: Provide platform data for mmc

Message ID 20170426080710.7166-5-lokeshvutla@ti.com
State Accepted
Commit 4548bc8d0a299a194f4b8322d66207150590b9dc
Delegated to: Tom Rini
Headers show

Commit Message

Lokesh Vutla April 26, 2017, 8:07 a.m. UTC
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
Changes since v1:
- Update base addresses for MMC0 and MMC1
 board/ti/am335x/board.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Comments

Adam Ford April 26, 2017, 11:11 a.m. UTC | #1
On Wed, Apr 26, 2017 at 3:07 AM, Lokesh Vutla <lokeshvutla@ti.com> wrote:
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
> Changes since v1:
> - Update base addresses for MMC0 and MMC1
>  board/ti/am335x/board.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c
> index 3e842d3187..6786229680 100644
> --- a/board/ti/am335x/board.c
> +++ b/board/ti/am335x/board.c
> @@ -9,6 +9,7 @@
>   */
>
>  #include <common.h>
> +#include <dm.h>
>  #include <errno.h>
>  #include <spl.h>
>  #include <serial.h>
> @@ -26,6 +27,7 @@
>  #include <asm/emif.h>
>  #include <asm/gpio.h>
>  #include <asm/omap_sec_common.h>
> +#include <asm/omap_mmc.h>
>  #include <i2c.h>
>  #include <miiphy.h>
>  #include <cpsw.h>
> @@ -892,3 +894,33 @@ void board_fit_image_post_process(void **p_image, size_t *p_size)
>         secure_boot_verify_image(p_image, p_size);
>  }
>  #endif
> +
> +#if !CONFIG_IS_ENABLED(OF_CONTROL)
> +static const struct omap_hsmmc_plat am335x_mmc0_platdata = {
> +       .base_addr = (struct hsmmc *)OMAP_HSMMC1_BASE,
> +       .cfg.host_caps = MMC_MODE_HS_52MHz | MMC_MODE_HS | MMC_MODE_4BIT,
> +       .cfg.f_min = 400000,
> +       .cfg.f_max = 52000000,
> +       .cfg.voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195,
> +       .cfg.b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT,
> +};

Do you need to address the offset factor here if you're going to
rebase from my series?  My series removed the offset from struct hsmmc
because I assummed (maybe wrongly) that everyone who was using DM_MMC
was using OF_CONFIG.  Since I assumed that people where using
OF_CONFIG, I removed the offset from the structure and moved it
elsewhere.

 struct hsmmc {
-#ifdef CONFIG_DM_MMC
- unsigned char res0[0x100];
-#endif

If you're going under the assumption that people might use DM_MMC and
without OF_CONTROL, I am concerned this may break because the offset
was removed.  I think that may be what Tony was trying to say.

If we update my patch to put the offset back, when OF_CONTROL is not
present, then it may break the functionality of OMAP3 users want to
use DM_MMC without OF_CONTROL which means, then the ifdef would grow.

struct hsmmc {
#if (defined(CONFIG_DM_MMC) && !defined(CONFIG_OF_CONTROL) &&
!defined(CONFIG_OMAP34XX))
unsigned char res0[0x100];
#endif

And that gets really ugly and part of what I was trying to avoid.

I wonder if we can somehow add the appropriate offset here like

 .base_addr = (struct hsmmc *) (OMAP_HSMMC1_BASE + 0x100)

I am just thinking out loud, so forgive the excessive chatter, but
since I don't have am33xx hardware, I can only give my ideas without
testing.


> +
> +U_BOOT_DEVICE(am335x_mmc0) = {
> +       .name = "omap_hsmmc",
> +       .platdata = &am335x_mmc0_platdata,
> +};
> +
> +static const struct omap_hsmmc_plat am335x_mmc1_platdata = {
> +       .base_addr = (struct hsmmc *)OMAP_HSMMC2_BASE,

See above on base_addr.

> +       .cfg.host_caps = MMC_MODE_HS_52MHz | MMC_MODE_HS | MMC_MODE_8BIT,
> +       .cfg.f_min = 400000,
> +       .cfg.f_max = 52000000,
> +       .cfg.voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195,
> +       .cfg.b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT,
> +};
> +
> +U_BOOT_DEVICE(am335x_mmc1) = {
> +       .name = "omap_hsmmc",
> +       .platdata = &am335x_mmc1_platdata,
> +};
> +#endif
> --
> 2.11.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Lokesh Vutla April 26, 2017, 11:13 a.m. UTC | #2
On Wednesday 26 April 2017 04:41 PM, Adam Ford wrote:
> On Wed, Apr 26, 2017 at 3:07 AM, Lokesh Vutla <lokeshvutla@ti.com> wrote:
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>> Changes since v1:
>> - Update base addresses for MMC0 and MMC1
>>  board/ti/am335x/board.c | 32 ++++++++++++++++++++++++++++++++
>>  1 file changed, 32 insertions(+)
>>
>> diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c
>> index 3e842d3187..6786229680 100644
>> --- a/board/ti/am335x/board.c
>> +++ b/board/ti/am335x/board.c
>> @@ -9,6 +9,7 @@
>>   */
>>
>>  #include <common.h>
>> +#include <dm.h>
>>  #include <errno.h>
>>  #include <spl.h>
>>  #include <serial.h>
>> @@ -26,6 +27,7 @@
>>  #include <asm/emif.h>
>>  #include <asm/gpio.h>
>>  #include <asm/omap_sec_common.h>
>> +#include <asm/omap_mmc.h>
>>  #include <i2c.h>
>>  #include <miiphy.h>
>>  #include <cpsw.h>
>> @@ -892,3 +894,33 @@ void board_fit_image_post_process(void **p_image, size_t *p_size)
>>         secure_boot_verify_image(p_image, p_size);
>>  }
>>  #endif
>> +
>> +#if !CONFIG_IS_ENABLED(OF_CONTROL)
>> +static const struct omap_hsmmc_plat am335x_mmc0_platdata = {
>> +       .base_addr = (struct hsmmc *)OMAP_HSMMC1_BASE,
>> +       .cfg.host_caps = MMC_MODE_HS_52MHz | MMC_MODE_HS | MMC_MODE_4BIT,
>> +       .cfg.f_min = 400000,
>> +       .cfg.f_max = 52000000,
>> +       .cfg.voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195,
>> +       .cfg.b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT,
>> +};
> 
> Do you need to address the offset factor here if you're going to
> rebase from my series?  My series removed the offset from struct hsmmc

OMAP_HSMMC1_BASE includes the offset. This is currently being used in
driver as well for non-DM cases.

Thanks and regards,
Lokesh
Adam Ford April 26, 2017, 11:56 a.m. UTC | #3
On Wed, Apr 26, 2017 at 6:13 AM, Lokesh Vutla <lokeshvutla@ti.com> wrote:
>
>
> On Wednesday 26 April 2017 04:41 PM, Adam Ford wrote:
>> On Wed, Apr 26, 2017 at 3:07 AM, Lokesh Vutla <lokeshvutla@ti.com> wrote:
>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>> ---
>>> Changes since v1:
>>> - Update base addresses for MMC0 and MMC1
>>>  board/ti/am335x/board.c | 32 ++++++++++++++++++++++++++++++++
>>>  1 file changed, 32 insertions(+)
>>>
>>> diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c
>>> index 3e842d3187..6786229680 100644
>>> --- a/board/ti/am335x/board.c
>>> +++ b/board/ti/am335x/board.c
>>> @@ -9,6 +9,7 @@
>>>   */
>>>
>>>  #include <common.h>
>>> +#include <dm.h>
>>>  #include <errno.h>
>>>  #include <spl.h>
>>>  #include <serial.h>
>>> @@ -26,6 +27,7 @@
>>>  #include <asm/emif.h>
>>>  #include <asm/gpio.h>
>>>  #include <asm/omap_sec_common.h>
>>> +#include <asm/omap_mmc.h>
>>>  #include <i2c.h>
>>>  #include <miiphy.h>
>>>  #include <cpsw.h>
>>> @@ -892,3 +894,33 @@ void board_fit_image_post_process(void **p_image, size_t *p_size)
>>>         secure_boot_verify_image(p_image, p_size);
>>>  }
>>>  #endif
>>> +
>>> +#if !CONFIG_IS_ENABLED(OF_CONTROL)
>>> +static const struct omap_hsmmc_plat am335x_mmc0_platdata = {
>>> +       .base_addr = (struct hsmmc *)OMAP_HSMMC1_BASE,
>>> +       .cfg.host_caps = MMC_MODE_HS_52MHz | MMC_MODE_HS | MMC_MODE_4BIT,
>>> +       .cfg.f_min = 400000,
>>> +       .cfg.f_max = 52000000,
>>> +       .cfg.voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195,
>>> +       .cfg.b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT,
>>> +};
>>
>> Do you need to address the offset factor here if you're going to
>> rebase from my series?  My series removed the offset from struct hsmmc
>
> OMAP_HSMMC1_BASE includes the offset. This is currently being used in
> driver as well for non-DM cases.

Great!

thanks

>
> Thanks and regards,
> Lokesh
>
Tom Rini April 26, 2017, 11:58 a.m. UTC | #4
On Wed, Apr 26, 2017 at 01:37:08PM +0530, Lokesh Vutla wrote:

> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>
Adam Ford April 28, 2017, 5:40 p.m. UTC | #5
On Wed, Apr 26, 2017 at 6:58 AM, Tom Rini <trini@konsulko.com> wrote:
> On Wed, Apr 26, 2017 at 01:37:08PM +0530, Lokesh Vutla wrote:
>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>
> Reviewed-by: Tom Rini <trini@konsulko.com>
>
> --
> Tom
>

Tom - Do you know if his series and/or my OMAP3 device tree series
with MMC changes will be accepted?  I haven't heard or seen any
feedback on either other than that's already been in here.

adam
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>
Tom Rini April 28, 2017, 5:43 p.m. UTC | #6
On Fri, Apr 28, 2017 at 12:40:30PM -0500, Adam Ford wrote:
> On Wed, Apr 26, 2017 at 6:58 AM, Tom Rini <trini@konsulko.com> wrote:
> > On Wed, Apr 26, 2017 at 01:37:08PM +0530, Lokesh Vutla wrote:
> >
> >> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> >
> > Reviewed-by: Tom Rini <trini@konsulko.com>
> 
> Tom - Do you know if his series and/or my OMAP3 device tree series
> with MMC changes will be accepted?  I haven't heard or seen any
> feedback on either other than that's already been in here.

I think we'll take these in with the next release (v2017.07).
Adam Ford April 28, 2017, 5:43 p.m. UTC | #7
On Fri, Apr 28, 2017 at 12:43 PM, Tom Rini <trini@konsulko.com> wrote:
> On Fri, Apr 28, 2017 at 12:40:30PM -0500, Adam Ford wrote:
>> On Wed, Apr 26, 2017 at 6:58 AM, Tom Rini <trini@konsulko.com> wrote:
>> > On Wed, Apr 26, 2017 at 01:37:08PM +0530, Lokesh Vutla wrote:
>> >
>> >> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> >
>> > Reviewed-by: Tom Rini <trini@konsulko.com>
>>
>> Tom - Do you know if his series and/or my OMAP3 device tree series
>> with MMC changes will be accepted?  I haven't heard or seen any
>> feedback on either other than that's already been in here.
>
> I think we'll take these in with the next release (v2017.07).
>

OK. Cool.  Thanks


> --
> Tom
Tom Rini May 12, 2017, 5:19 p.m. UTC | #8
On Wed, Apr 26, 2017 at 01:37:08PM +0530, Lokesh Vutla wrote:

> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!
diff mbox

Patch

diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c
index 3e842d3187..6786229680 100644
--- a/board/ti/am335x/board.c
+++ b/board/ti/am335x/board.c
@@ -9,6 +9,7 @@ 
  */
 
 #include <common.h>
+#include <dm.h>
 #include <errno.h>
 #include <spl.h>
 #include <serial.h>
@@ -26,6 +27,7 @@ 
 #include <asm/emif.h>
 #include <asm/gpio.h>
 #include <asm/omap_sec_common.h>
+#include <asm/omap_mmc.h>
 #include <i2c.h>
 #include <miiphy.h>
 #include <cpsw.h>
@@ -892,3 +894,33 @@  void board_fit_image_post_process(void **p_image, size_t *p_size)
 	secure_boot_verify_image(p_image, p_size);
 }
 #endif
+
+#if !CONFIG_IS_ENABLED(OF_CONTROL)
+static const struct omap_hsmmc_plat am335x_mmc0_platdata = {
+	.base_addr = (struct hsmmc *)OMAP_HSMMC1_BASE,
+	.cfg.host_caps = MMC_MODE_HS_52MHz | MMC_MODE_HS | MMC_MODE_4BIT,
+	.cfg.f_min = 400000,
+	.cfg.f_max = 52000000,
+	.cfg.voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195,
+	.cfg.b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT,
+};
+
+U_BOOT_DEVICE(am335x_mmc0) = {
+	.name = "omap_hsmmc",
+	.platdata = &am335x_mmc0_platdata,
+};
+
+static const struct omap_hsmmc_plat am335x_mmc1_platdata = {
+	.base_addr = (struct hsmmc *)OMAP_HSMMC2_BASE,
+	.cfg.host_caps = MMC_MODE_HS_52MHz | MMC_MODE_HS | MMC_MODE_8BIT,
+	.cfg.f_min = 400000,
+	.cfg.f_max = 52000000,
+	.cfg.voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195,
+	.cfg.b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT,
+};
+
+U_BOOT_DEVICE(am335x_mmc1) = {
+	.name = "omap_hsmmc",
+	.platdata = &am335x_mmc1_platdata,
+};
+#endif