On 22.01.24 13:29, Krzysztof Kozlowski wrote: > On 16/01/2024 08:29, Javier Carrasco wrote: >> On 15.01.24 21:43, Krzysztof Kozlowski wrote: >>> On 15/01/2024 20:43, Javier Carrasco wrote: >>>> On 15.01.24 19:11, Krzysztof Kozlowski wrote: >>>>> On 15/01/2024 17:24, Javier Carrasco wrote: >>>>>> Do you mean that the XVF3500 should not be represented as a platform >>>>>> device and instead it should turn into an USB device represented as a >>>>>> node of an USB controller? Something like this (Rockchip SoC): >>>>>> >>>>>> &usb_host1_xhci { >>>>>> ... >>>>>> >>>>>> xvf3500 { >>>>>> ... >>>>>> }; >>>>>> }; >>>>>> >>>>>> Did I get you right or is that not the correct representation? Thank you >>>>>> again. >>>>> >>>>> I believe it should be just like onboard hub. I don't understand why >>>>> onboard hub was limited to hub, because other USB devices also could be >>>>> designed similarly by hardware folks :/ >>>>> >>>>> And if we talk about Linux drivers, then your current solution does not >>>>> support suspend/resume and device unbind. >>>>> >>>>> Best regards, >>>>> Krzysztof >>>>> >>>> >>>> Actually this series is an attempt to get rid of a misuse of the >>>> onboard_usb_hub driver by a device that is not a HUB, but requires the >>>> platform-part of that driver for the initialization. >>> >>> That's just naming issue, isn't it? >>> >>>> >>>> What would be the best approach to provide support upstream? Should I >>>> turn this driver into a generic USB driver that does what the >>>> platform-part of the onboard HUB does? Or are we willing to accept >>> >>> No, because you did not solve the problems I mentioned. This is neither >>> accurate hardware description nor proper Linux driver model handling PM >>> and unbind. >>> >> You mentioned the PM handling twice, but I am not sure what you mean. >> The driver provides callbacks for SIMPLE_DEV_PM_OPS, which I tested in >> freeze and memory power states with positive results. On the other hand, >> I suppose that you insisted for a good reason, so I would be grateful if >> you could show me what I am doing wrong. The macro pattern was taken >> from other devices under sound/, which also check CONFIG_PM_SLEEP, >> but maybe I took a bad example or missed something. > > You have two Linux devices: USB device and platform device. The platform > device controls power of USB device. If platform device goes to some > variant of sleep (PRM, system PM) before the USB device, how will USB > device react? Will it work? I doubt. > > You have no ordering / dependencies / device links between these devices > thus possible problems. > Thank you for clarifying this point. I only covered the platform device and such device links are missing. I will opt for the inclusion in onboard_usb_hub, which already covers both devices. >>>> non-HUB devices in the onboard_usb_hub driver even though it supports >>>> more operations? >>>> >>>> I am adding linux-usb to this thread in case someone has other suggestions. >>> >>> I don't see any difference between this device and onboard hub. The >>> concept and the problem is the same. Therefore either treat it as as >>> onboard hub or come with USB-version of PCI power sequencing. >>> >> I have nothing against adding this device to onboard_usb_hub as long as >> it is valid upstream, so no conflicts arise with new additions to the >> list (which was the trigger for all of this with v6.7). That is > > I am sorry, but we talk here only about upstream. I don't know therefore > what "valid upstream" means. You cannot send a patch which is "not valid > upstream". I mean, technically you can, but this would be waste of our > time and receive rather annoyed responses. > I meant valid upstream, so sending such patch makes sense to the community (and not only to us). So far you are the source of feedback upstream, so I will follow your advice. >> obviously the most trivial solution and as you said, it is justs a >> naming issue because the power sequence is not HUB-specific. > > I would ack it. Others? No clue, I also do not remember full story > behind onboard USB hub and why it was called "hub" instead of "onboard > USB device". > > Best regards, > Krzysztof > Thank you and best regards, Javier Carrasco