| Message ID | 20180704191759.12577-3-linus.walleij@linaro.org |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [OpenWrt-Devel,1/3] ARM: dts: Add WAN ethernet port to the SQ201 | expand |
On Wed, Jul 04, 2018 at 09:17:59PM +0200, Linus Walleij wrote: > The Storlink Gemini324 EV-Board also known as Storm > Semiconductor SL93512R_BRD is ground zero for the Gemini > devices. We add a device tree so we can support it, it > turns out to be pretty trivial. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > arch/arm/boot/dts/Makefile | 1 + > arch/arm/boot/dts/gemini-sl93512r.dts | 325 ++++++++++++++++++++++++++ > 2 files changed, 326 insertions(+) > create mode 100644 arch/arm/boot/dts/gemini-sl93512r.dts > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > index 37a3de760d40..a10ef98c6d75 100644 > --- a/arch/arm/boot/dts/Makefile > +++ b/arch/arm/boot/dts/Makefile > @@ -200,6 +200,7 @@ dtb-$(CONFIG_ARCH_GEMINI) += \ > gemini-dlink-dns-313.dtb \ > gemini-nas4220b.dtb \ > gemini-rut1xx.dtb \ > + gemini-sl93512r.dtb \ > gemini-sq201.dtb \ > gemini-wbd111.dtb \ > gemini-wbd222.dtb > diff --git a/arch/arm/boot/dts/gemini-sl93512r.dts b/arch/arm/boot/dts/gemini-sl93512r.dts > new file mode 100644 > index 000000000000..6160538bbb54 > --- /dev/null > +++ b/arch/arm/boot/dts/gemini-sl93512r.dts > @@ -0,0 +1,325 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Device Tree file for the Storm Semiconductor SL93512R_BRD > + * Gemini reference design, also initially called > + * "Gemini324 EV-Board" before Storm acquired Storlink Semiconductor. > + * The series were later acquired by Cortina Systems. > + */ > + > +/dts-v1/; > + > +#include "gemini.dtsi" > +#include <dt-bindings/input/input.h> > + > +/ { > + model = "Storlink Semiconductor Gemini324 EV-Board / Storm Semiconductor SL93512R_BRD"; > + compatible = "storlink,gemini324", "storm,sl93512r", "cortina,gemini"; > + #address-cells = <1>; > + #size-cells = <1>; > + > + memory@0 { > + /* 64 MB Samsung K4H511638B */ > + device_type = "memory"; > + reg = <0x00000000 0x4000000>; > + }; > + > + chosen { > + bootargs = "console=ttyS0,19200n8 root=/dev/sda1 rw rootwait"; > + stdout-path = &uart0; Hi Linus You should put the baud rate as part of the stdout-patch, not in bootargs. > + mdio0: ethernet-phy { mdio0: mdio > + compatible = "virtual,mdio-gpio"; > + /* Uses MDC and MDIO */ > + gpios = <&gpio0 22 GPIO_ACTIVE_HIGH>, /* MDC */ > + <&gpio0 21 GPIO_ACTIVE_HIGH>; /* MDIO */ > + #address-cells = <1>; > + #size-cells = <0>; > + > + /* This is a Marvell 88E1111 ethernet transciever */ > + phy0: ethernet-phy@1 { > + reg = <1>; > + device_type = "ethernet-phy"; No device_type please. > + }; > + }; > + > + spi { > + compatible = "spi-gpio"; > + #address-cells = <1>; > + #size-cells = <0>; > + /* Check pin collisions */ > + gpio-sck = <&gpio1 28 GPIO_ACTIVE_HIGH>; > + gpio-miso = <&gpio1 30 GPIO_ACTIVE_HIGH>; > + gpio-mosi = <&gpio1 29 GPIO_ACTIVE_HIGH>; > + cs-gpios = <&gpio1 31 GPIO_ACTIVE_HIGH>; > + num-chipselects = <1>; > + > + switch@0 { > + compatible = "vitesse,vsc7385"; > + reg = <0>; > + /* Specified for 2.5 MHz or below */ > + spi-max-frequency = <2500000>; > + gpio-controller; > + #gpio-cells = <2>; > + > + 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"; > + }; > + vsc: port@6 { > + reg = <6>; > + label = "cpu"; > + ethernet = <&gmac1>; > + phy-mode = "rgmii"; > + fixed-link { > + speed = <1000>; > + full-duplex; > + pause; > + }; > + }; > + }; > + }; > + }; > + ethernet@60000000 { > + status = "okay"; > + > + ethernet-port@0 { > + phy-mode = "rgmii"; > + phy-handle = <&phy0>; > + }; > + ethernet-port@1 { > + phy-mode = "rgmii"; > + phy-handle = <&vsc>; This looks odd. The switch port is not a PHY. Normally you use a fixed-phy. Andrew
On Wed, Jul 4, 2018 at 10:56 PM Andrew Lunn <andrew@lunn.ch> wrote: > On Wed, Jul 04, 2018 at 09:17:59PM +0200, Linus Walleij wrote: > > + vsc: port@6 { > > + reg = <6>; > > + label = "cpu"; > > + ethernet = <&gmac1>; > > + phy-mode = "rgmii"; > > + fixed-link { > > + speed = <1000>; > > + full-duplex; > > + pause; > > + }; So this DSA side is fine like so. (...) > > + ethernet@60000000 { > > + status = "okay"; > > + > > + ethernet-port@0 { > > + phy-mode = "rgmii"; > > + phy-handle = <&phy0>; > > + }; > > + ethernet-port@1 { > > + phy-mode = "rgmii"; > > + phy-handle = <&vsc>; > > This looks odd. The switch port is not a PHY. Normally you use a > fixed-phy. I tried some stuff but can't figure this out any other way that also works :( I guess you mean a fixed-link as the kernel has no references to fixed-phy. This works: ethernet-port@1 { phy-mode = "rgmii"; phy-handle = <&vsc>; fixed-link { speed = <1000>; full-duplex; pause; }; }; Is it better? It is still using phy-handle... I couldn't find a way to live without that. But maybe this was all you expected? phy-mode and phy-handle still OK despite being named phy-something? Or do you have some example of how it should look? Yours, Linus Walleij
> > > + vsc: port@6 { > > > + reg = <6>; > > > + label = "cpu"; > > > + ethernet = <&gmac1>; > > > + phy-mode = "rgmii"; > > > + fixed-link { > > > + speed = <1000>; > > > + full-duplex; > > > + pause; > > > + }; > > So this DSA side is fine like so. Hi Linus Yes, this is how i normally do it, if the defaults don't work. Most DSA drivers will configure the CPU port to its maximum speed. But if you need to slow it down, you can use a fixed-link. e.g. i have boards with a 1Gbps switch connected to a SoC with 100Mbps Ethernet. So i need a fixed link with speed=<100>. > (...) > > > > + ethernet@60000000 { > > > + status = "okay"; > > > + > > > + ethernet-port@0 { > > > + phy-mode = "rgmii"; > > > + phy-handle = <&phy0>; > > > + }; > > > + ethernet-port@1 { > > > + phy-mode = "rgmii"; > > > + phy-handle = <&vsc>; > > > > This looks odd. The switch port is not a PHY. Normally you use a > > fixed-phy. > > I tried some stuff but can't figure this out any other way that also > works :( > > I guess you mean a fixed-link as the kernel has no references > to fixed-phy. Sorry, yes, i meant fixed link. > This works: > > ethernet-port@1 { > phy-mode = "rgmii"; > phy-handle = <&vsc>; > fixed-link { > speed = <1000>; > full-duplex; > pause; > }; > }; > > Is it better? It is still using phy-handle... I couldn't find a way to live > without that. But maybe this was all you expected? phy-mode > and phy-handle still OK despite being named phy-something? > > Or do you have some example of how it should look? arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts ð0 { phy-mode = "rgmii-id"; status = "okay"; fixed-link { speed = <1000>; full-duplex; }; }; phy-mode is O.K, since you need to set RGMII. phy-handle should not be used, since the fixed-link takes its place. But it could be your MAC driver has problems, does not correctly support fixed link, and has phy-handle as being mandatory, when it should not be. Which MAC driver is this? Andrew
On Fri, Jul 6, 2018 at 12:37 AM Andrew Lunn <andrew@lunn.ch> wrote: > arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts > > ð0 { > phy-mode = "rgmii-id"; > status = "okay"; > > fixed-link { > speed = <1000>; > full-duplex; > }; > }; > > phy-mode is O.K, since you need to set RGMII. OK hm maybe we should have "link-mode" as an alternative as it makes it less stressful syntactically... > phy-handle should not be used, since the fixed-link takes its > place. But it could be your MAC driver has problems, does not > correctly support fixed link, and has phy-handle as being mandatory, > when it should not be. > > Which MAC driver is this? This is drivers/net/ethernet/cortina/gemini.c My own driver code so the errors are likely mine... the callback trail is usually: gemini_ethernet_port_probe() or .ndo_open() -> gmac_open() I guess that is normal as net_device_ops doesn't have any .adjust_link() callback or so. then: if (!netdev->phydev) gmac_setup_phy(netdev); gmac_setup_phy() of_phy_get_and_connect(netdev, dev->of_node, gmac_speed_set); gmac_speed_set() As long as a "phydev" is carrying over the fixed link info it should be fine I think, this just sets up the RGMII speed in this case, but it doesn't get opened or something. Yours, Linus Walleij
On Sun, Jul 08, 2018 at 09:53:39PM +0200, Linus Walleij wrote: > On Fri, Jul 6, 2018 at 12:37 AM Andrew Lunn <andrew@lunn.ch> wrote: > > > arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts > > > > ð0 { > > phy-mode = "rgmii-id"; > > status = "okay"; > > > > fixed-link { > > speed = <1000>; > > full-duplex; > > }; > > }; > > > > phy-mode is O.K, since you need to set RGMII. > > OK hm maybe we should have "link-mode" as an alternative > as it makes it less stressful syntactically... > > > phy-handle should not be used, since the fixed-link takes its > > place. But it could be your MAC driver has problems, does not > > correctly support fixed link, and has phy-handle as being mandatory, > > when it should not be. > > > > Which MAC driver is this? > > This is > drivers/net/ethernet/cortina/gemini.c > > My own driver code so the errors are likely mine... Hi Linus drivers/net/ethernet/cortina/gemini.c does not appear to have anything to actually parse the fixed-link properties. I would probably extend of_phy_get_and_connect() to do this parsing. Something like this, which is not even compile tested: @@ -367,14 +367,24 @@ struct phy_device *of_phy_get_and_connect(struct net_device *dev, phy_interface_t iface; struct device_node *phy_np; struct phy_device *phy; + int ret; - iface = of_get_phy_mode(np); - if (iface < 0) - return NULL; + if (of_phy_is_fixed_link(np)) { + ret = of_phy_register_fixed_link(np); + if (ret < 0) { + dev_err(dev, "broken fixed-link specification\n"); + return NULL; + } + phy_np = of_node_get(np); + } else { + iface = of_get_phy_mode(np); + if (iface < 0) + return NULL; - phy_np = of_parse_phandle(np, "phy-handle", 0); - if (!phy_np) - return NULL; + phy_np = of_parse_phandle(np, "phy-handle", 0); + if (!phy_np) + return NULL; + } phy = of_phy_connect(dev, phy_np, hndlr, 0, iface); Andrew
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 37a3de760d40..a10ef98c6d75 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -200,6 +200,7 @@ dtb-$(CONFIG_ARCH_GEMINI) += \ gemini-dlink-dns-313.dtb \ gemini-nas4220b.dtb \ gemini-rut1xx.dtb \ + gemini-sl93512r.dtb \ gemini-sq201.dtb \ gemini-wbd111.dtb \ gemini-wbd222.dtb diff --git a/arch/arm/boot/dts/gemini-sl93512r.dts b/arch/arm/boot/dts/gemini-sl93512r.dts new file mode 100644 index 000000000000..6160538bbb54 --- /dev/null +++ b/arch/arm/boot/dts/gemini-sl93512r.dts @@ -0,0 +1,325 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Device Tree file for the Storm Semiconductor SL93512R_BRD + * Gemini reference design, also initially called + * "Gemini324 EV-Board" before Storm acquired Storlink Semiconductor. + * The series were later acquired by Cortina Systems. + */ + +/dts-v1/; + +#include "gemini.dtsi" +#include <dt-bindings/input/input.h> + +/ { + model = "Storlink Semiconductor Gemini324 EV-Board / Storm Semiconductor SL93512R_BRD"; + compatible = "storlink,gemini324", "storm,sl93512r", "cortina,gemini"; + #address-cells = <1>; + #size-cells = <1>; + + memory@0 { + /* 64 MB Samsung K4H511638B */ + device_type = "memory"; + reg = <0x00000000 0x4000000>; + }; + + chosen { + bootargs = "console=ttyS0,19200n8 root=/dev/sda1 rw rootwait"; + stdout-path = &uart0; + }; + + gpio_keys { + compatible = "gpio-keys"; + + button-wps { + debounce-interval = <50>; + wakeup-source; + linux,code = <KEY_WPS_BUTTON>; + label = "WPS"; + /* Conflict with NAND flash */ + gpios = <&gpio0 17 GPIO_ACTIVE_LOW>; + }; + + button-setup { + debounce-interval = <50>; + wakeup-source; + linux,code = <KEY_SETUP>; + label = "factory reset"; + /* Conflict with NAND flash */ + gpios = <&gpio0 18 GPIO_ACTIVE_LOW>; + }; + }; + + leds { + compatible = "gpio-leds"; + led-green-harddisk { + label = "sq201:green:harddisk"; + /* Conflict with LCD (no problem) */ + gpios = <&gpio0 16 GPIO_ACTIVE_LOW>; + default-state = "off"; + linux,default-trigger = "disk-activity"; + }; + led-green-wireless { + label = "sq201:green:wireless"; + /* Conflict with NAND flash CE0 (no problem) */ + gpios = <&gpio0 17 GPIO_ACTIVE_LOW>; + default-state = "on"; + linux,default-trigger = "heartbeat"; + }; + }; + + mdio0: ethernet-phy { + compatible = "virtual,mdio-gpio"; + /* Uses MDC and MDIO */ + gpios = <&gpio0 22 GPIO_ACTIVE_HIGH>, /* MDC */ + <&gpio0 21 GPIO_ACTIVE_HIGH>; /* MDIO */ + #address-cells = <1>; + #size-cells = <0>; + + /* This is a Marvell 88E1111 ethernet transciever */ + phy0: ethernet-phy@1 { + reg = <1>; + device_type = "ethernet-phy"; + }; + }; + + spi { + compatible = "spi-gpio"; + #address-cells = <1>; + #size-cells = <0>; + /* Check pin collisions */ + gpio-sck = <&gpio1 28 GPIO_ACTIVE_HIGH>; + gpio-miso = <&gpio1 30 GPIO_ACTIVE_HIGH>; + gpio-mosi = <&gpio1 29 GPIO_ACTIVE_HIGH>; + cs-gpios = <&gpio1 31 GPIO_ACTIVE_HIGH>; + num-chipselects = <1>; + + switch@0 { + compatible = "vitesse,vsc7385"; + reg = <0>; + /* Specified for 2.5 MHz or below */ + spi-max-frequency = <2500000>; + gpio-controller; + #gpio-cells = <2>; + + 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"; + }; + vsc: port@6 { + reg = <6>; + label = "cpu"; + ethernet = <&gmac1>; + phy-mode = "rgmii"; + fixed-link { + speed = <1000>; + full-duplex; + pause; + }; + }; + }; + }; + }; + + + soc { + flash@30000000 { + status = "okay"; + /* 16MB of flash */ + reg = <0x30000000 0x01000000>; + + partition@0 { + label = "BOOT"; + reg = <0x00000000 0x00020000>; + read-only; + }; + partition@120000 { + label = "Kern"; + reg = <0x00020000 0x00300000>; + }; + partition@320000 { + label = "Ramdisk"; + reg = <0x00320000 0x00600000>; + }; + partition@920000 { + label = "Application"; + reg = <0x00920000 0x00600000>; + }; + partition@f20000 { + label = "VCTL"; + reg = <0x00f20000 0x00020000>; + read-only; + }; + partition@f40000 { + label = "CurConf"; + reg = <0x00f40000 0x000a0000>; + read-only; + }; + partition@fe0000 { + label = "FIS directory"; + reg = <0x00fe0000 0x00020000>; + read-only; + }; + }; + + syscon: syscon@40000000 { + pinctrl { + /* + * gpio0egrp cover line 16 used by HD LED + * gpio0fgrp cover line 17, 18 used by wireless LED and reset button + * gpio0hgrp cover line 21, 22 used by MDIO for Marvell PHY + * gpio0kgrp cover line 31 used by USB LED + */ + gpio0_default_pins: pinctrl-gpio0 { + mux { + function = "gpio0"; + groups = "gpio0egrp", + "gpio0fgrp", + "gpio0hgrp"; + }; + }; + /* + * gpio1dgrp cover lines used by SPI for + * the Vitesse chip (28-31) + */ + gpio1_default_pins: pinctrl-gpio1 { + mux { + function = "gpio1"; + groups = "gpio1dgrp"; + }; + }; + pinctrl-gmii { + mux { + function = "gmii"; + groups = "gmii_gmac0_grp", "gmii_gmac1_grp"; + }; + /* Control pad skew comes from sl_switch.c in the vendor code */ + conf0 { + pins = "P10 GMAC1 TXC"; + skew-delay = <5>; + }; + conf1 { + pins = "V11 GMAC1 TXEN"; + skew-delay = <7>; + }; + conf2 { + pins = "T11 GMAC1 RXC"; + skew-delay = <8>; + }; + conf3 { + pins = "U11 GMAC1 RXDV"; + skew-delay = <7>; + }; + conf4 { + pins = "V7 GMAC0 TXC"; + skew-delay = <10>; + }; + conf5 { + pins = "P8 GMAC0 TXEN"; + skew-delay = <7>; /* 5 at another place? */ + }; + conf6 { + pins = "T8 GMAC0 RXC"; + skew-delay = <15>; + }; + conf7 { + pins = "R8 GMAC0 RXDV"; + skew-delay = <0>; + }; + conf8 { + /* The data lines all have default skew */ + pins = "U8 GMAC0 RXD0", "V8 GMAC0 RXD1", + "P9 GMAC0 RXD2", "R9 GMAC0 RXD3", + "R11 GMAC1 RXD0", "P11 GMAC1 RXD1", + "V12 GMAC1 RXD2", "U12 GMAC1 RXD3", + "R10 GMAC1 TXD0", "T10 GMAC1 TXD1", + "U10 GMAC1 TXD2", "V10 GMAC1 TXD3"; + skew-delay = <7>; + }; + /* Appears in sl351x_gmac.c in the vendor code */ + conf9 { + pins = "U7 GMAC0 TXD0", "T7 GMAC0 TXD1", + "R7 GMAC0 TXD2", "P7 GMAC0 TXD3"; + skew-delay = <5>; + }; + }; + }; + }; + + /* Both interfaces brought out on SATA connectors */ + sata: sata@46000000 { + cortina,gemini-ata-muxmode = <0>; + cortina,gemini-enable-sata-bridge; + status = "okay"; + }; + + gpio0: gpio@4d000000 { + pinctrl-names = "default"; + pinctrl-0 = <&gpio0_default_pins>; + }; + + gpio1: gpio@4e000000 { + pinctrl-names = "default"; + pinctrl-0 = <&gpio1_default_pins>; + }; + + pci@50000000 { + status = "okay"; + interrupt-map-mask = <0xf800 0 0 7>; + interrupt-map = + <0x4800 0 0 1 &pci_intc 0>, /* Slot 9 */ + <0x4800 0 0 2 &pci_intc 1>, + <0x4800 0 0 3 &pci_intc 2>, + <0x4800 0 0 4 &pci_intc 3>, + <0x5000 0 0 1 &pci_intc 1>, /* Slot 10 */ + <0x5000 0 0 2 &pci_intc 2>, + <0x5000 0 0 3 &pci_intc 3>, + <0x5000 0 0 4 &pci_intc 0>, + <0x5800 0 0 1 &pci_intc 2>, /* Slot 11 */ + <0x5800 0 0 2 &pci_intc 3>, + <0x5800 0 0 3 &pci_intc 0>, + <0x5800 0 0 4 &pci_intc 1>, + <0x6000 0 0 1 &pci_intc 3>, /* Slot 12 */ + <0x6000 0 0 2 &pci_intc 0>, + <0x6000 0 0 3 &pci_intc 1>, + <0x6000 0 0 4 &pci_intc 2>; + }; + + ethernet@60000000 { + status = "okay"; + + ethernet-port@0 { + phy-mode = "rgmii"; + phy-handle = <&phy0>; + }; + ethernet-port@1 { + phy-mode = "rgmii"; + phy-handle = <&vsc>; + }; + }; + + ata@63000000 { + status = "okay"; + }; + + ata@63400000 { + status = "okay"; + }; + }; +};
The Storlink Gemini324 EV-Board also known as Storm Semiconductor SL93512R_BRD is ground zero for the Gemini devices. We add a device tree so we can support it, it turns out to be pretty trivial. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/gemini-sl93512r.dts | 325 ++++++++++++++++++++++++++ 2 files changed, 326 insertions(+) create mode 100644 arch/arm/boot/dts/gemini-sl93512r.dts