> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > Sent: Sunday, March 19, 2023 6:13 AM > To: Frank Li <frank.li@xxxxxxx>; shawnguo@xxxxxxxxxx > Cc: devicetree@xxxxxxxxxxxxxxx; festevam@xxxxxxxxx; imx@xxxxxxxxxxxxxxx; > kernel@xxxxxxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; linux- > kernel@xxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx > 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@xxxxxxxxxx> > >> Sent: Friday, March 17, 2023 4:09 AM > >> To: Frank Li <frank.li@xxxxxxx>; shawnguo@xxxxxxxxxx > >> Cc: devicetree@xxxxxxxxxxxxxxx; festevam@xxxxxxxxx; > imx@xxxxxxxxxxxxxxx; > >> kernel@xxxxxxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; linux-arm- > >> kernel@xxxxxxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; linux- > >> kernel@xxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx > >> 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@xxxxxxx> > >>> --- > >>> .../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@xxxxxxx> > >>> + > >>> +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
