lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <e9d9c67d-e113-4a10-bd18-0d013fe7ea92@lunn.ch>
Date: Thu, 1 Aug 2024 15:14:46 +0200
From: Andrew Lunn <andrew@...n.ch>
To: "Frank.Sae" <Frank.Sae@...or-comm.com>
Cc: Krzysztof Kozlowski <krzk@...nel.org>, hkallweit1@...il.com,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, linux@...linux.org.uk, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	yuanlai.cui@...or-comm.com, hua.sun@...or-comm.com,
	xiaoyong.li@...or-comm.com, suting.hu@...or-comm.com,
	jie.han@...or-comm.com
Subject: Re: [PATCH 1/2] dt-bindings: net: motorcomm: Add chip mode cfg

On Thu, Aug 01, 2024 at 02:49:12AM -0700, Frank.Sae wrote:
> 
> On 7/27/24 02:25, Krzysztof Kozlowski wrote:
> 
>     On 27/07/2024 11:20, Frank.Sae wrote:
> 
>          The motorcomm phy (yt8821) supports the ability to
>          config the chip mode of serdes.
>          The yt8821 serdes could be set to AUTO_BX2500_SGMII or
>          FORCE_BX2500.
>          In AUTO_BX2500_SGMII mode, SerDes
>          speed is determined by UTP, if UTP link up
>          at 2.5GBASE-T, SerDes will work as
>          2500BASE-X, if UTP link up at
>          1000BASE-T/100BASE-Tx/10BASE-T, SerDes will work
>          as SGMII.
>          In FORCE_BX2500, SerDes always works
>          as 2500BASE-X.
> 
>     Very weird wrapping.
> 
>     Please wrap commit message according to Linux coding style / submission
>     process (neither too early nor over the limit):
>     https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
> 
> 
>         Signed-off-by: Frank.Sae <Frank.Sae@...or-comm.com>
> 
>     Didn't you copy user-name as you name?
> 
> sorry, not understand your mean.
> 
>         ---
>          .../bindings/net/motorcomm,yt8xxx.yaml          | 17 +++++++++++++++++
>          1 file changed, 17 insertions(+)
> 
>     Also, your threading is completely broken. Use git send-email or b4.
> 
> sorry, not understand your mean of threading broken. the patch used git
> send-email.

Your indentation of replies it also very odd!

> 
>         +      0: AUTO_BX2500_SGMII
>         +      1: FORCE_BX2500
>         +      In AUTO_BX2500_SGMII mode, serdes speed is determined by UTP,
>         +      if UTP link up at 2.5GBASE-T, serdes will work as 2500BASE-X,
>         +      if UTP link up at 1000BASE-T/100BASE-Tx/10BASE-T, serdes will
>         +      work as SGMII.
>         +      In FORCE_BX2500 mode, serdes always works as 2500BASE-X.
> 
> 
>     Explain why this is even needed and why "auto" is not correct in all
>     cases. In commit msg or property description.
> 
> yt8821 phy does not support strapping to config the serdes mode, so config the
> serdes mode by dts instead.

Strapping does not matter. You can set it on probe.

> even if auto 2500base-x serdes mode is default mode after phy hard reset, and
> auto as default must be make sense, but from most our customers's feedback,
> force 2500base-x serdes mode is used in project usually to adapt to mac's serdes
> settings. for customer's convenience and use simplicity, force 2500base-x serdes
> mode selected as default here.
 
If you are using phylink correctly, the customer should never
know. Both the MAC and the PHY enumerate the capabilities and phylink
will tell you what mode to change to.

I still think this property should be removed, default to auto, and
let phylink tell you if something else should be used.

> 
>         +    $ref: /schemas/types.yaml#/definitions/uint8
> 
>     Make it a string, not uint8.
> 
> 
> why do you suggest string, the property value(uint8) will be wrote to phy
> register.

Device tree is not a list of values to poke into registers. It is a
textual description of the hardware. The driver probably needs to
apply conversions to get register values. e.g. delay are in ns,
voltages are in millivolts, but register typically don't use such
units.

	Andrew

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ