[U-Boot] UCLASS_MISC bug

[Jean-Jacques Hiblot]

Simon,

On 22/03/2019 08:53, Simon Glass wrote:
> Hi,
>
> On Tue, 19 Mar 2019 at 23:29, Jean-Jacques Hiblot <jjhiblot at ti.com> wrote:
>> + Simon Glass
>>
>>
>> Hi Serguey,
>>
>> On 15/03/2019 22:55, Sergey Kubushyn wrote:
>>> I would like to point that this was not a very good idea:
>>>
>>> === Cut ===
>>> --- uboot-imx-next/drivers/misc/misc-uclass.c    2018-12-20
>>> 20:35:22.505180339 -0800
>>> +++ u-boot-imx/drivers/misc/misc-uclass.c    2019-03-13
>>> 11:50:19.408982814 -0700
>>> @@ -68,4 +68,7 @@ int misc_set_enabled(struct udevice *dev
>>>   UCLASS_DRIVER(misc) = {
>>>       .id        = UCLASS_MISC,
>>>       .name        = "misc",
>>> +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
>>> +    .post_bind    = dm_scan_fdt_dev,
>>> +#endif
>>>   };
>>> === Cut ===
>>>
>>> The problem comes up with all those "glue" layers like e.g. in
>>> usb/dwc3 and
>>> everywhere else.
>>>
>>> Those "glues" primarily bind drivers to e.g. USB interfaces like e.g.
>>> dwc3-generic does. That is where it should stop because glue knows better
>>> what to bind. However, after glue has bound appropriate drivers to their
>>> interfaces that post_bind scans FDT again and finds yet another bunch of
>>> drivers that it binds on top of what glue already bound. This happens
>>> e.g.
>>> with DWC3 drivers that glues bind to interfaces depending on their
>>> declared
>>> roles. However both peripheral and XHCI drivers are compatible with
>>> "snps,dwc3" so they are bound on top creating a huge mess. Here is a
>>> dm tree
>>> fragment of its action:
>>>
>>> === Cut ===
>>>   phy          0  [ + ]   imx8mq_usb_phy        |-- phy at 381f0040
>>>   misc         0  [ + ]   imx8m_dwc3_glue       |-- usb at 38100000
>>>   usb          0  [   ]   imx8m_dwc3_periphera  |   |-- dwc3
>>>   usb          0  [   ]   xhci-dwc3             |   `-- dwc3
>>>   phy          1  [   ]   imx8mq_usb_phy        |-- phy at 382f0040
>>>   misc         1  [   ]   imx8m_dwc3_glue       |-- usb at 38200000
>>>   usb          1  [   ]   xhci-dwc3             |   |-- dwc3
>>>   usb          2  [   ]   xhci-dwc3             |   `-- dwc3
>>> === Cut ===
>>>
>>> The glue bound imx8m_dwc3_peripheral to USB0 then post_bind found yet
>>> another "compatible" driver, xhci-dwc3 and bound it to the same
>>> interface.
>>>
>>> Then glue bound xhci-dwc3 to USB1 but post_bind found yet another
>>> "compatible" driver, xhci-dwc3 and bound it again so there are TWO xhci
>>> drivers bound to the same interface.
>>>
>>> This is simplified picture -- I removed "compatible" from my peripheral
>>> driver so it only comes up once, under USB0. If I leave that "compatible"
>>> with "snps,dwc3" there everything gets even weirder :(
>>>
>>> That diff above is the latest 2019.04-rc3 u-boot-imx-next vs 2019.01-rc
>>> something release.
>>>
>>> We either need some additional special flag to disable that post_bind
>>> where
>>> it is not needed or create a special new class for those binding
>>> "glues" or
>>> this change should be reverted.
>> We can also see the problem on TI platforms.
>>
>> Tested with a am57xx evm
> Can you check for an existing device and not create a new one in that case?
>
> Maybe use device_find_first_child_by_uclass() ?

If I'm not mistaken, that would not solve the issue because the device 
are bound in the bind() callback of the misc driver.

One solution would be to use another UCLASS that really does not do 
anything (UCLASS_NOP  for example). It would be ok because we don't use 
any of the features offered by the MISC UCLASS

Another solution would be to use a flag stored in the uclass priv data 
to tell not bind devices in the post_bind callback.

I prefer the first approach.

JJ


>
> Regards,
> Simon
>
>
>> JJ
>>
>>> P.S. I'm going to post imx8m USB glue, USB PHY, USB peripheral initial
>>> drivers probably Monday late night as they still need some cleanup. It
>>> all
>>> works with full Linux Kernel DTS files, picks power domains from there.
>>>
>>> Don't know what to do with my board-specific DTS as there is (and
>>> probably
>>> will not be) our board submitted -- it is not secret but used in our
>>> devices
>>> only and not available to the public -- so it doesn't make sense to
>>> post a
>>> full DTS. Will probably send a fragment.
>>>
>>> ---
>>> ******************************************************************
>>> *  KSI at home    KOI8 Net  < >  The impossible we do immediately.  *
>>> *  Las Vegas   NV, USA   < >  Miracles require 24-hour notice.   *
>>> ******************************************************************
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> https://lists.denx.de/listinfo/u-boot