* [PATCH v3 0/2] Add motorcomm phy pad-driver-strength-cfg support @ 2023-05-26 9:05 Samin Guo 2023-05-26 9:05 ` [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg Samin Guo 2023-05-26 9:05 ` [PATCH v3 2/2] net: phy: motorcomm: Add pad drive strength cfg support Samin Guo 0 siblings, 2 replies; 12+ messages in thread From: Samin Guo @ 2023-05-26 9:05 UTC (permalink / raw) To: linux-kernel, devicetree, netdev, Peter Geis, Frank Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Heiner Kallweit, Russell King, Samin Guo, Yanhong Wang The motorcomm phy (YT8531) supports the ability to adjust the drive strength of the rx_clk/rx_data, and the default strength may not be suitable for all boards. So add configurable options to better match the boards.(e.g. StarFive VisionFive 2) The first patch adds a description of dt-bingding, and the second patch adds YT8531's parsing and settings for pad-driver-strength-cfg. 'Magic numbers' are used because we haven't been able to get real units from motorcomm yet, but similar usage has been found in Documentation/devicetree/bindings/net/qca,ar803x.yaml. qca,clk-out-strength: description: Clock output driver strength. $ref: /schemas/types.yaml#/definitions/uint32 enum: [0, 1, 2] Changes since v2: Patch 2: - Readjusted the order of YT8531_RGMII_xxx to below YTPHY_PAD_DRIVE_STRENGTH_REG (by Frank Sae) - Reversed Christmas tree, sort these longest first, shortest last (by Andrew Lunn) - Rebased on tag v6.4 Changes since v1: Patch 1: - Renamed "rx-xxx-driver-strength" to "motorcomm,rx-xxx-driver-strength" (by Frank Sae) Patch 2: - Added default values for rxc/rxd driver strength (by Frank Sea/Andrew Lunn) - Added range checking when val is in DT (by Frank Sea/Andrew Lunn) Previous versions: v1 - https://patchwork.kernel.org/project/netdevbpf/cover/20230426063541.15378-1-samin.guo@starfivetech.com v2 - https://patchwork.kernel.org/project/netdevbpf/cover/20230505090558.2355-1-samin.guo@starfivetech.com Samin Guo (2): dt-bindings: net: motorcomm: Add pad driver strength cfg net: phy: motorcomm: Add pad drive strength cfg support .../bindings/net/motorcomm,yt8xxx.yaml | 12 +++++ drivers/net/phy/motorcomm.c | 46 +++++++++++++++++++ 2 files changed, 58 insertions(+) base-commit: 3201bee3a7171617da7cdbce06c428fb628c9944 -- 2.17.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg 2023-05-26 9:05 [PATCH v3 0/2] Add motorcomm phy pad-driver-strength-cfg support Samin Guo @ 2023-05-26 9:05 ` Samin Guo 2023-05-26 19:41 ` Conor Dooley 2023-05-26 9:05 ` [PATCH v3 2/2] net: phy: motorcomm: Add pad drive strength cfg support Samin Guo 1 sibling, 1 reply; 12+ messages in thread From: Samin Guo @ 2023-05-26 9:05 UTC (permalink / raw) To: linux-kernel, devicetree, netdev, Peter Geis, Frank Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Heiner Kallweit, Russell King, Samin Guo, Yanhong Wang The motorcomm phy (YT8531) supports the ability to adjust the drive strength of the rx_clk/rx_data, the value range of pad driver strength is 0 to 7. Signed-off-by: Samin Guo <samin.guo@starfivetech.com> --- .../devicetree/bindings/net/motorcomm,yt8xxx.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml index 157e3bbcaf6f..29a1997a1577 100644 --- a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml @@ -52,6 +52,18 @@ properties: for a timer. type: boolean + motorcomm,rx-clk-driver-strength: + description: drive strength of rx_clk pad. + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] + default: 3 + + motorcomm,rx-data-driver-strength: + description: drive strength of rx_data/rx_ctl rgmii pad. + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] + default: 3 + motorcomm,tx-clk-adj-enabled: description: | This configuration is mainly to adapt to VF2 with JH7110 SoC. -- 2.17.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg 2023-05-26 9:05 ` [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg Samin Guo @ 2023-05-26 19:41 ` Conor Dooley 2023-05-29 3:46 ` Guo Samin 0 siblings, 1 reply; 12+ messages in thread From: Conor Dooley @ 2023-05-26 19:41 UTC (permalink / raw) To: Samin Guo Cc: linux-kernel, devicetree, netdev, Peter Geis, Frank, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Heiner Kallweit, Russell King, Yanhong Wang [-- Attachment #1: Type: text/plain, Size: 1769 bytes --] On Fri, May 26, 2023 at 05:05:01PM +0800, Samin Guo wrote: > The motorcomm phy (YT8531) supports the ability to adjust the drive > strength of the rx_clk/rx_data, the value range of pad driver > strength is 0 to 7. > > Signed-off-by: Samin Guo <samin.guo@starfivetech.com> > --- > .../devicetree/bindings/net/motorcomm,yt8xxx.yaml | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml > index 157e3bbcaf6f..29a1997a1577 100644 > --- a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml > +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml > @@ -52,6 +52,18 @@ properties: > for a timer. > type: boolean > > + motorcomm,rx-clk-driver-strength: > + description: drive strength of rx_clk pad. > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] I think you should use minimum & maximum instead of these listed out enums. You have also had this comment since v1 & were reminded of it on v2 by Krzysztof: "What do the numbers mean? What are the units? mA?" This information should go into the binding, not sit in a thread on a mailing list that noone will look at when trying to write a DT :( Thanks, Conor. > + default: 3 > + > + motorcomm,rx-data-driver-strength: > + description: drive strength of rx_data/rx_ctl rgmii pad. > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] > + default: 3 > + > motorcomm,tx-clk-adj-enabled: > description: | > This configuration is mainly to adapt to VF2 with JH7110 SoC. > -- > 2.17.1 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg 2023-05-26 19:41 ` Conor Dooley @ 2023-05-29 3:46 ` Guo Samin 2023-06-20 3:09 ` Guo Samin 0 siblings, 1 reply; 12+ messages in thread From: Guo Samin @ 2023-05-29 3:46 UTC (permalink / raw) To: Conor Dooley Cc: linux-kernel, devicetree, netdev, Peter Geis, Frank, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Heiner Kallweit, Russell King, Yanhong Wang Re: [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg From: Conor Dooley <conor@kernel.org> to: Samin Guo <samin.guo@starfivetech.com> data: 2023/5/27 > On Fri, May 26, 2023 at 05:05:01PM +0800, Samin Guo wrote: >> The motorcomm phy (YT8531) supports the ability to adjust the drive >> strength of the rx_clk/rx_data, the value range of pad driver >> strength is 0 to 7. >> >> Signed-off-by: Samin Guo <samin.guo@starfivetech.com> >> --- >> .../devicetree/bindings/net/motorcomm,yt8xxx.yaml | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml >> index 157e3bbcaf6f..29a1997a1577 100644 >> --- a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml >> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml >> @@ -52,6 +52,18 @@ properties: >> for a timer. >> type: boolean >> >> + motorcomm,rx-clk-driver-strength: >> + description: drive strength of rx_clk pad. >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] > > I think you should use minimum & maximum instead of these listed out > enums. Thanks Conor, This can be improved in the next version. You have also had this comment since v1 & were reminded of it on > v2 by Krzysztof: "What do the numbers mean? What are the units? mA?" > The good news is that we just got some data about units from Motorcomm. Maybe I can post the data show of the unit later after I get the complete data. > This information should go into the binding, not sit in a thread on a > mailing list that noone will look at when trying to write a DT :( > > Thanks, > Conor. > Yes,when we have the complete 'unit' data, it will be placed in DT. Best regards, Samin >> + default: 3 >> + >> + motorcomm,rx-data-driver-strength: >> + description: drive strength of rx_data/rx_ctl rgmii pad. >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] >> + default: 3 >> + >> motorcomm,tx-clk-adj-enabled: >> description: | >> This configuration is mainly to adapt to VF2 with JH7110 SoC. >> -- >> 2.17.1 >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg 2023-05-29 3:46 ` Guo Samin @ 2023-06-20 3:09 ` Guo Samin 2023-06-20 8:26 ` Conor Dooley 2023-06-20 13:29 ` Andrew Lunn 0 siblings, 2 replies; 12+ messages in thread From: Guo Samin @ 2023-06-20 3:09 UTC (permalink / raw) To: Andrew Lunn, Conor Dooley Cc: linux-kernel, devicetree, netdev, Peter Geis, Frank, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Heiner Kallweit, Russell King, Yanhong Wang Re: [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg From: Guo Samin <samin.guo@starfivetech.com> to: Conor Dooley <conor@kernel.org>; Andrew Lunn <andrew@lunn.ch> data: 2023/5/29 > Re: [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg > From: Conor Dooley <conor@kernel.org> > to: Samin Guo <samin.guo@starfivetech.com> > data: 2023/5/27 > >> On Fri, May 26, 2023 at 05:05:01PM +0800, Samin Guo wrote: >>> The motorcomm phy (YT8531) supports the ability to adjust the drive >>> strength of the rx_clk/rx_data, the value range of pad driver >>> strength is 0 to 7. >>> >>> Signed-off-by: Samin Guo <samin.guo@starfivetech.com> >>> --- >>> .../devicetree/bindings/net/motorcomm,yt8xxx.yaml | 12 ++++++++++++ >>> 1 file changed, 12 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml >>> index 157e3bbcaf6f..29a1997a1577 100644 >>> --- a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml >>> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml >>> @@ -52,6 +52,18 @@ properties: >>> for a timer. >>> type: boolean >>> >>> + motorcomm,rx-clk-driver-strength: >>> + description: drive strength of rx_clk pad. >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] >> >> I think you should use minimum & maximum instead of these listed out >> enums. > > Thanks Conor, This can be improved in the next version. > > You have also had this comment since v1 & were reminded of it on >> v2 by Krzysztof: "What do the numbers mean? What are the units? mA?" >> > > > The good news is that we just got some data about units from Motorcomm. > Maybe I can post the data show of the unit later after I get the complete data. > Hi Andrew & Conor, Sorry, haven't updated in a while. I just got the detailed data of Driver Strength(DS) from Motorcomm , which applies to both rx_clk and rx_data. |----------------------| | ds map table | |----------------------| | DS(3b) | Current (mA)| |--------|-------------| | 000 | 1.20 | | 001 | 2.10 | | 010 | 2.70 | | 011 | 2.91 | | 100 | 3.11 | | 101 | 3.60 | | 110 | 3.97 | | 111 | 4.35 | |--------|-------------| Since these currents are not integer values and have no regularity, it is not very good to use in the drive/dts in my opinion. Therefore, I tend to continue to use DS(0-7) in dts/driver, and adding a description of the current value corresponding to DS in dt-bindings. Like This: + motorcomm,rx-clk-driver-strength: + description: drive strength of rx_clk pad. + |----------------------| + | rx_clk ds map table | + |----------------------| + | DS(3b) | Current (mA)| + | 000 | 1.20 | + | 001 | 2.10 | + | 010 | 2.70 | + | 011 | 2.91 | + | 100 | 3.11 | + | 101 | 3.60 | + | 110 | 3.97 | + | 111 | 4.35 | + |--------|-------------| + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] + default: 3 + + motorcomm,rx-data-driver-strength: + description: drive strength of rx_data/rx_ctl rgmii pad. + |----------------------| + | rx_data ds map table | + |----------------------| + | DS(3b) | Current (mA)| + | 000 | 1.20 | + | 001 | 2.10 | + | 010 | 2.70 | + | 011 | 2.91 | + | 100 | 3.11 | + | 101 | 3.60 | + | 110 | 3.97 | + | 111 | 4.35 | + |--------|-------------| + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] + default: 3 + Or use minimum & maximum instead of these listed out enums(Suggested by Conor) + motorcomm,rx-clk-driver-strength: + $ref: /schemas/types.yaml#/definitions/uint32 + default: 3 + minimum: 0 + maximum: 7 + description: drive strength of rx_clk pad. + |----------------------| + | rx_clk ds map table | + |----------------------| + | DS(3b) | Current (mA)| + | 000 | 1.20 | + | 001 | 2.10 | + | 010 | 2.70 | + | 011 | 2.91 | + | 100 | 3.11 | + | 101 | 3.60 | + | 110 | 3.97 | + | 111 | 4.35 | + |--------|-------------| + + motorcomm,rx-data-driver-strength: + $ref: /schemas/types.yaml#/definitions/uint32 + default: 3 + minimum: 0 + maximum: 7 + description: drive strength of rx_data/rx_ctl rgmii pad. + |----------------------| + | rx_data ds map table | + |----------------------| + | DS(3b) | Current (mA)| + | 000 | 1.20 | + | 001 | 2.10 | + | 010 | 2.70 | + | 011 | 2.91 | + | 100 | 3.11 | + | 101 | 3.60 | + | 110 | 3.97 | + | 111 | 4.35 | + |--------|-------------| + Looking forward to your suggestions. Best regards, Samin > > >> This information should go into the binding, not sit in a thread on a >> mailing list that noone will look at when trying to write a DT :( >> >> Thanks, >> Conor. >> > > Yes,when we have the complete 'unit' data, it will be placed in DT. > > Best regards, > Samin > >>> + default: 3 >>> + >>> + motorcomm,rx-data-driver-strength: >>> + description: drive strength of rx_data/rx_ctl rgmii pad. >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] >>> + default: 3 >>> + >>> motorcomm,tx-clk-adj-enabled: >>> description: | >>> This configuration is mainly to adapt to VF2 with JH7110 SoC. >>> -- >>> 2.17.1 >>> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg 2023-06-20 3:09 ` Guo Samin @ 2023-06-20 8:26 ` Conor Dooley 2023-06-20 8:55 ` Krzysztof Kozlowski 2023-06-20 13:29 ` Andrew Lunn 1 sibling, 1 reply; 12+ messages in thread From: Conor Dooley @ 2023-06-20 8:26 UTC (permalink / raw) To: Guo Samin Cc: Andrew Lunn, Conor Dooley, linux-kernel, devicetree, netdev, Peter Geis, Frank, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Heiner Kallweit, Russell King, Yanhong Wang [-- Attachment #1: Type: text/plain, Size: 3111 bytes --] Hey, On Tue, Jun 20, 2023 at 11:09:52AM +0800, Guo Samin wrote: > From: Guo Samin <samin.guo@starfivetech.com> > > From: Conor Dooley <conor@kernel.org> > >> On Fri, May 26, 2023 at 05:05:01PM +0800, Samin Guo wrote: > >>> The motorcomm phy (YT8531) supports the ability to adjust the drive > >>> strength of the rx_clk/rx_data, the value range of pad driver > >>> strength is 0 to 7. > >>> + motorcomm,rx-clk-driver-strength: > >>> + description: drive strength of rx_clk pad. > >>> + $ref: /schemas/types.yaml#/definitions/uint32 > >>> + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] > >> > >> I think you should use minimum & maximum instead of these listed out > >> enums. > > You have also had this comment since v1 & were reminded of it on > >> v2 by Krzysztof: "What do the numbers mean? What are the units? mA?" > > The good news is that we just got some data about units from Motorcomm. > > Maybe I can post the data show of the unit later after I get the complete data. > Sorry, haven't updated in a while. NW chief. > I just got the detailed data of Driver Strength(DS) from Motorcomm , which applies to both rx_clk and rx_data. > > |----------------------| > | ds map table | > |----------------------| > | DS(3b) | Current (mA)| > |--------|-------------| > | 000 | 1.20 | > | 001 | 2.10 | > | 010 | 2.70 | > | 011 | 2.91 | > | 100 | 3.11 | > | 101 | 3.60 | > | 110 | 3.97 | > | 111 | 4.35 | > |--------|-------------| > > Since these currents are not integer values and have no regularity, > it is not very good to use in the drive/dts in my opinion. Who says you have to use mA? What about uA? > Therefore, I tend to continue to use DS(0-7) in dts/driver, and adding > a description of the current value corresponding to DS in dt-bindings. I think this goes against not putting register values into the dts & that the accurate description of the hardware are the currents. > Like This: > > + motorcomm,rx-clk-driver-strength: > + description: drive strength of rx_clk pad. You need "description: |" to preserve the formatting if you add tables, but I don't think that this is a good idea. Put the values in here that describe the hardware (IOW the currents) and then you don't need to have this table. > + |----------------------| > + | rx_clk ds map table | > + |----------------------| > + | DS(3b) | Current (mA)| > + | 000 | 1.20 | > + | 001 | 2.10 | > + | 010 | 2.70 | > + | 011 | 2.91 | > + | 100 | 3.11 | > + | 101 | 3.60 | > + | 110 | 3.97 | > + | 111 | 4.35 | > + |--------|-------------| > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] > + default: 3 > + > Or use minimum & maximum instead of these listed out enums With the actual current values, enum rather than min + max. Cheers, Conor. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg 2023-06-20 8:26 ` Conor Dooley @ 2023-06-20 8:55 ` Krzysztof Kozlowski 2023-06-21 3:03 ` Guo Samin 0 siblings, 1 reply; 12+ messages in thread From: Krzysztof Kozlowski @ 2023-06-20 8:55 UTC (permalink / raw) To: Conor Dooley, Guo Samin Cc: Andrew Lunn, Conor Dooley, linux-kernel, devicetree, netdev, Peter Geis, Frank, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Heiner Kallweit, Russell King, Yanhong Wang On 20/06/2023 10:26, Conor Dooley wrote: > Hey, > > On Tue, Jun 20, 2023 at 11:09:52AM +0800, Guo Samin wrote: >> From: Guo Samin <samin.guo@starfivetech.com> >>> From: Conor Dooley <conor@kernel.org> >>>> On Fri, May 26, 2023 at 05:05:01PM +0800, Samin Guo wrote: >>>>> The motorcomm phy (YT8531) supports the ability to adjust the drive >>>>> strength of the rx_clk/rx_data, the value range of pad driver >>>>> strength is 0 to 7. > >>>>> + motorcomm,rx-clk-driver-strength: >>>>> + description: drive strength of rx_clk pad. >>>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>>> + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] >>>> >>>> I think you should use minimum & maximum instead of these listed out >>>> enums. > >>> You have also had this comment since v1 & were reminded of it on >>>> v2 by Krzysztof: "What do the numbers mean? What are the units? mA?" > >>> The good news is that we just got some data about units from Motorcomm. >>> Maybe I can post the data show of the unit later after I get the complete data. > >> Sorry, haven't updated in a while. > > NW chief. > >> I just got the detailed data of Driver Strength(DS) from Motorcomm , which applies to both rx_clk and rx_data. >> >> |----------------------| >> | ds map table | >> |----------------------| >> | DS(3b) | Current (mA)| >> |--------|-------------| >> | 000 | 1.20 | >> | 001 | 2.10 | >> | 010 | 2.70 | >> | 011 | 2.91 | >> | 100 | 3.11 | >> | 101 | 3.60 | >> | 110 | 3.97 | >> | 111 | 4.35 | >> |--------|-------------| >> >> Since these currents are not integer values and have no regularity, There is no mA unit in DT schema, so I don't see what by "not integer values". 1200 uA is an integer. >> it is not very good to use in the drive/dts in my opinion. > > Who says you have to use mA? What about uA? Yep > >> Therefore, I tend to continue to use DS(0-7) in dts/driver, and adding >> a description of the current value corresponding to DS in dt-bindings. > > I think this goes against not putting register values into the dts & > that the accurate description of the hardware are the currents. For vendor properties register values are often accepted, but logical unit is much more readable in the DTS. Also allows further customization or extending when new variant appears. You cannot do extend a property easily when it holds a register value, without changing the meaning per variant. > >> Like This: >> >> + motorcomm,rx-clk-driver-strength: >> + description: drive strength of rx_clk pad. > > You need "description: |" to preserve the formatting if you add tables, > but I don't think that this is a good idea. Put the values in here that > describe the hardware (IOW the currents) and then you don't need to have > this table. > >> + |----------------------| >> + | rx_clk ds map table | >> + |----------------------| >> + | DS(3b) | Current (mA)| >> + | 000 | 1.20 | >> + | 001 | 2.10 | >> + | 010 | 2.70 | >> + | 011 | 2.91 | >> + | 100 | 3.11 | >> + | 101 | 3.60 | >> + | 110 | 3.97 | >> + | 111 | 4.35 | >> + |--------|-------------| >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] >> + default: 3 >> + > >> Or use minimum & maximum instead of these listed out enums > > With the actual current values, enum rather than min + max. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg 2023-06-20 8:55 ` Krzysztof Kozlowski @ 2023-06-21 3:03 ` Guo Samin 0 siblings, 0 replies; 12+ messages in thread From: Guo Samin @ 2023-06-21 3:03 UTC (permalink / raw) To: Krzysztof Kozlowski, Conor Dooley Cc: Andrew Lunn, Conor Dooley, linux-kernel, devicetree, netdev, Peter Geis, Frank, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Heiner Kallweit, Russell King, Yanhong Wang -------- 原始信息 -------- 主题: Re: [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 收件人: Conor Dooley <conor.dooley@microchip.com>, Guo Samin <samin.guo@starfivetech.com> 日期: 2023/6/20 > On 20/06/2023 10:26, Conor Dooley wrote: >> Hey, >> >> On Tue, Jun 20, 2023 at 11:09:52AM +0800, Guo Samin wrote: >>> From: Guo Samin <samin.guo@starfivetech.com> >>>> From: Conor Dooley <conor@kernel.org> >>>>> On Fri, May 26, 2023 at 05:05:01PM +0800, Samin Guo wrote: >>>>>> The motorcomm phy (YT8531) supports the ability to adjust the drive >>>>>> strength of the rx_clk/rx_data, the value range of pad driver >>>>>> strength is 0 to 7. >> >>>>>> + motorcomm,rx-clk-driver-strength: >>>>>> + description: drive strength of rx_clk pad. >>>>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>>>> + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] >>>>> >>>>> I think you should use minimum & maximum instead of these listed out >>>>> enums. >> >>>> You have also had this comment since v1 & were reminded of it on >>>>> v2 by Krzysztof: "What do the numbers mean? What are the units? mA?" >> >>>> The good news is that we just got some data about units from Motorcomm. >>>> Maybe I can post the data show of the unit later after I get the complete data. >> >>> Sorry, haven't updated in a while. >> >> NW chief. >> >>> I just got the detailed data of Driver Strength(DS) from Motorcomm , which applies to both rx_clk and rx_data. >>> >>> |----------------------| >>> | ds map table | >>> |----------------------| >>> | DS(3b) | Current (mA)| >>> |--------|-------------| >>> | 000 | 1.20 | >>> | 001 | 2.10 | >>> | 010 | 2.70 | >>> | 011 | 2.91 | >>> | 100 | 3.11 | >>> | 101 | 3.60 | >>> | 110 | 3.97 | >>> | 111 | 4.35 | >>> |--------|-------------| >>> >>> Since these currents are not integer values and have no regularity, > > There is no mA unit in DT schema, so I don't see what by "not integer > values". 1200 uA is an integer. > >>> it is not very good to use in the drive/dts in my opinion. >> >> Who says you have to use mA? What about uA? > > Yep > >> >>> Therefore, I tend to continue to use DS(0-7) in dts/driver, and adding >>> a description of the current value corresponding to DS in dt-bindings. >> >> I think this goes against not putting register values into the dts & >> that the accurate description of the hardware are the currents. > > For vendor properties register values are often accepted, but logical > unit is much more readable in the DTS. Also allows further customization > or extending when new variant appears. You cannot do extend a property > easily when it holds a register value, without changing the meaning per > variant. > Hi Conor & Krzysztof, Thanks for taking the time to review. Yes, uA can be used to avoid the problem of decimals. In addition to this, we need to deal with the DS under different IOPAD. these values were existed at IO 1.8V. When the IO is set to 3.3V, the values will change. Chip_Config (EXT_0xA001) |Bit |Symbol |Access |Default |Description | |5:4 |Cfg_ldo |RW |0x0 |Rgmii ldo voltage and RGMII/MDC/MDIO PAD's level shifter control. Depends on strapping.| |2'b11: 1.8v 2'b10: 1.8v 2'b01: 2.5v 2'b00: 3.3v | |----------------------| | ds map table(1.8V) | |----------------------| | DS(3b) | Current (mA)| | 000 | 1.20 | | 001 | 2.10 | | 010 | 2.70 | | 011 | 2.91 | | 100 | 3.11 | | 101 | 3.60 | | 110 | 3.97 | | 111 | 4.35 | |--------|-------------| |----------------------| | ds map table(3.3V) | |----------------------| | DS(3b) | Current (mA)| | 000 | 3.07 | | 001 | 4.08 | | 010 | 4.37 | | 011 | 4.68 | | 100 | 5.02 | | 101 | 5.45 | | 110 | 5.74 | | 111 | 6.14 | |--------|-------------| Best regards, Samin >> >>> Like This: >>> >>> + motorcomm,rx-clk-driver-strength: >>> + description: drive strength of rx_clk pad. >> >> You need "description: |" to preserve the formatting if you add tables, >> but I don't think that this is a good idea. Put the values in here that >> describe the hardware (IOW the currents) and then you don't need to have >> this table. >> >>> + |----------------------| >>> + | rx_clk ds map table | >>> + |----------------------| >>> + | DS(3b) | Current (mA)| >>> + | 000 | 1.20 | >>> + | 001 | 2.10 | >>> + | 010 | 2.70 | >>> + | 011 | 2.91 | >>> + | 100 | 3.11 | >>> + | 101 | 3.60 | >>> + | 110 | 3.97 | >>> + | 111 | 4.35 | >>> + |--------|-------------| >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] >>> + default: 3 >>> + >> >>> Or use minimum & maximum instead of these listed out enums >> >> With the actual current values, enum rather than min + max. > > > > Best regards, > Krzysztof > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg 2023-06-20 3:09 ` Guo Samin 2023-06-20 8:26 ` Conor Dooley @ 2023-06-20 13:29 ` Andrew Lunn 2023-06-21 3:28 ` Guo Samin 1 sibling, 1 reply; 12+ messages in thread From: Andrew Lunn @ 2023-06-20 13:29 UTC (permalink / raw) To: Guo Samin Cc: Conor Dooley, linux-kernel, devicetree, netdev, Peter Geis, Frank, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Heiner Kallweit, Russell King, Yanhong Wang > I just got the detailed data of Driver Strength(DS) from Motorcomm , > which applies to both rx_clk and rx_data. > > |----------------------| > | ds map table | > |----------------------| > | DS(3b) | Current (mA)| > |--------|-------------| > | 000 | 1.20 | > | 001 | 2.10 | > | 010 | 2.70 | > | 011 | 2.91 | > | 100 | 3.11 | > | 101 | 3.60 | > | 110 | 3.97 | > | 111 | 4.35 | > |--------|-------------| > > Since these currents are not integer values Integers is not a problem. Simply use uA. > and have no regularity, it is not very good to use in the drive/dts > in my opinion. I think they are fine to use. Add a lookup table, microamps to register value. Return -EINVAL if the requested value is not in the table. List the valid values in the schema, so the checker tool might point out problems. Andrew ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg 2023-06-20 13:29 ` Andrew Lunn @ 2023-06-21 3:28 ` Guo Samin 2023-06-21 14:13 ` Andrew Lunn 0 siblings, 1 reply; 12+ messages in thread From: Guo Samin @ 2023-06-21 3:28 UTC (permalink / raw) To: Andrew Lunn Cc: Conor Dooley, linux-kernel, devicetree, netdev, Peter Geis, Frank, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Heiner Kallweit, Russell King, Yanhong Wang Re: [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg From: Andrew Lunn <andrew@lunn.ch> to: Guo Samin <samin.guo@starfivetech.com> data: 2023/6/20 >> I just got the detailed data of Driver Strength(DS) from Motorcomm , >> which applies to both rx_clk and rx_data. >> >> |----------------------| >> | ds map table | >> |----------------------| >> | DS(3b) | Current (mA)| >> |--------|-------------| >> | 000 | 1.20 | >> | 001 | 2.10 | >> | 010 | 2.70 | >> | 011 | 2.91 | >> | 100 | 3.11 | >> | 101 | 3.60 | >> | 110 | 3.97 | >> | 111 | 4.35 | >> |--------|-------------| >> >> Since these currents are not integer values > > Integers is not a problem. Simply use uA. > >> and have no regularity, it is not very good to use in the drive/dts >> in my opinion. > > I think they are fine to use. Add a lookup table, microamps to > register value. Return -EINVAL if the requested value is not in the > table. List the valid values in the schema, so the checker tool might > point out problems. > > Andrew Thanks Andrew, I'll use a lookup table to try. Another thing we need to deal with DS under different IO voltages(1.8V/2.5V/3.3V). The IO voltage can be configured via a hardware pull-up resistor (visionfive 2 is configured to 1.8V by default), and then the IO voltage can be obtained or set through the register(0xA001) Chip_Config (EXT_0xA001) |Bit |Symbol |Access |Default |Description | |5:4 |Cfg_ldo |RW |0x0 |Rgmii ldo voltage and RGMII/MDC/MDIO PAD's level shifter control. Depends on strapping.| |2'b11: 1.8v 2'b10: 1.8v 2'b01: 2.5v 2'b00: 3.3v | |----------------------| | ds map table(1.8V) | |----------------------| | DS(3b) | Current (mA)| | 000 | 1.20 | | 001 | 2.10 | | 010 | 2.70 | | 011 | 2.91 | | 100 | 3.11 | | 101 | 3.60 | | 110 | 3.97 | | 111 | 4.35 | |--------|-------------| |----------------------| | ds map table(3.3V) | |----------------------| | DS(3b) | Current (mA)| | 000 | 3.07 | | 001 | 4.08 | | 010 | 4.37 | | 011 | 4.68 | | 100 | 5.02 | | 101 | 5.45 | | 110 | 5.74 | | 111 | 6.14 | |--------|-------------| (The current value of 2.5V is not available to us now) When we need to deal with current values at different voltages, using register values in drives may be simpler, comparing the current value. Of course, I will also follow your suggestion and use the current value + lookup table to implement a solution. Thanks! Best regards, Samin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg 2023-06-21 3:28 ` Guo Samin @ 2023-06-21 14:13 ` Andrew Lunn 0 siblings, 0 replies; 12+ messages in thread From: Andrew Lunn @ 2023-06-21 14:13 UTC (permalink / raw) To: Guo Samin Cc: Conor Dooley, linux-kernel, devicetree, netdev, Peter Geis, Frank, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Heiner Kallweit, Russell King, Yanhong Wang > When we need to deal with current values at different voltages, > using register values in drives may be simpler, comparing the > current value. Register values in DT are only allowed in the worst case if its all undocumented vendor magic. You have the needed documentation, and its easy code to write, lookup a value in a table. Andrew ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 2/2] net: phy: motorcomm: Add pad drive strength cfg support 2023-05-26 9:05 [PATCH v3 0/2] Add motorcomm phy pad-driver-strength-cfg support Samin Guo 2023-05-26 9:05 ` [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg Samin Guo @ 2023-05-26 9:05 ` Samin Guo 1 sibling, 0 replies; 12+ messages in thread From: Samin Guo @ 2023-05-26 9:05 UTC (permalink / raw) To: linux-kernel, devicetree, netdev, Peter Geis, Frank Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Heiner Kallweit, Russell King, Samin Guo, Yanhong Wang The motorcomm phy (YT8531) supports the ability to adjust the drive strength of the rx_clk/rx_data, and the default strength may not be suitable for all boards. So add configurable options to better match the boards.(e.g. StarFive VisionFive 2) Signed-off-by: Samin Guo <samin.guo@starfivetech.com> --- drivers/net/phy/motorcomm.c | 46 +++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c index 2fa5a90e073b..e5d6b491046e 100644 --- a/drivers/net/phy/motorcomm.c +++ b/drivers/net/phy/motorcomm.c @@ -236,6 +236,15 @@ */ #define YTPHY_WCR_TYPE_PULSE BIT(0) +#define YTPHY_PAD_DRIVE_STRENGTH_REG 0xA010 +#define YT8531_RGMII_RXC_DS_DEFAULT 0x3 +#define YT8531_RGMII_RXC_DS_MAX 0x7 +#define YT8531_RGMII_RXC_DS GENMASK(15, 13) +#define YT8531_RGMII_RXD_DS_DEFAULT 0x3 +#define YT8531_RGMII_RXD_DS_MAX 0x7 +#define YT8531_RGMII_RXD_DS_LOW GENMASK(5, 4) /* Bit 1/0 of rxd_ds */ +#define YT8531_RGMII_RXD_DS_HI BIT(12) /* Bit 2 of rxd_ds */ + #define YTPHY_SYNCE_CFG_REG 0xA012 #define YT8521_SCR_SYNCE_ENABLE BIT(5) /* 1b0 output 25m clock @@ -1494,6 +1503,7 @@ static int yt8521_config_init(struct phy_device *phydev) static int yt8531_config_init(struct phy_device *phydev) { struct device_node *node = phydev->mdio.dev.of_node; + u32 ds, val; int ret; ret = ytphy_rgmii_clk_delay_config_with_lock(phydev); @@ -1518,6 +1528,42 @@ static int yt8531_config_init(struct phy_device *phydev) return ret; } + ds = YT8531_RGMII_RXC_DS_DEFAULT; + if (!of_property_read_u32(node, "motorcomm,rx-clk-driver-strength", &val)) { + if (val > YT8531_RGMII_RXC_DS_MAX) + return -EINVAL; + + ds = val; + } + + ret = ytphy_modify_ext_with_lock(phydev, + YTPHY_PAD_DRIVE_STRENGTH_REG, + YT8531_RGMII_RXC_DS, + FIELD_PREP(YT8531_RGMII_RXC_DS, ds)); + if (ret < 0) + return ret; + + ds = FIELD_PREP(YT8531_RGMII_RXD_DS_LOW, YT8531_RGMII_RXD_DS_DEFAULT); + if (!of_property_read_u32(node, "motorcomm,rx-data-driver-strength", &val)) { + if (val > YT8531_RGMII_RXD_DS_MAX) + return -EINVAL; + + if (val > FIELD_MAX(YT8531_RGMII_RXD_DS_LOW)) { + ds = val & FIELD_MAX(YT8531_RGMII_RXD_DS_LOW); + ds = FIELD_PREP(YT8531_RGMII_RXD_DS_LOW, ds); + ds |= YT8531_RGMII_RXD_DS_HI; + } else { + ds = FIELD_PREP(YT8531_RGMII_RXD_DS_LOW, val); + } + } + + ret = ytphy_modify_ext_with_lock(phydev, + YTPHY_PAD_DRIVE_STRENGTH_REG, + YT8531_RGMII_RXD_DS_LOW | YT8531_RGMII_RXD_DS_HI, + ds); + if (ret < 0) + return ret; + return 0; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-06-21 14:14 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-26 9:05 [PATCH v3 0/2] Add motorcomm phy pad-driver-strength-cfg support Samin Guo 2023-05-26 9:05 ` [PATCH v3 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg Samin Guo 2023-05-26 19:41 ` Conor Dooley 2023-05-29 3:46 ` Guo Samin 2023-06-20 3:09 ` Guo Samin 2023-06-20 8:26 ` Conor Dooley 2023-06-20 8:55 ` Krzysztof Kozlowski 2023-06-21 3:03 ` Guo Samin 2023-06-20 13:29 ` Andrew Lunn 2023-06-21 3:28 ` Guo Samin 2023-06-21 14:13 ` Andrew Lunn 2023-05-26 9:05 ` [PATCH v3 2/2] net: phy: motorcomm: Add pad drive strength cfg support Samin Guo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).