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

[Conor Dooley]

On Wed, Jul 03, 2024 at 07:06:54PM -0400, Frank Li wrote:
> From: Ran Wang <ran.wang_1@xxxxxxx>
> 
> 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@xxxxxxx>
> Reviewed-by: Jun Li <jun.li@xxxxxxx>
> Signed-off-by: Frank Li <Frank.Li@xxxxxxx>
> ---
>  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
>

Attachment: signature.asc
Description: PGP signature