On Mon, May 09, 2022 at 05:13:05PM +0000, Vladimir Oltean wrote:
> Hi Colin,
>
> On Sun, May 08, 2022 at 11:52:57AM -0700, Colin Foster wrote:
> > The patch set in general is to add support for the VSC7512, and
> > eventually the VSC7511, VSC7513 and VSC7514 devices controlled over
> > SPI. The driver is believed to be fully functional for the internal
> > phy ports (0-3) on the VSC7512. It is not yet functional for SGMII,
> > QSGMII, and SerDes ports.
> >
> > I have mentioned previously:
> > The hardware setup I'm using for development is a beaglebone black, with
> > jumpers from SPI0 to the microchip VSC7512 dev board. The microchip dev
> > board has been modified to not boot from flash, but wait for SPI. An
> > ethernet cable is connected from the beaglebone ethernet to port 0 of
> > the dev board.
> >
> > The relevant sections of the device tree I'm using for the VSC7512 is
> > below. Notably the SGPIO LEDs follow link status and speed from network
> > triggers.
> >
> > In order to make this work, I have modified the cpsw driver, and now the
> > cpsw_new driver, to allow for frames over 1500 bytes. Otherwise the
> > tagging protocol will not work between the beaglebone and the VSC7512. I
> > plan to eventually try to get those changes in mainline, but I don't
> > want to get distracted from my initial goal. I also had to change
> > bonecommon.dtsi to avoid using VLAN 0.
>
> This ti,dual-emac-pvid thing is a really odd thing to put in the device
> tree. But what's the problem with VLAN 0 anyway?
Ahh, I see that was an exchange between me and Grygorii Strashko that
wasn't public. Looking now, it might be VLAN 1...
I'd see "failed to initialize vlan filtering" when I ran "ip link set
dev swp1 master br0" because the default bridge vlan conflicted with
slave_data->dual_emac_res_vlan = port_id;
(drivers/net/ethernet/ti/cpsw_new.c, around line 1325)
My initial attempt was to just change cpsw_port1
ti,dual-emac-pvid=<12>; but that didn't change the behavior. Maybe if I
went back to it again, seeing as I'm much older and wiser than I was
before, I could find the correct device tree solution... Ideally I think
I should have the ability to not enable cpsw_port1 and be good.
But I think the magic was really just to set
slave_data->dual_emac_res_vlan = 10 + port_id; to avoid conflicts.
This became an issue at 5.15, when cpsw_new was rolled in to the .dtsis
I've been using.
>
> >
> > I believe much of the MFD sections are very near feature-complete,
> > whereas the switch section will require ongoing work to enable
> > additional ports / features. This could lead to a couple potential
> > scenarios:
> >
> > The first being patches 1-8 being split into a separate patch set, while
> > patches 9-16 remain in the RFC state. This would offer the pinctrl /
> > sgpio / mdio controller functionality, but no switch control until it is
> > ready.
> >
> > The second would assume the current state of the switch driver is
> > acceptable (or at least very near so) and the current patch set gets an
> > official PATCH set (with minor changes as necessary - e.g. squashing
> > patch 16 into 14). That might be ambitious.
> >
> > The third would be to keep this patch set in RFC until switch
> > functionality is more complete. I'd understand if this was the desired
> > path... but it would mean me having to bug more reviewers.
>
> Considering that the merge window is approaching, I'd say get the
> non-DSA stuff accepted until then, then repost the DSA stuff in ~3 weeks
> from now as non-RFC, once v5.18 is cut and the development for v5.20
> (or whatever the number will be) begins.
That's the approach I'd prefer as well.
>
> > / {
> > vscleds {
> > compatible = "gpio-leds";
> > vscled@0 {
> > label = "port0led";
> > gpios = <&sgpio_out1 0 0 GPIO_ACTIVE_LOW>;
> > default-state = "off";
> > linux,default-trigger = "ocelot-miim0.2.auto-mii:00:link";
> > };
> > vscled@1 {
> > label = "port0led1";
> > gpios = <&sgpio_out1 0 1 GPIO_ACTIVE_LOW>;
> > default-state = "off";
> > linux,default-trigger = "ocelot-miim0.2.auto-mii:00:1Gbps";
> > };
> > [ ... ]
> > };
> > };
> >
> > &spi0 {
> > #address-cells = <1>;
> > #size-cells = <0>;
> > status = "okay";
> >
> > ocelot-chip@0 {
> > compatible = "mscc,vsc7512_mfd_spi";
>
> Can you use hyphens instead of underscores in this compatible string?
>
> > spi-max-frequency = <2500000>;
> > reg = <0>;
> >
> > ethernet-switch@0 {
>
> I don't think the switch node should have any address?
>
> > compatible = "mscc,vsc7512-ext-switch";
> > ports {
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > port@0 {
> > reg = <0>;
> > label = "cpu";
> > status = "okay";
> > ethernet = <&mac_sw>;
> > phy-handle = <&sw_phy0>;
> > phy-mode = "internal";
> > };
> >
> > port@1 {
> > reg = <1>;
> > label = "swp1";
> > status = "okay";
> > phy-handle = <&sw_phy1>;
> > phy-mode = "internal";
> > };
> > };
> > };
> >
> > mdio0: mdio0@0 {
>
> This is going to be interesting. Some drivers with multiple MDIO buses
> create an "mdios" container with #address-cells = <1> and put the MDIO
> bus nodes under that. Others create an "mdio" node and an "mdio0" node
> (and no address for either of them).
>
> The problem with the latter approach is that
> Documentation/devicetree/bindings/net/mdio.yaml does not accept the
> "mdio0"/"mdio1" node name for an MDIO bus.
Hmm... That'll be interesting indeed. The 7514
(arch/mips/boot/dts/mscc/ocelot.dtsi) is where I undoubtedly started.
Is there an issue with the 7514, or is it just an issue with my
implementation, which should be:
mdio0: mdio@0 {
instead of mdio0@0?
>
> > compatible = "mscc,ocelot-miim";
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > sw_phy0: ethernet-phy@0 {
> > reg = <0x0>;
> > };
> >
> > sw_phy1: ethernet-phy@1 {
> > reg = <0x1>;
> > };
> >
> > sw_phy2: ethernet-phy@2 {
> > reg = <0x2>;
> > };
> >
> > sw_phy3: ethernet-phy@3 {
> > reg = <0x3>;
> > };
> > };
> >
> > mdio1: mdio1@1 {
> > compatible = "mscc,ocelot-miim";
> > pinctrl-names = "default";
> > pinctrl-0 = <&miim1>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > sw_phy4: ethernet-phy@4 {
> > reg = <0x4>;
> > };
> >
> > sw_phy5: ethernet-phy@5 {
> > reg = <0x5>;
> > };
> >
> > sw_phy6: ethernet-phy@6 {
> > reg = <0x6>;
> > };
> >
> > sw_phy7: ethernet-phy@7 {
> > reg = <0x7>;
> > };
> > };
> >
> > gpio: pinctrl@0 {
>
> Similar thing with the address. All these @0 addresses actually conflict
> with each other.
>
> > compatible = "mscc,ocelot-pinctrl";
> > gpio-controller;
> > #gpio_cells = <2>;
> > gpio-ranges = <&gpio 0 0 22>;
> >
> > led_shift_reg_pins: led-shift-reg-pins {
> > pins = "GPIO_0", "GPIO_1", "GPIO_2", "GPIO_3";
> > function = "sg0";
> > };
> >
> > miim1: miim1 {
> > pins = "GPIO_14", "GPIO_15";
> > function = "miim";
> > };
> > };
> >
> > sgpio: sgpio {
>
> And mixing nodes with addresses with nodes without addresses is broken too.
>
> > compatible = "mscc,ocelot-sgpio";
> > #address-cells = <1>;
> > #size-cells = <0>;
> > bus-frequency=<12500000>;
> > clocks = <&ocelot_clock>;
> > microchip,sgpio-port-ranges = <0 15>;
> > pinctrl-names = "default";
> > pinctrl-0 = <&led_shift_reg_pins>;
> >
> > sgpio_in0: sgpio@0 {
> > compatible = "microchip,sparx5-sgpio-bank";
> > reg = <0>;
> > gpio-controller;
> > #gpio-cells = <3>;
> > ngpios = <64>;
> > };
> >
> > sgpio_out1: sgpio@1 {
> > compatible = "microchip,sparx5-sgpio-bank";
> > reg = <1>;
> > gpio-controller;
> > #gpio-cells = <3>;
> > ngpios = <64>;
> > };
> > };
> > };
> > };
> >
> > And I'll include the relevant dmesg prints - I don't love the "invalid
> > resource" prints, as they seem to be misleading. They're a byproduct of
> > looking for IO resources before falling back to REG.
> >
> > [ 0.000000] Booting Linux on physical CPU 0x0
> > [ 0.000000] Linux version 5.18.0-rc5-01295-g47053e327c52 (X@X) (arm-linux-gnueabi-gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0, GNU ld (GNU Binutils for Ubuntu) 2.34) #630 SMP PREEMPT Sun May 8 10:56:51 PDT 2022
> > ...
> > [ 2.829319] pinctrl-ocelot ocelot-pinctrl.0.auto: DMA mask not set
>
> Why does this get printed, if you put a dump_stack() in of_dma_configure_id()?
I'll run that tonight.
>
> > [ 2.835718] pinctrl-ocelot ocelot-pinctrl.0.auto: invalid resource
> > [ 2.842717] gpiochip_find_base: found new base at 2026
> > [ 2.842774] gpio gpiochip4: (ocelot-gpio): created GPIO range 0->21 ==> ocelot-pinctrl.0.auto PIN 0->21
> > [ 2.845693] gpio gpiochip4: (ocelot-gpio): added GPIO chardev (254:4)
> > [ 2.845828] gpio gpiochip4: registered GPIOs 2026 to 2047 on ocelot-gpio
> > [ 2.845855] pinctrl-ocelot ocelot-pinctrl.0.auto: driver registered
> > [ 2.855925] pinctrl-microchip-sgpio ocelot-sgpio.1.auto: DMA mask not set
> > [ 2.863089] pinctrl-microchip-sgpio ocelot-sgpio.1.auto: invalid resource
> > [ 2.870801] gpiochip_find_base: found new base at 1962
> > [ 2.871528] gpio_stub_drv gpiochip5: (ocelot-sgpio.1.auto-input): added GPIO chardev (254:5)
> > [ 2.871666] gpio_stub_drv gpiochip5: registered GPIOs 1962 to 2025 on ocelot-sgpio.1.auto-input
> > [ 2.872364] gpiochip_find_base: found new base at 1898
> > [ 2.873244] gpio_stub_drv gpiochip6: (ocelot-sgpio.1.auto-output): added GPIO chardev (254:6)
> > [ 2.873354] gpio_stub_drv gpiochip6: registered GPIOs 1898 to 1961 on ocelot-sgpio.1.auto-output
> > [ 2.881148] mscc-miim ocelot-miim0.2.auto: DMA mask not set
> > [ 2.886929] mscc-miim ocelot-miim0.2.auto: invalid resource
> > [ 2.893738] mdio_bus ocelot-miim0.2.auto-mii: GPIO lookup for consumer reset
> > [ 2.893769] mdio_bus ocelot-miim0.2.auto-mii: using device tree for GPIO lookup
> > [ 2.893802] of_get_named_gpiod_flags: can't parse 'reset-gpios' property of node '/ocp/interconnect@48000000/segment@0/target-module@30000/spi@0/ocelot-chip@0/mdio0[0]'
> > [ 2.893898] of_get_named_gpiod_flags: can't parse 'reset-gpio' property of node '/ocp/interconnect@48000000/segment@0/target-module@30000/spi@0/ocelot-chip@0/mdio0[0]'
> > [ 2.893996] mdio_bus ocelot-miim0.2.auto-mii: using lookup tables for GPIO lookup
> > [ 2.894012] mdio_bus ocelot-miim0.2.auto-mii: No GPIO consumer reset found
> > [ 3.395738] mdio_bus ocelot-miim0.2.auto-mii:00: GPIO lookup for consumer reset
> > [ 3.395777] mdio_bus ocelot-miim0.2.auto-mii:00: using device tree for GPIO lookup
> > [ 3.395840] of_get_named_gpiod_flags: can't parse 'reset-gpios' property of node '/ocp/interconnect@48000000/segment@0/target-module@30000/spi@0/ocelot-chip@0/mdio0/ethernet-phy@0[0]'
> > [ 3.395959] of_get_named_gpiod_flags: can't parse 'reset-gpio' property of node '/ocp/interconnect@48000000/segment@0/target-module@30000/spi@0/ocelot-chip@0/mdio0/ethernet-phy@0[0]'
> > [ 3.396069] mdio_bus ocelot-miim0.2.auto-mii:00: using lookup tables for GPIO lookup
> > [ 3.396086] mdio_bus ocelot-miim0.2.auto-mii:00: No GPIO consumer reset found
> > ...
> > [ 3.449187] ocelot-ext-switch ocelot-ext-switch.4.auto: DMA mask not set
> > [ 5.336880] ocelot-ext-switch ocelot-ext-switch.4.auto: PHY [ocelot-miim0.2.auto-mii:00] driver [Generic PHY] (irq=POLL)
> > [ 5.349087] ocelot-ext-switch ocelot-ext-switch.4.auto: configuring for phy/internal link mode
> > [ 5.363619] ocelot-ext-switch ocelot-ext-switch.4.auto swp1 (uninitialized): PHY [ocelot-miim0.2.auto-mii:01] driver [Generic PHY] (irq=POLL)
> > [ 5.381396] ocelot-ext-switch ocelot-ext-switch.4.auto swp2 (uninitialized): PHY [ocelot-miim0.2.auto-mii:02] driver [Generic PHY] (irq=POLL)
> > [ 5.398525] ocelot-ext-switch ocelot-ext-switch.4.auto swp3 (uninitialized): PHY [ocelot-miim0.2.auto-mii:03] driver [Generic PHY] (irq=POLL)
>
> Do the PHYs not have a specific driver?
When I have the other four ports defined, those correctly find the
vsc85xx driver, perform serdes calibration, etc. (assuming I have that
phy support compiled in) The internal phys I believe have always just
been using a generic driver.
>
> > [ 5.422048] device eth0 entered promiscuous mode
> > [ 5.426785] DSA: tree 0 setup
> > ...
> > [ 7.450067] ocelot-ext-switch ocelot-ext-switch.4.auto: Link is Up - 100Mbps/Full - flow control off
> > [ 21.556395] cpsw-switch 4a100000.switch: starting ndev. mode: dual_mac
> > [ 21.648564] SMSC LAN8710/LAN8720 4a101000.mdio:00: attached PHY driver (mii_bus:phy_addr=4a101000.mdio:00, irq=POLL)
> > [ 21.667970] 8021q: adding VLAN 0 to HW filter on device eth0
> > [ 21.705360] ocelot-ext-switch ocelot-ext-switch.4.auto swp1: configuring for phy/internal link mode
> > [ 22.018230] ocelot-ext-switch ocelot-ext-switch.4.auto: Link is Down
> > [ 23.771740] cpsw-switch 4a100000.switch eth0: Link is Up - 100Mbps/Full - flow control off
> > [ 24.090929] ocelot-ext-switch ocelot-ext-switch.4.auto: Link is Up - 100Mbps/Full - flow control off
> > [ 25.853021] ocelot-ext-switch ocelot-ext-switch.4.auto swp1: Link is Up - 1Gbps/Full - flow control rx/tx
> >
> >
> > RFC history:
> > v1 (accidentally named vN)
> > * Initial architecture. Not functional
> > * General concepts laid out
> >
> > v2
> > * Near functional. No CPU port communication, but control over all
> > external ports
> > * Cleaned up regmap implementation from v1
> >
> > v3
> > * Functional
> > * Shared MDIO transactions routed through mdio-mscc-miim
> > * CPU / NPI port enabled by way of vsc7512_enable_npi_port /
> > felix->info->enable_npi_port
> > * NPI port tagging functional - Requires a CPU port driver that supports
> > frames of 1520 bytes. Verified with a patch to the cpsw driver
> >
> > v4
> > * Functional
> > * Device tree fixes
> > * Add hooks for pinctrl-ocelot - some functionality by way of sysfs
> > * Add hooks for pinctrl-microsemi-sgpio - not yet fully functional
> > * Remove lynx_pcs interface for a generic phylink_pcs. The goal here
> > is to have an ocelot_pcs that will work for each configuration of
> > every port.
> >
> > v5
> > * Restructured to MFD
> > * Several commits were split out, submitted, and accepted
> > * pinctrl-ocelot believed to be fully functional (requires commits
> > from the linux-pinctrl tree)
> > * External MDIO bus believed to be fully functional
> >
> > v6
> > * Applied several suggestions from the last RFC from Lee Jones. I
> > hope I didn't miss anything.
> > * Clean up MFD core - SPI interaction. They no longer use callbacks.
> > * regmaps get registered to the child device, and don't attempt to
> > get shared. It seems if a regmap is to be shared, that should be
> > solved with syscon, not dev or mfd.
> >
> > v7
> > * Applied as much as I could from Lee and Vladimir's suggestions. As
> > always, the feedback is greatly appreciated!
> > * Remove "ocelot_spi" container complication
> > * Move internal MDIO bus from ocelot_ext to MFD, with a devicetree
> > change to match
> > * Add initial HSIO support
> > * Switch to IORESOURCE_REG for resource definitions
> >
> > v8
> > * Applied another round of suggestions from Lee and Vladimir
> > * Utilize regmap bus reads, which speeds bulk transfers up by an
>
> bus -> bulk?
Either is probably valid. Here I'm referencing struct regmap_bus,
so _regmap_bus_read allows the utilization of bulk transfers for stats.
>
> > order of magnitude
> > * Add two additional patches to utilize phylink_generic_validate
> > * Changed GPL V2 to GPL in licenses where applicable (checkpatch)
> > * Remove initial hsio/serdes changes from the RFC
> >
> >
> > Colin Foster (16):
> > pinctrl: ocelot: allow pinctrl-ocelot to be loaded as a module
> > pinctrl: microchip-sgpio: allow sgpio driver to be used as a module
> > net: ocelot: add interface to get regmaps when exernally controlled
> > net: mdio: mscc-miim: add ability to be used in a non-mmio
> > configuration
> > pinctrl: ocelot: add ability to be used in a non-mmio configuration
> > pinctrl: microchip-sgpio: add ability to be used in a non-mmio
> > configuration
> > resource: add define macro for register address resources
> > mfd: ocelot: add support for the vsc7512 chip via spi
> > net: mscc: ocelot: expose ocelot wm functions
> > net: dsa: felix: add configurable device quirks
> > net: mscc: ocelot: expose regfield definition to be used by other
> > drivers
> > net: mscc: ocelot: expose stats layout definition to be used by other
> > drivers
> > net: mscc: ocelot: expose vcap_props structure
> > net: dsa: ocelot: add external ocelot switch control
> > net: dsa: felix: add phylink_get_caps capability
> > net: dsa: ocelot: utilize phylink_generic_validate
> >
> > drivers/mfd/Kconfig | 18 +
> > drivers/mfd/Makefile | 2 +
> > drivers/mfd/ocelot-core.c | 138 ++++++++
> > drivers/mfd/ocelot-spi.c | 311 +++++++++++++++++
> > drivers/mfd/ocelot.h | 34 ++
> > drivers/net/dsa/ocelot/Kconfig | 14 +
> > drivers/net/dsa/ocelot/Makefile | 5 +
> > drivers/net/dsa/ocelot/felix.c | 29 +-
> > drivers/net/dsa/ocelot/felix.h | 3 +
> > drivers/net/dsa/ocelot/felix_vsc9959.c | 1 +
> > drivers/net/dsa/ocelot/ocelot_ext.c | 366 +++++++++++++++++++++
> > drivers/net/dsa/ocelot/seville_vsc9953.c | 1 +
> > drivers/net/ethernet/mscc/ocelot_devlink.c | 31 ++
> > drivers/net/ethernet/mscc/ocelot_vsc7514.c | 230 +------------
> > drivers/net/ethernet/mscc/vsc7514_regs.c | 200 +++++++++++
> > drivers/net/mdio/mdio-mscc-miim.c | 31 +-
> > drivers/pinctrl/Kconfig | 4 +-
> > drivers/pinctrl/pinctrl-microchip-sgpio.c | 26 +-
> > drivers/pinctrl/pinctrl-ocelot.c | 35 +-
> > include/linux/ioport.h | 5 +
> > include/soc/mscc/ocelot.h | 19 ++
> > include/soc/mscc/vsc7514_regs.h | 6 +
> > 22 files changed, 1251 insertions(+), 258 deletions(-)
> > create mode 100644 drivers/mfd/ocelot-core.c
> > create mode 100644 drivers/mfd/ocelot-spi.c
> > create mode 100644 drivers/mfd/ocelot.h
> > create mode 100644 drivers/net/dsa/ocelot/ocelot_ext.c
> >
> > --
> > 2.25.1
> >
Re: [RFC v8 net-next 00/16] add support for VSC7512 control over SPI
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
- Subject: Re: [RFC v8 net-next 00/16] add support for VSC7512 control over SPI
- From: Colin Foster <colin.foster@xxxxxxxxxxxxxxxx>
- Date: Mon, 9 May 2022 18:43:50 -0700
- In-reply-to: <20220509171304.hfh5rbynt4qtr6m4@skbuf>
- References: <20220508185313.2222956-1-colin.foster@in-advantage.com> <20220509171304.hfh5rbynt4qtr6m4@skbuf>
- References:
- [RFC v8 net-next 00/16] add support for VSC7512 control over SPI
- From: Colin Foster
- Re: [RFC v8 net-next 00/16] add support for VSC7512 control over SPI
- From: Vladimir Oltean
- [RFC v8 net-next 00/16] add support for VSC7512 control over SPI
- Prev by Date: Re: [PATCH v2 3/5] gpio: gpiolib: Allow free() callback to be overridden
- Next by Date: Re: [PATCH v2 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller driver
- Previous by thread: Re: [RFC v8 net-next 00/16] add support for VSC7512 control over SPI
- Next by thread: Re: [RFC v8 net-next 00/16] add support for VSC7512 control over SPI
- Index(es):
 |