linux-kernel - RE: [PATCH 1/2] dt-bindings: phy: imx8mq-usb: add phy tuning properties

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <PA4PR04MB9640B33A3B4A2EFC9F3EBBE489659@PA4PR04MB9640.eurprd04.prod.outlook.com>
Date:   Wed, 26 Apr 2023 10:23:51 +0000
From:   Jun Li <jun.li@....com>
To:     Johannes Zink <j.zink@...gutronix.de>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        "vkoul@...nel.org" <vkoul@...nel.org>,
        "kishon@...nel.org" <kishon@...nel.org>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        dl-linux-imx <linux-imx@....com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>,
        Bough Chen <haibo.chen@....com>,
        "linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: RE: [PATCH 1/2] dt-bindings: phy: imx8mq-usb: add phy tuning
 properties



> -----Original Message-----
> From: Johannes Zink <j.zink@...gutronix.de>
> Sent: Tuesday, April 11, 2023 11:23 PM
> To: Jun Li <jun.li@....com>; Krzysztof Kozlowski
> <krzysztof.kozlowski@...aro.org>; vkoul@...nel.org; kishon@...nel.org;
> shawnguo@...nel.org; s.hauer@...gutronix.de; kernel@...gutronix.de;
> festevam@...il.com; dl-linux-imx <linux-imx@....com>; robh+dt@...nel.org;
> krzysztof.kozlowski+dt@...aro.org; Bough Chen <haibo.chen@....com>;
> linux-phy@...ts.infradead.org; linux-arm-kernel@...ts.infradead.org;
> linux-kernel@...r.kernel.org; devicetree@...r.kernel.org
> 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@...gutronix.de>
> > > Sent: Tuesday, April 11, 2023 10:23 PM
> > > To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>;
> > > vkoul@...nel.org; kishon@...nel.org; shawnguo@...nel.org;
> > > s.hauer@...gutronix.de; kernel@...gutronix.de; festevam@...il.com;
> > > dl-linux-imx <linux-imx@....com>; robh+dt@...nel.org;
> > > krzysztof.kozlowski+dt@...aro.org; Jun Li <jun.li@....com>; Bough
> > > Chen <haibo.chen@....com>; linux-phy@...ts.infradead.org;
> > > linux-arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org;
> > > devicetree@...r.kernel.org
> > > 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    |