diff mbox series

[2/3] dt-bindings: usb: ci-hdrc-usb2: add samsung,picophy-rise-fall-time-adjust property

Message ID 20230626092952.1115834-2-xu.yang_2@nxp.com (mailing list archive)
State Superseded
Headers show
Series [1/3] usb: chipidea: imx: improve logic if samsung,picophy-* parameter is 0 | expand

Commit Message

Xu Yang June 26, 2023, 9:29 a.m. UTC
The samsung,picophy-rise-fall-time-adjust property can help to adjust the
rise/fall times of the high-speed transmitter waveform. The value can be
0~3.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 .../devicetree/bindings/usb/ci-hdrc-usb2.yaml          | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Krzysztof Kozlowski June 26, 2023, 3:44 p.m. UTC | #1
On 26/06/2023 11:29, Xu Yang wrote:
> The samsung,picophy-rise-fall-time-adjust property can help to adjust the
> rise/fall times of the high-speed transmitter waveform. The value can be
> 0~3.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml          | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> index 782402800d4a..d84c66c342ac 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> @@ -292,6 +292,16 @@ properties:
>      minimum: 0x0
>      maximum: 0xf
>  
> +  samsung,picophy-rise-fall-time-adjust:
> +    description:
> +      HS Transmitter Rise/Fall Time Adjustment. Adjust the rise/fall times

Adjust with/by what? What are the units?

> +      of the high-speed transmitter waveform. The range is from 0x0 to 0x3,
> +      the default value is 0x1. Details can refer to TXRISETUNE0 bit of

default: 1

Don't repeat constraints in free form text.

> +      USBNC_n_PHY_CFG1.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0x0
> +    maximum: 0x3

Why "samsung" prefix? If this is specific to samsung, make it
specific/narrowed like other properties.

There are no Samsung compatibles here, so what is exactly here made by
Samsung? Which device?

Best regards,
Krzysztof
Xu Yang June 27, 2023, 3:10 a.m. UTC | #2
Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Monday, June 26, 2023 11:45 PM
> To: Xu Yang <xu.yang_2@nxp.com>; peter.chen@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> gregkh@linuxfoundation.org
> Cc: conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>; Peng Fan
> <peng.fan@nxp.com>; linux-usb@vger.kernel.org; devicetree@vger.kernel.org; Jun Li <jun.li@nxp.com>
> Subject: [EXT] Re: [PATCH 2/3] dt-bindings: usb: ci-hdrc-usb2: add samsung,picophy-rise-fall-time-adjust property
> 
> 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
> 
> 
> On 26/06/2023 11:29, Xu Yang wrote:
> > The samsung,picophy-rise-fall-time-adjust property can help to adjust the
> > rise/fall times of the high-speed transmitter waveform. The value can be
> > 0~3.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml          | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
> hdrc-usb2.yaml
> > index 782402800d4a..d84c66c342ac 100644
> > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> > @@ -292,6 +292,16 @@ properties:
> >      minimum: 0x0
> >      maximum: 0xf
> >
> > +  samsung,picophy-rise-fall-time-adjust:
> > +    description:
> > +      HS Transmitter Rise/Fall Time Adjustment. Adjust the rise/fall times
> 
> Adjust with/by what? What are the units?

This property is used to adjust the rise/fall time of the high-speed
transmitter waveform. It has no unit. According to the description of
USBNC_n_PHY_CFG1 register, the rise/fall time will be increased or
decreased by a certain percentage relative to design default time if
a value is given to this property.

The actions as below:
  - 0: -10%
  - 1: design default 
  - 2: +15%
  - 3: +20%

> 
> > +      of the high-speed transmitter waveform. The range is from 0x0 to 0x3,
> > +      the default value is 0x1. Details can refer to TXRISETUNE0 bit of
> 
> default: 1
> 
> Don't repeat constraints in free form text.

Okay, will remove.

> 
> > +      USBNC_n_PHY_CFG1.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    minimum: 0x0
> > +    maximum: 0x3
> 
> Why "samsung" prefix? If this is specific to samsung, make it
> specific/narrowed like other properties.
> 
> There are no Samsung compatibles here, so what is exactly here made by
> Samsung? Which device?

Reviewed the design, this is not specific to samsung. I will rename it to
fsl,picophy-rise-fall-time-adjust since it's related to imx chip.

Thanks,
Xu Yang

> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski June 27, 2023, 6:23 a.m. UTC | #3
On 27/06/2023 05:10, Xu Yang wrote:
> Hi Krzysztof,
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: Monday, June 26, 2023 11:45 PM
>> To: Xu Yang <xu.yang_2@nxp.com>; peter.chen@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
>> gregkh@linuxfoundation.org
>> Cc: conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>; Peng Fan
>> <peng.fan@nxp.com>; linux-usb@vger.kernel.org; devicetree@vger.kernel.org; Jun Li <jun.li@nxp.com>
>> Subject: [EXT] Re: [PATCH 2/3] dt-bindings: usb: ci-hdrc-usb2: add samsung,picophy-rise-fall-time-adjust property
>>
>> 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
>>
>>
>> On 26/06/2023 11:29, Xu Yang wrote:
>>> The samsung,picophy-rise-fall-time-adjust property can help to adjust the
>>> rise/fall times of the high-speed transmitter waveform. The value can be
>>> 0~3.
>>>
>>> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>>> ---
>>>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml          | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
>> hdrc-usb2.yaml
>>> index 782402800d4a..d84c66c342ac 100644
>>> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>>> @@ -292,6 +292,16 @@ properties:
>>>      minimum: 0x0
>>>      maximum: 0xf
>>>
>>> +  samsung,picophy-rise-fall-time-adjust:
>>> +    description:
>>> +      HS Transmitter Rise/Fall Time Adjustment. Adjust the rise/fall times
>>
>> Adjust with/by what? What are the units?
> 
> This property is used to adjust the rise/fall time of the high-speed
> transmitter waveform. It has no unit. According to the description of
> USBNC_n_PHY_CFG1 register, the rise/fall time will be increased or
> decreased by a certain percentage relative to design default time if
> a value is given to this property.
> 
> The actions as below:
>   - 0: -10%
>   - 1: design default 
>   - 2: +15%
>   - 3: +20%

Include it then in the description or even make the property -percent:
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml


Best regards,
Krzysztof
Xu Yang June 27, 2023, 9:16 a.m. UTC | #4
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Tuesday, June 27, 2023 2:24 PM
> To: Xu Yang <xu.yang_2@nxp.com>; peter.chen@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> gregkh@linuxfoundation.org
> Cc: conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>; Peng Fan
> <peng.fan@nxp.com>; linux-usb@vger.kernel.org; devicetree@vger.kernel.org; Jun Li <jun.li@nxp.com>
> Subject: Re: [EXT] Re: [PATCH 2/3] dt-bindings: usb: ci-hdrc-usb2: add samsung,picophy-rise-fall-time-adjust property
>
> 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
>
>
> On 27/06/2023 05:10, Xu Yang wrote:
> > Hi Krzysztof,
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> Sent: Monday, June 26, 2023 11:45 PM
> >> To: Xu Yang <xu.yang_2@nxp.com>; peter.chen@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> >> gregkh@linuxfoundation.org
> >> Cc: conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>; Peng
> Fan
> >> <peng.fan@nxp.com>; linux-usb@vger.kernel.org; devicetree@vger.kernel.org; Jun Li <jun.li@nxp.com>
> >> Subject: [EXT] Re: [PATCH 2/3] dt-bindings: usb: ci-hdrc-usb2: add samsung,picophy-rise-fall-time-adjust property
> >>
> >> 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
> >>
> >>
> >> On 26/06/2023 11:29, Xu Yang wrote:
> >>> The samsung,picophy-rise-fall-time-adjust property can help to adjust the
> >>> rise/fall times of the high-speed transmitter waveform. The value can be
> >>> 0~3.
> >>>
> >>> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >>> ---
> >>>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml          | 10 ++++++++++
> >>>  1 file changed, 10 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
> >> hdrc-usb2.yaml
> >>> index 782402800d4a..d84c66c342ac 100644
> >>> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> >>> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> >>> @@ -292,6 +292,16 @@ properties:
> >>>      minimum: 0x0
> >>>      maximum: 0xf
> >>>
> >>> +  samsung,picophy-rise-fall-time-adjust:
> >>> +    description:
> >>> +      HS Transmitter Rise/Fall Time Adjustment. Adjust the rise/fall times
> >>
> >> Adjust with/by what? What are the units?
> >
> > This property is used to adjust the rise/fall time of the high-speed
> > transmitter waveform. It has no unit. According to the description of
> > USBNC_n_PHY_CFG1 register, the rise/fall time will be increased or
> > decreased by a certain percentage relative to design default time if
> > a value is given to this property.
> >
> > The actions as below:
> >   - 0: -10%
> >   - 1: design default
> >   - 2: +15%
> >   - 3: +20%
>
> Include it then in the description or even make the property -percent:

Okay, will add this in v2.

Thanks,
Xu Yang

> https://github.com/devicetree-org/dt-
> schema%2Fblob%2Fmain%2Fdtschema%2Fschemas%2Fproperty-
> units.yaml&data=05%7C01%7Cxu.yang_2%40nxp.com%7C9ba7b99a7b6849ec138e08db76d71211%7C686ea1d3bc2b4c6fa92
> cd99c5c301635%7C0%7C0%7C638234438320292819%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2lu
> MzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=j3mETXe6ZMmeDxFbZ3N1hQaIb4wSt3IrIIbxm8f%2F
> S1g%3D&reserved=0
>
>
> Best regards,
> Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
index 782402800d4a..d84c66c342ac 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
@@ -292,6 +292,16 @@  properties:
     minimum: 0x0
     maximum: 0xf
 
+  samsung,picophy-rise-fall-time-adjust:
+    description:
+      HS Transmitter Rise/Fall Time Adjustment. Adjust the rise/fall times
+      of the high-speed transmitter waveform. The range is from 0x0 to 0x3,
+      the default value is 0x1. Details can refer to TXRISETUNE0 bit of
+      USBNC_n_PHY_CFG1.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0x0
+    maximum: 0x3
+
   usb-phy:
     description: phandle for the PHY device. Use "phys" instead.
     $ref: /schemas/types.yaml#/definitions/phandle