> -----Original Message----- > From: Johannes Zink <j.zink@xxxxxxxxxxxxxx> > Sent: Tuesday, April 11, 2023 11:23 PM > To: Jun Li <jun.li@xxxxxxx>; Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx>; vkoul@xxxxxxxxxx; kishon@xxxxxxxxxx; > shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; > festevam@xxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; robh+dt@xxxxxxxxxx; > krzysztof.kozlowski+dt@xxxxxxxxxx; Bough Chen <haibo.chen@xxxxxxx>; > linux-phy@xxxxxxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 1/2] dt-bindings: phy: imx8mq-usb: add phy tuning > properties > > Hi Jun, > > On Tue, 2023-04-11 at 14:59 +0000, Jun Li wrote: > > > > > > > -----Original Message----- > > > From: Johannes Zink <j.zink@xxxxxxxxxxxxxx> > > > Sent: Tuesday, April 11, 2023 10:23 PM > > > To: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>; > > > vkoul@xxxxxxxxxx; kishon@xxxxxxxxxx; shawnguo@xxxxxxxxxx; > > > s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; > > > dl-linux-imx <linux-imx@xxxxxxx>; robh+dt@xxxxxxxxxx; > > > krzysztof.kozlowski+dt@xxxxxxxxxx; Jun Li <jun.li@xxxxxxx>; Bough > > > Chen <haibo.chen@xxxxxxx>; linux-phy@xxxxxxxxxxxxxxxxxxx; > > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > > > devicetree@xxxxxxxxxxxxxxx > > > Subject: Re: [PATCH 1/2] dt-bindings: phy: imx8mq-usb: add phy > > > tuning properties > > > > > > Hi Krzystof, > > > > > > thank you for your explanations. As I'm still quite new to writing > > > bindings, I still have some questions: > > > > > > On Fri, 2023-04-07 at 11:03 +0200, Krzysztof Kozlowski wrote: > > > > On 05/04/2023 14:14, Johannes Zink wrote: > > > > > Hi Krysztof, > > > > > > > > > > thanks for your review, please find my questions below. > > > > > > > > > > On Wed, 2023-04-05 at 13:51 +0200, Krzysztof Kozlowski wrote: > > > > > > [snip] > > > > > > > A phandle to the regulator for USB VBUS. > > > > > > > > > > > > > > + fsl,phy-tx-vref-tune: > > > > > > > + description: > > > > > > > + HS DC Voltage level adjustment > > > > > > > > > > > > "Level" in what units? > > > > > > > > > > > > > > > > The datasheet just shows percent, ranging from -6 to +24%, in 2% > > > > > increments. What unit would you suggest? > > > > > > > > percent > > > > > > > https://gith/ > > > > > > > ub.com%2Fdevicetree-org%2Fdt- > > > schema%2Fblob%2Fmain%2Fdtschema%2Fschemas > > > > %2Fproperty- > > > > units.yaml&data=05%7C01%7Cjun.li%40nxp.com%7Ca2e1e5bb6a78 > > > 4 > > > > > > > de5941d08db3a9847d0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63 > > > 816 > > > > > > > 8197947407580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > > > 2lu > > > > > > > MzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=kY4dXkOHRs > > > 9k% > > > > 2BeUQ5iixLYsNC8UotIgq6eOPGjbf01o%3D&reserved=0 > > > > > > I am still a bit confused how to use this properly. How can I > > > restrict the values to multiples of 2 in order to avoid illegal > > > values? > > > > > > At the moment the only thing I could come up with is something like > > > > > > fsl,phy-tx-vref-tune-percent: > > > description: | > > > Adjusts the high-speed DC level voltage > > > $ref: /schemas/types.yaml#/definitions/int32 > > > minimum: -6 > > > maximum: 24 > > > default: 0 > > > > > > Does something like this work? I am not quite sure if I am on the > > > right track here, especially as this requires a signed int, of which > > > I have not seen many examples so far. > > > > > > Also, as far as the description is concerned: This is almost the > > > entire information I there is in the datasheet. As I try to upstream > > > some of the vendor downstream patches, I do not have any additional > > > information. > > > > > > > > > > > > > > > > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > > > > + enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, > > > > > > > 14, > > > > > > > 15, > > > > > > > 16] > > > > > > > + > > > > > > > + fsl,phy-tx-rise-tune: > > > > > > > + description: > > > > > > > + HS Transmitter Rise/Fall Time Adjustment > > > > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > > > > + enum: [0, 1, 2, 3] > > > > > > > + > > > > > > > + fsl,phy-tx-preemp-amp-tune: > > > > > > > + description: > > > > > > > + HS Transmitter Pre-Emphasis Current Control > > > > > > > > > > > > If this is current then use standard unit suffixes. > > > > > > > > > > According to the datasheet this is in "unit amonts" of 600uA, > > > > > basically 0x600uA, 1x600uA etc. Should I just suffix it with uA > > > > > then? > > > > > > > > Yes > > > > > > > > > > > https://gith/ > > > > > > > ub.com%2Fdevicetree-org%2Fdt- > > > schema%2Fblob%2Fmain%2Fdtschema%2Fschemas > > > > %2Fproperty- > > > > units.yaml&data=05%7C01%7Cjun.li%40nxp.com%7Ca2e1e5bb6a78 > > > 4 > > > > > > > de5941d08db3a9847d0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63 > > > 816 > > > > > > > 8197947407580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > > > 2lu > > > > > > > MzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=kY4dXkOHRs > > > 9k% > > > > 2BeUQ5iixLYsNC8UotIgq6eOPGjbf01o%3D&reserved=0 > > > > > > > > The register values can work sometimes fine but also do not scale > > > > at all. For any other variant all the meanings will differ. Any > > > > other > > > > IMX8 > > > > phy will need new bindings and new description/values for your > > > > register-like-fields. > > > > > > I think this particular property should work, probably its something > > > like > > > > > > fsl,phy-tx-preemp-amp-tune-microamps: > > > description: | > > > Transmitter Pre-Emphasis Current Control > > > Controls the amount of current sourced to DPn and DMn after a > > > J-to- > > > K or K-to-J transition. > > > $ref: /schemas/types.yaml#/definitions/uint32 > > > minimum: 0 > > > maximum: 1800 > > > default: 0 > > > > > > What's the right way to communicate that the value is in multiples > > > of 600uA and that this is only an approximate Value? Add some > > > free-text to the description? > > > > > > > > > For some other properties, such as fsl,phy-pcs-tx-swing-full or > > > fsl,phy-pcs-tx-deemph-3p5db the datasheet provides no information at > > > all, neither on the unit nor on a valid range. What is the proper > > > way for something like them (I try to get some of the freescale > > > downstream patches to mainline, but they did not even provide > > > bindings for their > > > driver...) > > > > I will check with internal design team for those not well documented > > properties. > > > > That's great, thanks! Here is the feedback from our design team: Per the USB specification, the TX de-emphasis value is 3.5dB nominal. Only pcs_tx_deemph_3p5db is used for USB3. pcs_tx_deemph_6db[5:0] is not used. pcs_tx_deemph_3p5db[5:0] range is between 0dB to -36dB (unit step in decimal 1) pcs_tx_swing_full[6:0] range is from 0 to full scale 127(decimal), with unit step in decimal 1. TX amplitude is determined by ( voltage corresponding to tx_vboost_lvl) * (pcs_tx_swing_full +1 )/128 The actual TX swing value should be decided by characterization with the package and PCB. Li Jun > > Johannes > > > Li Jun > > > > > > > > > For fsl,phy-comp-dis-tune-percent, the actual values to not map well > > > to integer amount of percent, but I have not found a permill in > > > property- units. > > > Also, as the steps appear quite arbitrary large, what is the correct > > > way of restricting the values to valid values that the hardware can > > > actually support? As reference, I have only seen stuff like the > > > st,trim-hs- current in > > > Documentation/devicetree/bindings/phy/phy-stm32- > > > usbphyc.yaml so far... > > > > > > Thanks for helping me and best regards Johannes > > > > > > > > > > > > > > Best regards, > > > > Krzysztof > > > > > > > > > > > > > > > > > > -- > > > Pengutronix e.K. | Johannes Zink | > > > Steuerwalder Str. 21 | > > > https://ww/ > > > w.p%2F&data=05%7C01%7Cjun.li%40nxp.com%7C4cfd371e93084ee8c43908db3aa > > > 0a30c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63816823382425151 > > > 8%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi > > > I6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=EsH%2B63B32iGAJEzk9Tc > > > BYdcMFqnVLzr1D8t9D%2Bl0J5o%3D&reserved=0 > > > engutronix.de%2F&data=05%7C01%7Cjun.li%40nxp.com%7Ca2e1e5bb6a784de5 > > > 941d > > > 08db3a9847d0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638168197 > > > 9474 > > > 07580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL > > > CJBT > > > iI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Dq06I6tprUib5nOmp4p > > > DFdY > > > MgYULn8MLj5iHwVlQHMY%3D&reserved=0 | > > > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > > > Amtsgericht Hildesheim, HRA 2686| Fax: +49-5121-206917-5555 | > > > > > > > > -- > Pengutronix e.K. | Johannes Zink | > Steuerwalder Str. 21 | > https://www.p/ > engutronix.de%2F&data=05%7C01%7Cjun.li%40nxp.com%7C4cfd371e93084ee8c439 > 08db3aa0a30c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6381682338242 > 51518%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBT > iI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=80YR6G0A347G%2FMI9ksARn > VSqXVQXORt8pNBQjRoIBVY%3D&reserved=0 | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686| Fax: +49-5121-206917-5555 |
