* [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added
@ 2017-09-07 21:11 Tristram.Ha
2017-09-07 21:54 ` Andrew Lunn
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Tristram.Ha @ 2017-09-07 21:11 UTC (permalink / raw)
To: andrew, muvarov, pavel, nathan.leigh.conrad, vivien.didelot,
f.fainelli, netdev, linux-kernel, Woojung.Huh
From: Tristram Ha <Tristram.Ha@microchip.com>
Add other KSZ switches support so that patch check does not complain.
Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
Documentation/devicetree/bindings/net/dsa/ksz.txt | 117 ++++++++++++----------
1 file changed, 62 insertions(+), 55 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt b/Documentation/devicetree/bindings/net/dsa/ksz.txt
index 0ab8b39..34af0e0 100644
--- a/Documentation/devicetree/bindings/net/dsa/ksz.txt
+++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
@@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches
Required properties:
-- compatible: For external switch chips, compatible string must be exactly one
- of: "microchip,ksz9477"
+- compatible: Should be "microchip,ksz9477" for KSZ9477 chip,
+ "microchip,ksz8795" for KSZ8795 chip,
+ "microchip,ksz8794" for KSZ8794 chip,
+ "microchip,ksz8765" for KSZ8765 chip,
+ "microchip,ksz8895" for KSZ8895 chip,
+ "microchip,ksz8864" for KSZ8864 chip,
+ "microchip,ksz8873" for KSZ8873 chip,
+ "microchip,ksz8863" for KSZ8863 chip,
+ "microchip,ksz8463" for KSZ8463 chip
See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional required and optional properties.
@@ -13,60 +20,60 @@ Examples:
Ethernet switch connected via SPI to the host, CPU port wired to eth0:
- eth0: ethernet@10001000 {
- fixed-link {
- speed = <1000>;
- full-duplex;
- };
- };
+ eth0: ethernet@10001000 {
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
- spi1: spi@f8008000 {
- pinctrl-0 = <&pinctrl_spi_ksz>;
- cs-gpios = <&pioC 25 0>;
- id = <1>;
- status = "okay";
+ spi1: spi@f8008000 {
+ cs-gpios = <&pioC 25 0>;
+ id = <1>;
+ status = "okay";
- ksz9477: ksz9477@0 {
- compatible = "microchip,ksz9477";
- reg = <0>;
+ ksz9477: ksz9477@0 {
+ compatible = "microchip,ksz9477";
+ reg = <0>;
- spi-max-frequency = <44000000>;
- spi-cpha;
- spi-cpol;
+ spi-max-frequency = <44000000>;
+ spi-cpha;
+ spi-cpol;
+
+ status = "okay";
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ label = "lan1";
+ };
+ port@1 {
+ reg = <1>;
+ label = "lan2";
+ };
+ port@2 {
+ reg = <2>;
+ label = "lan3";
+ };
+ port@3 {
+ reg = <3>;
+ label = "lan4";
+ };
+ port@4 {
+ reg = <4>;
+ label = "lan5";
+ };
+ port@5 {
+ reg = <5>;
+ label = "cpu";
+ ethernet = <ð0>;
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+ };
+ };
+ };
- status = "okay";
- ports {
- #address-cells = <1>;
- #size-cells = <0>;
- port@0 {
- reg = <0>;
- label = "lan1";
- };
- port@1 {
- reg = <1>;
- label = "lan2";
- };
- port@2 {
- reg = <2>;
- label = "lan3";
- };
- port@3 {
- reg = <3>;
- label = "lan4";
- };
- port@4 {
- reg = <4>;
- label = "lan5";
- };
- port@5 {
- reg = <5>;
- label = "cpu";
- ethernet = <ð0>;
- fixed-link {
- speed = <1000>;
- full-duplex;
- };
- };
- };
- };
- };
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-07 21:11 [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added Tristram.Ha @ 2017-09-07 21:54 ` Andrew Lunn 2017-09-08 13:32 ` Maxim Uvarov 2017-09-08 9:04 ` Pavel Machek 2017-09-08 18:48 ` Florian Fainelli 2 siblings, 1 reply; 18+ messages in thread From: Andrew Lunn @ 2017-09-07 21:54 UTC (permalink / raw) To: Tristram.Ha Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli, netdev, linux-kernel, Woojung.Huh > -- compatible: For external switch chips, compatible string must be exactly one > - of: "microchip,ksz9477" > +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip, > + "microchip,ksz8795" for KSZ8795 chip, > + "microchip,ksz8794" for KSZ8794 chip, > + "microchip,ksz8765" for KSZ8765 chip, > + "microchip,ksz8895" for KSZ8895 chip, > + "microchip,ksz8864" for KSZ8864 chip, > + "microchip,ksz8873" for KSZ8873 chip, > + "microchip,ksz8863" for KSZ8863 chip, > + "microchip,ksz8463" for KSZ8463 chip This part of this patch should be in a patch of the series that actually adds support for these chips. Don't document chips until you actually support them. > See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional required and optional properties. > @@ -13,60 +20,60 @@ Examples: > > Ethernet switch connected via SPI to the host, CPU port wired to eth0: > > - eth0: ethernet@10001000 { > - fixed-link { > - speed = <1000>; > - full-duplex; > - }; > - }; > + eth0: ethernet@10001000 { > + fixed-link { > + speed = <1000>; > + full-duplex; > + }; > + }; > > - spi1: spi@f8008000 { > - pinctrl-0 = <&pinctrl_spi_ksz>; > - cs-gpios = <&pioC 25 0>; > - id = <1>; > - status = "okay"; > + spi1: spi@f8008000 { > + cs-gpios = <&pioC 25 0>; > + id = <1>; > + status = "okay"; > > - ksz9477: ksz9477@0 { > - compatible = "microchip,ksz9477"; > - reg = <0>; > + ksz9477: ksz9477@0 { > + compatible = "microchip,ksz9477"; > + reg = <0>; > > - spi-max-frequency = <44000000>; > - spi-cpha; > - spi-cpol; > + spi-max-frequency = <44000000>; > + spi-cpha; > + spi-cpol; > + > + status = "okay"; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + port@0 { > + reg = <0>; > + label = "lan1"; > + }; > + port@1 { > + reg = <1>; > + label = "lan2"; > + }; > + port@2 { > + reg = <2>; > + label = "lan3"; > + }; > + port@3 { > + reg = <3>; > + label = "lan4"; > + }; > + port@4 { > + reg = <4>; > + label = "lan5"; > + }; > + port@5 { > + reg = <5>; > + label = "cpu"; > + ethernet = <ð0>; > + fixed-link { > + speed = <1000>; > + full-duplex; > + }; > + }; > + }; > + }; > + }; > > - status = "okay"; > - ports { > - #address-cells = <1>; > - #size-cells = <0>; > - port@0 { > - reg = <0>; > - label = "lan1"; > - }; > - port@1 { > - reg = <1>; > - label = "lan2"; > - }; > - port@2 { > - reg = <2>; > - label = "lan3"; > - }; > - port@3 { > - reg = <3>; > - label = "lan4"; > - }; > - port@4 { > - reg = <4>; > - label = "lan5"; > - }; > - port@5 { > - reg = <5>; > - label = "cpu"; > - ethernet = <ð0>; > - fixed-link { > - speed = <1000>; > - full-duplex; > - }; > - }; > - }; > - }; > - }; This part however is a nice cleanup. You can submit this patch as a separate patch, once netdev has opened again in about 10 days time. Andrew ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-07 21:54 ` Andrew Lunn @ 2017-09-08 13:32 ` Maxim Uvarov 2017-09-08 14:12 ` Andrew Lunn 0 siblings, 1 reply; 18+ messages in thread From: Maxim Uvarov @ 2017-09-08 13:32 UTC (permalink / raw) To: Andrew Lunn Cc: Tristram.Ha, Pavel Machek, Nathan Conrad, Vivien Didelot, Florian Fainelli, netdev, linux-kernel, Woojung.Huh 2017-09-08 0:54 GMT+03:00 Andrew Lunn <andrew@lunn.ch>: >> -- compatible: For external switch chips, compatible string must be exactly one >> - of: "microchip,ksz9477" >> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip, >> + "microchip,ksz8795" for KSZ8795 chip, >> + "microchip,ksz8794" for KSZ8794 chip, >> + "microchip,ksz8765" for KSZ8765 chip, >> + "microchip,ksz8895" for KSZ8895 chip, >> + "microchip,ksz8864" for KSZ8864 chip, >> + "microchip,ksz8873" for KSZ8873 chip, >> + "microchip,ksz8863" for KSZ8863 chip, >> + "microchip,ksz8463" for KSZ8463 chip > all that chips have the same spi access to get chip id on probe(). I prefer common microship,ksz-spi rather than somebody will always maintain that list. Maxim. > This part of this patch should be in a patch of the series that > actually adds support for these chips. Don't document chips until you > actually support them. > >> See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional required and optional properties. >> @@ -13,60 +20,60 @@ Examples: >> >> Ethernet switch connected via SPI to the host, CPU port wired to eth0: >> >> - eth0: ethernet@10001000 { >> - fixed-link { >> - speed = <1000>; >> - full-duplex; >> - }; >> - }; >> + eth0: ethernet@10001000 { >> + fixed-link { >> + speed = <1000>; >> + full-duplex; >> + }; >> + }; >> >> - spi1: spi@f8008000 { >> - pinctrl-0 = <&pinctrl_spi_ksz>; >> - cs-gpios = <&pioC 25 0>; >> - id = <1>; >> - status = "okay"; >> + spi1: spi@f8008000 { >> + cs-gpios = <&pioC 25 0>; >> + id = <1>; >> + status = "okay"; >> >> - ksz9477: ksz9477@0 { >> - compatible = "microchip,ksz9477"; >> - reg = <0>; >> + ksz9477: ksz9477@0 { >> + compatible = "microchip,ksz9477"; >> + reg = <0>; >> >> - spi-max-frequency = <44000000>; >> - spi-cpha; >> - spi-cpol; >> + spi-max-frequency = <44000000>; >> + spi-cpha; >> + spi-cpol; >> + >> + status = "okay"; >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + port@0 { >> + reg = <0>; >> + label = "lan1"; >> + }; >> + port@1 { >> + reg = <1>; >> + label = "lan2"; >> + }; >> + port@2 { >> + reg = <2>; >> + label = "lan3"; >> + }; >> + port@3 { >> + reg = <3>; >> + label = "lan4"; >> + }; >> + port@4 { >> + reg = <4>; >> + label = "lan5"; >> + }; >> + port@5 { >> + reg = <5>; >> + label = "cpu"; >> + ethernet = <ð0>; >> + fixed-link { >> + speed = <1000>; >> + full-duplex; >> + }; >> + }; >> + }; >> + }; >> + }; >> >> - status = "okay"; >> - ports { >> - #address-cells = <1>; >> - #size-cells = <0>; >> - port@0 { >> - reg = <0>; >> - label = "lan1"; >> - }; >> - port@1 { >> - reg = <1>; >> - label = "lan2"; >> - }; >> - port@2 { >> - reg = <2>; >> - label = "lan3"; >> - }; >> - port@3 { >> - reg = <3>; >> - label = "lan4"; >> - }; >> - port@4 { >> - reg = <4>; >> - label = "lan5"; >> - }; >> - port@5 { >> - reg = <5>; >> - label = "cpu"; >> - ethernet = <ð0>; >> - fixed-link { >> - speed = <1000>; >> - full-duplex; >> - }; >> - }; >> - }; >> - }; >> - }; > > This part however is a nice cleanup. You can submit this patch as a > separate patch, once netdev has opened again in about 10 days time. > > Andrew -- Best regards, Maxim Uvarov ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-08 13:32 ` Maxim Uvarov @ 2017-09-08 14:12 ` Andrew Lunn 2017-09-08 18:40 ` Tristram.Ha 0 siblings, 1 reply; 18+ messages in thread From: Andrew Lunn @ 2017-09-08 14:12 UTC (permalink / raw) To: Maxim Uvarov Cc: Tristram.Ha, Pavel Machek, Nathan Conrad, Vivien Didelot, Florian Fainelli, netdev, linux-kernel, Woojung.Huh On Fri, Sep 08, 2017 at 04:32:35PM +0300, Maxim Uvarov wrote: > 2017-09-08 0:54 GMT+03:00 Andrew Lunn <andrew@lunn.ch>: > >> -- compatible: For external switch chips, compatible string must be exactly one > >> - of: "microchip,ksz9477" > >> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip, > >> + "microchip,ksz8795" for KSZ8795 chip, > >> + "microchip,ksz8794" for KSZ8794 chip, > >> + "microchip,ksz8765" for KSZ8765 chip, > >> + "microchip,ksz8895" for KSZ8895 chip, > >> + "microchip,ksz8864" for KSZ8864 chip, > >> + "microchip,ksz8873" for KSZ8873 chip, > >> + "microchip,ksz8863" for KSZ8863 chip, > >> + "microchip,ksz8463" for KSZ8463 chip > > > > all that chips have the same spi access to get chip id on probe(). I > prefer common microship,ksz-spi rather than somebody will always > maintain that list. The Marvell DSA driver is similar. The compatibility string tells you enough to go find the switch ID in the switch itself. I suppose this comes down to, is there going to be one SPI driver for all the devices, or lots of drivers? In general, DSA has one driver for lots of devices. The mv88e6xxx supports around 25 devices. The b53 has around 17, etc. So i would suggest one driver supporting all the different devices. Andrew ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-08 14:12 ` Andrew Lunn @ 2017-09-08 18:40 ` Tristram.Ha 2017-09-08 18:45 ` Florian Fainelli ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Tristram.Ha @ 2017-09-08 18:40 UTC (permalink / raw) To: andrew, muvarov Cc: pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli, netdev, linux-kernel, Woojung.Huh > -----Original Message----- > From: Andrew Lunn [mailto:andrew@lunn.ch] > Sent: Friday, September 08, 2017 7:12 AM > To: Maxim Uvarov > Cc: Tristram Ha - C24268; Pavel Machek; Nathan Conrad; Vivien Didelot; Florian > Fainelli; netdev; linux-kernel@vger.kernel.org; Woojung Huh - C21699 > Subject: Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new > drivers can be added > > On Fri, Sep 08, 2017 at 04:32:35PM +0300, Maxim Uvarov wrote: > > 2017-09-08 0:54 GMT+03:00 Andrew Lunn <andrew@lunn.ch>: > > >> -- compatible: For external switch chips, compatible string must be exactly > one > > >> - of: "microchip,ksz9477" > > >> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip, > > >> + "microchip,ksz8795" for KSZ8795 chip, > > >> + "microchip,ksz8794" for KSZ8794 chip, > > >> + "microchip,ksz8765" for KSZ8765 chip, > > >> + "microchip,ksz8895" for KSZ8895 chip, > > >> + "microchip,ksz8864" for KSZ8864 chip, > > >> + "microchip,ksz8873" for KSZ8873 chip, > > >> + "microchip,ksz8863" for KSZ8863 chip, > > >> + "microchip,ksz8463" for KSZ8463 chip > > > > > > > all that chips have the same spi access to get chip id on probe(). I > > prefer common microship,ksz-spi rather than somebody will always > > maintain that list. > > The Marvell DSA driver is similar. The compatibility string tells you > enough to go find the switch ID in the switch itself. > > I suppose this comes down to, is there going to be one SPI driver for > all the devices, or lots of drivers? In general, DSA has one driver > for lots of devices. The mv88e6xxx supports around 25 devices. The b53 > has around 17, etc. > > So i would suggest one driver supporting all the different devices. > There will be 5 drivers to support these devices: ksz9477.c - KSZ9893/KSZ9897/KSZ9567/KSZ9566/KSZ9477 ksz8795.c - KSZ8795/KSZ8795/KSZ8765 ksz8895.c - KSZ8895/KSZ8864 ksz8863.c - KSZ8863/KSZ8873 ksz8463.c - KSZ8463 These chips have different SPI access mechanisms, MIB counter reading, and register set. These can be combined into one single driver using function pointers, at least for ksz8795/ksz8895/ksz8863/ksz8463. My only concern is the memory footprint. The customer may not want a big driver to cover all the switches while only one is used. Out of topic I have a question to ask the community regarding the DSA port creation: Port 1 can be specified using the reg parameter specifying 0; port 2, 1; and so on. The KSZ8794 is a variant of KSZ8795 with port 4 disabled. So lan1 = 0, lan2 = 1, lan3 = 2, cpu = 4. But our company Marketing does not want to promote that fact but treat KSZ8794 as a distinct product. So lan1 = 0, lan2 = 1, lan3 = 2, cpu = 3. Is this okay to hide this information inside the driver? This is manageable for KSZ8794 but for KSZ8864 the first port is disabled: lan1 = 1, lan2 = 2, lan3 = 3, cpu = 4. I am not sure whether DSA has or will have a way to display the port mapping to regular users. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-08 18:40 ` Tristram.Ha @ 2017-09-08 18:45 ` Florian Fainelli 2017-09-08 19:01 ` Andrew Lunn 2017-09-08 21:53 ` Pavel Machek 2 siblings, 0 replies; 18+ messages in thread From: Florian Fainelli @ 2017-09-08 18:45 UTC (permalink / raw) To: Tristram.Ha, andrew, muvarov Cc: pavel, nathan.leigh.conrad, vivien.didelot, netdev, linux-kernel, Woojung.Huh On 09/08/2017 11:40 AM, Tristram.Ha@microchip.com wrote: >> -----Original Message----- >> From: Andrew Lunn [mailto:andrew@lunn.ch] >> Sent: Friday, September 08, 2017 7:12 AM >> To: Maxim Uvarov >> Cc: Tristram Ha - C24268; Pavel Machek; Nathan Conrad; Vivien Didelot; Florian >> Fainelli; netdev; linux-kernel@vger.kernel.org; Woojung Huh - C21699 >> Subject: Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new >> drivers can be added >> >> On Fri, Sep 08, 2017 at 04:32:35PM +0300, Maxim Uvarov wrote: >>> 2017-09-08 0:54 GMT+03:00 Andrew Lunn <andrew@lunn.ch>: >>>>> -- compatible: For external switch chips, compatible string must be exactly >> one >>>>> - of: "microchip,ksz9477" >>>>> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip, >>>>> + "microchip,ksz8795" for KSZ8795 chip, >>>>> + "microchip,ksz8794" for KSZ8794 chip, >>>>> + "microchip,ksz8765" for KSZ8765 chip, >>>>> + "microchip,ksz8895" for KSZ8895 chip, >>>>> + "microchip,ksz8864" for KSZ8864 chip, >>>>> + "microchip,ksz8873" for KSZ8873 chip, >>>>> + "microchip,ksz8863" for KSZ8863 chip, >>>>> + "microchip,ksz8463" for KSZ8463 chip >>>> >>> >>> all that chips have the same spi access to get chip id on probe(). I >>> prefer common microship,ksz-spi rather than somebody will always >>> maintain that list. >> >> The Marvell DSA driver is similar. The compatibility string tells you >> enough to go find the switch ID in the switch itself. >> >> I suppose this comes down to, is there going to be one SPI driver for >> all the devices, or lots of drivers? In general, DSA has one driver >> for lots of devices. The mv88e6xxx supports around 25 devices. The b53 >> has around 17, etc. >> >> So i would suggest one driver supporting all the different devices. >> > > There will be 5 drivers to support these devices: > > ksz9477.c - KSZ9893/KSZ9897/KSZ9567/KSZ9566/KSZ9477 > ksz8795.c - KSZ8795/KSZ8795/KSZ8765 > ksz8895.c - KSZ8895/KSZ8864 > ksz8863.c - KSZ8863/KSZ8873 > ksz8463.c - KSZ8463 > > These chips have different SPI access mechanisms, MIB counter reading, > and register set. These can be combined into one single driver using > function pointers, at least for ksz8795/ksz8895/ksz8863/ksz8463. My > only concern is the memory footprint. The customer may not want a > big driver to cover all the switches while only one is used. > > Out of topic I have a question to ask the community regarding the DSA > port creation: > > Port 1 can be specified using the reg parameter specifying 0; port 2, 1; > and so on. The KSZ8794 is a variant of KSZ8795 with port 4 disabled. > So > lan1 = 0, lan2 = 1, lan3 = 2, cpu = 4. > But our company Marketing does not want to promote that fact but treat > KSZ8794 as a distinct product. > So > lan1 = 0, lan2 = 1, lan3 = 2, cpu = 3. > Is this okay to hide this information inside the driver? This is manageable > for KSZ8794 but for KSZ8864 the first port is disabled: > lan1 = 1, lan2 = 2, lan3 = 3, cpu = 4. What dictates all of that is ultimately Device Tree because it defines the port mapping, including where the CPU port is. Once your driver knows which chip it is "talking to" you could always have the driver issue warnings and indicate that the Device Tree is malformed if the user-specified port mapping does not match what the HW supports. > > I am not sure whether DSA has or will have a way to display the port > mapping to regular users. DSA does display the port mapping of user facing ports under the standard sysfs attribute /sys/class/net/*/phys_port_name. CPU port mapping is not displayed because there is no CPU-port network device (intentionally). -- Florian ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-08 18:40 ` Tristram.Ha 2017-09-08 18:45 ` Florian Fainelli @ 2017-09-08 19:01 ` Andrew Lunn 2017-09-08 19:05 ` Florian Fainelli 2017-09-08 21:50 ` Pavel Machek 2017-09-08 21:53 ` Pavel Machek 2 siblings, 2 replies; 18+ messages in thread From: Andrew Lunn @ 2017-09-08 19:01 UTC (permalink / raw) To: Tristram.Ha Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli, netdev, linux-kernel, Woojung.Huh > > So i would suggest one driver supporting all the different devices. > > There will be 5 drivers to support these devices: > > ksz9477.c - KSZ9893/KSZ9897/KSZ9567/KSZ9566/KSZ9477 > ksz8795.c - KSZ8795/KSZ8795/KSZ8765 > ksz8895.c - KSZ8895/KSZ8864 > ksz8863.c - KSZ8863/KSZ8873 > ksz8463.c - KSZ8463 > > These chips have different SPI access mechanisms, MIB counter reading, > and register set. These can be combined into one single driver using > function pointers, at least for ksz8795/ksz8895/ksz8863/ksz8463. My > only concern is the memory footprint. The customer may not want a > big driver to cover all the switches while only one is used. If memory footprint is your problem, make it a compile time choice which devices are supported within the one driver. In practice, you will find most distributions just enable them all. > Out of topic I have a question to ask the community regarding the DSA > port creation: > > Port 1 can be specified using the reg parameter specifying 0; port 2, 1; > and so on. The KSZ8794 is a variant of KSZ8795 with port 4 disabled. > So > lan1 = 0, lan2 = 1, lan3 = 2, cpu = 4. > But our company Marketing does not want to promote that fact but treat > KSZ8794 as a distinct product. > So > lan1 = 0, lan2 = 1, lan3 = 2, cpu = 3. > Is this okay to hide this information inside the driver? This is manageable > for KSZ8794 but for KSZ8864 the first port is disabled: > lan1 = 1, lan2 = 2, lan3 = 3, cpu = 4. > > I am not sure whether DSA has or will have a way to display the port > mapping to regular users. So the port number to name is determined in the device tree. It depends on the board how the ports are named. I always suggest going by the label on the box. clearfog is a good example of this, 0 - lan5, 1 - lan4, etc. So for the KSZ8864, ensure reg = 0 causes an error. Does the hardware force which port is used for the CPU? I've boards with Marvell devices where the CPU is port 0, or port 6, or port 7. The hardware does not care. So we don't force anything. As for KSZ8794 vs KSZ8795, is there different IDs in the silicon? It is these IDs you are using, not the compatible string, to determine how the drive the silicon. You can trust the ID in the silicon. Anything else can be wrong. Andrew ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-08 19:01 ` Andrew Lunn @ 2017-09-08 19:05 ` Florian Fainelli 2017-09-08 21:50 ` Pavel Machek 1 sibling, 0 replies; 18+ messages in thread From: Florian Fainelli @ 2017-09-08 19:05 UTC (permalink / raw) To: Andrew Lunn, Tristram.Ha Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, netdev, linux-kernel, Woojung.Huh On 09/08/2017 12:01 PM, Andrew Lunn wrote: >>> So i would suggest one driver supporting all the different devices. >> >> There will be 5 drivers to support these devices: >> >> ksz9477.c - KSZ9893/KSZ9897/KSZ9567/KSZ9566/KSZ9477 >> ksz8795.c - KSZ8795/KSZ8795/KSZ8765 >> ksz8895.c - KSZ8895/KSZ8864 >> ksz8863.c - KSZ8863/KSZ8873 >> ksz8463.c - KSZ8463 >> >> These chips have different SPI access mechanisms, MIB counter reading, >> and register set. These can be combined into one single driver using >> function pointers, at least for ksz8795/ksz8895/ksz8863/ksz8463. My >> only concern is the memory footprint. The customer may not want a >> big driver to cover all the switches while only one is used. > > If memory footprint is your problem, make it a compile time choice > which devices are supported within the one driver. In practice, you > will find most distributions just enable them all. > >> Out of topic I have a question to ask the community regarding the DSA >> port creation: >> >> Port 1 can be specified using the reg parameter specifying 0; port 2, 1; >> and so on. The KSZ8794 is a variant of KSZ8795 with port 4 disabled. >> So >> lan1 = 0, lan2 = 1, lan3 = 2, cpu = 4. >> But our company Marketing does not want to promote that fact but treat >> KSZ8794 as a distinct product. >> So >> lan1 = 0, lan2 = 1, lan3 = 2, cpu = 3. >> Is this okay to hide this information inside the driver? This is manageable >> for KSZ8794 but for KSZ8864 the first port is disabled: >> lan1 = 1, lan2 = 2, lan3 = 3, cpu = 4. >> >> I am not sure whether DSA has or will have a way to display the port >> mapping to regular users. > > So the port number to name is determined in the device tree. It > depends on the board how the ports are named. I always suggest going > by the label on the box. clearfog is a good example of this, 0 - lan5, > 1 - lan4, etc. > > So for the KSZ8864, ensure reg = 0 causes an error. > > Does the hardware force which port is used for the CPU? I've boards > with Marvell devices where the CPU is port 0, or port 6, or port > 7. The hardware does not care. So we don't force anything. > > As for KSZ8794 vs KSZ8795, is there different IDs in the silicon? It > is these IDs you are using, not the compatible string, to determine > how the drive the silicon. You can trust the ID in the > silicon. Anything else can be wrong. You can't always trust the silicon to report the right revision, just like marketing people in semiconductors tend to like to play tricks and pretend that the same chip is the same, but it is not quite, and a new tape-out was not possible, so it has the same ID, including the revision. This is of course not what should be done, but it happens all the time. Describe in Device Tree with a proper compatible string, which may end-up being treated the same way by the driver, and if it happens that the silicon is not wrong, and can be differentiated, then you can always resort to that to warn the user the wrong compatible was specified, or that this is not going to work, or anything really. -- Florian ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-08 19:01 ` Andrew Lunn 2017-09-08 19:05 ` Florian Fainelli @ 2017-09-08 21:50 ` Pavel Machek 1 sibling, 0 replies; 18+ messages in thread From: Pavel Machek @ 2017-09-08 21:50 UTC (permalink / raw) To: Andrew Lunn Cc: Tristram.Ha, muvarov, nathan.leigh.conrad, vivien.didelot, f.fainelli, netdev, linux-kernel, Woojung.Huh [-- Attachment #1: Type: text/plain, Size: 1357 bytes --] On Fri 2017-09-08 21:01:22, Andrew Lunn wrote: > > > So i would suggest one driver supporting all the different devices. > > > > There will be 5 drivers to support these devices: > > > > ksz9477.c - KSZ9893/KSZ9897/KSZ9567/KSZ9566/KSZ9477 > > ksz8795.c - KSZ8795/KSZ8795/KSZ8765 > > ksz8895.c - KSZ8895/KSZ8864 > > ksz8863.c - KSZ8863/KSZ8873 > > ksz8463.c - KSZ8463 > > > > These chips have different SPI access mechanisms, MIB counter reading, > > and register set. These can be combined into one single driver using > > function pointers, at least for ksz8795/ksz8895/ksz8863/ksz8463. My > > only concern is the memory footprint. The customer may not want a > > big driver to cover all the switches while only one is used. > > If memory footprint is your problem, make it a compile time choice > which devices are supported within the one driver. In practice, you > will find most distributions just enable them all. I have to side with Tristram here. The register layouts are so different that single driver does not make sense. What could make sense is single function, compiled 5 times, based on different includes; same source code but 5 different binaries. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-08 18:40 ` Tristram.Ha 2017-09-08 18:45 ` Florian Fainelli 2017-09-08 19:01 ` Andrew Lunn @ 2017-09-08 21:53 ` Pavel Machek 2 siblings, 0 replies; 18+ messages in thread From: Pavel Machek @ 2017-09-08 21:53 UTC (permalink / raw) To: Tristram.Ha Cc: andrew, muvarov, nathan.leigh.conrad, vivien.didelot, f.fainelli, netdev, linux-kernel, Woojung.Huh [-- Attachment #1: Type: text/plain, Size: 1314 bytes --] Hi! > There will be 5 drivers to support these devices: > > ksz9477.c - KSZ9893/KSZ9897/KSZ9567/KSZ9566/KSZ9477 > ksz8795.c - KSZ8795/KSZ8795/KSZ8765 > ksz8895.c - KSZ8895/KSZ8864 Could we see the 8895 driver, please? > Out of topic I have a question to ask the community regarding the DSA > port creation: > > Port 1 can be specified using the reg parameter specifying 0; port 2, 1; > and so on. The KSZ8794 is a variant of KSZ8795 with port 4 disabled. > So > lan1 = 0, lan2 = 1, lan3 = 2, cpu = 4. > But our company Marketing does not want to promote that fact but treat > KSZ8794 as a distinct product. > So > lan1 = 0, lan2 = 1, lan3 = 2, cpu = 3. > Is this okay to hide this information inside the driver? This is manageable > for KSZ8794 but for KSZ8864 the first port is disabled: > lan1 = 1, lan2 = 2, lan3 = 3, cpu = 4. > > I am not sure whether DSA has or will have a way to display the port > mapping to regular users. Kernel is not a place to play marketing games, and people reading the kernel sources are not targets of your marketing department. Please let us just see the underlying hardware, as is. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-07 21:11 [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added Tristram.Ha 2017-09-07 21:54 ` Andrew Lunn @ 2017-09-08 9:04 ` Pavel Machek 2017-09-08 18:48 ` Florian Fainelli 2 siblings, 0 replies; 18+ messages in thread From: Pavel Machek @ 2017-09-08 9:04 UTC (permalink / raw) To: Tristram.Ha Cc: andrew, muvarov, nathan.leigh.conrad, vivien.didelot, f.fainelli, netdev, linux-kernel, Woojung.Huh [-- Attachment #1: Type: text/plain, Size: 1666 bytes --] On Thu 2017-09-07 21:11:45, Tristram.Ha@microchip.com wrote: > From: Tristram Ha <Tristram.Ha@microchip.com> > > Add other KSZ switches support so that patch check does not complain. Nice; unlike Andrew I believe that adding compatible strings is ok before the support is added. Device tree description is Linux-independend. But... you need to Cc: device tree maintainers on this one. They usually take some time to respond, so it is nice to do this early. > index 0ab8b39..34af0e0 100644 > --- a/Documentation/devicetree/bindings/net/dsa/ksz.txt > +++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt > @@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches > > Required properties: > > -- compatible: For external switch chips, compatible string must be exactly one > - of: "microchip,ksz9477" > +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip, > + "microchip,ksz8795" for KSZ8795 chip, > + "microchip,ksz8794" for KSZ8794 chip, > + "microchip,ksz8765" for KSZ8765 chip, > + "microchip,ksz8895" for KSZ8895 chip, > + "microchip,ksz8864" for KSZ8864 chip, > + "microchip,ksz8873" for KSZ8873 chip, > + "microchip,ksz8863" for KSZ8863 chip, > + "microchip,ksz8463" for KSZ8463 chip > > See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional required and optional properties. And you may want to wrap long line here, unless that's email client playing another trick. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-07 21:11 [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added Tristram.Ha 2017-09-07 21:54 ` Andrew Lunn 2017-09-08 9:04 ` Pavel Machek @ 2017-09-08 18:48 ` Florian Fainelli 2017-09-08 19:00 ` Maxim Uvarov 2 siblings, 1 reply; 18+ messages in thread From: Florian Fainelli @ 2017-09-08 18:48 UTC (permalink / raw) To: Tristram.Ha, andrew, muvarov, pavel, nathan.leigh.conrad, vivien.didelot, netdev, linux-kernel, Woojung.Huh On 09/07/2017 02:11 PM, Tristram.Ha@microchip.com wrote: > From: Tristram Ha <Tristram.Ha@microchip.com> > > Add other KSZ switches support so that patch check does not complain. > > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com> > --- > Documentation/devicetree/bindings/net/dsa/ksz.txt | 117 ++++++++++++---------- > 1 file changed, 62 insertions(+), 55 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt b/Documentation/devicetree/bindings/net/dsa/ksz.txt > index 0ab8b39..34af0e0 100644 > --- a/Documentation/devicetree/bindings/net/dsa/ksz.txt > +++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt > @@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches > > Required properties: > > -- compatible: For external switch chips, compatible string must be exactly one > - of: "microchip,ksz9477" > +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip, > + "microchip,ksz8795" for KSZ8795 chip, > + "microchip,ksz8794" for KSZ8794 chip, > + "microchip,ksz8765" for KSZ8765 chip, > + "microchip,ksz8895" for KSZ8895 chip, > + "microchip,ksz8864" for KSZ8864 chip, > + "microchip,ksz8873" for KSZ8873 chip, > + "microchip,ksz8863" for KSZ8863 chip, > + "microchip,ksz8463" for KSZ8463 chip It becomes pretty obvious there is a 1 to 1 mapping between the compatible name and what you should be using it for so specifying ksz8795 for KSZ8795 chip is really redundant. You could just list all compatible strings that you support and change the verbiage to be: compatible: Should be one of: "microchip,ksz9477" ... "microchip,ksz8463" > > See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional required and optional properties. > @@ -13,60 +20,60 @@ Examples: > > Ethernet switch connected via SPI to the host, CPU port wired to eth0: > > - eth0: ethernet@10001000 { > - fixed-link { > - speed = <1000>; > - full-duplex; > - }; > - }; > + eth0: ethernet@10001000 { > + fixed-link { > + speed = <1000>; > + full-duplex; > + }; > + }; This is a good clean up, but it would probably belong in a separate patch that you would do before adding compatible strings for instance. > > - spi1: spi@f8008000 { > - pinctrl-0 = <&pinctrl_spi_ksz>; > - cs-gpios = <&pioC 25 0>; > - id = <1>; > - status = "okay"; > + spi1: spi@f8008000 { > + cs-gpios = <&pioC 25 0>; > + id = <1>; > + status = "okay"; > > - ksz9477: ksz9477@0 { > - compatible = "microchip,ksz9477"; > - reg = <0>; > + ksz9477: ksz9477@0 { > + compatible = "microchip,ksz9477"; > + reg = <0>; > > - spi-max-frequency = <44000000>; > - spi-cpha; > - spi-cpol; > + spi-max-frequency = <44000000>; > + spi-cpha; > + spi-cpol; > + > + status = "okay"; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + port@0 { > + reg = <0>; > + label = "lan1"; > + }; > + port@1 { > + reg = <1>; > + label = "lan2"; > + }; > + port@2 { > + reg = <2>; > + label = "lan3"; > + }; > + port@3 { > + reg = <3>; > + label = "lan4"; > + }; > + port@4 { > + reg = <4>; > + label = "lan5"; > + }; > + port@5 { > + reg = <5>; > + label = "cpu"; > + ethernet = <ð0>; > + fixed-link { > + speed = <1000>; > + full-duplex; > + }; > + }; > + }; > + }; > + }; > > - status = "okay"; > - ports { > - #address-cells = <1>; > - #size-cells = <0>; > - port@0 { > - reg = <0>; > - label = "lan1"; > - }; > - port@1 { > - reg = <1>; > - label = "lan2"; > - }; > - port@2 { > - reg = <2>; > - label = "lan3"; > - }; > - port@3 { > - reg = <3>; > - label = "lan4"; > - }; > - port@4 { > - reg = <4>; > - label = "lan5"; > - }; > - port@5 { > - reg = <5>; > - label = "cpu"; > - ethernet = <ð0>; > - fixed-link { > - speed = <1000>; > - full-duplex; > - }; > - }; > - }; > - }; > - }; > -- Florian ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-08 18:48 ` Florian Fainelli @ 2017-09-08 19:00 ` Maxim Uvarov 2017-09-08 19:48 ` Tristram.Ha 0 siblings, 1 reply; 18+ messages in thread From: Maxim Uvarov @ 2017-09-08 19:00 UTC (permalink / raw) To: Florian Fainelli Cc: Tristram.Ha, Andrew Lunn, Pavel Machek, Nathan Conrad, Vivien Didelot, netdev, linux-kernel, Woojung.Huh 2017-09-08 21:48 GMT+03:00 Florian Fainelli <f.fainelli@gmail.com>: > On 09/07/2017 02:11 PM, Tristram.Ha@microchip.com wrote: >> From: Tristram Ha <Tristram.Ha@microchip.com> >> >> Add other KSZ switches support so that patch check does not complain. >> >> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com> >> --- >> Documentation/devicetree/bindings/net/dsa/ksz.txt | 117 ++++++++++++---------- >> 1 file changed, 62 insertions(+), 55 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt b/Documentation/devicetree/bindings/net/dsa/ksz.txt >> index 0ab8b39..34af0e0 100644 >> --- a/Documentation/devicetree/bindings/net/dsa/ksz.txt >> +++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt >> @@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches >> >> Required properties: >> >> -- compatible: For external switch chips, compatible string must be exactly one >> - of: "microchip,ksz9477" >> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip, >> + "microchip,ksz8795" for KSZ8795 chip, >> + "microchip,ksz8794" for KSZ8794 chip, >> + "microchip,ksz8765" for KSZ8765 chip, >> + "microchip,ksz8895" for KSZ8895 chip, >> + "microchip,ksz8864" for KSZ8864 chip, >> + "microchip,ksz8873" for KSZ8873 chip, >> + "microchip,ksz8863" for KSZ8863 chip, >> + "microchip,ksz8463" for KSZ8463 chip > Tristram, does any of this devices support chaining? Maxim. > It becomes pretty obvious there is a 1 to 1 mapping between the > compatible name and what you should be using it for so specifying > ksz8795 for KSZ8795 chip is really redundant. > > You could just list all compatible strings that you support and change > the verbiage to be: > > compatible: Should be one of: > "microchip,ksz9477" > ... > "microchip,ksz8463" >> >> See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional required and optional properties. >> @@ -13,60 +20,60 @@ Examples: >> >> Ethernet switch connected via SPI to the host, CPU port wired to eth0: >> >> - eth0: ethernet@10001000 { >> - fixed-link { >> - speed = <1000>; >> - full-duplex; >> - }; >> - }; >> + eth0: ethernet@10001000 { >> + fixed-link { >> + speed = <1000>; >> + full-duplex; >> + }; >> + }; > > This is a good clean up, but it would probably belong in a separate > patch that you would do before adding compatible strings for instance. > >> >> - spi1: spi@f8008000 { >> - pinctrl-0 = <&pinctrl_spi_ksz>; >> - cs-gpios = <&pioC 25 0>; >> - id = <1>; >> - status = "okay"; >> + spi1: spi@f8008000 { >> + cs-gpios = <&pioC 25 0>; >> + id = <1>; >> + status = "okay"; >> >> - ksz9477: ksz9477@0 { >> - compatible = "microchip,ksz9477"; >> - reg = <0>; >> + ksz9477: ksz9477@0 { >> + compatible = "microchip,ksz9477"; >> + reg = <0>; >> >> - spi-max-frequency = <44000000>; >> - spi-cpha; >> - spi-cpol; >> + spi-max-frequency = <44000000>; >> + spi-cpha; >> + spi-cpol; >> + >> + status = "okay"; >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + port@0 { >> + reg = <0>; >> + label = "lan1"; >> + }; >> + port@1 { >> + reg = <1>; >> + label = "lan2"; >> + }; >> + port@2 { >> + reg = <2>; >> + label = "lan3"; >> + }; >> + port@3 { >> + reg = <3>; >> + label = "lan4"; >> + }; >> + port@4 { >> + reg = <4>; >> + label = "lan5"; >> + }; >> + port@5 { >> + reg = <5>; >> + label = "cpu"; >> + ethernet = <ð0>; >> + fixed-link { >> + speed = <1000>; >> + full-duplex; >> + }; >> + }; >> + }; >> + }; >> + }; >> >> - status = "okay"; >> - ports { >> - #address-cells = <1>; >> - #size-cells = <0>; >> - port@0 { >> - reg = <0>; >> - label = "lan1"; >> - }; >> - port@1 { >> - reg = <1>; >> - label = "lan2"; >> - }; >> - port@2 { >> - reg = <2>; >> - label = "lan3"; >> - }; >> - port@3 { >> - reg = <3>; >> - label = "lan4"; >> - }; >> - port@4 { >> - reg = <4>; >> - label = "lan5"; >> - }; >> - port@5 { >> - reg = <5>; >> - label = "cpu"; >> - ethernet = <ð0>; >> - fixed-link { >> - speed = <1000>; >> - full-duplex; >> - }; >> - }; >> - }; >> - }; >> - }; >> > > > -- > Florian -- Best regards, Maxim Uvarov ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-08 19:00 ` Maxim Uvarov @ 2017-09-08 19:48 ` Tristram.Ha 2017-09-08 19:54 ` Florian Fainelli 2017-09-11 7:53 ` Maxim Uvarov 0 siblings, 2 replies; 18+ messages in thread From: Tristram.Ha @ 2017-09-08 19:48 UTC (permalink / raw) To: muvarov Cc: andrew, pavel, nathan.leigh.conrad, vivien.didelot, netdev, linux-kernel, Woojung.Huh, f.fainelli > -----Original Message----- > From: Maxim Uvarov [mailto:muvarov@gmail.com] > Sent: Friday, September 08, 2017 12:00 PM > To: Florian Fainelli > Cc: Tristram Ha - C24268; Andrew Lunn; Pavel Machek; Nathan Conrad; Vivien > Didelot; netdev; linux-kernel@vger.kernel.org; Woojung Huh - C21699 > Subject: Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new > drivers can be added > > 2017-09-08 21:48 GMT+03:00 Florian Fainelli <f.fainelli@gmail.com>: > > On 09/07/2017 02:11 PM, Tristram.Ha@microchip.com wrote: > >> From: Tristram Ha <Tristram.Ha@microchip.com> > >> > >> Add other KSZ switches support so that patch check does not complain. > >> > >> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com> > >> --- > >> Documentation/devicetree/bindings/net/dsa/ksz.txt | 117 > >> ++++++++++++---------- > >> 1 file changed, 62 insertions(+), 55 deletions(-) > >> > >> diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt > >> b/Documentation/devicetree/bindings/net/dsa/ksz.txt > >> index 0ab8b39..34af0e0 100644 > >> --- a/Documentation/devicetree/bindings/net/dsa/ksz.txt > >> +++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt > >> @@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches > >> > >> Required properties: > >> > >> -- compatible: For external switch chips, compatible string must be > >> exactly one > >> - of: "microchip,ksz9477" > >> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip, > >> + "microchip,ksz8795" for KSZ8795 chip, > >> + "microchip,ksz8794" for KSZ8794 chip, > >> + "microchip,ksz8765" for KSZ8765 chip, > >> + "microchip,ksz8895" for KSZ8895 chip, > >> + "microchip,ksz8864" for KSZ8864 chip, > >> + "microchip,ksz8873" for KSZ8873 chip, > >> + "microchip,ksz8863" for KSZ8863 chip, > >> + "microchip,ksz8463" for KSZ8463 chip > > > > > Tristram, does any of this devices support chaining? > > Maxim. They do not if you mean daisy-chaining the switches together. There is always a problem that once tail tagging mode is enabled sending a frame through the MAC without going through the DSA layer will cause the frame to be dropped. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-08 19:48 ` Tristram.Ha @ 2017-09-08 19:54 ` Florian Fainelli 2017-09-08 20:07 ` Tristram.Ha 2017-09-11 7:53 ` Maxim Uvarov 1 sibling, 1 reply; 18+ messages in thread From: Florian Fainelli @ 2017-09-08 19:54 UTC (permalink / raw) To: Tristram.Ha, muvarov Cc: andrew, pavel, nathan.leigh.conrad, vivien.didelot, netdev, linux-kernel, Woojung.Huh On 09/08/2017 12:48 PM, Tristram.Ha@microchip.com wrote: >> -----Original Message----- >> From: Maxim Uvarov [mailto:muvarov@gmail.com] >> Sent: Friday, September 08, 2017 12:00 PM >> To: Florian Fainelli >> Cc: Tristram Ha - C24268; Andrew Lunn; Pavel Machek; Nathan Conrad; Vivien >> Didelot; netdev; linux-kernel@vger.kernel.org; Woojung Huh - C21699 >> Subject: Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new >> drivers can be added >> >> 2017-09-08 21:48 GMT+03:00 Florian Fainelli <f.fainelli@gmail.com>: >>> On 09/07/2017 02:11 PM, Tristram.Ha@microchip.com wrote: >>>> From: Tristram Ha <Tristram.Ha@microchip.com> >>>> >>>> Add other KSZ switches support so that patch check does not complain. >>>> >>>> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com> >>>> --- >>>> Documentation/devicetree/bindings/net/dsa/ksz.txt | 117 >>>> ++++++++++++---------- >>>> 1 file changed, 62 insertions(+), 55 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt >>>> b/Documentation/devicetree/bindings/net/dsa/ksz.txt >>>> index 0ab8b39..34af0e0 100644 >>>> --- a/Documentation/devicetree/bindings/net/dsa/ksz.txt >>>> +++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt >>>> @@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches >>>> >>>> Required properties: >>>> >>>> -- compatible: For external switch chips, compatible string must be >>>> exactly one >>>> - of: "microchip,ksz9477" >>>> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip, >>>> + "microchip,ksz8795" for KSZ8795 chip, >>>> + "microchip,ksz8794" for KSZ8794 chip, >>>> + "microchip,ksz8765" for KSZ8765 chip, >>>> + "microchip,ksz8895" for KSZ8895 chip, >>>> + "microchip,ksz8864" for KSZ8864 chip, >>>> + "microchip,ksz8873" for KSZ8873 chip, >>>> + "microchip,ksz8863" for KSZ8863 chip, >>>> + "microchip,ksz8463" for KSZ8463 chip >>> >> >> >> Tristram, does any of this devices support chaining? >> >> Maxim. > > They do not if you mean daisy-chaining the switches together. Marvell tags allow you to specify both a port and switch index destination after setting up an appropriate routing table, I am assuming this is not supported. What happens though if I connect two KSZ switches ones to another say: eth0 -> KSZ8463 -> KSZ8463 Will the first switch terminates KSZ tag if it sees two tags encapsulated in another, something like this: | MAC DA | MAC SA | .... payload | Inner KSZ tag | Inner KSZ tag | FCS | > > There is always a problem that once tail tagging mode is enabled > sending a frame through the MAC without going through the DSA > layer will cause the frame to be dropped. Yes, once the master network device is used for DSA, it is still usable directly by e.g: applications, but it won't do anything if the switch is configured such that it drops ingressing frames not having the proper tag. We documented that here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/networking/dsa/dsa.txt#n275 -- Florian ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-08 19:54 ` Florian Fainelli @ 2017-09-08 20:07 ` Tristram.Ha 2017-09-08 20:58 ` Florian Fainelli 0 siblings, 1 reply; 18+ messages in thread From: Tristram.Ha @ 2017-09-08 20:07 UTC (permalink / raw) To: f.fainelli Cc: andrew, pavel, nathan.leigh.conrad, vivien.didelot, netdev, linux-kernel, Woojung.Huh, muvarov > -----Original Message----- > From: Florian Fainelli [mailto:f.fainelli@gmail.com] > Sent: Friday, September 08, 2017 12:54 PM > To: Tristram Ha - C24268; muvarov@gmail.com > Cc: andrew@lunn.ch; pavel@ucw.cz; nathan.leigh.conrad@gmail.com; > vivien.didelot@savoirfairelinux.com; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; Woojung Huh - C21699 > Subject: Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new > drivers can be added > > On 09/08/2017 12:48 PM, Tristram.Ha@microchip.com wrote: > >> -----Original Message----- > >> From: Maxim Uvarov [mailto:muvarov@gmail.com] > >> Sent: Friday, September 08, 2017 12:00 PM > >> To: Florian Fainelli > >> Cc: Tristram Ha - C24268; Andrew Lunn; Pavel Machek; Nathan Conrad; Vivien > >> Didelot; netdev; linux-kernel@vger.kernel.org; Woojung Huh - C21699 > >> Subject: Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that > new > >> drivers can be added > >> > >> 2017-09-08 21:48 GMT+03:00 Florian Fainelli <f.fainelli@gmail.com>: > >>> On 09/07/2017 02:11 PM, Tristram.Ha@microchip.com wrote: > >>>> From: Tristram Ha <Tristram.Ha@microchip.com> > >>>> > >>>> Add other KSZ switches support so that patch check does not complain. > >>>> > >>>> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com> > >>>> --- > >>>> Documentation/devicetree/bindings/net/dsa/ksz.txt | 117 > >>>> ++++++++++++---------- > >>>> 1 file changed, 62 insertions(+), 55 deletions(-) > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt > >>>> b/Documentation/devicetree/bindings/net/dsa/ksz.txt > >>>> index 0ab8b39..34af0e0 100644 > >>>> --- a/Documentation/devicetree/bindings/net/dsa/ksz.txt > >>>> +++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt > >>>> @@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches > >>>> > >>>> Required properties: > >>>> > >>>> -- compatible: For external switch chips, compatible string must be > >>>> exactly one > >>>> - of: "microchip,ksz9477" > >>>> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip, > >>>> + "microchip,ksz8795" for KSZ8795 chip, > >>>> + "microchip,ksz8794" for KSZ8794 chip, > >>>> + "microchip,ksz8765" for KSZ8765 chip, > >>>> + "microchip,ksz8895" for KSZ8895 chip, > >>>> + "microchip,ksz8864" for KSZ8864 chip, > >>>> + "microchip,ksz8873" for KSZ8873 chip, > >>>> + "microchip,ksz8863" for KSZ8863 chip, > >>>> + "microchip,ksz8463" for KSZ8463 chip > >>> > >> > >> > >> Tristram, does any of this devices support chaining? > >> > >> Maxim. > > > > They do not if you mean daisy-chaining the switches together. > > Marvell tags allow you to specify both a port and switch index > destination after setting up an appropriate routing table, I am assuming > this is not supported. > > What happens though if I connect two KSZ switches ones to another say: > > eth0 > -> KSZ8463 > -> KSZ8463 > > Will the first switch terminates KSZ tag if it sees two tags > encapsulated in another, something like this: > > | MAC DA | MAC SA | .... payload | Inner KSZ tag | Inner KSZ tag | FCS | > In theory it is doable by adding more tags and remember which port is connected to the cpu port of another switch, but there is no switch forwarding and everything is handled by software. > > > > There is always a problem that once tail tagging mode is enabled > > sending a frame through the MAC without going through the DSA > > layer will cause the frame to be dropped. > > Yes, once the master network device is used for DSA, it is still usable > directly by e.g: applications, but it won't do anything if the switch is > configured such that it drops ingressing frames not having the proper > tag. We documented that here: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docume > ntation/networking/dsa/dsa.txt#n275 > -- As the DSA was developed for the Marvell switches I assumed they can forward frame without a tag. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-08 20:07 ` Tristram.Ha @ 2017-09-08 20:58 ` Florian Fainelli 0 siblings, 0 replies; 18+ messages in thread From: Florian Fainelli @ 2017-09-08 20:58 UTC (permalink / raw) To: Tristram.Ha Cc: andrew, pavel, nathan.leigh.conrad, vivien.didelot, netdev, linux-kernel, Woojung.Huh, muvarov On 09/08/2017 01:07 PM, Tristram.Ha@microchip.com wrote: >> -----Original Message----- >> From: Florian Fainelli [mailto:f.fainelli@gmail.com] >> Sent: Friday, September 08, 2017 12:54 PM >> To: Tristram Ha - C24268; muvarov@gmail.com >> Cc: andrew@lunn.ch; pavel@ucw.cz; nathan.leigh.conrad@gmail.com; >> vivien.didelot@savoirfairelinux.com; netdev@vger.kernel.org; linux- >> kernel@vger.kernel.org; Woojung Huh - C21699 >> Subject: Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new >> drivers can be added >> >> On 09/08/2017 12:48 PM, Tristram.Ha@microchip.com wrote: >>>> -----Original Message----- >>>> From: Maxim Uvarov [mailto:muvarov@gmail.com] >>>> Sent: Friday, September 08, 2017 12:00 PM >>>> To: Florian Fainelli >>>> Cc: Tristram Ha - C24268; Andrew Lunn; Pavel Machek; Nathan Conrad; Vivien >>>> Didelot; netdev; linux-kernel@vger.kernel.org; Woojung Huh - C21699 >>>> Subject: Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that >> new >>>> drivers can be added >>>> >>>> 2017-09-08 21:48 GMT+03:00 Florian Fainelli <f.fainelli@gmail.com>: >>>>> On 09/07/2017 02:11 PM, Tristram.Ha@microchip.com wrote: >>>>>> From: Tristram Ha <Tristram.Ha@microchip.com> >>>>>> >>>>>> Add other KSZ switches support so that patch check does not complain. >>>>>> >>>>>> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com> >>>>>> --- >>>>>> Documentation/devicetree/bindings/net/dsa/ksz.txt | 117 >>>>>> ++++++++++++---------- >>>>>> 1 file changed, 62 insertions(+), 55 deletions(-) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt >>>>>> b/Documentation/devicetree/bindings/net/dsa/ksz.txt >>>>>> index 0ab8b39..34af0e0 100644 >>>>>> --- a/Documentation/devicetree/bindings/net/dsa/ksz.txt >>>>>> +++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt >>>>>> @@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches >>>>>> >>>>>> Required properties: >>>>>> >>>>>> -- compatible: For external switch chips, compatible string must be >>>>>> exactly one >>>>>> - of: "microchip,ksz9477" >>>>>> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip, >>>>>> + "microchip,ksz8795" for KSZ8795 chip, >>>>>> + "microchip,ksz8794" for KSZ8794 chip, >>>>>> + "microchip,ksz8765" for KSZ8765 chip, >>>>>> + "microchip,ksz8895" for KSZ8895 chip, >>>>>> + "microchip,ksz8864" for KSZ8864 chip, >>>>>> + "microchip,ksz8873" for KSZ8873 chip, >>>>>> + "microchip,ksz8863" for KSZ8863 chip, >>>>>> + "microchip,ksz8463" for KSZ8463 chip >>>>> >>>> >>>> >>>> Tristram, does any of this devices support chaining? >>>> >>>> Maxim. >>> >>> They do not if you mean daisy-chaining the switches together. >> >> Marvell tags allow you to specify both a port and switch index >> destination after setting up an appropriate routing table, I am assuming >> this is not supported. >> >> What happens though if I connect two KSZ switches ones to another say: >> >> eth0 >> -> KSZ8463 >> -> KSZ8463 >> >> Will the first switch terminates KSZ tag if it sees two tags >> encapsulated in another, something like this: >> >> | MAC DA | MAC SA | .... payload | Inner KSZ tag | Inner KSZ tag | FCS | >> > > In theory it is doable by adding more tags and remember which port > is connected to the cpu port of another switch, but there is no switch > forwarding and everything is handled by software. Fair enough. > >>> >>> There is always a problem that once tail tagging mode is enabled >>> sending a frame through the MAC without going through the DSA >>> layer will cause the frame to be dropped. >> >> Yes, once the master network device is used for DSA, it is still usable >> directly by e.g: applications, but it won't do anything if the switch is >> configured such that it drops ingressing frames not having the proper >> tag. We documented that here: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docume >> ntation/networking/dsa/dsa.txt#n275 >> -- > > As the DSA was developed for the Marvell switches I assumed they can > forward frame without a tag. Once you use tags with your switch, eth0/master netdev/conduit interface no longer has a purpose as a normal interface because we create per-port network devices. That means if you want to send packets towards Port 0 you use the proper network interface. If you create a bridge, you will use brX as the network device to send/receive packets from, in all cases, the packets originate from the CPU and the frame ingresses the switch with the proper information within the tag to target the vector of ports. If you have tags enabled and you use eth0 to send packets towards the switch, there are not that many options, either you have the proper tag, and the switch will forward to the right port which can be the CPU port itself if you want, but why do that? When tags are not enabled (e.g: b53) that is slightly different, eth0/master/conduit remains usable as a normal interface would, but unless you start adding VLAN tags, you cannot quite differentiate where the traffic from LAN to CPU is coming from, so this has limited usefulness. Hope this clears things. -- Florian ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added 2017-09-08 19:48 ` Tristram.Ha 2017-09-08 19:54 ` Florian Fainelli @ 2017-09-11 7:53 ` Maxim Uvarov 1 sibling, 0 replies; 18+ messages in thread From: Maxim Uvarov @ 2017-09-11 7:53 UTC (permalink / raw) To: Tristram.Ha Cc: Andrew Lunn, Pavel Machek, Nathan Conrad, Vivien Didelot, netdev, linux-kernel, Woojung.Huh, Florian Fainelli 2017-09-08 22:48 GMT+03:00 <Tristram.Ha@microchip.com>: >> -----Original Message----- >> From: Maxim Uvarov [mailto:muvarov@gmail.com] >> Sent: Friday, September 08, 2017 12:00 PM >> To: Florian Fainelli >> Cc: Tristram Ha - C24268; Andrew Lunn; Pavel Machek; Nathan Conrad; Vivien >> Didelot; netdev; linux-kernel@vger.kernel.org; Woojung Huh - C21699 >> Subject: Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new >> drivers can be added >> >> 2017-09-08 21:48 GMT+03:00 Florian Fainelli <f.fainelli@gmail.com>: >> > On 09/07/2017 02:11 PM, Tristram.Ha@microchip.com wrote: >> >> From: Tristram Ha <Tristram.Ha@microchip.com> >> >> >> >> Add other KSZ switches support so that patch check does not complain. >> >> >> >> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com> >> >> --- >> >> Documentation/devicetree/bindings/net/dsa/ksz.txt | 117 >> >> ++++++++++++---------- >> >> 1 file changed, 62 insertions(+), 55 deletions(-) >> >> >> >> diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt >> >> b/Documentation/devicetree/bindings/net/dsa/ksz.txt >> >> index 0ab8b39..34af0e0 100644 >> >> --- a/Documentation/devicetree/bindings/net/dsa/ksz.txt >> >> +++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt >> >> @@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches >> >> >> >> Required properties: >> >> >> >> -- compatible: For external switch chips, compatible string must be >> >> exactly one >> >> - of: "microchip,ksz9477" >> >> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip, >> >> + "microchip,ksz8795" for KSZ8795 chip, >> >> + "microchip,ksz8794" for KSZ8794 chip, >> >> + "microchip,ksz8765" for KSZ8765 chip, >> >> + "microchip,ksz8895" for KSZ8895 chip, >> >> + "microchip,ksz8864" for KSZ8864 chip, >> >> + "microchip,ksz8873" for KSZ8873 chip, >> >> + "microchip,ksz8863" for KSZ8863 chip, >> >> + "microchip,ksz8463" for KSZ8463 chip >> > >> >> >> Tristram, does any of this devices support chaining? >> >> Maxim. > > They do not if you mean daisy-chaining the switches together. > > There is always a problem that once tail tagging mode is enabled > sending a frame through the MAC without going through the DSA > layer will cause the frame to be dropped. > Tistram, as Florian answered before by "chaining" in my question I meant milti chip DSA. I.e. when several chips represent one DSA instance and all interfaces joined to the same bridge. Bridge code take care about fdb, mdb, vlan configuration for all chips. If packet supposed to be forward across chips it will not go to cpu, Only cpus related traffic goes to cpu (like broad casts, fdb mac entry, unknown packet, stp/BPDU). I.e. milti chip DSA allows you to work with several chips as one big virtual chip. How to make it happen depends on hardware possibilities. Might be several tail tags, but in that case other chips have to know how to work with it. Or smart vlans configuration, but it that case cross chip vlan should not overlap with system vlans. In marvel it's done that each tag has chip id which can be programmed from mdio, and also each chip knows how to work with that tag and their place in dsa chain - "up-link" and "down-link". For ksz* I did not find any notes that such configuration is supported. How ever it might be doable with some smart software settings. -- Best regards, Maxim Uvarov ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2017-09-11 7:53 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-09-07 21:11 [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added Tristram.Ha 2017-09-07 21:54 ` Andrew Lunn 2017-09-08 13:32 ` Maxim Uvarov 2017-09-08 14:12 ` Andrew Lunn 2017-09-08 18:40 ` Tristram.Ha 2017-09-08 18:45 ` Florian Fainelli 2017-09-08 19:01 ` Andrew Lunn 2017-09-08 19:05 ` Florian Fainelli 2017-09-08 21:50 ` Pavel Machek 2017-09-08 21:53 ` Pavel Machek 2017-09-08 9:04 ` Pavel Machek 2017-09-08 18:48 ` Florian Fainelli 2017-09-08 19:00 ` Maxim Uvarov 2017-09-08 19:48 ` Tristram.Ha 2017-09-08 19:54 ` Florian Fainelli 2017-09-08 20:07 ` Tristram.Ha 2017-09-08 20:58 ` Florian Fainelli 2017-09-11 7:53 ` Maxim Uvarov
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).