| 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
| ||
|
Message-ID: <MWHPR11MB1693771FDBEFA284B490DF71EF299@MWHPR11MB1693.namprd11.prod.outlook.com>
Date: Mon, 17 Oct 2022 20:00:13 +0000
From: <Jerry.Ray@...rochip.com>
To: <olteanv@...il.com>
CC: <andrew@...n.ch>, <vivien.didelot@...il.com>,
<f.fainelli@...il.com>, <davem@...emloft.net>,
<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
<robh+dt@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>,
<UNGLinuxDriver@...rochip.com>, <netdev@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: RE: [net-next][PATCH v4] dt-bindings: dsa: Add lan9303 yaml
>> ---
>> v3->v4:
>> - Addressed v3 community feedback
>
>More specifically?
>
- Old lan9303.txt file is totally removed rather than containing text that
redirects the user to the microchip,lan9303.yaml source file.
- Drop "Tree Bindings" from title
- Drop quotes from dsa.yaml reference line.
- Modified the compatible second enum to include a second string.
(( I now realize this is not what was being asked for and have made
it a single enum with 4 items, removing the oneOf. ))
- Drop "gpio specifier for a" in reset-gpois description
- added a default: property to the reset-duration item and set it to 200.
- Drop "0" from the ethernet name. Split the MDIO and I2C examples into
two so that the number is no longer needed.
- Placed the reg property to be directly following the compatible string
in the mdio node.
>> +examples:
>> + - |
>> + #include <dt-bindings/gpio/gpio.h>
>> +
>> + // Ethernet switch connected via mdio to the host
>> + ethernet {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + phy-handle = <&lan9303switch>;
>> + phy-mode = "rmii";
>> + fixed-link {
>> + speed = <100>;
>> + full-duplex;
>> + };
>
>I see the phy-handle to the switch is inherited from the .txt dt-binding,
>but I don't understand it. The switch is an mdio_device, not a phy_device,
>so what will this do?
>
>Also, any reasonable host driver will error out if it finds a phy-handle
>and a fixed-link in its OF node. So one of phy-handle or fixed-link must
>be dropped, they are bogus.
>
>Even better, just stick to the mdio node as root and drop the DSA master
>OF node, like other DSA dt-binding examples do. You can have dangling
>phandles, so "ethernet = <ðernet>" below is not an issue.
>
I can remove the phy-handle, but I'm trying to establish the link between
this ethernet port and port0 (the CPU port) of the lan9303. The lan9303
acts as the phy for this ethernet port and I want to force the speed and
duplex of the link to be 100 / full-duplex.
>> + mdio {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + lan9303switch: switch@0 {
>> + compatible = "smsc,lan9303-mdio";
>> + reg = <0>;
>> + dsa,member = <0 0>;
>
>Redundant, please remove.
>
Okay. I can remove the "dsa,member = <0,0>;" line.
>> + ethernet-ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + port@0 {
>> + reg = <0>;
>> + phy-mode = "rmii";
>
>FWIW, RMII has a MAC mode and a PHY mode. Two RMII interfaces connected
>in MAC mode to one another don't work. You'll have problems if you also
>have an RMII PHY connected to one of the xMII ports, and you describe
>phy-mode = "rmii" for both. There exists a "rev-rmii" phy-mode to denote
>an RMII interface working in PHY mode. Wonder if you should be using
>that here.
>
"rev-rmii" does make more sense. And yes, in this configuration the rmii
port of the lan9303 is acting as the PHY end.
>> + ethernet = <ðernet>;
>> + fixed-link {
>> + speed = <100>;
>> + full-duplex;
>> + };
>> + };
>> + port@1 {
>> + reg = <1>;
>> + max-speed = <100>;
>> + label = "lan1";
>> + };
>> + port@2 {
>> + reg = <2>;
>> + max-speed = <100>;
>> + label = "lan2";
>> + };
>> + };
>> + };
>> + };
>> + };
>> +
>> + - |
>> + #include <dt-bindings/gpio/gpio.h>
>> +
>> + // Ethernet switch connected via i2c to the host
>> + ethernet {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + phy-mode = "rmii";
>> + speed = <100>;
>> + fixed-link {
>> + full-duplex;
>> + };
>> + };
>
>No need for this node.
>
Without this, what does the port0 entry below have to point to?
How do you establish the device tree linkage between the ethenet
MAC and the rev-rmii PHY it connects to?
>> +
>> + i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + lan9303: switch@1a {
>> + compatible = "smsc,lan9303-i2c";
>> + reg = <0x1a>;
>> + ethernet-ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + port@0 {
>> + reg = <0>;
>> + phy-mode = "rmii";
>> + ethernet = <ðernet>;
>> + fixed-link {
>> + speed = <100>;
>> + full-duplex;
>> + };
>> + };
>> + port@1 {
>> + reg = <1>;
>> + max-speed = <100>;
>> + label = "lan1";
>> + };
>> + port@2 {
>> + reg = <2>;
>> + max-speed = <100>;
>> + label = "lan2";
>> + };
>> + };
>> + };
>> + };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 5d58b55c5ae5..89055ff2838a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -13386,6 +13386,14 @@ L: netdev@...r.kernel.org
>> S: Maintained
>> F: drivers/net/ethernet/microchip/lan743x_*
>>
>> +MICROCHIP LAN9303/LAN9354 ETHERNET SWITCH DRIVER
>> +M: Jerry Ray <jerry.ray@...rochip.com>
>> +M: UNGLinuxDriver@...rochip.com
>> +L: netdev@...r.kernel.org
>> +S: Maintained
>> +F: Documentation/devicetree/bindings/net/dsa/microchip,lan9303.yaml
>> +F: drivers/net/dsa/lan9303*
>> +
>
>Separate patch please? Changes to the MAINTAINERS file get applied to
>the "net" tree.
>
>> MICROCHIP LAN966X ETHERNET DRIVER
>> M: Horatiu Vultur <horatiu.vultur@...rochip.com>
>> M: UNGLinuxDriver@...rochip.com
>> --
>> 2.25.1
>>
>
Regards,
Jerry.
Powered by blists - more mailing lists