Re: [RFC PATCH v3 net-next 09/10] net: dsa: ocelot: felix: add support for VSC75XX control over SPI

From: Colin Foster
Date: Sun Aug 15 2021 - 16:42:38 EST


On Sat, Aug 14, 2021 at 03:02:11PM +0300, Vladimir Oltean wrote:
> On Sat, Aug 14, 2021 at 02:43:29PM +0300, Vladimir Oltean wrote:
> > The issue is that the registers for the PCS1G block look nothing like
> > the MDIO clause 22 layout, so anything that tries to map the struct
> > ocelot_pcs over a struct mdio_device is going to look like a horrible
> > shoehorn.
> >
> > For that we might need Russell's assistance.
> >
> > The documentation is at:
> > http://ww1.microchip.com/downloads/en/DeviceDoc/VMDS-10489.pdf
> > search for "Information about the registers for this product is available in the attached file."
> > and then open the PDF embedded within the PDF.
>
> In fact I do notice now that as long as you don't use any of the
> optional phylink_mii_c22_pcs_* helpers in your PCS driver, then
> struct phylink_pcs has pretty much zero dependency on struct mdio_device,
> which means that I'm wrong and it should be completely within reach to
> write a dedicated PCS driver for this hardware.
>
> As to how to make the common felix.c work with different implementations
> of struct phylink_pcs, one thing that certainly has to change is that
> struct felix should hold a struct phylink_pcs **pcs and not a
> struct lynx_pcs **pcs.
>
> Does this mean that we should refactor lynx_pcs_create() to return a
> struct phylink_pcs * instead of struct lynx_pcs *, and lynx_pcs_destroy()
> to receive the struct phylink_pcs *, use container_of() and free the
> larger struct lynx_pcs *? Yes, probably.
>
> If you feel uncomfortable with this, I can try to refactor lynx_pcs to
> make it easier to accomodate a different PCS driver in felix.

I believe I'll need to rebase this commit before I send it out to the
maintainers, but is this what you had in mind?

I also came across some curious code in Seville where it is callocing a
struct phy_device * array instead of struct lynx_pcs *. I'm not sure if
that's technically a bug or if the thought is "a pointer array is a
pointer array."