[RFC] dm: uclass: add functions to get device by platdata
Simon Glass
sjg at chromium.org
Fri May 8 03:36:57 CEST 2020
Hi Walter,
On Wed, 6 May 2020 at 06:57, Walter Lozano <walter.lozano at collabora.com> wrote:
>
> Hi Simon,
>
> On 23/4/20 00:38, Walter Lozano wrote:
> > Hi Simon,
> >
> > On 21/4/20 14:36, Simon Glass wrote:
> >> Hi Walter,
> >>
> >> On Fri, 17 Apr 2020 at 15:18, Walter Lozano
> >> <walter.lozano at collabora.com> wrote:
> >>> Hi Simon,
> >>>
> >>> On 9/4/20 16:36, Simon Glass wrote:
> >>>> HI Walter,
> >>>>
> >>>> On Thu, 9 Apr 2020 at 12:57, Walter Lozano
> >>>> <walter.lozano at collabora.com> wrote:
> >>>>> Hi Simon,
> >>>>>
> >>>>> On 8/4/20 00:14, Simon Glass wrote:
> >>>>>> Hi Walter,
> >>>>>>
> >>>>>> On Tue, 7 Apr 2020 at 14:05, Walter
> >>>>>> Lozano<walter.lozano at collabora.com> wrote:
> >>>>>>> Hi Simon,
> >>>>>>>
> >>>>>>> On 6/4/20 00:43, Simon Glass wrote:
> >>>>>>>> Hi Walter,
> >>>>>>>>
> >>>>>>>> On Mon, 9 Mar 2020 at 12:27, Walter
> >>>>>>>> Lozano<walter.lozano at collabora.com> wrote:
> >>>>>>>>> Hi Simon
> >>>>>>>>>
> >>>>>>>>> On 6/3/20 17:32, Simon Glass wrote:
> >>>>>>>>>> Hi Walter,
> >>>>>>>>>>
> >>>>>>>>>> On Fri, 6 Mar 2020 at 09:10, Walter
> >>>>>>>>>> Lozano<walter.lozano at collabora.com> wrote:
> >>>>>>>>>>> Hi Simon,
> >>>>>>>>>>>
> >>>>>>>>>>> Thanks again for taking the time to check my comments.
> >>>>>>>>>>>
> >>>>>>>>>>> On 6/3/20 10:17, Simon Glass wrote:
> >>>>>>>>>>>> Hi Walter,
> >>>>>>>>>>>>
> >>>>>>>>>>>> On Thu, 5 Mar 2020 at 06:54, Walter
> >>>>>>>>>>>> Lozano<walter.lozano at collabora.com> wrote:
> >>>>>>>>>>>>> Hi Simon,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Thanks for taking the time to check for my comments
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> On 4/3/20 20:11, Simon Glass wrote:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>> Hi Walter,
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> On Wed, 4 Mar 2020 at 12:40, Walter
> >>>>>>>>>>>>>> Lozano<walter.lozano at collabora.com> wrote:
> >>>>>>>>>>>>>>> When OF_PLATDATA is enabled DT information is parsed and
> >>>>>>>>>>>>>>> platdata
> >>>>>>>>>>>>>>> structures are populated. In this context the links
> >>>>>>>>>>>>>>> between DT nodes are
> >>>>>>>>>>>>>>> represented as pointers to platdata structures, and
> >>>>>>>>>>>>>>> there is no clear way
> >>>>>>>>>>>>>>> to access to the device which owns the structure.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> This patch implements a set of functions:
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> - device_find_by_platdata
> >>>>>>>>>>>>>>> - uclass_find_device_by_platdata
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> to access to the device.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Signed-off-by: Walter Lozano<walter.lozano at collabora.com>
> >>>>>>>>>>>>>>> ---
> >>>>>>>>>>>>>>> drivers/core/device.c | 19
> >>>>>>>>>>>>>>> +++++++++++++++++++
> >>>>>>>>>>>>>>> drivers/core/uclass.c | 34
> >>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++++
> >>>>>>>>>>>>>>> include/dm/device.h | 2 ++
> >>>>>>>>>>>>>>> include/dm/uclass-internal.h | 3 +++
> >>>>>>>>>>>>>>> include/dm/uclass.h | 2 ++
> >>>>>>>>>>>>>>> 5 files changed, 60 insertions(+)
> >>>>>>>>>>>>>> This is interesting. Could you also add the motivation
> >>>>>>>>>>>>>> for this? It's
> >>>>>>>>>>>>>> not clear to me who would call this function.
> >>>>>>>>>>>>> I have been reviewing the OF_PLATDATA support as an R&D
> >>>>>>>>>>>>> project, in this context, in order to have
> >>>>>>>>>>>>> a better understanding on the possibilities and
> >>>>>>>>>>>>> limitations I decided to add its support to iMX6,
> >>>>>>>>>>>>> more particularly to the MMC drivers. The link issue
> >>>>>>>>>>>>> arises when I tried to setup the GPIO for
> >>>>>>>>>>>>> Card Detection, which is trivial when DT is available.
> >>>>>>>>>>>>> However, when OF_PLATDATA is enabled
> >>>>>>>>>>>>> this seems, at least for me, not straightforward.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> In order to overcome this limitation I think that having a
> >>>>>>>>>>>>> set of functions to find/get devices
> >>>>>>>>>>>>> based on platdata could be useful. Of course, there might
> >>>>>>>>>>>>> be a better approach/idea, so that is
> >>>>>>>>>>>>> the motivation for this RFC.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> An example of the usage could be
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> #if CONFIG_IS_ENABLED(DM_GPIO)
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> struct udevice *gpiodev;
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> ret =
> >>>>>>>>>>>>> uclass_get_device_by_platdata(UCLASS_GPIO, (void
> >>>>>>>>>>>>> *)dtplat->cd_gpios->node, &gpiodev);
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> if (ret)
> >>>>>>>>>>>>> return ret;
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> ret = gpio_dev_request_index(gpiodev,
> >>>>>>>>>>>>> gpiodev->name, "cd-gpios",
> >>>>>>>>>>>>> dtplat->cd_gpios->arg[0], GPIOD_IS_IN,
> >>>>>>>>>>>>> dtplat->cd_gpios->arg[1], &priv->cd_gpio);
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> if (ret)
> >>>>>>>>>>>>> return ret;
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> #endif
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> This is part of my current work, a series of patches to
> >>>>>>>>>>>>> add OF_PLATDATA support as explained.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Does this make sense to you?
> >>>>>>>>>>>> Not yet :-)
> >>>>>>>>>>>>
> >>>>>>>>>>>> What is the context of this call? Typically dtplat is only
> >>>>>>>>>>>> available
> >>>>>>>>>>>> in the driver that includes it.
> >>>>>>>>>>> Sorry for not being clear enough. I'm working in a patchset
> >>>>>>>>>>> that needs
> >>>>>>>>>>> some clean up, that is the reason I didn't send it yet. I'll
> >>>>>>>>>>> try to
> >>>>>>>>>>> clarify, but if you think it could be useful to share it,
> >>>>>>>>>>> please let me
> >>>>>>>>>>> know.
> >>>>>>>>>>>
> >>>>>>>>>>>> What driver is the above code in? Is it for MMC that needs
> >>>>>>>>>>>> a GPIO to
> >>>>>>>>>>>> function? I'll assume it is for now.
> >>>>>>>>>>> The driver on which I'm working in is
> >>>>>>>>>>> drivers/mmc/fsl_esdhc_imx.c, I'm
> >>>>>>>>>>> adding support for OF_PLATDATA to it, and in this sense
> >>>>>>>>>>> trying to get
> >>>>>>>>>>> the GPIOs used for CD to be requested.
> >>>>>>>>>>>
> >>>>>>>>>>>> Then the weird thing is that we are accessing the dtplat of
> >>>>>>>>>>>> another
> >>>>>>>>>>>> device. It's a clever technique but I wonder if we can find
> >>>>>>>>>>>> another
> >>>>>>>>>>>> way.
> >>>>>>>>>>>>
> >>>>>>>>>>>> If you see drivers/mmc/rockchip_sdhci.c it has:
> >>>>>>>>>>>>
> >>>>>>>>>>>> ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &clk);
> >>>>>>>>>>>>
> >>>>>>>>>>>> So I wonder if we need gpio_dev_request_by_platdata()?
> >>>>>>>>>>> Thanks for pointing to this example, as I saw it before
> >>>>>>>>>>> starting to work
> >>>>>>>>>>> on these functions and had some doubts. I'll use it in the next
> >>>>>>>>>>> paragraph to share my thoughts and the motivation of my work.
> >>>>>>>>>>>
> >>>>>>>>>>> From my understanding, clk_get_by_index_platdata in
> >>>>>>>>>>> this context can
> >>>>>>>>>>> only get a UCLASS_CLK device with id == 0. Is this correct?
> >>>>>>>>>>>
> >>>>>>>>>>> If it is so, this will only allow us to use this function if
> >>>>>>>>>>> we know in
> >>>>>>>>>>> advance that the UCLASS_CLK device has index 0.
> >>>>>>>>>>>
> >>>>>>>>>>> How can we get the correct UCLASS_CLK device in case of
> >>>>>>>>>>> multiple instances?
> >>>>>>>>>> We actually can't support that at present. I think we would
> >>>>>>>>>> need to
> >>>>>>>>>> change the property to be an array, like:
> >>>>>>>>>>
> >>>>>>>>>> clocks = [
> >>>>>>>>>> [&clk1, CLK_ID_SPI],
> >>>>>>>>>> [&clk1, CLK_ID_I2C, 23],
> >>>>>>>>>> ]
> >>>>>>>>>>
> >>>>>>>>>> which would need a fancier dtoc. Perhaps we should start by
> >>>>>>>>>> upstreaming that tool.
> >>>>>>>>> In this case, are you suggesting to replace CLK_ID_SPI and
> >>>>>>>>> CLK_ID_I2C
> >>>>>>>>> with a integer calculated by dtoc based on the clocks entries
> >>>>>>>>> available?
> >>>>>>>>> If I understand correctly, what we need is to get the index
> >>>>>>>>> parameter to
> >>>>>>>>> feed the function uclass_find_device. Is this correct?
> >>>>>>>> No, I was thinking that it would be the same CLK_ID_SPI value
> >>>>>>>> from the binding.
> >>>>>>>>
> >>>>>>>> We currently have (see the 'rock' board):
> >>>>>>>>
> >>>>>>>> struct dtd_rockchip_rk3188_uart {
> >>>>>>>> fdt32_t clock_frequency;
> >>>>>>>> struct phandle_1_arg clocks[2];
> >>>>>>>> fdt32_t interrupts[3];
> >>>>>>>> fdt32_t reg[2];
> >>>>>>>> fdt32_t reg_io_width;
> >>>>>>>> fdt32_t reg_shift;
> >>>>>>>> };
> >>>>>>>>
> >>>>>>>> So the phandle_1_arg is similar to what you want. It could use
> >>>>>>>> phandle_2_arg.
> >>>>>>>>
> >>>>>>>> Since the array has two elements, there is room for two clocks.
> >>>>>>> Now is clear, thanks.
> >>>>>>>
> >>>>>>>>>>> I understand that we need a way to use the link information
> >>>>>>>>>>> present in
> >>>>>>>>>>> platdata. However I could not find a way to get the actual
> >>>>>>>>>>> index of the
> >>>>>>>>>>> UCLASS_CLK device. In this context, I thought that the
> >>>>>>>>>>> simplest but
> >>>>>>>>>>> still valid approach could be to find the right device based
> >>>>>>>>>>> on the
> >>>>>>>>>>> struct platdata pointer it owns.
> >>>>>>>>>> The index should be the first value after the phandle. If you
> >>>>>>>>>> check
> >>>>>>>>>> the output of dtoc you should see it.
> >>>>>>>>>>
> >>>>>>>>>>> So in my understanding, your code could be more generic in
> >>>>>>>>>>> this way
> >>>>>>>>>>>
> >>>>>>>>>>> diff --git a/drivers/clk/clk-uclass.c
> >>>>>>>>>>> b/drivers/clk/clk-uclass.c
> >>>>>>>>>>> index 71878474eb..61041bb3b8 100644
> >>>>>>>>>>> --- a/drivers/clk/clk-uclass.c
> >>>>>>>>>>> +++ b/drivers/clk/clk-uclass.c
> >>>>>>>>>>> @@ -25,14 +25,12 @@ static inline const struct clk_ops
> >>>>>>>>>>> *clk_dev_ops(struct udevice *dev)
> >>>>>>>>>>>
> >>>>>>>>>>> #if CONFIG_IS_ENABLED(OF_CONTROL)
> >>>>>>>>>>> # if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>>>>>>>>> -int clk_get_by_index_platdata(struct udevice *dev, int index,
> >>>>>>>>>>> - struct phandle_1_arg *cells,
> >>>>>>>>>>> struct clk *clk)
> >>>>>>>>>>> +int clk_get_by_platdata(struct udevice *dev, struct
> >>>>>>>>>>> phandle_1_arg *cells,
> >>>>>>>>>>> + struct clk *clk)
> >>>>>>>>>>> {
> >>>>>>>>>>> int ret;
> >>>>>>>>>>>
> >>>>>>>>>>> - if (index != 0)
> >>>>>>>>>>> - return -ENOSYS;
> >>>>>>>>>>> - ret = uclass_get_device(UCLASS_CLK, 0, &clk->dev);
> >>>>>>>>>>> + ret = uclass_get_device_by_platdata(UCLASS_CLK (void
> >>>>>>>>>>> *)cells->node, &clk->dev);
> >>>>>>>>>>> if (ret)
> >>>>>>>>>>> return ret;
> >>>>>>>>>>> clk->id = cells[0].arg[0];
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> I understand there could be a more elegant way, which I
> >>>>>>>>>>> still don't see,
> >>>>>>>>>>> that is the motivation of this RFC.
> >>>>>>>>>>>
> >>>>>>>>>>> What do you think?
> >>>>>>>>>> Yes I see, but I think it is better to enhance dtoc if
> >>>>>>>>>> needed, to give
> >>>>>>>>>> us the info we need.
> >>>>>>>>> I understand. When I first reviewed the work to be done and
> >>>>>>>>> dtoc tool
> >>>>>>>>> source code I asked myself some questions. Let me share my
> >>>>>>>>> thoughts
> >>>>>>>>> using rock_defconfig as reference.
> >>>>>>>>>
> >>>>>>>>> The link information regarding phandles is processed by dtoc
> >>>>>>>>> tool. By
> >>>>>>>>> default the phandle_id is converted to fdt32_t, but in case of
> >>>>>>>>> clocks
> >>>>>>>>> the function
> >>>>>>>>>
> >>>>>>>>> get_phandle_argc(self, prop, node_name)
> >>>>>>>>>
> >>>>>>>>> resolves the link and generates a pointer to the dt_struct of
> >>>>>>>>> the linked
> >>>>>>>>> node.
> >>>>>>>>>
> >>>>>>>>> So without the special treatment for clocks a dt_struct looks
> >>>>>>>>> like
> >>>>>>>>>
> >>>>>>>>> static const struct dtd_rockchip_rk3188_dmc
> >>>>>>>>> dtv_dmc_at_20020000 = {
> >>>>>>>>> .clocks = {0x2, 0x160, 0x2, 0x161},
> >>>>>>>>>
> >>>>>>>>> And with the special treatment the phandle_id gets converted
> >>>>>>>>> to a pointer
> >>>>>>>>>
> >>>>>>>>> static const struct dtd_rockchip_rk3188_dmc
> >>>>>>>>> dtv_dmc_at_20020000 = {
> >>>>>>>>> .clocks = {
> >>>>>>>>> {&dtv_clock_controller_at_20000000, {352}},
> >>>>>>>>> {&dtv_clock_controller_at_20000000, {353}},},
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> This approach was what encouraged me to try to find the device
> >>>>>>>>> which
> >>>>>>>>> owns dtv_clock_controller_at_20000000 pointer or something
> >>>>>>>>> similar.
> >>>>>>>> I think at present it is very simple. E.g. see
> >>>>>>>> clk_get_by_index_platdata() which only supports a 1-arg phandle
> >>>>>>>> and
> >>>>>>>> always uses the first available clock device.
> >>>>>>>>
> >>>>>>>>> However, I was also thinking that other possibility is to keep
> >>>>>>>>> dtoc
> >>>>>>>>> simple and don't process this information at all, leaving the
> >>>>>>>>> phandle_id, and also adding the phandle_id in each node
> >>>>>>>>> dt_struct, in
> >>>>>>>>> order to be able to get/find the device by phandle_id.
> >>>>>>>>>
> >>>>>>>>> I understand that this approach is NOT what you thought it was
> >>>>>>>>> the best
> >>>>>>>>> for some reason I am not aware of.
> >>>>>>>> Well you don't have the device tree with of-platdata, so you
> >>>>>>>> cannot
> >>>>>>>> look up a phandle. I suppose we could add the phandle into the
> >>>>>>>> structures but we would need to know how to find it generically.
> >>>>>>>>
> >>>>>>>>> So in my mind there should be a generic way to get/find a
> >>>>>>>>> device based
> >>>>>>>>> on some information in the dt_struct, valid for clocks, gpios
> >>>>>>>>> and any
> >>>>>>>>> other type of device/node as the phandle_id. In the specific
> >>>>>>>>> case I'm
> >>>>>>>>> working on, the cd-gpios property should allow us to get/find
> >>>>>>>>> the gpio
> >>>>>>>>> device to check for the status of the input gpio.
> >>>>>>>> OK I see.
> >>>>>>>>
> >>>>>>>> DM_GET_DRIVER() is a compile-time way to find a driver. I
> >>>>>>>> wonder if we
> >>>>>>>> could have a compile-time way to find a device?
> >>>>>>>>
> >>>>>>>> It should be possible since each U_BOOT_DEVICE() get s a symbol
> >>>>>>>> and
> >>>>>>>> the symbols are named methodically. So we can actually find the
> >>>>>>>> device
> >>>>>>>> at compile time.
> >>>>>>>>
> >>>>>>>> So how about we have DM_GET_DEVICE(name) in that field. That
> >>>>>>>> way you
> >>>>>>>> have the device pointer available, with no lookup needed.
> >>>>>>> I found this approach very interesting. Let me investigate it
> >>>>>>> and I'll
> >>>>>>> get back to you. I really appreciate this suggestion, I hope to
> >>>>>>> come
> >>>>>>> with something useful.
> >>>>>> Yes me too...
> >>>>>>
> >>>>>> But sadly I don't think it is enough. It points to the driver data,
> >>>>>> the data *used* to create the device, but not the device itself,
> >>>>>> which
> >>>>>> is dynamically allocated with malloc().
> >>>>>>
> >>>>>> The good news is that it is a compile-time check, so there is some
> >>>>>> value in the idea. Good to avoid runtime failure if possible.
> >>>>>>
> >>>>>> One option would be to add a pointer at run-time from the driver
> >>>>>> data
> >>>>>> to the device, for of-platdata. That way we could follow a
> >>>>>> pointer and
> >>>>>> find the device. It would be easy enough to do.
> >>>>> Thank you so much for sharing all these ideas. I hope to make good
> >>>>> use
> >>>>> of these suggestions. I think I'm following your idea, however
> >>>>> this will
> >>>>> be clearer when I start to work on this, hopefully next week, and
> >>>>> probably will come back to you with some silly questions.
> >>>>>
> >>>>> At this point what I see
> >>>>>
> >>>>> - I like the compile-time check, you have showed me that benefits
> >>>>> with
> >>>>> several of your previous patches, thanks for that.
> >>>>>
> >>>>> - If we need to add a pointer from driver data to the device, why not
> >>>>> add this pointer to struct platdata instead?
> >>>> Unfortunately struct udevice is allocated at runtime and so the
> >>>> address is not available at compile-time.
> >>>>
> >>>> I suppose we could statically allocate the 'struct udevice' things in
> >>>> the dt-platdata.c file and then track them down in device_bind(),
> >>>> avoiding the malloc().
> >>>>
> >>>> But it might be easier (and less different) to add a pointer at
> >>>> runtime in device_bind().
> >>> Let me check if I understand you correctly, as I might I have
> >>> misunderstood you previously. Again I will use your example to have a
> >>> reference
> >>>
> >>> What I see is that I have access to a pointer to
> >>> dtd_rockchip_rk3188_cru, by &dtv_clock_controller_at_20000000, so I
> >>> understand that the idea is to extend the dtd_rockchip_rk3188_cru to
> >>> include a pointer to the device which uses it. This pointer, as you
> >>> described should be filled at runtime, in device_bind(). So in order to
> >>> to this we have to
> >>>
> >>> - Tweak dtoc to add this new pointer
> >>>
> >>> - Populate this data on device_bind()
> >>>
> >>> If this is not correct, could you please point me to the correct
> >>> suggestion using your example?
> >> I am not suggesting extending dtd_rockchip_rk3188_cru, but to struct
> >> driver_info. Something like:
> >>
> >> struct udevice *dev;
> >>
> >> which points to the actual device. It would not be set by dtoc, but
> >> device_bind() could assign it.
> >
> > Thanks for the explanation, I think I now fully understand your
> > approach. I will work on it and let you know.
> >
> >
> > Again, thanks for your time.
> >
>
> I have finally managed to have some time to work on this feature, sorry
> for the long delay.
>
>
> In order to test this approach I've
>
> - added DM_GET_DEVICE
>
> - updated dtoc in order to use DM_GET_DEVICE when populated phandle
>
> with this in new features the spl/dts/dt-platdata.c looks like
>
> static const struct dtd_rockchip_rk3188_dmc dtv_dmc_at_20020000 = {
> .clocks = {
> {DM_GET_DEVICE(clock_controller_at_20000000), {352}},
> {DM_GET_DEVICE(clock_controller_at_20000000), {353}},},
> .reg = {0x20020000, 0x3fc, 0x20040000, 0x294},
> .rockchip_cru = 0x2,
> .rockchip_grf = 0x7,
> .rockchip_noc = 0x14,
> .rockchip_pctl_timing = {0x12c, 0xc8, 0x1f4, 0x1e, 0x4e, 0x4, 0x69, 0x6,
> 0x3, 0x0, 0x6, 0x5, 0xc, 0x10, 0x6, 0x4,
> 0x4, 0x5, 0x4, 0x200, 0x3, 0xa, 0x40, 0x0,
> 0x1, 0x5, 0x5, 0x3, 0xc, 0x1e, 0x100, 0x0,
> 0x4, 0x0},
> .rockchip_phy_timing = {0x208c6690, 0x690878, 0x10022a00, 0x220, 0x40, 0x0, 0x0},
> .rockchip_pmu = 0x13,
> .rockchip_sdram_params = {0x24716310, 0x0, 0x2, 0x11e1a300, 0x3, 0x9, 0x0},
> };
>
> which I think is what you suggested or at least in that direction.
>
> However, this code does not compile due to
>
> In file included from include/command.h:14:0,
> from include/image.h:45,
> from include/common.h:40,
> from spl/dts/dt-platdata.c:7:
> include/linker_lists.h:206:2: error: braced-group within expression allowed only inside a function
> ({ \
> ^
> include/dm/platdata.h:48:2: note: in expansion of macro ‘ll_entry_get’
> ll_entry_get(struct driver_info, __name, driver_info)
> ^~~~~~~~~~~~
> spl/dts/dt-platdata.c:50:5: note: in expansion of macro ‘DM_GET_DEVICE’
> {DM_GET_DEVICE(clock_controller_at_20000000), {352}},
> ^~~~~~~~~~~~~
>
> which is clear when examining the code for *ll_entry_get*
Yes...unfortunately the devices are allocated at run-time. It is the
driver struct that is allocated at build-time.
>
> I haven't found a proper fix for this, the options I currently see are:
>
> 1- Populate phandle data with the device name which matches the
> U_BOOT_DEVICE entry, and do a runtime lookup. This is not a nice
> approach as it uses strings, as we previously discussed.
>
> 2- Populate the phandle with the struct driver_info at runtime, with
> some code generated by dtoc on spl/dts/dt-platdata.c, something like
>
> void populate_phandle(void) {
> dtv_dmc_at_20020000.clocks[0].node = DM_GET_DEVICE(clock_controller_at_20000000);
> dtv_dmc_at_20020000.clocks[1].node = DM_GET_DEVICE(clock_controller_at_20000000);
> }
>
>
> the additional issue is that I would need to drop then const from
> dtv_dmc_at_20020000
Yes that seems like the right idea. The 'const' is probably not a good
idea anyway, if it stops us linking up the data structures.
If you'd like me to fiddle with it a bit, point me to your patches and
I can have a try. It seems like you are close though.
>
> 3- Keep my original approach
>
>
> I would like, if possible, to know your opinions and suggestions. Thanks
> in advance.
Regards,
Simon
More information about the U-Boot
mailing list