Date:   Tue, 5 Jul 2022 13:30:15 -0700
From:   Colin Foster <colin.foster@...advantage.com>
To:     Rob Herring <robh@...nel.org>
Cc:     devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-gpio@...r.kernel.org,
        Vladimir Oltean <vladimir.oltean@....com>,
        Lee Jones <lee.jones@...aro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Lars Povlsen <lars.povlsen@...rochip.com>,
        Steen Hegelund <Steen.Hegelund@...rochip.com>,
        UNGLinuxDriver@...rochip.com,
        Linus Walleij <linus.walleij@...aro.org>,
        Wolfram Sang <wsa@...nel.org>,
        Terry Bowman <terry.bowman@....com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        katie.morris@...advantage.com
Subject: Re: [PATCH v12 net-next 0/9] add support for VSC7512 control over SPI

On Tue, Jul 05, 2022 at 02:24:22PM -0600, Rob Herring wrote:
> On Fri, Jul 01, 2022 at 12:26:00PM -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. Specifically this patch set enables pinctrl, serial gpio expander
> > access, and control of an internal and an external MDIO bus.
> > 
> > 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. Network functionality will be included in a future patch set.
> > 
> > The device tree I'm using is included in the documentation, so I'll not
> > include that in this cover letter. I have exported the serial GPIOs to the
> > LEDs, and verified functionality via
> > "echo heartbeat > sys/class/leds/port0led/trigger"
> > 
> > / {
> > 	vscleds {
> > 		compatible = "gpio-leds";
> > 		vscled@0 {
> > 			label = "port0led";
> > 			gpios = <&sgpio_out1 0 0 GPIO_ACTIVE_LOW>;
> > 			default-state = "off";
> > 		};
> > 		vscled@1 {
> > 			label = "port0led1";
> > 			gpios = <&sgpio_out1 0 1 GPIO_ACTIVE_LOW>;
> > 			default-state = "off";
> > 		};
> > [ ... ]
> > 	};
> > };
> > 
> > [    0.000000] Booting Linux on physical CPU 0x0
> > [    0.000000] Linux version 5.19.0-rc3-00745-g30c05ffbecdc (arm-linux-gnueabi-gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0, GNU ld (GNU Binutils for Ubuntu) 2.34) #826 SMP PREEMPT Fri Jul 1 11:26:44 PDT 2022
> > ...
> > [    1.952616] pinctrl-ocelot ocelot-pinctrl.0.auto: DMA mask not set
> > [    1.956522] pinctrl-ocelot ocelot-pinctrl.0.auto: driver registered
> > [    1.967188] pinctrl-microchip-sgpio ocelot-sgpio.1.auto: DMA mask not set
> > [    1.983763] mscc-miim ocelot-miim0.2.auto: DMA mask not set
> > [    3.020687] mscc-miim ocelot-miim1.3.auto: DMA mask not set
> > 
> > 
> > I only have hardware to test the last patch, so any testers are welcome.
> > I've been extra cautious about the ocelot_regmap_from_resource helper
> > function, both before and after the last patch. I accidentally broke it
> > in the past and would like to avoid doing so again.
> > 
> > 
> > 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
> >       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
> > 
> > v9
> >     * Submitting as a PATCH instead of an RFC
> >     * Remove switch functionality - will be a separate patch set
> >     * Remove Kconfig tristate module options
> >     * Another round of suggestions from Lee, Vladimir, and Andy. Many
> >       thanks!
> >     * Add documentation
> >     * Update maintainers
> > 
> > v10
> >     * Fix warming by removing unused function
> > 
> > v11
> >     * Suggestions from Rob and Andy. Thanks!
> >     * Add pinctrl module functionality back and fixing those features
> >     * Fix aarch64 compiler error
> > 
> > v12
> >     * Suggestions from Vladimir, Andy, Randy, and Rob. Thanks as always!
> 
> Not all that useful as a changelog. I have no idea what I told you as 
> that was probably 100s of reviews ago. When writing changelogs for patch 
> revisions, you need to describe what changed. And it's best to put that 
> into the relevant patch. IOW, I want to know what I said to change so I 
> know what I need to look at again in particular.
> 
> And now that I've found v11, 'suggestions from Rob' isn't really 
> accurate as you fixed errors reported by running the tools.
> 
> Rob

Good point - I'll be more clear going forward.

Powered by blists - more mailing lists