[1/2] dt-bindings: phy: imx8mq-usb: add phy tuning properties

Message ID 20230405112118.1256151-2-j.zink@pengutronix.de
State New
Headers
Series Add phy tuning support for imx8mq-usb |

Commit Message

Johannes Zink April 5, 2023, 11:21 a.m. UTC
  Add optional properties for tuning of usb phy.

Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
 .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)
  

Comments

Krzysztof Kozlowski April 5, 2023, 11:51 a.m. UTC | #1
On 05/04/2023 13:21, Johannes Zink wrote:
> Add optional properties for tuning of usb phy.
> 
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> ---
>  .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> index e6f9f5540cc3..f452a41b4f32 100644
> --- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> @@ -35,6 +35,46 @@ properties:
>      description:
>        A phandle to the regulator for USB VBUS.
>  
> +  fsl,phy-tx-vref-tune:
> +    description:
> +      HS DC Voltage level adjustment

"Level" in what units?

> +    $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.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 2, 3]
> +
> +  fsl,phy-tx-vboost-level:
> +    description:
> +      TX Voltage Boost Level
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 2, 3]
> +
> +  fsl,phy-comp-dis-tune:
> +    description:
> +      Disconnect Threshold Adjustment
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 2, 3, 4, 5, 6, 7]
> +
> +  fsl,phy-pcs-tx-deemph-3p5db:
> +    description:
> +      TX De-Emphasis at 3.5 dB
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  fsl,phy-pcs-tx-swing-full:
> +    description:
> +      TX Amplitude

I have feeling you just pasted here short titles from datasheet. They
are not that helpful.



Best regards,
Krzysztof
  
Johannes Zink April 5, 2023, 12:14 p.m. UTC | #2
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?

> > +    $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?

> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [0, 1, 2, 3]
> > +
> > +  fsl,phy-tx-vboost-level:
> > +    description:
> > +      TX Voltage Boost Level
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [0, 2, 3]
> > +
> > +  fsl,phy-comp-dis-tune:
> > +    description:
> > +      Disconnect Threshold Adjustment
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [0, 1, 2, 3, 4, 5, 6, 7]
> > +
> > +  fsl,phy-pcs-tx-deemph-3p5db:
> > +    description:
> > +      TX De-Emphasis at 3.5 dB
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  fsl,phy-pcs-tx-swing-full:
> > +    description:
> > +      TX Amplitude
> 
> I have feeling you just pasted here short titles from datasheet. They
> are not that helpful.

ack, I will add some more text from the register description, since I
have no other source of information on these.

Best regards
Johannes

> 
> 
> 
> Best regards,
> Krzysztof
> 
> 
>
  
Krzysztof Kozlowski April 7, 2023, 9:03 a.m. UTC | #3
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://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml

> 
>>> +    $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://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml

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.

Best regards,
Krzysztof
  
Johannes Zink April 11, 2023, 2:22 p.m. UTC | #4
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://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml

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://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
> 
> 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...)


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
> 
> 
>
  
Jun Li April 11, 2023, 2:59 p.m. UTC | #5
> -----Original Message-----
> From: Johannes Zink <j.zink@pengutronix.de>
> Sent: Tuesday, April 11, 2023 10:23 PM
> To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>;
> vkoul@kernel.org; kishon@kernel.org; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> dl-linux-imx <linux-imx@nxp.com>; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; Jun Li <jun.li@nxp.com>; Bough Chen
> <haibo.chen@nxp.com>; linux-phy@lists.infradead.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicetree@vger.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%7C63816
> >
> 8197947407580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2lu
> >
> MzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=kY4dXkOHRs9k%
> > 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%7C63816
> >
> 8197947407580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2lu
> >
> MzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=kY4dXkOHRs9k%
> > 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.

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://www.p/
> engutronix.de%2F&data=05%7C01%7Cjun.li%40nxp.com%7Ca2e1e5bb6a784de5941d
> 08db3a9847d0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6381681979474
> 07580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBT
> iI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Dq06I6tprUib5nOmp4pDFdY
> MgYULn8MLj5iHwVlQHMY%3D&reserved=0    |
> 31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
> Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |
  
Johannes Zink April 11, 2023, 3:22 p.m. UTC | #6
Hi Jun,

On Tue, 2023-04-11 at 14:59 +0000, Jun Li wrote:
> 
> 
> > -----Original Message-----
> > From: Johannes Zink <j.zink@pengutronix.de>
> > Sent: Tuesday, April 11, 2023 10:23 PM
> > To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>;
> > vkoul@kernel.org; kishon@kernel.org; shawnguo@kernel.org;
> > s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> > dl-linux-imx <linux-imx@nxp.com>; robh+dt@kernel.org;
> > krzysztof.kozlowski+dt@linaro.org; Jun Li <jun.li@nxp.com>; Bough
> > Chen
> > <haibo.chen@nxp.com>; linux-phy@lists.infradead.org;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > devicetree@vger.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!

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://www.p/
> > 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    |
> 
> 
>
  
Rob Herring April 12, 2023, 1:39 p.m. UTC | #7
On Tue, Apr 11, 2023 at 04:22:37PM +0200, Johannes Zink wrote:
> 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://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
> 
> 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

Note that with standard unit suffixes, you don't need a type.

>   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.

We'd have to change the type for -percent to signed. That's possible, 
but for vendor specific properties there's not much advantage to use 
standard units instead of just using the register values directly.

Rob
  
Johannes Zink April 12, 2023, 2:32 p.m. UTC | #8
Hi Rob,

On 4/12/23 15:39, Rob Herring wrote:
> On Tue, Apr 11, 2023 at 04:22:37PM +0200, Johannes Zink wrote:
>> 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://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
>>
>> 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
> 
> Note that with standard unit suffixes, you don't need a type.
> 
>>    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.
> 
> We'd have to change the type for -percent to signed. That's possible,
> but for vendor specific properties there's not much advantage to use
> standard units instead of just using the register values directly.
> 

I don't have any objections to that, this is pretty much what I sent in 
my v1 patch <20230405112118.1256151-2-j.zink@pengutronix.de>, but 
Krzysztof requested to change the vendor specific properties to use 
property-units.

Would something along the lines of the st,trim-hs-current on 
Documentation/devicetree/bindings/phy/phy-stm32-usbphyc.yaml be 
acceptable (i.e. use an enum and annotate the meaning of the values in 
the description)?

I will, nevertheless, try to make the descriptions a bit more verbose in 
my v2 (wherever the datasheet gives me proper informations), as 
Krzysztof requested.

Best regards
Johannes

> Rob
>
  
Johannes Zink April 18, 2023, 9:42 a.m. UTC | #9
Hi Rob, hi Krzysztof,

On 4/12/23 16:32, Johannes Zink wrote:
> Hi Rob,
> 
> On 4/12/23 15:39, Rob Herring wrote:
>> On Tue, Apr 11, 2023 at 04:22:37PM +0200, Johannes Zink wrote:
>>> 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://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
>>>
>>> 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
>>
>> Note that with standard unit suffixes, you don't need a type.
>>
>>>    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.
>>
>> We'd have to change the type for -percent to signed. That's possible,
>> but for vendor specific properties there's not much advantage to use
>> standard units instead of just using the register values directly.
>>
> 
> I don't have any objections to that, this is pretty much what I sent in 
> my v1 patch <20230405112118.1256151-2-j.zink@pengutronix.de>, but 
> Krzysztof requested to change the vendor specific properties to use 
> property-units.
> 
> Would something along the lines of the st,trim-hs-current on 
> Documentation/devicetree/bindings/phy/phy-stm32-usbphyc.yaml be 
> acceptable (i.e. use an enum and annotate the meaning of the values in 
> the description)?
> 
> I will, nevertheless, try to make the descriptions a bit more verbose in 
> my v2 (wherever the datasheet gives me proper informations), as 
> Krzysztof requested.

gentle ping - any opinions on this? Shall I just send a V2 along the 
lines of the phy-stm32-usbphy.c?

Best regards
Johannes

> 
> Best regards
> Johannes
> 
>> Rob
>>
>
  
Jun Li April 26, 2023, 10:23 a.m. UTC | #10
> -----Original Message-----
> From: Johannes Zink <j.zink@pengutronix.de>
> Sent: Tuesday, April 11, 2023 11:23 PM
> To: Jun Li <jun.li@nxp.com>; Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org>; vkoul@kernel.org; kishon@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; Bough Chen <haibo.chen@nxp.com>;
> linux-phy@lists.infradead.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; devicetree@vger.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@pengutronix.de>
> > > Sent: Tuesday, April 11, 2023 10:23 PM
> > > To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>;
> > > vkoul@kernel.org; kishon@kernel.org; shawnguo@kernel.org;
> > > s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> > > dl-linux-imx <linux-imx@nxp.com>; robh+dt@kernel.org;
> > > krzysztof.kozlowski+dt@linaro.org; Jun Li <jun.li@nxp.com>; Bough
> > > Chen <haibo.chen@nxp.com>; linux-phy@lists.infradead.org;
> > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > > devicetree@vger.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    |
  
Johannes Zink May 4, 2023, 7:32 a.m. UTC | #11
Hi Rob, hi Krzysztof,

[snip]
 > gentle ping - any opinions on this? Shall I just send a V2 along the 
lines of the phy-stm32-usbphy.c?

This would be something like (example for the trim-hs-current, taken 
from pyh-stm32-usbphyc):

       st,trim-hs-current:
         description: |
           Controls HS driver current trimming for choke compensation
           - <0> = 18.87 mA target current / nominal + 0%
           - <1> = 19.165 mA target current / nominal + 1.56%
           - <2> = 19.46 mA target current / nominal + 3.12%
           - <3> = 19.755 mA target current / nominal + 4.68%
           - <4> = 20.05 mA target current / nominal + 6.24%
           - <5> = 20.345 mA target current / nominal + 7.8%
           - <6> = 20.64 mA target current / nominal + 9.36%
           - <7> = 20.935 mA target current / nominal + 10.92%
           - <8> = 21.23 mA target current / nominal + 12.48%
           - <9> = 21.525 mA target current / nominal + 14.04%
           - <10> = 21.82 mA target current / nominal + 15.6%
           - <11> = 22.115 mA target current / nominal + 17.16%
           - <12> = 22.458 mA target current / nominal + 19.01%
           - <13> = 22.755 mA target current / nominal + 20.58%
           - <14> = 23.052 mA target current / nominal + 22.16%
           - <15> = 23.348 mA target current / nominal + 23.73%
         $ref: /schemas/types.yaml#/definitions/uint32
         minimum: 0
         maximum: 15
         default: 0

If you think something along these lines is acceptable, I would like to 
prepare and send a V2.

Best regards
Johannes

[snip]
  
Krzysztof Kozlowski May 4, 2023, 8:13 a.m. UTC | #12
On 04/05/2023 09:32, Johannes Zink wrote:
> Hi Rob, hi Krzysztof,
> 
> [snip]
>  > gentle ping - any opinions on this? Shall I just send a V2 along the 
> lines of the phy-stm32-usbphy.c?
> 
> This would be something like (example for the trim-hs-current, taken 
> from pyh-stm32-usbphyc):
> 
>        st,trim-hs-current:
>          description: |
>            Controls HS driver current trimming for choke compensation
>            - <0> = 18.87 mA target current / nominal + 0%
>            - <1> = 19.165 mA target current / nominal + 1.56%
>            - <2> = 19.46 mA target current / nominal + 3.12%
>            - <3> = 19.755 mA target current / nominal + 4.68%
>            - <4> = 20.05 mA target current / nominal + 6.24%
>            - <5> = 20.345 mA target current / nominal + 7.8%
>            - <6> = 20.64 mA target current / nominal + 9.36%
>            - <7> = 20.935 mA target current / nominal + 10.92%
>            - <8> = 21.23 mA target current / nominal + 12.48%
>            - <9> = 21.525 mA target current / nominal + 14.04%
>            - <10> = 21.82 mA target current / nominal + 15.6%
>            - <11> = 22.115 mA target current / nominal + 17.16%
>            - <12> = 22.458 mA target current / nominal + 19.01%
>            - <13> = 22.755 mA target current / nominal + 20.58%
>            - <14> = 23.052 mA target current / nominal + 22.16%
>            - <15> = 23.348 mA target current / nominal + 23.73%
>          $ref: /schemas/types.yaml#/definitions/uint32
>          minimum: 0
>          maximum: 15
>          default: 0
> 
> If you think something along these lines is acceptable, I would like to 
> prepare and send a V2.
> 

Go with Rob's approach, so something as you wrote above.

Best regards,
Krzysztof
  

Patch

diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
index e6f9f5540cc3..f452a41b4f32 100644
--- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
@@ -35,6 +35,46 @@  properties:
     description:
       A phandle to the regulator for USB VBUS.
 
+  fsl,phy-tx-vref-tune:
+    description:
+      HS DC Voltage level adjustment
+    $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
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2, 3]
+
+  fsl,phy-tx-vboost-level:
+    description:
+      TX Voltage Boost Level
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 2, 3]
+
+  fsl,phy-comp-dis-tune:
+    description:
+      Disconnect Threshold Adjustment
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2, 3, 4, 5, 6, 7]
+
+  fsl,phy-pcs-tx-deemph-3p5db:
+    description:
+      TX De-Emphasis at 3.5 dB
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  fsl,phy-pcs-tx-swing-full:
+    description:
+      TX Amplitude
+    $ref: /schemas/types.yaml#/definitions/uint32
+
 required:
   - compatible
   - reg