On Thu, May 06, 2021 at 11:34:18AM -0700, Colin Foster wrote: > Vladimir, Thank you very much for the feedback. I appreciate your time > and your patience when dealing with my many blunders. This is a large > learning experience for me. You haven't seen mine... > On Thu, May 06, 2021 at 01:22:04PM +0300, Vladimir Oltean wrote: > > On Mon, May 03, 2021 at 10:11:27PM -0700, Colin Foster wrote: > > > Add support for control for VSC75XX chips over SPI control. Starting with the > > > VSC9959 code, this will utilize a spi bus instead of PCIe or memory-mapped IO to > > > control the chip. > > > > > > Signed-off-by: Colin Foster <colin.foster@xxxxxxxxxxxxxxxx> > > > --- > > > arch/arm/boot/dts/rpi-vsc7512-spi-overlay.dts | 124 ++ > > > drivers/net/dsa/ocelot/Kconfig | 11 + > > > drivers/net/dsa/ocelot/Makefile | 5 + > > > drivers/net/dsa/ocelot/felix_vsc7512_spi.c | 1214 +++++++++++++++++ > > > include/soc/mscc/ocelot.h | 15 + > > > 5 files changed, 1369 insertions(+) > > > create mode 100644 arch/arm/boot/dts/rpi-vsc7512-spi-overlay.dts > > > create mode 100644 drivers/net/dsa/ocelot/felix_vsc7512_spi.c > > > > > > diff --git a/drivers/net/dsa/ocelot/Kconfig b/drivers/net/dsa/ocelot/Kconfig > > > index 932b6b6fe817..2db147ce9fe7 100644 > > > --- a/drivers/net/dsa/ocelot/Kconfig > > > +++ b/drivers/net/dsa/ocelot/Kconfig > > > @@ -14,6 +14,17 @@ config NET_DSA_MSCC_FELIX > > > This driver supports the VSC9959 (Felix) switch, which is embedded as > > > a PCIe function of the NXP LS1028A ENETC RCiEP. > > > > > > +config NET_DSA_MSCC_FELIX_SPI > > > + tristate "Ocelot / Felix Ethernet SPI switch support" > > > + depends on NET_DSA && SPI > > > + depends on NET_VENDOR_MICROSEMI > > > + select MSCC_OCELOT_SWITCH_LIB > > > + select NET_DSA_TAG_OCELOT_8021Q > > > + select NET_DSA_TAG_OCELOT > > > + select PCS_LYNX > > > > You most probably don't need to select PCS_LYNX (that's an NXP thing). > > For that reason, you might want to call your module NET_DSA_MSCC_OCELOT_SPI. > > The "felix" name is just the name of the common DSA driver and of the > > VSC9959 chip. VSC7512 is probably called Ocelot-1 according to Microchip > > marketing. > > Thank you. I couldn't find where "Felix" and "Seville" actually came > from. My next RFC submission will include these changes. felix = code name for the VSC9959 from NXP LS1028A, derivative of Ocelot-1 seville = code name for the VSC9953 from NXP T1040, derivative of Serval-1 The common DSA driver layer is called felix because felix was the first switch it supported. But since probing is now handled individually and the PCIe probing of felix is done in a different driver compared to the platform device probing of seville, you can name your VSC7512 driver in any way you feel appropriate. > > I'm surprised that the reset procedure for VSC7512 is different than > > what is implemented in ocelot_reset() for VSC7514. > > I will look more into this and repair as needed. I'm also considering > the ability to use a GPIO as an external reset, which might negate any > reset procedure the MIPS would be doing. I think some memories need to be initialized too, not sure if you can achieve that with a reset pin only. > > You must be working on an old kernel or something. There's also a > > watermark decode function which you can reuse from ocelot (ocelot_wm_dec). > > Yes, my proof-of-concept was from a 5.10 kernel. I brought it up to > mainline before submitting the patch and must have missed this. Per the > documentation, I'll base my next submission entirely on the latest 5.12 > release. What you actually mean when you set the patch subject prefix to "PATCH net-next" is that your patches can be applied to the current master branch of this tree, not on any release tree: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/ The official kernel tag v5.12 has just been released, and kernel v5.13 entered release candidate status (starting with rc1 and ending with rc7 or rc8, depending on how many fixes it gets). The development process happens simultaneously with the release candidates for the official v5.13 release, but new development is done for v5.14 only (that's why it is called "next"). When Linus decides that the v5.13 release candidates are mature enough to print the v5.13 final release tag, development for v5.14 stops too, the maintainers stop accepting development patches for a little while, everybody prepares a pull request, there is a "memory barrier" until the official v5.14 rc1 is released which contains all pull requests with new features from the maintainers' development trees, and then these "next" trees reopen for development that will be included in v5.15. This is a bit oversimplified, but hopefully it should give you an idea why sometimes you are told to rebase on a different tree, or why you can't send patches right now, etc. > > > +static int felix_spi_remove(struct spi_device *pdev) > > > +{ > > > + struct felix *felix; > > > + > > > + felix = spi_get_drvdata(pdev); > > > + > > > + return 0; > > > +} > > > > You got bored of writing code here? :) > > A bad habit of mine. I'm trying to test the code as I'm writing it. > Communication doesn't work, so I can't get past probe. I'm excited for > the day I can see this memory leak in action. > > I'll repair this for RFC V2, regardless of whether I can test it. By testing, you mean this? echo spiN.M > /sys/bus/spi/drivers/vsc7512/unbind echo spiN.M > /sys/bus/spi/drivers/vsc7512/bind (where of course, N is the bus number and M is the chip select)