Re: [PATCH v3 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings

[Krzysztof Kozlowski]

On 21/03/2023 16:19, Frank Li wrote:
> NXP imx8qm integrates 1 cdns3 IP. This is glue layer device bindings.
> 
> Signed-off-by: Frank Li <Frank.Li@xxxxxxx>


Subject is still not fixed.

> ---
> Change from v2 to v3
> - Drop two fixed frequency clocks, it is system reset value, no need set now.
> If need, futher work/discuss on driver or dts change. It will not block this
> basic enablement work.
> - Drop lable
> - Drop some descriptions
> - Reg as second property.
> 
> Change from v1 to v2.
> - new add binding doc
> 
>  .../bindings/usb/fsl,imx8qm-cdns3.yaml        | 114 ++++++++++++++++++
>  1 file changed, 114 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml b/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml
> new file mode 100644
> index 000000000000..d876e3dab608
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml
> @@ -0,0 +1,112 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (c) 2020 NXP
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/fsl,imx8qm-cdns3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP iMX8QM Soc USB Controller
> +
> +maintainers:
> +  - Frank Li <Frank.Li@xxxxxxx>
> +
> +properties:
> +  compatible:
> +    const: fsl,imx8qm-usb3
> +
> +  reg:
> +    items:
> +      - description: Register set for iMX USB3 Platform Control
> +
> +  "#address-cells":
> +    enum: [ 1, 2 ]
> +
> +  "#size-cells":
> +    enum: [ 1, 2 ]
> +
> +  ranges: true
> +
> +  clocks:
> +    items:
> +      - description: Standby clock. Used during ultra low power states.
> +      - description: USB bus clock for usb3 controller.
> +      - description: AXI clock for AXI interface.
> +      - description: ipg clock for register access.
> +      - description: Core clock for usb3 controller.
> +
> +  clock-names:
> +    items:
> +      - const: usb3_lpm_clk
> +      - const: usb3_bus_clk
> +      - const: usb3_aclk
> +      - const: usb3_ipg_clk
> +      - const: usb3_core_pclk
> +
> +  assigned-clocks:
> +    items:
> +      - description: Phandle and clock specifoer of IMX_SC_PM_CLK_MST_BUS.

Drop useless pieces so "Phandle and clock specifoer of " and name the
hardware, not the syntax.

> +
> +  assigned-clock-rates:
> +    items:
> +      - description: Should be in Range 100 - 600 Mhz.

That's better but I still do not understand why do you need it in the
bindings. You never actually answered this question.

Best regards,
Krzysztof