linux-kernel - Re: [PATCH v2 03/22] usb: dwc3: glue: Allow more fine grained control over mode switches

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] [day] [month] [year] [list]

Message-ID: <5bae4ebe-dd42-4e84-9ee6-c9f6a88f7db5@kernel.org>
Date: Sat, 20 Sep 2025 13:48:12 +0200
From: Sven Peter <sven@...nel.org>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Felipe Balbi <balbi@...nel.org>,
 Janne Grunau <j@...nau.net>, Alyssa Rosenzweig <alyssa@...enzweig.io>,
 Neal Gompa <neal@...pa.dev>, Vinod Koul <vkoul@...nel.org>,
 Kishon Vijay Abraham I <kishon@...nel.org>,
 Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
 Philipp Zabel <p.zabel@...gutronix.de>, Frank Li <Frank.Li@....com>,
 Ran Wang <ran.wang_1@....com>, Peter Chen <peter.chen@....com>,
 "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "asahi@...ts.linux.dev" <asahi@...ts.linux.dev>,
 "linux-arm-kernel@...ts.infradead.org"
 <linux-arm-kernel@...ts.infradead.org>,
 "linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>
Subject: Re: [PATCH v2 03/22] usb: dwc3: glue: Allow more fine grained control
 over mode switches

Hi,

On 19.09.25 23:40, Thinh Nguyen wrote:
> On Sat, Sep 06, 2025, Sven Peter wrote:
>> We need fine grained control over mode switched on the DWC3 controller
>> present on Apple Silicon. Export core, host and gadget init and exit,
>> ptrcap and susphy control functions. Also introduce an additional
>> parameter to probe_data that allows to skip the final initialization
>> step that would bring up host or gadget mode.
>>
>> Signed-off-by: Sven Peter <sven@...nel.org>
>> ---
>>   drivers/usb/dwc3/core.c   | 16 +++++++++++-----
>>   drivers/usb/dwc3/gadget.c |  2 ++
>>   drivers/usb/dwc3/glue.h   | 14 ++++++++++++++
>>   drivers/usb/dwc3/host.c   |  2 ++
>>   4 files changed, 29 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 8002c23a5a02acb8f3e87b2662a53998a4cf4f5c..18056fac44c8732278a650ac2be8b493892c92dd 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -132,6 +132,7 @@ void dwc3_enable_susphy(struct dwc3 *dwc, bool enable)
>>   		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>>   	}
>>   }
>> +EXPORT_SYMBOL_GPL(dwc3_enable_susphy);
>>   
>>   void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode, bool ignore_susphy)
>>   {
>> @@ -157,6 +158,7 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode, bool ignore_susphy)
>>   
>>   	dwc->current_dr_role = mode;
>>   }
>> +EXPORT_SYMBOL_GPL(dwc3_set_prtcap);
> 
> I'm hesitant to export this as is. This function may change the susphy
> bits and expect them to be restored later. It's not meant to be a
> standalone use. At least, we should document how it should be used along
> with the other newly added interfaces.


Sure, I can otherwise also open-code the susphy change inside 
dwc3_apple_phy_set_mode anyway if you prefer to keep this private to the 
dwc3 core. I should restore it there to the original value anyway I 
guess after phy_set_mode.



>>   
>>   static void __dwc3_set_mode(struct work_struct *work)

[...]

>>   int dwc3_gadget_suspend(struct dwc3 *dwc)
>>   {
>> diff --git a/drivers/usb/dwc3/glue.h b/drivers/usb/dwc3/glue.h
>> index 2efd00e763be4fc51911f32d43054059e61fb43a..633268c76fe4c7fdc312c9705dfa7cf7ccf3544c 100644
>> --- a/drivers/usb/dwc3/glue.h
>> +++ b/drivers/usb/dwc3/glue.h
>> @@ -15,16 +15,30 @@
>>    * @res: resource for the DWC3 core mmio region
>>    * @ignore_clocks_and_resets: clocks and resets defined for the device should
>>    *		be ignored by the DWC3 core, as they are managed by the glue
>> + * @skip_core_init_mode: skip the finial initialization of the target mode, as
> 
> finial -> final?

Whoops, yes, I thought I ran a spell checker over this because I usually 
add a lot of typos but must've forgotten :-)


> 
>> + *		it must be managed by the glue
>>    */

[...]

>>   
>> +int dwc3_core_init(struct dwc3 *dwc);
>> +void dwc3_core_exit(struct dwc3 *dwc);
>> +
>> +int dwc3_host_init(struct dwc3 *dwc);
>> +void dwc3_host_exit(struct dwc3 *dwc);
>> +int dwc3_gadget_init(struct dwc3 *dwc);
>> +void dwc3_gadget_exit(struct dwc3 *dwc);
>> +
>> +void dwc3_enable_susphy(struct dwc3 *dwc, bool enable);
>> +void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode, bool ignore_susphy);
>> +
> 
> We should document these interfaces. The dwc3_core_probe() does all of
> the above in the proper order. It's not obvious why these are needed and
> how they should be used.

Very good point, I'll add documentation for all of these!


Thanks for the review,


Sven