Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sat, 14 Aug 2021 14:43:29 +0300
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Colin Foster <colin.foster@in-advantage.com>
Cc:     andrew@lunn.ch, vivien.didelot@gmail.com, f.fainelli@gmail.com,
        davem@davemloft.net, kuba@kernel.org, robh+dt@kernel.org,
        claudiu.manoil@nxp.com, alexandre.belloni@bootlin.com,
        UNGLinuxDriver@microchip.com, hkallweit1@gmail.com,
        linux@armlinux.org.uk, netdev@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v3 net-next 09/10] net: dsa: ocelot: felix: add
 support for VSC75XX control over SPI
Message-ID: <20210814114329.mycpcfwoqpqxzsyl@skbuf>
References: <20210814025003.2449143-1-colin.foster@in-advantage.com>
 <20210814025003.2449143-10-colin.foster@in-advantage.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210814025003.2449143-10-colin.foster@in-advantage.com>
Precedence: bulk

On Fri, Aug 13, 2021 at 07:50:02PM -0700, Colin Foster wrote:
> +/* Code taken from ocelot_adjust_link. Since we don't have a phydev, and
> + * therefore a phydev->link associated with the NPI port, it needs to be enabled
> + * blindly.
> + */

This makes no sense. You do have a phylink associated with the NPI port,
see for yourself, all of felix_phylink_mac_link_up, felix_phylink_mac_config,
felix_phylink_validate get called for the NPI port.

The trouble, really, is that what is done in felix_phylink_mac_link_up
is not sufficient for your hardware. The felix_vsc9959 and seville_vsc9953
drivers are Microchip switches integrated with NXP PCS, and the NXP PCS
has a dedicated driver in drivers/net/pcs/pcs-lynx.c.

So you won't see any of the PCS1G writes in the common driver, because
NXP integrations of these switches don't have that block.

This is not the proper way to do things. You are "fixing" SGMII for the
NPI/CPU port by pretending it's an NPI port issue, but in reality all
the other ports that use SGMII need the same treatment.

What we might need is a dedicated PCS driver for the VSC7512 switch, and
a way for the felix driver to interchangeably work with either struct
lynx_pcs or struct ocelot_pcs (or whatever it's going to be called).

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.