Re: [PATCH v12 net-next 0/9] add support for VSC7512 control over SPI

Colin Foster
2022-07-05T20:30:30Z

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.