linux-kernel - Re: [PATCH 2/3] ASoC: dt-bindings: xmos,xvf3500: add bindings for XMOS XVF3500

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 22 Jan 2024 13:35:30 +0100
From: Javier Carrasco <javier.carrasco@...fvision.net>
To: Takashi Iwai <tiwai@...e.de>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
 Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>, Liam Girdwood <lgirdwood@...il.com>,
 Mark Brown <broonie@...nel.org>, Jaroslav Kysela <perex@...ex.cz>,
 Takashi Iwai <tiwai@...e.com>, Rob Herring <robh@...nel.org>,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-sound@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCH 2/3] ASoC: dt-bindings: xmos,xvf3500: add bindings for
 XMOS XVF3500

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