[U-Boot] UCLASS_MISC bug

[Jean-Jacques Hiblot]

Simon, Sergey,

On 22/03/2019 17:33, Jean-Jacques Hiblot wrote:
> 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

A patch being worth a thousand words, I posted a patch for such a UCLASS 
: https://patchwork.ozlabs.org/patch/1061383/

JJ


>
> 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
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot