[v5,1/4] omap: mmc: Avoid using libfdt with of-platdata

Message ID 20200427012435.254270-2-sjg@chromium.org
State New
Headers show
Series
  • of-platdata: Avoid building libfdt
Related show

Commit Message

Simon Glass April 27, 2020, 1:24 a.m.
At present this driver is enabled in SPL on omapl138_lcdk, which uses
of-platdata. The driver needs to be ported to use of-platdata properly.
For now, avoid a build error by returning an error.

Signed-off-by: Simon Glass <sjg at chromium.org>
---

Changes in v5: None
Changes in v4: None
Changes in v3: None

 drivers/mmc/davinci_mmc.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Peng Fan April 27, 2020, 5:33 a.m. | #1
> Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
> 
> At present this driver is enabled in SPL on omapl138_lcdk, which uses
> of-platdata. The driver needs to be ported to use of-platdata properly.
> For now, avoid a build error by returning an error.
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>

Acked-by: Peng Fan <peng.fan at nxp.com>
Tom Rini April 27, 2020, 6:59 p.m. | #2
On Mon, Apr 27, 2020 at 05:33:41AM +0000, Peng Fan wrote:
> > Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
> > 
> > At present this driver is enabled in SPL on omapl138_lcdk, which uses
> > of-platdata. The driver needs to be ported to use of-platdata properly.
> > For now, avoid a build error by returning an error.
> > 
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> 
> Acked-by: Peng Fan <peng.fan at nxp.com>

Since the board maintainer is on CC and I believe that platform is still
actively used in testing, I want to see this fixed rather than turned in
to a run-time error.  Thanks!
Lokesh Vutla April 28, 2020, 4:17 a.m. | #3
+Faiz,

On 28/04/20 12:29 AM, Tom Rini wrote:
> On Mon, Apr 27, 2020 at 05:33:41AM +0000, Peng Fan wrote:
>>> Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
>>>
>>> At present this driver is enabled in SPL on omapl138_lcdk, which uses
>>> of-platdata. The driver needs to be ported to use of-platdata properly.
>>> For now, avoid a build error by returning an error.
>>>
>>> Signed-off-by: Simon Glass <sjg at chromium.org>

Does this break the boot on omap l138?

Thanks and regards,
Lokesh

>>
>> Acked-by: Peng Fan <peng.fan at nxp.com>
> 
> Since the board maintainer is on CC and I believe that platform is still
> actively used in testing, I want to see this fixed rather than turned in
> to a run-time error.  Thanks!
>
Faiz Abbas April 28, 2020, 7 a.m. | #4
+Bartosz

On 28/04/20 9:47 am, Lokesh Vutla wrote:
> +Faiz,
> 
> On 28/04/20 12:29 AM, Tom Rini wrote:
>> On Mon, Apr 27, 2020 at 05:33:41AM +0000, Peng Fan wrote:
>>>> Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
>>>>
>>>> At present this driver is enabled in SPL on omapl138_lcdk, which uses
>>>> of-platdata. The driver needs to be ported to use of-platdata properly.
>>>> For now, avoid a build error by returning an error.
>>>>
>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
> 
> Does this break the boot on omap l138?
> 

I don't have a board at hand to test this out. Bartosz can you help test this with
omapl138?

Thanks,
Faiz
Bartosz Golaszewski April 30, 2020, 11:43 a.m. | #5
wt., 28 kwi 2020 o 09:01 Faiz Abbas <faiz_abbas at ti.com> napisa?(a):
>
> +Bartosz
>
> On 28/04/20 9:47 am, Lokesh Vutla wrote:
> > +Faiz,
> >
> > On 28/04/20 12:29 AM, Tom Rini wrote:
> >> On Mon, Apr 27, 2020 at 05:33:41AM +0000, Peng Fan wrote:
> >>>> Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
> >>>>
> >>>> At present this driver is enabled in SPL on omapl138_lcdk, which uses
> >>>> of-platdata. The driver needs to be ported to use of-platdata properly.
> >>>> For now, avoid a build error by returning an error.
> >>>>
> >>>> Signed-off-by: Simon Glass <sjg at chromium.org>
> >
> > Does this break the boot on omap l138?
> >
>
> I don't have a board at hand to test this out. Bartosz can you help test this with
> omapl138?
>
> Thanks,
> Faiz

Hi Faiz,

I can confirm - this *does* break the mmc boot on da850-lcdk.

Bart
Tom Rini May 1, 2020, 6:32 p.m. | #6
On Thu, Apr 30, 2020 at 01:43:30PM +0200, Bartosz Golaszewski wrote:
> wt., 28 kwi 2020 o 09:01 Faiz Abbas <faiz_abbas at ti.com> napisa?(a):
> >
> > +Bartosz
> >
> > On 28/04/20 9:47 am, Lokesh Vutla wrote:
> > > +Faiz,
> > >
> > > On 28/04/20 12:29 AM, Tom Rini wrote:
> > >> On Mon, Apr 27, 2020 at 05:33:41AM +0000, Peng Fan wrote:
> > >>>> Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
> > >>>>
> > >>>> At present this driver is enabled in SPL on omapl138_lcdk, which uses
> > >>>> of-platdata. The driver needs to be ported to use of-platdata properly.
> > >>>> For now, avoid a build error by returning an error.
> > >>>>
> > >>>> Signed-off-by: Simon Glass <sjg at chromium.org>
> > >
> > > Does this break the boot on omap l138?
> > >
> >
> > I don't have a board at hand to test this out. Bartosz can you help test this with
> > omapl138?
> >
> > Thanks,
> > Faiz
> 
> Hi Faiz,
> 
> I can confirm - this *does* break the mmc boot on da850-lcdk.

So who is going to fix the driver to unblock Simon's series?
Bartosz Golaszewski May 4, 2020, 7:10 a.m. | #7
pt., 1 maj 2020 o 20:32 Tom Rini <trini at konsulko.com> napisa?(a):
>
> On Thu, Apr 30, 2020 at 01:43:30PM +0200, Bartosz Golaszewski wrote:
> > wt., 28 kwi 2020 o 09:01 Faiz Abbas <faiz_abbas at ti.com> napisa?(a):
> > >
> > > +Bartosz
> > >
> > > On 28/04/20 9:47 am, Lokesh Vutla wrote:
> > > > +Faiz,
> > > >
> > > > On 28/04/20 12:29 AM, Tom Rini wrote:
> > > >> On Mon, Apr 27, 2020 at 05:33:41AM +0000, Peng Fan wrote:
> > > >>>> Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
> > > >>>>
> > > >>>> At present this driver is enabled in SPL on omapl138_lcdk, which uses
> > > >>>> of-platdata. The driver needs to be ported to use of-platdata properly.
> > > >>>> For now, avoid a build error by returning an error.
> > > >>>>
> > > >>>> Signed-off-by: Simon Glass <sjg at chromium.org>
> > > >
> > > > Does this break the boot on omap l138?
> > > >
> > >
> > > I don't have a board at hand to test this out. Bartosz can you help test this with
> > > omapl138?
> > >
> > > Thanks,
> > > Faiz
> >
> > Hi Faiz,
> >
> > I can confirm - this *does* break the mmc boot on da850-lcdk.
>
> So who is going to fix the driver to unblock Simon's series?
>

Is this something that will take a lot of work? What exactly needs
doing? I'm not sure what "use of-platdata properly" means.

Bart
Simon Glass May 4, 2020, 1:14 p.m. | #8
Hi Bart,

On Mon, 4 May 2020 at 01:10, Bartosz Golaszewski <brgl at bgdev.pl> wrote:
>
> pt., 1 maj 2020 o 20:32 Tom Rini <trini at konsulko.com> napisa?(a):
> >
> > On Thu, Apr 30, 2020 at 01:43:30PM +0200, Bartosz Golaszewski wrote:
> > > wt., 28 kwi 2020 o 09:01 Faiz Abbas <faiz_abbas at ti.com> napisa?(a):
> > > >
> > > > +Bartosz
> > > >
> > > > On 28/04/20 9:47 am, Lokesh Vutla wrote:
> > > > > +Faiz,
> > > > >
> > > > > On 28/04/20 12:29 AM, Tom Rini wrote:
> > > > >> On Mon, Apr 27, 2020 at 05:33:41AM +0000, Peng Fan wrote:
> > > > >>>> Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
> > > > >>>>
> > > > >>>> At present this driver is enabled in SPL on omapl138_lcdk, which uses
> > > > >>>> of-platdata. The driver needs to be ported to use of-platdata properly.
> > > > >>>> For now, avoid a build error by returning an error.
> > > > >>>>
> > > > >>>> Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > >
> > > > > Does this break the boot on omap l138?
> > > > >
> > > >
> > > > I don't have a board at hand to test this out. Bartosz can you help test this with
> > > > omapl138?
> > > >
> > > > Thanks,
> > > > Faiz
> > >
> > > Hi Faiz,
> > >
> > > I can confirm - this *does* break the mmc boot on da850-lcdk.
> >
> > So who is going to fix the driver to unblock Simon's series?
> >
>
> Is this something that will take a lot of work? What exactly needs
> doing? I'm not sure what "use of-platdata properly" means.

This board is defining CONFIG_SPL_OF_PLATDATA which means that device
tree is not available in SPL. Instead you need to use a C structure
created by dtoc. It basically involves creating that struct and
getting the data from that instead of calling the DT functions. I
expect it will take 2-4 hours to figure out, code and test.

See of-plat.rst for full documentation. There are quite a few examples for mmc:

grep PLATDATA drivers/mmc/*.c
drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/ftsdc010_mci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/mxsmmc.c: debug("OF_PLATDATA: regs: 0x%p bw: %d clkid: %d
non_removable: %d\n",
drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
!CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
!CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
!CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
!CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
!CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/rockchip_dw_mmc.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
drivers/mmc/rockchip_sdhci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)

Regards,
Simon
Faiz Abbas May 5, 2020, 6:50 a.m. | #9
Hi,

On 04/05/20 6:44 pm, Simon Glass wrote:
> Hi Bart,
> 
> On Mon, 4 May 2020 at 01:10, Bartosz Golaszewski <brgl at bgdev.pl> wrote:
>>
>> pt., 1 maj 2020 o 20:32 Tom Rini <trini at konsulko.com> napisa?(a):
>>>
>>> On Thu, Apr 30, 2020 at 01:43:30PM +0200, Bartosz Golaszewski wrote:
>>>> wt., 28 kwi 2020 o 09:01 Faiz Abbas <faiz_abbas at ti.com> napisa?(a):
>>>>>
>>>>> +Bartosz
>>>>>
>>>>> On 28/04/20 9:47 am, Lokesh Vutla wrote:
>>>>>> +Faiz,
>>>>>>
>>>>>> On 28/04/20 12:29 AM, Tom Rini wrote:
>>>>>>> On Mon, Apr 27, 2020 at 05:33:41AM +0000, Peng Fan wrote:
>>>>>>>>> Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
>>>>>>>>>
>>>>>>>>> At present this driver is enabled in SPL on omapl138_lcdk, which uses
>>>>>>>>> of-platdata. The driver needs to be ported to use of-platdata properly.
>>>>>>>>> For now, avoid a build error by returning an error.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>>>>>
>>>>>> Does this break the boot on omap l138?
>>>>>>
>>>>>
>>>>> I don't have a board at hand to test this out. Bartosz can you help test this with
>>>>> omapl138?
>>>>>
>>>>> Thanks,
>>>>> Faiz
>>>>
>>>> Hi Faiz,
>>>>
>>>> I can confirm - this *does* break the mmc boot on da850-lcdk.
>>>
>>> So who is going to fix the driver to unblock Simon's series?
>>>
>>
>> Is this something that will take a lot of work? What exactly needs
>> doing? I'm not sure what "use of-platdata properly" means.
> 
> This board is defining CONFIG_SPL_OF_PLATDATA which means that device
> tree is not available in SPL. Instead you need to use a C structure
> created by dtoc. It basically involves creating that struct and
> getting the data from that instead of calling the DT functions. I
> expect it will take 2-4 hours to figure out, code and test.
> 
> See of-plat.rst for full documentation. There are quite a few examples for mmc:
> 
> grep PLATDATA drivers/mmc/*.c
> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/ftsdc010_mci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/mxsmmc.c: debug("OF_PLATDATA: regs: 0x%p bw: %d clkid: %d
> non_removable: %d\n",
> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> !CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> !CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> !CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> !CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> !CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/rockchip_dw_mmc.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> drivers/mmc/rockchip_sdhci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> 

I was able to get a setup to work on. Will post a fix for this soon.

Thanks,
Faiz
Bartosz Golaszewski May 5, 2020, 4:17 p.m. | #10
wt., 5 maj 2020 o 08:50 Faiz Abbas <faiz_abbas at ti.com> napisa?(a):
>
> Hi,
>
> On 04/05/20 6:44 pm, Simon Glass wrote:
> > Hi Bart,
> >
> > On Mon, 4 May 2020 at 01:10, Bartosz Golaszewski <brgl at bgdev.pl> wrote:
> >>
> >> pt., 1 maj 2020 o 20:32 Tom Rini <trini at konsulko.com> napisa?(a):
> >>>
> >>> On Thu, Apr 30, 2020 at 01:43:30PM +0200, Bartosz Golaszewski wrote:
> >>>> wt., 28 kwi 2020 o 09:01 Faiz Abbas <faiz_abbas at ti.com> napisa?(a):
> >>>>>
> >>>>> +Bartosz
> >>>>>
> >>>>> On 28/04/20 9:47 am, Lokesh Vutla wrote:
> >>>>>> +Faiz,
> >>>>>>
> >>>>>> On 28/04/20 12:29 AM, Tom Rini wrote:
> >>>>>>> On Mon, Apr 27, 2020 at 05:33:41AM +0000, Peng Fan wrote:
> >>>>>>>>> Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
> >>>>>>>>>
> >>>>>>>>> At present this driver is enabled in SPL on omapl138_lcdk, which uses
> >>>>>>>>> of-platdata. The driver needs to be ported to use of-platdata properly.
> >>>>>>>>> For now, avoid a build error by returning an error.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
> >>>>>>
> >>>>>> Does this break the boot on omap l138?
> >>>>>>
> >>>>>
> >>>>> I don't have a board at hand to test this out. Bartosz can you help test this with
> >>>>> omapl138?
> >>>>>
> >>>>> Thanks,
> >>>>> Faiz
> >>>>
> >>>> Hi Faiz,
> >>>>
> >>>> I can confirm - this *does* break the mmc boot on da850-lcdk.
> >>>
> >>> So who is going to fix the driver to unblock Simon's series?
> >>>
> >>
> >> Is this something that will take a lot of work? What exactly needs
> >> doing? I'm not sure what "use of-platdata properly" means.
> >
> > This board is defining CONFIG_SPL_OF_PLATDATA which means that device
> > tree is not available in SPL. Instead you need to use a C structure
> > created by dtoc. It basically involves creating that struct and
> > getting the data from that instead of calling the DT functions. I
> > expect it will take 2-4 hours to figure out, code and test.
> >
> > See of-plat.rst for full documentation. There are quite a few examples for mmc:
> >
> > grep PLATDATA drivers/mmc/*.c
> > drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/ftsdc010_mci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/mxsmmc.c: debug("OF_PLATDATA: regs: 0x%p bw: %d clkid: %d
> > non_removable: %d\n",
> > drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> > !CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> > !CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> > !CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> > !CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> > !CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/rockchip_dw_mmc.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> > drivers/mmc/rockchip_sdhci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> >
>
> I was able to get a setup to work on. Will post a fix for this soon.
>
> Thanks,
> Faiz

Thanks Faiz! Let me know if you need some help testing it.

Bart
Faiz Abbas May 14, 2020, 8:19 a.m. | #11
Simon,

On 05/05/20 12:20 pm, Faiz Abbas wrote:
> Hi,
> 
> On 04/05/20 6:44 pm, Simon Glass wrote:
>> Hi Bart,
>>
>> On Mon, 4 May 2020 at 01:10, Bartosz Golaszewski <brgl at bgdev.pl> wrote:
>>>
>>> pt., 1 maj 2020 o 20:32 Tom Rini <trini at konsulko.com> napisa?(a):
>>>>
>>>> On Thu, Apr 30, 2020 at 01:43:30PM +0200, Bartosz Golaszewski wrote:
>>>>> wt., 28 kwi 2020 o 09:01 Faiz Abbas <faiz_abbas at ti.com> napisa?(a):
>>>>>>
>>>>>> +Bartosz
>>>>>>
>>>>>> On 28/04/20 9:47 am, Lokesh Vutla wrote:
>>>>>>> +Faiz,
>>>>>>>
>>>>>>> On 28/04/20 12:29 AM, Tom Rini wrote:
>>>>>>>> On Mon, Apr 27, 2020 at 05:33:41AM +0000, Peng Fan wrote:
>>>>>>>>>> Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
>>>>>>>>>>
>>>>>>>>>> At present this driver is enabled in SPL on omapl138_lcdk, which uses
>>>>>>>>>> of-platdata. The driver needs to be ported to use of-platdata properly.
>>>>>>>>>> For now, avoid a build error by returning an error.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>>>>>>
>>>>>>> Does this break the boot on omap l138?
>>>>>>>
>>>>>>
>>>>>> I don't have a board at hand to test this out. Bartosz can you help test this with
>>>>>> omapl138?
>>>>>>
>>>>>> Thanks,
>>>>>> Faiz
>>>>>
>>>>> Hi Faiz,
>>>>>
>>>>> I can confirm - this *does* break the mmc boot on da850-lcdk.
>>>>
>>>> So who is going to fix the driver to unblock Simon's series?
>>>>
>>>
>>> Is this something that will take a lot of work? What exactly needs
>>> doing? I'm not sure what "use of-platdata properly" means.
>>
>> This board is defining CONFIG_SPL_OF_PLATDATA which means that device
>> tree is not available in SPL. Instead you need to use a C structure
>> created by dtoc. It basically involves creating that struct and
>> getting the data from that instead of calling the DT functions. I
>> expect it will take 2-4 hours to figure out, code and test.
>>
>> See of-plat.rst for full documentation. There are quite a few examples for mmc:
>>
>> grep PLATDATA drivers/mmc/*.c
>> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/ftsdc010_mci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/mxsmmc.c: debug("OF_PLATDATA: regs: 0x%p bw: %d clkid: %d
>> non_removable: %d\n",
>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
>> !CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
>> !CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
>> !CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
>> !CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
>> !CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/rockchip_dw_mmc.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>> drivers/mmc/rockchip_sdhci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
>>

In all the examples above, platdata reg filed is directly being used for
to assign a register base address but looking at davinci platdata that is generated,
spl/dts/dt-platdata.c:

static const struct dtd_simple_bus dtv_soc_at_1c00000 = {
        .model                  = "da850",
        .ranges                 = {0x0, 0x1c00000, 0x400000},
};
U_BOOT_DEVICE(soc_at_1c00000) = {
        .name           = "simple_bus",
        .platdata       = &dtv_soc_at_1c00000,
        .platdata_size  = sizeof(dtv_soc_at_1c00000),
};

static const struct dtd_ti_da830_uart dtv_serial_at_10d000 = {
        .power_domains          = {0xa, 0xd},
        .reg                    = {0x10d000, 0x100},
        .reg_io_width           = 0x4,
        .reg_shift              = 0x2,
};
U_BOOT_DEVICE(serial_at_10d000) = {
        .name           = "ti_da830_uart",
        .platdata       = &dtv_serial_at_10d000,
        .platdata_size  = sizeof(dtv_serial_at_10d000),
};

static const struct dtd_ti_da830_mmc dtv_mmc_at_40000 = {
        .bus_width              = 0x4,
        .cap_mmc_highspeed      = true,
        .cap_sd_highspeed       = true,
        .cd_gpios               = {0x16, 0x40, 0x1},
        .dma_names              = {"rx", "tx"},
        .dmas                   = {0x14, 0x10, 0x0, 0x14, 0x11, 0x0},
        .max_frequency          = 0x2faf080,
        .reg                    = {0x40000, 0x1000},
};
U_BOOT_DEVICE(mmc_at_40000) = {
        .name           = "ti_da830_mmc",
        .platdata       = &dtv_mmc_at_40000,
        .platdata_size  = sizeof(dtv_mmc_at_40000),
};

I need the base address of the MMC device (dtd_ti_da830_mmc dtv_mmc_at_40000)
reg = 0x40000 to be translated with the simple_bus ranges above. How do I do
this without a device tree as there is no parent-child relationship defined
between the structures? Or at least that is my understanding.

Looks like I will have to drop OF_PLATDATA and use hardcoded U_BOOT_DEVICE()
declarations for this.

Thanks,
Faiz
Tom Rini May 14, 2020, 2:59 p.m. | #12
On Thu, May 14, 2020 at 01:49:37PM +0530, Faiz Abbas wrote:
> Simon,
> 
> On 05/05/20 12:20 pm, Faiz Abbas wrote:
> > Hi,
> > 
> > On 04/05/20 6:44 pm, Simon Glass wrote:
> >> Hi Bart,
> >>
> >> On Mon, 4 May 2020 at 01:10, Bartosz Golaszewski <brgl at bgdev.pl> wrote:
> >>>
> >>> pt., 1 maj 2020 o 20:32 Tom Rini <trini at konsulko.com> napisa?(a):
> >>>>
> >>>> On Thu, Apr 30, 2020 at 01:43:30PM +0200, Bartosz Golaszewski wrote:
> >>>>> wt., 28 kwi 2020 o 09:01 Faiz Abbas <faiz_abbas at ti.com> napisa?(a):
> >>>>>>
> >>>>>> +Bartosz
> >>>>>>
> >>>>>> On 28/04/20 9:47 am, Lokesh Vutla wrote:
> >>>>>>> +Faiz,
> >>>>>>>
> >>>>>>> On 28/04/20 12:29 AM, Tom Rini wrote:
> >>>>>>>> On Mon, Apr 27, 2020 at 05:33:41AM +0000, Peng Fan wrote:
> >>>>>>>>>> Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
> >>>>>>>>>>
> >>>>>>>>>> At present this driver is enabled in SPL on omapl138_lcdk, which uses
> >>>>>>>>>> of-platdata. The driver needs to be ported to use of-platdata properly.
> >>>>>>>>>> For now, avoid a build error by returning an error.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
> >>>>>>>
> >>>>>>> Does this break the boot on omap l138?
> >>>>>>>
> >>>>>>
> >>>>>> I don't have a board at hand to test this out. Bartosz can you help test this with
> >>>>>> omapl138?
> >>>>>>
> >>>>>> Thanks,
> >>>>>> Faiz
> >>>>>
> >>>>> Hi Faiz,
> >>>>>
> >>>>> I can confirm - this *does* break the mmc boot on da850-lcdk.
> >>>>
> >>>> So who is going to fix the driver to unblock Simon's series?
> >>>>
> >>>
> >>> Is this something that will take a lot of work? What exactly needs
> >>> doing? I'm not sure what "use of-platdata properly" means.
> >>
> >> This board is defining CONFIG_SPL_OF_PLATDATA which means that device
> >> tree is not available in SPL. Instead you need to use a C structure
> >> created by dtoc. It basically involves creating that struct and
> >> getting the data from that instead of calling the DT functions. I
> >> expect it will take 2-4 hours to figure out, code and test.
> >>
> >> See of-plat.rst for full documentation. There are quite a few examples for mmc:
> >>
> >> grep PLATDATA drivers/mmc/*.c
> >> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/ftsdc010_mci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/mxsmmc.c: debug("OF_PLATDATA: regs: 0x%p bw: %d clkid: %d
> >> non_removable: %d\n",
> >> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/rockchip_dw_mmc.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/rockchip_sdhci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>
> 
> In all the examples above, platdata reg filed is directly being used for
> to assign a register base address but looking at davinci platdata that is generated,
> spl/dts/dt-platdata.c:
> 
> static const struct dtd_simple_bus dtv_soc_at_1c00000 = {
>         .model                  = "da850",
>         .ranges                 = {0x0, 0x1c00000, 0x400000},
> };
> U_BOOT_DEVICE(soc_at_1c00000) = {
>         .name           = "simple_bus",
>         .platdata       = &dtv_soc_at_1c00000,
>         .platdata_size  = sizeof(dtv_soc_at_1c00000),
> };
> 
> static const struct dtd_ti_da830_uart dtv_serial_at_10d000 = {
>         .power_domains          = {0xa, 0xd},
>         .reg                    = {0x10d000, 0x100},
>         .reg_io_width           = 0x4,
>         .reg_shift              = 0x2,
> };
> U_BOOT_DEVICE(serial_at_10d000) = {
>         .name           = "ti_da830_uart",
>         .platdata       = &dtv_serial_at_10d000,
>         .platdata_size  = sizeof(dtv_serial_at_10d000),
> };
> 
> static const struct dtd_ti_da830_mmc dtv_mmc_at_40000 = {
>         .bus_width              = 0x4,
>         .cap_mmc_highspeed      = true,
>         .cap_sd_highspeed       = true,
>         .cd_gpios               = {0x16, 0x40, 0x1},
>         .dma_names              = {"rx", "tx"},
>         .dmas                   = {0x14, 0x10, 0x0, 0x14, 0x11, 0x0},
>         .max_frequency          = 0x2faf080,
>         .reg                    = {0x40000, 0x1000},
> };
> U_BOOT_DEVICE(mmc_at_40000) = {
>         .name           = "ti_da830_mmc",
>         .platdata       = &dtv_mmc_at_40000,
>         .platdata_size  = sizeof(dtv_mmc_at_40000),
> };
> 
> I need the base address of the MMC device (dtd_ti_da830_mmc dtv_mmc_at_40000)
> reg = 0x40000 to be translated with the simple_bus ranges above. How do I do
> this without a device tree as there is no parent-child relationship defined
> between the structures? Or at least that is my understanding.
> 
> Looks like I will have to drop OF_PLATDATA and use hardcoded U_BOOT_DEVICE()
> declarations for this.

I'm sure the TRM for those chips is readily available in public even,
you should be able to work it out from there, yes?
Faiz Abbas May 14, 2020, 3:25 p.m. | #13
Hi Tom,

On 14/05/20 8:29 pm, Tom Rini wrote:
> On Thu, May 14, 2020 at 01:49:37PM +0530, Faiz Abbas wrote:
>> Simon,
>>
>> On 05/05/20 12:20 pm, Faiz Abbas wrote:
>>> Hi,
>>>
>>> On 04/05/20 6:44 pm, Simon Glass wrote:
>>>> Hi Bart,
>>>>
>>>> On Mon, 4 May 2020 at 01:10, Bartosz Golaszewski <brgl at bgdev.pl> wrote:
>>>>>
>>>>> pt., 1 maj 2020 o 20:32 Tom Rini <trini at konsulko.com> napisa?(a):
>>>>>>
>>>>>> On Thu, Apr 30, 2020 at 01:43:30PM +0200, Bartosz Golaszewski wrote:
>>>>>>> wt., 28 kwi 2020 o 09:01 Faiz Abbas <faiz_abbas at ti.com> napisa?(a):
>>>>>>>>
>>>>>>>> +Bartosz
>>>>>>>>
>>>>>>>> On 28/04/20 9:47 am, Lokesh Vutla wrote:
>>>>>>>>> +Faiz,
>>>>>>>>>
>>>>>>>>> On 28/04/20 12:29 AM, Tom Rini wrote:
>>>>>>>>>> On Mon, Apr 27, 2020 at 05:33:41AM +0000, Peng Fan wrote:
>>>>>>>>>>>> Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
>>>>>>>>>>>>
...
>>>>>>>
>>>>>>> I can confirm - this *does* break the mmc boot on da850-lcdk.
>>>>>>
>>>>>> So who is going to fix the driver to unblock Simon's series?
>>>>>>
>>>>>
>>>>> Is this something that will take a lot of work? What exactly needs
>>>>> doing? I'm not sure what "use of-platdata properly" means.
>>>>
>>>> This board is defining CONFIG_SPL_OF_PLATDATA which means that device
>>>> tree is not available in SPL. Instead you need to use a C structure
>>>> created by dtoc. It basically involves creating that struct and
>>>> getting the data from that instead of calling the DT functions. I
>>>> expect it will take 2-4 hours to figure out, code and test.
>>>>
>>>> See of-plat.rst for full documentation. There are quite a few examples for mmc:
>>>>
>>>> grep PLATDATA drivers/mmc/*.c
>>>> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/ftsdc010_mci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/mxsmmc.c: debug("OF_PLATDATA: regs: 0x%p bw: %d clkid: %d
>>>> non_removable: %d\n",
>>>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
>>>> !CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
>>>> !CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
>>>> !CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
>>>> !CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
>>>> !CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/rockchip_dw_mmc.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
>>>> drivers/mmc/rockchip_sdhci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
>>>>
>>
>> In all the examples above, platdata reg filed is directly being used for
>> to assign a register base address but looking at davinci platdata that is generated,
>> spl/dts/dt-platdata.c:
>>
>> static const struct dtd_simple_bus dtv_soc_at_1c00000 = {
>>         .model                  = "da850",
>>         .ranges                 = {0x0, 0x1c00000, 0x400000},
>> };
>> U_BOOT_DEVICE(soc_at_1c00000) = {
>>         .name           = "simple_bus",
>>         .platdata       = &dtv_soc_at_1c00000,
>>         .platdata_size  = sizeof(dtv_soc_at_1c00000),
>> };
>>
>> static const struct dtd_ti_da830_uart dtv_serial_at_10d000 = {
>>         .power_domains          = {0xa, 0xd},
>>         .reg                    = {0x10d000, 0x100},
>>         .reg_io_width           = 0x4,
>>         .reg_shift              = 0x2,
>> };
>> U_BOOT_DEVICE(serial_at_10d000) = {
>>         .name           = "ti_da830_uart",
>>         .platdata       = &dtv_serial_at_10d000,
>>         .platdata_size  = sizeof(dtv_serial_at_10d000),
>> };
>>
>> static const struct dtd_ti_da830_mmc dtv_mmc_at_40000 = {
>>         .bus_width              = 0x4,
>>         .cap_mmc_highspeed      = true,
>>         .cap_sd_highspeed       = true,
>>         .cd_gpios               = {0x16, 0x40, 0x1},
>>         .dma_names              = {"rx", "tx"},
>>         .dmas                   = {0x14, 0x10, 0x0, 0x14, 0x11, 0x0},
>>         .max_frequency          = 0x2faf080,
>>         .reg                    = {0x40000, 0x1000},
>> };
>> U_BOOT_DEVICE(mmc_at_40000) = {
>>         .name           = "ti_da830_mmc",
>>         .platdata       = &dtv_mmc_at_40000,
>>         .platdata_size  = sizeof(dtv_mmc_at_40000),
>> };
>>
>> I need the base address of the MMC device (dtd_ti_da830_mmc dtv_mmc_at_40000)
>> reg = 0x40000 to be translated with the simple_bus ranges above. How do I do
>> this without a device tree as there is no parent-child relationship defined
>> between the structures? Or at least that is my understanding.
>>
>> Looks like I will have to drop OF_PLATDATA and use hardcoded U_BOOT_DEVICE()
>> declarations for this.
> 
> I'm sure the TRM for those chips is readily available in public even,
> you should be able to work it out from there, yes?
> 

The problem is not getting the offset. We already know it from the device tree. The
issue is that of-platdata doesn't support address translation (yet?). Is there a
way to do this cleanly using the generated device structures of platdata? Otherwise
as I said I will need to disable OF_PLATDATA and declare static C structures in
the board file.

Thanks,
Faiz
Tom Rini May 14, 2020, 3:45 p.m. | #14
On Thu, May 14, 2020 at 08:55:01PM +0530, Faiz Abbas wrote:
> Hi Tom,
> 
> On 14/05/20 8:29 pm, Tom Rini wrote:
> > On Thu, May 14, 2020 at 01:49:37PM +0530, Faiz Abbas wrote:
> >> Simon,
> >>
> >> On 05/05/20 12:20 pm, Faiz Abbas wrote:
> >>> Hi,
> >>>
> >>> On 04/05/20 6:44 pm, Simon Glass wrote:
> >>>> Hi Bart,
> >>>>
> >>>> On Mon, 4 May 2020 at 01:10, Bartosz Golaszewski <brgl at bgdev.pl> wrote:
> >>>>>
> >>>>> pt., 1 maj 2020 o 20:32 Tom Rini <trini at konsulko.com> napisa?(a):
> >>>>>>
> >>>>>> On Thu, Apr 30, 2020 at 01:43:30PM +0200, Bartosz Golaszewski wrote:
> >>>>>>> wt., 28 kwi 2020 o 09:01 Faiz Abbas <faiz_abbas at ti.com> napisa?(a):
> >>>>>>>>
> >>>>>>>> +Bartosz
> >>>>>>>>
> >>>>>>>> On 28/04/20 9:47 am, Lokesh Vutla wrote:
> >>>>>>>>> +Faiz,
> >>>>>>>>>
> >>>>>>>>> On 28/04/20 12:29 AM, Tom Rini wrote:
> >>>>>>>>>> On Mon, Apr 27, 2020 at 05:33:41AM +0000, Peng Fan wrote:
> >>>>>>>>>>>> Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
> >>>>>>>>>>>>
> ...
> >>>>>>>
> >>>>>>> I can confirm - this *does* break the mmc boot on da850-lcdk.
> >>>>>>
> >>>>>> So who is going to fix the driver to unblock Simon's series?
> >>>>>>
> >>>>>
> >>>>> Is this something that will take a lot of work? What exactly needs
> >>>>> doing? I'm not sure what "use of-platdata properly" means.
> >>>>
> >>>> This board is defining CONFIG_SPL_OF_PLATDATA which means that device
> >>>> tree is not available in SPL. Instead you need to use a C structure
> >>>> created by dtoc. It basically involves creating that struct and
> >>>> getting the data from that instead of calling the DT functions. I
> >>>> expect it will take 2-4 hours to figure out, code and test.
> >>>>
> >>>> See of-plat.rst for full documentation. There are quite a few examples for mmc:
> >>>>
> >>>> grep PLATDATA drivers/mmc/*.c
> >>>> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/ftsdc010_mci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/mxsmmc.c: debug("OF_PLATDATA: regs: 0x%p bw: %d clkid: %d
> >>>> non_removable: %d\n",
> >>>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >>>> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >>>> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >>>> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >>>> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >>>> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/rockchip_dw_mmc.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/rockchip_sdhci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>>
> >>
> >> In all the examples above, platdata reg filed is directly being used for
> >> to assign a register base address but looking at davinci platdata that is generated,
> >> spl/dts/dt-platdata.c:
> >>
> >> static const struct dtd_simple_bus dtv_soc_at_1c00000 = {
> >>         .model                  = "da850",
> >>         .ranges                 = {0x0, 0x1c00000, 0x400000},
> >> };
> >> U_BOOT_DEVICE(soc_at_1c00000) = {
> >>         .name           = "simple_bus",
> >>         .platdata       = &dtv_soc_at_1c00000,
> >>         .platdata_size  = sizeof(dtv_soc_at_1c00000),
> >> };
> >>
> >> static const struct dtd_ti_da830_uart dtv_serial_at_10d000 = {
> >>         .power_domains          = {0xa, 0xd},
> >>         .reg                    = {0x10d000, 0x100},
> >>         .reg_io_width           = 0x4,
> >>         .reg_shift              = 0x2,
> >> };
> >> U_BOOT_DEVICE(serial_at_10d000) = {
> >>         .name           = "ti_da830_uart",
> >>         .platdata       = &dtv_serial_at_10d000,
> >>         .platdata_size  = sizeof(dtv_serial_at_10d000),
> >> };
> >>
> >> static const struct dtd_ti_da830_mmc dtv_mmc_at_40000 = {
> >>         .bus_width              = 0x4,
> >>         .cap_mmc_highspeed      = true,
> >>         .cap_sd_highspeed       = true,
> >>         .cd_gpios               = {0x16, 0x40, 0x1},
> >>         .dma_names              = {"rx", "tx"},
> >>         .dmas                   = {0x14, 0x10, 0x0, 0x14, 0x11, 0x0},
> >>         .max_frequency          = 0x2faf080,
> >>         .reg                    = {0x40000, 0x1000},
> >> };
> >> U_BOOT_DEVICE(mmc_at_40000) = {
> >>         .name           = "ti_da830_mmc",
> >>         .platdata       = &dtv_mmc_at_40000,
> >>         .platdata_size  = sizeof(dtv_mmc_at_40000),
> >> };
> >>
> >> I need the base address of the MMC device (dtd_ti_da830_mmc dtv_mmc_at_40000)
> >> reg = 0x40000 to be translated with the simple_bus ranges above. How do I do
> >> this without a device tree as there is no parent-child relationship defined
> >> between the structures? Or at least that is my understanding.
> >>
> >> Looks like I will have to drop OF_PLATDATA and use hardcoded U_BOOT_DEVICE()
> >> declarations for this.
> > 
> > I'm sure the TRM for those chips is readily available in public even,
> > you should be able to work it out from there, yes?
> > 
> 
> The problem is not getting the offset. We already know it from the device tree. The
> issue is that of-platdata doesn't support address translation (yet?). Is there a
> way to do this cleanly using the generated device structures of platdata? Otherwise
> as I said I will need to disable OF_PLATDATA and declare static C structures in
> the board file.

Ah, sorry I misunderstood the problem.  I suspect U_BOOT_DEVICES is
probably the best path forward right now.
Simon Glass May 14, 2020, 4:01 p.m. | #15
Hi Faiz,

+Walter Lozano

On Thu, 14 May 2020 at 02:43, Faiz Abbas <faiz_abbas at ti.com> wrote:
>
> Simon,
>
> On 05/05/20 12:20 pm, Faiz Abbas wrote:
> > Hi,
> >
> > On 04/05/20 6:44 pm, Simon Glass wrote:
> >> Hi Bart,
> >>
> >> On Mon, 4 May 2020 at 01:10, Bartosz Golaszewski <brgl at bgdev.pl> wrote:
> >>>
> >>> pt., 1 maj 2020 o 20:32 Tom Rini <trini at konsulko.com> napisa?(a):
> >>>>
> >>>> On Thu, Apr 30, 2020 at 01:43:30PM +0200, Bartosz Golaszewski wrote:
> >>>>> wt., 28 kwi 2020 o 09:01 Faiz Abbas <faiz_abbas at ti.com> napisa?(a):
> >>>>>>
> >>>>>> +Bartosz
> >>>>>>
> >>>>>> On 28/04/20 9:47 am, Lokesh Vutla wrote:
> >>>>>>> +Faiz,
> >>>>>>>
> >>>>>>> On 28/04/20 12:29 AM, Tom Rini wrote:
> >>>>>>>> On Mon, Apr 27, 2020 at 05:33:41AM +0000, Peng Fan wrote:
> >>>>>>>>>> Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
> >>>>>>>>>>
> >>>>>>>>>> At present this driver is enabled in SPL on omapl138_lcdk, which uses
> >>>>>>>>>> of-platdata. The driver needs to be ported to use of-platdata properly.
> >>>>>>>>>> For now, avoid a build error by returning an error.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
> >>>>>>>
> >>>>>>> Does this break the boot on omap l138?
> >>>>>>>
> >>>>>>
> >>>>>> I don't have a board at hand to test this out. Bartosz can you help test this with
> >>>>>> omapl138?
> >>>>>>
> >>>>>> Thanks,
> >>>>>> Faiz
> >>>>>
> >>>>> Hi Faiz,
> >>>>>
> >>>>> I can confirm - this *does* break the mmc boot on da850-lcdk.
> >>>>
> >>>> So who is going to fix the driver to unblock Simon's series?
> >>>>
> >>>
> >>> Is this something that will take a lot of work? What exactly needs
> >>> doing? I'm not sure what "use of-platdata properly" means.
> >>
> >> This board is defining CONFIG_SPL_OF_PLATDATA which means that device
> >> tree is not available in SPL. Instead you need to use a C structure
> >> created by dtoc. It basically involves creating that struct and
> >> getting the data from that instead of calling the DT functions. I
> >> expect it will take 2-4 hours to figure out, code and test.
> >>
> >> See of-plat.rst for full documentation. There are quite a few examples for mmc:
> >>
> >> grep PLATDATA drivers/mmc/*.c
> >> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/ftsdc010_mci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/mxsmmc.c: debug("OF_PLATDATA: regs: 0x%p bw: %d clkid: %d
> >> non_removable: %d\n",
> >> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/rockchip_dw_mmc.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >> drivers/mmc/rockchip_sdhci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>
>
> In all the examples above, platdata reg filed is directly being used for
> to assign a register base address but looking at davinci platdata that is generated,
> spl/dts/dt-platdata.c:
>
> static const struct dtd_simple_bus dtv_soc_at_1c00000 = {
>         .model                  = "da850",
>         .ranges                 = {0x0, 0x1c00000, 0x400000},
> };
> U_BOOT_DEVICE(soc_at_1c00000) = {
>         .name           = "simple_bus",
>         .platdata       = &dtv_soc_at_1c00000,
>         .platdata_size  = sizeof(dtv_soc_at_1c00000),
> };
>
> static const struct dtd_ti_da830_uart dtv_serial_at_10d000 = {
>         .power_domains          = {0xa, 0xd},
>         .reg                    = {0x10d000, 0x100},
>         .reg_io_width           = 0x4,
>         .reg_shift              = 0x2,
> };
> U_BOOT_DEVICE(serial_at_10d000) = {
>         .name           = "ti_da830_uart",
>         .platdata       = &dtv_serial_at_10d000,
>         .platdata_size  = sizeof(dtv_serial_at_10d000),
> };
>
> static const struct dtd_ti_da830_mmc dtv_mmc_at_40000 = {
>         .bus_width              = 0x4,
>         .cap_mmc_highspeed      = true,
>         .cap_sd_highspeed       = true,
>         .cd_gpios               = {0x16, 0x40, 0x1},
>         .dma_names              = {"rx", "tx"},
>         .dmas                   = {0x14, 0x10, 0x0, 0x14, 0x11, 0x0},
>         .max_frequency          = 0x2faf080,
>         .reg                    = {0x40000, 0x1000},
> };
> U_BOOT_DEVICE(mmc_at_40000) = {
>         .name           = "ti_da830_mmc",
>         .platdata       = &dtv_mmc_at_40000,
>         .platdata_size  = sizeof(dtv_mmc_at_40000),
> };
>
> I need the base address of the MMC device (dtd_ti_da830_mmc dtv_mmc_at_40000)
> reg = 0x40000 to be translated with the simple_bus ranges above. How do I do
> this without a device tree as there is no parent-child relationship defined
> between the structures? Or at least that is my understanding.
>
> Looks like I will have to drop OF_PLATDATA and use hardcoded U_BOOT_DEVICE()
> declarations for this.

Four options I can think of:

1. Add support for translating to dtoc
2. Add support for parents to dtoc
3. Find the soc device (the one with the ranges) and write a function
to return the base address given the dtplat for that device.
4. Hard-code the address behind if CONFIG_IS_ENABLED(OF_PLATDATA)) for
your board

Regards,
Simon
Simon Glass May 14, 2020, 5:23 p.m. | #16
Hi Faiz,

On Thu, 14 May 2020 at 10:40, Faiz Abbas <faiz_abbas at ti.com> wrote:
>
> Hi Tom,
>
> On 14/05/20 8:29 pm, Tom Rini wrote:
> > On Thu, May 14, 2020 at 01:49:37PM +0530, Faiz Abbas wrote:
> >> Simon,
> >>
> >> On 05/05/20 12:20 pm, Faiz Abbas wrote:
> >>> Hi,
> >>>
> >>> On 04/05/20 6:44 pm, Simon Glass wrote:
> >>>> Hi Bart,
> >>>>
> >>>> On Mon, 4 May 2020 at 01:10, Bartosz Golaszewski <brgl at bgdev.pl> wrote:
> >>>>>
> >>>>> pt., 1 maj 2020 o 20:32 Tom Rini <trini at konsulko.com> napisa?(a):
> >>>>>>
> >>>>>> On Thu, Apr 30, 2020 at 01:43:30PM +0200, Bartosz Golaszewski wrote:
> >>>>>>> wt., 28 kwi 2020 o 09:01 Faiz Abbas <faiz_abbas at ti.com> napisa?(a):
> >>>>>>>>
> >>>>>>>> +Bartosz
> >>>>>>>>
> >>>>>>>> On 28/04/20 9:47 am, Lokesh Vutla wrote:
> >>>>>>>>> +Faiz,
> >>>>>>>>>
> >>>>>>>>> On 28/04/20 12:29 AM, Tom Rini wrote:
> >>>>>>>>>> On Mon, Apr 27, 2020 at 05:33:41AM +0000, Peng Fan wrote:
> >>>>>>>>>>>> Subject: [PATCH v5 1/4] omap: mmc: Avoid using libfdt with of-platdata
> >>>>>>>>>>>>
> ...
> >>>>>>>
> >>>>>>> I can confirm - this *does* break the mmc boot on da850-lcdk.
> >>>>>>
> >>>>>> So who is going to fix the driver to unblock Simon's series?
> >>>>>>
> >>>>>
> >>>>> Is this something that will take a lot of work? What exactly needs
> >>>>> doing? I'm not sure what "use of-platdata properly" means.
> >>>>
> >>>> This board is defining CONFIG_SPL_OF_PLATDATA which means that device
> >>>> tree is not available in SPL. Instead you need to use a C structure
> >>>> created by dtoc. It basically involves creating that struct and
> >>>> getting the data from that instead of calling the DT functions. I
> >>>> expect it will take 2-4 hours to figure out, code and test.
> >>>>
> >>>> See of-plat.rst for full documentation. There are quite a few examples for mmc:
> >>>>
> >>>> grep PLATDATA drivers/mmc/*.c
> >>>> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/ftsdc010_mci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/ftsdc010_mci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/mxsmmc.c: debug("OF_PLATDATA: regs: 0x%p bw: %d clkid: %d
> >>>> non_removable: %d\n",
> >>>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >>>> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/mxsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >>>> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >>>> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >>>> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/omap_hsmmc.c:#if CONFIG_IS_ENABLED(OF_CONTROL) &&
> >>>> !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/rockchip_dw_mmc.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/rockchip_dw_mmc.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/rockchip_sdhci.c:#if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>> drivers/mmc/rockchip_sdhci.c:#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>>
> >>
> >> In all the examples above, platdata reg filed is directly being used for
> >> to assign a register base address but looking at davinci platdata that is generated,
> >> spl/dts/dt-platdata.c:
> >>
> >> static const struct dtd_simple_bus dtv_soc_at_1c00000 = {
> >>         .model                  = "da850",
> >>         .ranges                 = {0x0, 0x1c00000, 0x400000},
> >> };
> >> U_BOOT_DEVICE(soc_at_1c00000) = {
> >>         .name           = "simple_bus",
> >>         .platdata       = &dtv_soc_at_1c00000,
> >>         .platdata_size  = sizeof(dtv_soc_at_1c00000),
> >> };
> >>
> >> static const struct dtd_ti_da830_uart dtv_serial_at_10d000 = {
> >>         .power_domains          = {0xa, 0xd},
> >>         .reg                    = {0x10d000, 0x100},
> >>         .reg_io_width           = 0x4,
> >>         .reg_shift              = 0x2,
> >> };
> >> U_BOOT_DEVICE(serial_at_10d000) = {
> >>         .name           = "ti_da830_uart",
> >>         .platdata       = &dtv_serial_at_10d000,
> >>         .platdata_size  = sizeof(dtv_serial_at_10d000),
> >> };
> >>
> >> static const struct dtd_ti_da830_mmc dtv_mmc_at_40000 = {
> >>         .bus_width              = 0x4,
> >>         .cap_mmc_highspeed      = true,
> >>         .cap_sd_highspeed       = true,
> >>         .cd_gpios               = {0x16, 0x40, 0x1},
> >>         .dma_names              = {"rx", "tx"},
> >>         .dmas                   = {0x14, 0x10, 0x0, 0x14, 0x11, 0x0},
> >>         .max_frequency          = 0x2faf080,
> >>         .reg                    = {0x40000, 0x1000},
> >> };
> >> U_BOOT_DEVICE(mmc_at_40000) = {
> >>         .name           = "ti_da830_mmc",
> >>         .platdata       = &dtv_mmc_at_40000,
> >>         .platdata_size  = sizeof(dtv_mmc_at_40000),
> >> };
> >>
> >> I need the base address of the MMC device (dtd_ti_da830_mmc dtv_mmc_at_40000)
> >> reg = 0x40000 to be translated with the simple_bus ranges above. How do I do
> >> this without a device tree as there is no parent-child relationship defined
> >> between the structures? Or at least that is my understanding.
> >>
> >> Looks like I will have to drop OF_PLATDATA and use hardcoded U_BOOT_DEVICE()
> >> declarations for this.
> >
> > I'm sure the TRM for those chips is readily available in public even,
> > you should be able to work it out from there, yes?
> >
>
> The problem is not getting the offset. We already know it from the device tree. The
> issue is that of-platdata doesn't support address translation (yet?). Is there a
> way to do this cleanly using the generated device structures of platdata? Otherwise
> as I said I will need to disable OF_PLATDATA and declare static C structures in
> the board file.

+Walter Lozano again

Four options I can think of:

1. Add support for translating to dtoc
2. Add support for parents to dtoc
3. Find the soc device (the one with the ranges) and write a function
to return the base address given the dtplat for that device.
4. Hard-code the address behind if CONFIG_IS_ENABLED(OF_PLATDATA)) for
your board

Regards,
Simon

Patch

diff --git a/drivers/mmc/davinci_mmc.c b/drivers/mmc/davinci_mmc.c
index ef5cd4e723..44903354ab 100644
--- a/drivers/mmc/davinci_mmc.c
+++ b/drivers/mmc/davinci_mmc.c
@@ -498,6 +498,12 @@  static int davinci_mmc_probe(struct udevice *dev)
 	cfg->b_max = DAVINCI_MAX_BLOCKS;
 	cfg->name = "da830-mmc";
 
+	/* FIXME: Cannot read from device tree with of-platdata */
+	if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
+		printf("Please fix this driver to use of-platdata");
+		return -ENOSYS;
+	}
+
 	priv->reg_base = (struct davinci_mmc_regs *)dev_read_addr(dev);
 	priv->input_clk = clk_get(DAVINCI_MMCSD_CLKID);