linux-kernel - Re: [PATCH 1/2] dt-bindings: usb: Add chip-specific compatible string 'fsl,ls-dwc3'

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]

Message-ID: <20240704-ladies-economy-1adc905930bf@wendy>
Date: Thu, 4 Jul 2024 07:57:52 +0100
From: Conor Dooley <conor.dooley@...rochip.com>
To: Frank Li <Frank.Li@....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>, Thinh Nguyen
	<Thinh.Nguyen@...opsys.com>, <linux-usb@...r.kernel.org>,
	<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<imx@...ts.linux.dev>, Ran Wang <ran.wang_1@....com>, Jun Li <jun.li@....com>
Subject: Re: [PATCH 1/2] dt-bindings: usb: Add chip-specific compatible
 string 'fsl,ls-dwc3'

On Wed, Jul 03, 2024 at 07:06:54PM -0400, Frank Li wrote:
> From: Ran Wang <ran.wang_1@....com>
> 
> Some Layerscape paltforms (such as LS1088A, LS2088A, etc) require update HW
> default cache type configuration to fix DWC3 init failure when applying
> property dma-coherent.

Your driver patch looks odd because it doesn't actually check if
dma-coherent is present before setting this configuration. Is it okay to
set this configuration when dma-coherent is not present?

> The cache type configuration is actually native feature of DWC3, not
> additional desgin (or glue layer) coming from SoC, so add compatible string
> 'fsl,ls-dwc3'.

You'd need to add specific compatibles here for the broken platforms,
otherwise it is not clear to users what platforms to actually use this
one.

> Signed-off-by: Ran Wang <ran.wang_1@....com>
> Reviewed-by: Jun Li <jun.li@....com>
> Signed-off-by: Frank Li <Frank.Li@....com>
> ---
>  Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> index 1cd0ca90127d9..02cb986d0fd32 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> @@ -32,6 +32,7 @@ properties:
>    compatible:
>      contains:
>        oneOf:
> +        - const: fsl,ls-dwc3

This clearly was not actually tested properly - your driver patch never
changes of_dwc3_match, so providing only this property won't work & your
expectation is actually that there is a fallback to snps,dwc3. The
binding does not allow that fallback as written.

Thanks,
Conor.

>          - const: snps,dwc3
>          - const: synopsys,dwc3
>            deprecated: true
> 
> -- 
> 2.34.1
> 

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)