On 22.01.24 13:01, Takashi Iwai wrote: > On Tue, 16 Jan 2024 08:29:04 +0100, > 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. > > FWIW, the patterns in sound/ are somewhat outdated and need to be > refreshed. Nowadays one should use DEFINE_SIMPLE_DEV_PM_OPS() instead > (that should work without ifdef). > > > thanks, > > Takashi Thank you for your feedback. I noticed that the pattern looks different, but given that many devices in sound/ still use that pattern, I just followed suit. In that case I will only use DEFINE_SIMPLE_DEV_PM_OPS. Thanks again and best regards, Javier Carrasco