lkml.org 
[lkml]   [2023]   [Mar]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings
    Date


    > -----Original Message-----
    > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
    > Sent: Sunday, March 19, 2023 6:13 AM
    > To: Frank Li <frank.li@nxp.com>; shawnguo@kernel.org
    > Cc: devicetree@vger.kernel.org; festevam@gmail.com; imx@lists.linux.dev;
    > kernel@pengutronix.de; krzysztof.kozlowski+dt@linaro.org; linux-arm-
    > kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; linux-
    > kernel@vger.kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de
    > Subject: Re: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add
    > imx8qm cdns3 glue bindings
    >
    > Caution: EXT Email
    >
    > On 17/03/2023 15:55, Frank Li wrote:
    > >
    > >
    > >> -----Original Message-----
    > >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
    > >> Sent: Friday, March 17, 2023 4:09 AM
    > >> To: Frank Li <frank.li@nxp.com>; shawnguo@kernel.org
    > >> Cc: devicetree@vger.kernel.org; festevam@gmail.com;
    > imx@lists.linux.dev;
    > >> kernel@pengutronix.de; krzysztof.kozlowski+dt@linaro.org; linux-arm-
    > >> kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; linux-
    > >> kernel@vger.kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de
    > >> Subject: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add
    > >> imx8qm cdns3 glue bindings
    > >>
    > >> Caution: EXT Email
    > >>
    > >> On 16/03/2023 22:27, Frank Li wrote:
    > >>> NXP imx8qm integrates 1 cdns3 IP. This is glue layer device bindings.
    > >>>
    > >>
    > >> Subject: drop second/last, redundant "bindings". The "dt-bindings"
    > >> prefix is already stating that these are bindings.
    > >>
    > >>> Signed-off-by: Frank Li <Frank.Li@nxp.com>
    > >>> ---
    > >>> .../bindings/usb/fsl,imx8qm-cdns3.yaml | 122
    > ++++++++++++++++++
    > >>> 1 file changed, 122 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..fc24df1e4483
    > >>> --- /dev/null
    > >>> +++ b/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml
    > >>> @@ -0,0 +1,122 @@
    > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
    > >>> +# Copyright (c) 2020 NXP
    > >>> +%YAML 1.2
    > >>> +---
    > >>> +$id:
    > >>
    > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevice
    > %2F&data=05%7C01%7Cfrank.li%40nxp.com%7Cc2d76d3694194fba130a08db
    > 286ae90e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6381482118
    > 66106607%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi
    > V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Lo
    > JUcnJnBaGjywN1zF%2BuUpFVUmldixTci96NPzVuio0%3D&reserved=0
    > >> tree.org%2Fschemas%2Fusb%2Ffsl%2Cimx8qm-
    > >>
    > cdns3.yaml%23&data=05%7C01%7CFrank.Li%40nxp.com%7Cac9af3d617dc4cf
    > >>
    > 14baf08db26c74b07%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
    > >>
    > 38146409617970248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
    > >>
    > AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C
    > >>
    > &sdata=EczZhjyMUGnnp7ZGfSvTj4lmOUuGlOtWYIsxxXIlNXw%3D&reserved
    > >> =0
    > >>> +$schema:
    > >>
    > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevice
    > %2F&data=05%7C01%7Cfrank.li%40nxp.com%7Cc2d76d3694194fba130a08db
    > 286ae90e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6381482118
    > 66106607%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi
    > V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Lo
    > JUcnJnBaGjywN1zF%2BuUpFVUmldixTci96NPzVuio0%3D&reserved=0
    > >> tree.org%2Fmeta-
    > >>
    > schemas%2Fcore.yaml%23&data=05%7C01%7CFrank.Li%40nxp.com%7Cac9a
    > >>
    > f3d617dc4cf14baf08db26c74b07%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C
    > >>
    > 0%7C0%7C638146409617970248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM
    > >>
    > C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000
    > >> %7C%7C%7C&sdata=uTNYuDm%2ByhZ56oQET2pX8sHpEqVvsUYtmOBCPX
    > EK
    > >> v40%3D&reserved=0
    > >>> +
    > >>> +title: NXP iMX8QM Soc USB Controller
    > >>> +
    > >>> +maintainers:
    > >>> + - Frank Li <Frank.Li@nxp.com>
    > >>> +
    > >>> +properties:
    > >>> + compatible:
    > >>> + const: fsl,imx8qm-usb3
    > >>> +
    > >>> + reg:
    > >>> + items:
    > >>> + - description: Address and length of the register set for iMX USB3
    > >> Platform Control
    > >>
    > >> Drop "Address and length of the"... or actually just maxItems: 1,
    > >> because the description is a bit obvious.
    > >>
    > >>> +
    > >>> + "#address-cells":
    > >>> + enum: [ 1, 2 ]
    > >>> +
    > >>> + "#size-cells":
    > >>> + enum: [ 1, 2 ]
    > >>> +
    > >>> + ranges: true
    > >>> +
    > >>> + clocks:
    > >>> + description:
    > >>> + A list of phandle and clock-specifier pairs for the clocks
    > >>> + listed in clock-names.
    > >>
    > >> Drop description.
    > >>
    > >>> + 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 specifier of IMX_SC_PM_CLK_PER.
    > >>> + - description: Phandle and clock specifoer of
    > IMX_SC_PM_CLK_MISC.
    > >>> + - description: Phandle and clock specifoer of
    > >> IMX_SC_PM_CLK_MST_BUS.
    > >>> +
    > >>> + assigned-clock-rates:
    > >>> + items:
    > >>> + - description: Must be 125 Mhz.
    > >>> + - description: Must be 12 Mhz.
    > >>> + - description: Must be 250 Mhz.
    > >>
    > >> I would argue that both properties above are not needed. If your
    > >> hardware requires fixed frequencies, clock provider can fix them, can't it?
    > >
    > > Clock provider don't know fixed value and turn on only used by client.
    >
    > So maybe fix the clock provider? Or this device driver? Requiring by
    > binding specific frequencies for every board is a bit redundant.

    It is not for every boards, it is common for a chip family. Only a place to set for
    QM and QXP.

    The similar case is network driver, which require a specific frequency at clock assign.
    Generally frequency is fixed, clock source name may change at difference chips.

    >
    > Best regards,
    > Krzysztof

    \
     
     \ /
      Last update: 2023-03-27 01:09    [W:8.421 / U:0.196 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site