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

On Sun, Aug 15, 2021 at 04:27:53PM -0700, Colin Foster wrote:

On Mon, Aug 16, 2021 at 12:14:54AM +0100, Russell King (Oracle) wrote:

quoted

quoted

diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index a84129d18007..d0b3f6be360f 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c

@@ -1046,7 +1046,7 @@ static int vsc9959_mdio_bus_alloc(struct ocelot *ocelot)
 	int rc;
 
 	felix->pcs = devm_kcalloc(dev, felix->info->num_ports,
-				  sizeof(struct lynx_pcs *),
+				  sizeof(struct phylink_pcs *),
 				  GFP_KERNEL);
 	if (!felix->pcs) {
 		dev_err(dev, "failed to allocate array for PCS PHYs\n");

@@ -1095,8 +1095,8 @@ static int vsc9959_mdio_bus_alloc(struct ocelot *ocelot)
 
 	for (port = 0; port < felix->info->num_ports; port++) {
 		struct ocelot_port *ocelot_port = ocelot->ports[port];
+		struct phylink_pcs *phylink;
 		struct mdio_device *pcs;
-		struct lynx_pcs *lynx;

Normally, "phylink" is used to refer to the main phylink data
structure, so I'm not too thrilled to see it getting re-used for the
PCS. However, as you have a variable called "pcs" already, I suppose
you don't have much choice.

That said, it would be nice to have consistent naming through at
least a single file, and you do have "pcs" below to refer to this
same thing.

Maybe using plpcs or ppcs would suffice? Or maybe use the "long name"
of phylink_pcs ?

I noticed this as well. It seems to me like the mdio_device variable
name of pcs is misleading, and perhaps should be "mdio" and phylink_pcs
should be pcs, or any of the alternatives you suggested.

Yes, we could alternatively could use mdiodev for mdio devices,
which would free up "pcs" for use with struct phylink_pcs.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!