linux-kernel - RE: [PATCH v2] usb: dwc3: core: enable CCI support for AMD-xilinx DWC3 controller

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:
 <MN0PR12MB59536B21229DE2FB64200B70B7092@MN0PR12MB5953.namprd12.prod.outlook.com>
Date: Mon, 15 Apr 2024 19:06:12 +0000
From: "Pandey, Radhey Shyam" <radhey.shyam.pandey@....com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>, Krzysztof Kozlowski
	<krzysztof.kozlowski@...aro.org>, Rob Herring <robh+dt@...nel.org>
CC: "Simek, Michal" <michal.simek@....com>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "gregkh@...uxfoundation.org"
	<gregkh@...uxfoundation.org>, "linux-usb@...r.kernel.org"
	<linux-usb@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "git (AMD-Xilinx)" <git@....com>
Subject: RE: [PATCH v2] usb: dwc3: core: enable CCI support for AMD-xilinx
 DWC3 controller

> -----Original Message-----
> From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
> Sent: Wednesday, March 20, 2024 6:18 AM
> To: Pandey, Radhey Shyam <radhey.shyam.pandey@....com>; Krzysztof
> Kozlowski <krzysztof.kozlowski@...aro.org>; Rob Herring
> <robh+dt@...nel.org>
> Cc: Thinh Nguyen <Thinh.Nguyen@...opsys.com>; Simek, Michal
> <michal.simek@....com>; devicetree@...r.kernel.org;
> gregkh@...uxfoundation.org; linux-usb@...r.kernel.org; linux-
> kernel@...r.kernel.org; git (AMD-Xilinx) <git@....com>
> Subject: Re: [PATCH v2] usb: dwc3: core: enable CCI support for AMD-xilinx
> DWC3 controller
> 
> +devicetree maintainers
> 
> On Mon, Mar 18, 2024, Pandey, Radhey Shyam wrote:
> > >
> > > Instead, perhaps we can do it as following:
> > >  * Keep the setting of the controller registers in the core
> > >  * Create a software_node to pass a software property to the core
> > Thanks. By software property you mean flags or caps that can be passed
> > glue drivers to dwc3 core driver ?
> >
> > dwc3_set_quirks(struct dwc3 *dwc, u64 flags);
> >
> > Defines quirks in core.h
> >
> > DWC3_FLAGS_COMMON
> > DWC3_XLNX_CCI
> > DWC3_XLNX_IPD
> > DWC3_REALTEK_RES_FIX
> >
> > Then based on these quirks/flags program it in core.c.
> > Is this approach fine and aligned with your thoughts?
> >
> 
> Not as a flag. Create 4 properties for GSBUSCFG0.DATRDREQINFO,
> DESRDREQINFO, DATWRREQINFO, and DESWRREQINFO in your glue driver.
> Pass them through your software node as PROPERTY_ENTRY_U16. The core
> will override the default coreConsultant value of GSBUSCFG0 based on these
> properties in dwc3_get_properties().

Thanks . It is clear to me now.
> 
> Check drivers/usb/dwc3/host.c for reference.
> 
> >
> > >
> > > These software properties will not be documented in the devicetree
> > > binding. Just document them in the driver core header. They are
> > > simply driver properties that get passed through software node.
> > >
> > > You can add the software node using device_add_software_node(). This
> > > can be done before calling of_platform_populate() in dwc3-xilinx
> > > (can be done in pltfm_init())
> > >
> > > Let me know if this works for you.
> > >
> 
> Hi Rob/Krzysztof,
> 
> Just want to check in with you for your opinion. To summarize my suggestion
> to Pandey, here are the key notes:
> * Platform specific settings are set in glue drivers (match through
>   compatible string)
> * These settings are set by controller registers that should only
>   be accessible in the dwc3 core
> * So, the suggestion is to pass these settings as properties using
>   software_node created from the glue driver to the dwc3 core
> * These properties will not be documented in the devicetree binding, but
>   only in the driver
> 
> We're already doing that to some properties such as
> "linux,sysdev_is_parent"
> 
> If this suggestion makes sense, would the prefix "linux," for linux specific
> binding or "snps," is a better fit?

Rob/Krzysztof: Please let me know your thoughts on Thinh's suggestion?
Based on it I can work and submit the next version.

Thanks,
Radhey