Hi Stefan, > -----Original Message----- > From: Stefan Wahren <stefan.wahren@xxxxxxxx> > Sent: Thursday, June 8, 2023 7:39 PM > To: Xu Yang <xu.yang_2@xxxxxxx>; robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx > Cc: shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; dl-linux-imx <linux- > imx@xxxxxxx>; linux-phy@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux- > usb@xxxxxxxxxxxxxxx; Jun Li <jun.li@xxxxxxx> > Subject: Re: [EXT] Re: [PATCH v2 1/2] dt-bindings: phy: mxs-usb-phy: convert to DT schema format > > Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the > message using the 'Report this email' button > > > Hi Xu, > > Am 08.06.23 um 12:30 schrieb Xu Yang: > > Hi Stefan, > > > >> -----Original Message----- > >> From: Stefan Wahren <stefan.wahren@xxxxxxxx> > >> Sent: Thursday, June 8, 2023 3:56 PM > >> To: Xu Yang <xu.yang_2@xxxxxxx>; robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx > >> Cc: shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; dl-linux-imx > <linux- > >> imx@xxxxxxx>; linux-phy@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > linux- > >> usb@xxxxxxxxxxxxxxx; Jun Li <jun.li@xxxxxxx> > >> Subject: [EXT] Re: [PATCH v2 1/2] dt-bindings: phy: mxs-usb-phy: convert to DT schema format > >> > >> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report > the > >> message using the 'Report this email' button > >> > >> > >> Hi Xu, > >> > >> Am 08.06.23 um 05:36 schrieb Xu Yang: > >>> Convert the binding to DT schema format. Besides, this also add other > >>> optional properties not contained in txt file. > >>> > >>> Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx> > >>> > >>> --- > >>> Changes in v2: > >>> - change filename to fsl,mxs-usbphy.yaml > >>> - add other optional properties > >>> - narrow fsl,anatop to imx6 > >>> - use additionalProperties > >>> --- > >>> .../bindings/phy/fsl,mxs-usbphy.yaml | 128 ++++++++++++++++++ > >>> .../devicetree/bindings/phy/mxs-usb-phy.txt | 33 ----- > >>> 2 files changed, 128 insertions(+), 33 deletions(-) > >>> create mode 100644 Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml > >>> delete mode 100644 Documentation/devicetree/bindings/phy/mxs-usb-phy.txt > >>> > >>> diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml > >> b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml > >>> new file mode 100644 > >>> index 000000000000..1b6b19fdf491 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml > >>> @@ -0,0 +1,128 @@ > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>> +%YAML 1.2 > >>> +--- > >>> +$id: > >> > http://devicetree.org/schemas/phy/fsl,mxs- > &data=05%7C01%7Cxu.yang_2%40nxp.com%7C35ccff605dbd46ac9d8608db6814f13f%7C686ea1d3bc2b4c6fa92cd99c5c3016 > 35%7C0%7C0%7C638218211379372106%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I > k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=gSKnnBeB7BrscJNdHEPWgALwvaSlnZrCbW2lo1K8D0s%3D&reserv > ed=0 > >> > usbphy.yaml%23&data=05%7C01%7Cxu.yang_2%40nxp.com%7C5df4d949f975469013b408db67f5d46c%7C686ea1d3bc2b4c > >> > 6fa92cd99c5c301635%7C0%7C0%7C638218077754788407%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjo > >> > iV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=8Sz8tK9GVQqE6ywVLpxPB8YDFQvygZvj6s1NjZk > >> hbzU%3D&reserved=0 > >>> +$schema: http://devicetree.org/meta- > &data=05%7C01%7Cxu.yang_2%40nxp.com%7C35ccff605dbd46ac9d8608db6814f13f%7C686ea1d3bc2b4c6fa92cd99c5c3016 > 35%7C0%7C0%7C638218211379372106%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I > k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=D3MKKJpLHdfLhK9ubiqQYGZ7ORNDOYbA%2FJOQX%2B3DoAg%3 > D&reserved=0 > >> > schemas%2Fcore.yaml%23&data=05%7C01%7Cxu.yang_2%40nxp.com%7C5df4d949f975469013b408db67f5d46c%7C686ea1 > >> > d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638218077754788407%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD > >> > AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=iaX16VnSJnvU%2F0tcnRgsdnTxMsD89 > >> 5r4WquGsCFt9Qo%3D&reserved=0 > >>> + > >>> +title: Freescale MXS USB Phy Device > >>> + > >>> +maintainers: > >>> + - Xu Yang <xu.yang_2@xxxxxxx> > >>> + > >>> +properties: > >>> + compatible: > >>> + oneOf: > >>> + - enum: > >>> + - fsl,imx23-usbphy > >>> + - fsl,vf610-usbphy > >>> + - fsl,imx7ulp-usbphy > >> > >> on the one side the fsl,imx7ulp-usbphy has an individual compatible > >> > >>> + - items: > >>> + - enum: > >>> + - fsl,imx28-usbphy > >>> + - fsl,imx6ul-usbphy > >>> + - fsl,imx6sl-usbphy > >>> + - fsl,imx6sx-usbphy > >>> + - fsl,imx6q-usbphy > >>> + - const: fsl,imx23-usbphy > >>> + - items: > >>> + - const: fsl,imx6sll-usbphy > >>> + - const: fsl,imx6ul-usbphy > >>> + - const: fsl,imx23-usbphy > >>> + - items: > >>> + - const: fsl,imx7ulp-usbphy > >>> + - const: fsl,imx6ul-usbphy > >> > >> on the other side this should be compatible to imx6ul. So at least one > >> definition seems to be unnecessary. > >> > >> Looking at usb/phy/phy-mxs-usb.c suggests me that fsl,imx7ulp-usbphy is > >> not directly compatible to fsl,imx6ul-usbphy, because the platform data > >> is different. So maybe the using dts* files should be fixed instead? > > > > The imx7ulp and imx6ul only has minor difference. In general, imx7ulp > > is compatilbe with imx6ul. We don't need to modify both dts file and > > this doc here. So the validation of existing dtb would not fail. > > The fact that according to the schema imx6ul needs fsl,anatop and > imx7ulp doesn't need it, let me think that the difference is not really > minor. > > Nevertheless the compatibles for imx7ulp-usbphy looks fishy to me, > because there are two ways to describe imx7ulp-usbphy ( with and without > fsl,imx6ul-usbphy ). From my understanding there should be only one way. > > In case you are just concerned about validation issues in this series: > it's acceptable to convert txt file to YAML and fix outstanding > validation issues within the series via separate patch. In my opinion > the goal is to get a proper DT schema and not just to avoid DT > validation warnings. I reviewed the dts files and driver again. It seems indeedly that imx7ulp-usbphy is diverged from imx6ul-usbphy today. Therefore, to keep this doc clear, "fsl,imx7ulp-usbphy", "fsl,imx6ul-usbphy" could be removed. Thanks for your input. I will remove the second one and keep the first one in V4. Thanks, Xu Yang > > Best regards > > > > > Thanks, > > Xu Yang > > > >> > >>> + - items: > >>> + - const: fsl,imx8dxl-usbphy > >>> + - const: fsl,imx7ulp-usbphy > >>> + > >>> + reg: > >>> + minItems: 1 > >>> + maxItems: 2 > >>> + > >>> + interrupts: > >>> + minItems: 1 > >>> + maxItems: 2 > >>> + > >>> + clocks: > >>> + maxItems: 1 > >>> + > >>> + '#phy-cells': > >>> + const: 0 > >>> + > >>> + power-domains: > >>> + maxItems: 1 > >>> + > >>> + fsl,anatop: > >>> + description: > >>> + phandle for anatop register, it is only for imx6 SoC series. > >>> + $ref: /schemas/types.yaml#/definitions/phandle > >>> + > >>> + phy-3p0-supply: > >>> + description: > >>> + One of USB PHY's power supply. Can be used to keep a good signal > >>> + quality. > >>> + > >>> + fsl,tx-cal-45-dn-ohms: > >>> + description: > >>> + Resistance (in ohms) of switchable high-speed trimming resistor > >>> + connected in parallel with the 45 ohm resistor that terminates > >>> + the DN output signal. > >>> + minimum: 35 > >>> + maximum: 54 > >>> + default: 45 > >>> + > >>> + fsl,tx-cal-45-dp-ohms: > >>> + description: > >>> + Resistance (in ohms) of switchable high-speed trimming resistor > >>> + connected in parallel with the 45 ohm resistor that terminates > >>> + the DP output signal. > >>> + minimum: 35 > >>> + maximum: 54 > >>> + default: 45 > >>> + > >>> + fsl,tx-d-cal: > >>> + description: > >>> + Current trimming value (as a percentage) of the 17.78 mA TX > >>> + reference current. > >>> + $ref: /schemas/types.yaml#/definitions/uint32 > >>> + minimum: 79 > >>> + maximum: 119 > >>> + default: 100 > >>> + > >>> +required: > >>> + - compatible > >>> + - reg > >>> + - clocks > >>> + > >>> +allOf: > >>> + - if: > >>> + properties: > >>> + compatible: > >>> + oneOf: > >>> + - enum: > >>> + - fsl,imx6sl-usbphy > >>> + - fsl,imx6sx-usbphy > >>> + - fsl,imx6sll-usbphy > >>> + - fsl,imx6q-usbphy > >>> + - fsl,vf610-usbphy > >>> + - items: > >>> + - const: fsl,imx6ul-usbphy > >>> + - const: fsl,imx23-usbphy > >>> + then: > >>> + required: > >>> + - fsl,anatop > >>> + > >>> +additionalProperties: false > >>> + > >>> +examples: > >>> + - | > >>> + #include <dt-bindings/interrupt-controller/arm-gic.h> > >>> + > >>> + usbphy1: usb-phy@20c9000 { > >>> + compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy"; > >>> + reg = <0x020c9000 0x1000>; > >>> + interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; > >>> + fsl,anatop = <&anatop>; > >>> + }; > >>> + > >>> +... > >>> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt > b/Documentation/devicetree/bindings/phy/mxs- > >> usb-phy.txt > >>> deleted file mode 100644 > >>> index 70c813b0755f..000000000000 > >>> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt > >>> +++ /dev/null > >>> @@ -1,33 +0,0 @@ > >>> -* Freescale MXS USB Phy Device > >>> - > >>> -Required properties: > >>> -- compatible: should contain: > >>> - * "fsl,imx23-usbphy" for imx23 and imx28 > >>> - * "fsl,imx6q-usbphy" for imx6dq and imx6dl > >>> - * "fsl,imx6sl-usbphy" for imx6sl > >>> - * "fsl,vf610-usbphy" for Vybrid vf610 > >>> - * "fsl,imx6sx-usbphy" for imx6sx > >>> - * "fsl,imx7ulp-usbphy" for imx7ulp > >>> - * "fsl,imx8dxl-usbphy" for imx8dxl > >>> - "fsl,imx23-usbphy" is still a fallback for other strings > >>> -- reg: Should contain registers location and length > >>> -- interrupts: Should contain phy interrupt > >>> -- fsl,anatop: phandle for anatop register, it is only for imx6 SoC series > >>> - > >>> -Optional properties: > >>> -- fsl,tx-cal-45-dn-ohms: Integer [35-54]. Resistance (in ohms) of switchable > >>> - high-speed trimming resistor connected in parallel with the 45 ohm resistor > >>> - that terminates the DN output signal. Default: 45 > >>> -- fsl,tx-cal-45-dp-ohms: Integer [35-54]. Resistance (in ohms) of switchable > >>> - high-speed trimming resistor connected in parallel with the 45 ohm resistor > >>> - that terminates the DP output signal. Default: 45 > >>> -- fsl,tx-d-cal: Integer [79-119]. Current trimming value (as a percentage) of > >>> - the 17.78mA TX reference current. Default: 100 > >>> - > >>> -Example: > >>> -usbphy1: usb-phy@20c9000 { > >>> - compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy"; > >>> - reg = <0x020c9000 0x1000>; > >>> - interrupts = <0 44 0x04>; > >>> - fsl,anatop = <&anatop>; > >>> -};
