From: Colin Foster <colin.foster@in-advantage.com>
To: Vladimir Oltean <olteanv@gmail.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
Date: Sun, 15 Aug 2021 13:41:49 -0700 [thread overview]
Message-ID: <20210815204149.GB3328995@euler> (raw)
In-Reply-To: <20210814120211.v2qjqgi6l3slnkq2@skbuf>
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."
From 323d2f68447c3532dba0d85c636ea14c66aa098f Mon Sep 17 00:00:00 2001
From: Colin Foster <colin.foster@in-advantage.com>
Date: Sun, 15 Aug 2021 13:07:47 -0700
Subject: [RFC PATCH v3 net-next] net: phy: lynx: refactor Lynx PCS module to
use generic phylink_pcs
Remove references to lynx_pcs structures so drivers like the Felix DSA
can reference alternate PCS drivers.
Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
drivers/net/dsa/ocelot/felix.h | 2 +-
drivers/net/dsa/ocelot/felix_vsc9959.c | 10 ++++-----
drivers/net/dsa/ocelot/seville_vsc9953.c | 12 +++++-----
.../net/ethernet/freescale/dpaa2/dpaa2-mac.c | 7 +++---
.../net/ethernet/freescale/dpaa2/dpaa2-mac.h | 3 +--
.../net/ethernet/freescale/enetc/enetc_pf.c | 12 +++++-----
.../net/ethernet/freescale/enetc/enetc_pf.h | 4 ++--
drivers/net/pcs/pcs-lynx.c | 22 +++++++++++++++----
include/linux/pcs-lynx.h | 9 +++-----
9 files changed, 46 insertions(+), 35 deletions(-)
diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
index c872705115bc..f51e9e8064fc 100644
--- a/drivers/net/dsa/ocelot/felix.h
+++ b/drivers/net/dsa/ocelot/felix.h
@@ -58,7 +58,7 @@ struct felix {
const struct felix_info *info;
struct ocelot ocelot;
struct mii_bus *imdio;
- struct lynx_pcs **pcs;
+ struct phylink_pcs **pcs;
resource_size_t switch_base;
resource_size_t imdio_base;
struct dsa_8021q_context *dsa_8021q_ctx;
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;
if (dsa_is_unused_port(felix->ds, port))
continue;
@@ -1108,13 +1108,13 @@ static int vsc9959_mdio_bus_alloc(struct ocelot *ocelot)
if (IS_ERR(pcs))
continue;
- lynx = lynx_pcs_create(pcs);
+ phylink = lynx_pcs_create(pcs);
if (!lynx) {
mdio_device_free(pcs);
continue;
}
- felix->pcs[port] = lynx;
+ felix->pcs[port] = phylink;
dev_info(dev, "Found PCS at internal MDIO address %d\n", port);
}
@@ -1128,7 +1128,7 @@ static void vsc9959_mdio_bus_free(struct ocelot *ocelot)
int port;
for (port = 0; port < ocelot->num_phys_ports; port++) {
- struct lynx_pcs *pcs = felix->pcs[port];
+ struct phylink_pcs *pcs = felix->pcs[port];
if (!pcs)
continue;
diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c
index 540cf5bc9c54..8200cc5dd24d 100644
--- a/drivers/net/dsa/ocelot/seville_vsc9953.c
+++ b/drivers/net/dsa/ocelot/seville_vsc9953.c
@@ -1007,7 +1007,7 @@ static int vsc9953_mdio_bus_alloc(struct ocelot *ocelot)
int rc;
felix->pcs = devm_kcalloc(dev, felix->info->num_ports,
- sizeof(struct phy_device *),
+ sizeof(struct phylink_pcs *),
GFP_KERNEL);
if (!felix->pcs) {
dev_err(dev, "failed to allocate array for PCS PHYs\n");
@@ -1029,8 +1029,8 @@ static int vsc9953_mdio_bus_alloc(struct ocelot *ocelot)
for (port = 0; port < felix->info->num_ports; port++) {
struct ocelot_port *ocelot_port = ocelot->ports[port];
int addr = port + 4;
+ struct phylink_pcs *phylink;
struct mdio_device *pcs;
- struct lynx_pcs *lynx;
if (dsa_is_unused_port(felix->ds, port))
continue;
@@ -1042,13 +1042,13 @@ static int vsc9953_mdio_bus_alloc(struct ocelot *ocelot)
if (IS_ERR(pcs))
continue;
- lynx = lynx_pcs_create(pcs);
+ phylink = lynx_pcs_create(pcs);
if (!lynx) {
mdio_device_free(pcs);
continue;
}
- felix->pcs[port] = lynx;
+ felix->pcs[port] = phylink;
dev_info(dev, "Found PCS at internal MDIO address %d\n", addr);
}
@@ -1062,12 +1062,12 @@ static void vsc9953_mdio_bus_free(struct ocelot *ocelot)
int port;
for (port = 0; port < ocelot->num_phys_ports; port++) {
- struct lynx_pcs *pcs = felix->pcs[port];
+ struct phylink_pcs *pcs = felix->pcs[port];
if (!pcs)
continue;
- mdio_device_free(pcs->mdio);
+ mdio_device_free(lynx_pcs_get_mdio(pcs));
lynx_pcs_destroy(pcs);
}
felix_mdio_bus_free(ocelot);
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
index ccaf7e35abeb..484f0d4efefe 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -270,10 +270,11 @@ static int dpaa2_pcs_create(struct dpaa2_mac *mac,
static void dpaa2_pcs_destroy(struct dpaa2_mac *mac)
{
- struct lynx_pcs *pcs = mac->pcs;
+ struct phylink_pcs *pcs = mac->pcs;
if (pcs) {
- struct device *dev = &pcs->mdio->dev;
+ struct mdio_device *mdio = lynx_get_mdio_device(pcs);
+ struct device *dev = &mdio->dev;
lynx_pcs_destroy(pcs);
put_device(dev);
mac->pcs = NULL;
@@ -336,7 +337,7 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
mac->phylink = phylink;
if (mac->pcs)
- phylink_set_pcs(mac->phylink, &mac->pcs->pcs);
+ phylink_set_pcs(mac->phylink, mac->pcs);
err = phylink_of_phy_connect(mac->phylink, dpmac_node, 0);
if (err) {
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
index 13d42dd58ec9..d1d22b52a960 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
@@ -7,7 +7,6 @@
#include <linux/of_mdio.h>
#include <linux/of_net.h>
#include <linux/phylink.h>
-#include <linux/pcs-lynx.h>
#include "dpmac.h"
#include "dpmac-cmd.h"
@@ -23,7 +22,7 @@ struct dpaa2_mac {
struct phylink *phylink;
phy_interface_t if_mode;
enum dpmac_link_type if_link_type;
- struct lynx_pcs *pcs;
+ struct phylink_pcs *pcs;
};
bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev,
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 31274325159a..cc2ca51ac984 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -823,7 +823,7 @@ static int enetc_imdio_create(struct enetc_pf *pf)
{
struct device *dev = &pf->si->pdev->dev;
struct enetc_mdio_priv *mdio_priv;
- struct lynx_pcs *pcs_lynx;
+ struct phylink_pcs *pcs_phylink;
struct mdio_device *pcs;
struct mii_bus *bus;
int err;
@@ -855,8 +855,8 @@ static int enetc_imdio_create(struct enetc_pf *pf)
goto unregister_mdiobus;
}
- pcs_lynx = lynx_pcs_create(pcs);
- if (!pcs_lynx) {
+ pcs_phylink = lynx_pcs_create(pcs);
+ if (!pcs_phylink) {
mdio_device_free(pcs);
err = -ENOMEM;
dev_err(dev, "cannot create lynx pcs (%d)\n", err);
@@ -864,7 +864,7 @@ static int enetc_imdio_create(struct enetc_pf *pf)
}
pf->imdio = bus;
- pf->pcs = pcs_lynx;
+ pf->pcs = pcs_phylink;
return 0;
@@ -878,7 +878,7 @@ static int enetc_imdio_create(struct enetc_pf *pf)
static void enetc_imdio_remove(struct enetc_pf *pf)
{
if (pf->pcs) {
- mdio_device_free(pf->pcs->mdio);
+ mdio_device_free(lynx_get_mdio_device(pf->pcs));
lynx_pcs_destroy(pf->pcs);
}
if (pf->imdio) {
@@ -977,7 +977,7 @@ static void enetc_pl_mac_config(struct phylink_config *config,
priv = netdev_priv(pf->si->ndev);
if (pf->pcs)
- phylink_set_pcs(priv->phylink, &pf->pcs->pcs);
+ phylink_set_pcs(priv->phylink, &pf->pcs);
}
static void enetc_force_rgmii_mac(struct enetc_hw *hw, int speed, int duplex)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.h b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
index 263946c51e37..c26bd66e4597 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
@@ -2,7 +2,7 @@
/* Copyright 2017-2019 NXP */
#include "enetc.h"
-#include <linux/pcs-lynx.h>
+#include <linux/phylink.h>
#define ENETC_PF_NUM_RINGS 8
@@ -46,7 +46,7 @@ struct enetc_pf {
struct mii_bus *mdio; /* saved for cleanup */
struct mii_bus *imdio;
- struct lynx_pcs *pcs;
+ struct phylink_pcs *pcs;
phy_interface_t if_mode;
struct phylink_config phylink_config;
diff --git a/drivers/net/pcs/pcs-lynx.c b/drivers/net/pcs/pcs-lynx.c
index af36cd647bf5..bdefcb36e913 100644
--- a/drivers/net/pcs/pcs-lynx.c
+++ b/drivers/net/pcs/pcs-lynx.c
@@ -22,6 +22,11 @@
#define IF_MODE_SPEED_MSK GENMASK(3, 2)
#define IF_MODE_HALF_DUPLEX BIT(4)
+struct lynx_pcs {
+ struct phylink_pcs pcs;
+ struct mdio_device *mdio;
+};
+
enum sgmii_speed {
SGMII_SPEED_10 = 0,
SGMII_SPEED_100 = 1,
@@ -30,6 +35,15 @@ enum sgmii_speed {
};
#define phylink_pcs_to_lynx(pl_pcs) container_of((pl_pcs), struct lynx_pcs, pcs)
+#define lynx_to_phylink_pcs(lynx) (&lynx->pcs)
+
+struct mdio_device *lynx_get_mdio_device(struct phylink_pcs *pcs)
+{
+ struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
+
+ return lynx->mdio;
+}
+EXPORT_SYMBOL(lynx_get_mdio_device);
static void lynx_pcs_get_state_usxgmii(struct mdio_device *pcs,
struct phylink_link_state *state)
@@ -329,7 +343,7 @@ static const struct phylink_pcs_ops lynx_pcs_phylink_ops = {
.pcs_link_up = lynx_pcs_link_up,
};
-struct lynx_pcs *lynx_pcs_create(struct mdio_device *mdio)
+struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio)
{
struct lynx_pcs *lynx_pcs;
@@ -341,13 +355,13 @@ struct lynx_pcs *lynx_pcs_create(struct mdio_device *mdio)
lynx_pcs->pcs.ops = &lynx_pcs_phylink_ops;
lynx_pcs->pcs.poll = true;
- return lynx_pcs;
+ return lynx_to_phylink_pcs(lynx_pcs);
}
EXPORT_SYMBOL(lynx_pcs_create);
-void lynx_pcs_destroy(struct lynx_pcs *pcs)
+void lynx_pcs_destroy(struct phylink_pcs *pcs)
{
- kfree(pcs);
+ kfree(phylink_pcs_to_lynx(pcs));
}
EXPORT_SYMBOL(lynx_pcs_destroy);
diff --git a/include/linux/pcs-lynx.h b/include/linux/pcs-lynx.h
index a6440d6ebe95..5712cc2ce775 100644
--- a/include/linux/pcs-lynx.h
+++ b/include/linux/pcs-lynx.h
@@ -9,13 +9,10 @@
#include <linux/mdio.h>
#include <linux/phylink.h>
-struct lynx_pcs {
- struct phylink_pcs pcs;
- struct mdio_device *mdio;
-};
+struct mdio_device *lynx_get_mdio_device(struct phylink_pcs *pcs);
-struct lynx_pcs *lynx_pcs_create(struct mdio_device *mdio);
+struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio);
-void lynx_pcs_destroy(struct lynx_pcs *pcs);
+void lynx_pcs_destroy(struct phylink_pcs *pcs);
#endif /* __LINUX_PCS_LYNX_H */
--
2.25.1
next prev parent reply other threads:[~2021-08-15 20:42 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-14 2:49 [RFC PATCH v3 net-next 00/10] add support for VSC75XX control over SPI Colin Foster
2021-08-14 2:49 ` [RFC PATCH v3 net-next 01/10] net: dsa: ocelot: remove unnecessary pci_bar variables Colin Foster
2021-08-14 11:07 ` Vladimir Oltean
2021-08-14 16:26 ` Colin Foster
2021-08-14 2:49 ` [RFC PATCH v3 net-next 02/10] net: mdio: mscc-miim: convert to a regmap implementation Colin Foster
2021-08-14 11:03 ` Vladimir Oltean
2021-08-20 16:53 ` Colin Foster
2021-08-14 2:49 ` [RFC PATCH v3 net-next 03/10] net: dsa: ocelot: felix: switch to mdio-mscc-miim driver for indirect mdio access Colin Foster
2021-08-14 2:49 ` [RFC PATCH v3 net-next 04/10] net: dsa: ocelot: felix: Remove requirement for PCS in felix devices Colin Foster
2021-08-14 2:49 ` [RFC PATCH v3 net-next 05/10] net: dsa: ocelot: felix: add interface for custom regmaps Colin Foster
2021-08-14 2:49 ` [RFC PATCH v3 net-next 06/10] net: mscc: ocelot: split register definitions to a separate file Colin Foster
2021-08-14 11:15 ` Vladimir Oltean
2021-08-14 16:33 ` Colin Foster
2021-08-14 16:56 ` Alexandre Belloni
2021-08-14 2:50 ` [RFC PATCH v3 net-next 07/10] net: mscc: ocelot: expose ocelot wm functions Colin Foster
2021-08-14 11:17 ` Vladimir Oltean
2021-08-14 16:36 ` Colin Foster
2021-08-14 2:50 ` [RFC PATCH v3 net-next 08/10] net: mscc: ocelot: felix: add ability to enable a CPU / NPI port Colin Foster
2021-08-14 2:50 ` [RFC PATCH v3 net-next 09/10] net: dsa: ocelot: felix: add support for VSC75XX control over SPI Colin Foster
2021-08-14 11:43 ` Vladimir Oltean
2021-08-14 12:02 ` Vladimir Oltean
2021-08-15 19:10 ` Colin Foster
2021-08-15 20:41 ` Colin Foster [this message]
2021-08-15 23:14 ` Russell King (Oracle)
2021-08-15 23:27 ` Colin Foster
2021-08-16 0:05 ` Russell King (Oracle)
2021-08-17 9:41 ` Vladimir Oltean
2021-08-15 21:35 ` Russell King (Oracle)
2021-08-14 2:50 ` [RFC PATCH v3 net-next 10/10] docs: devicetree: add documentation for the VSC7512 SPI device Colin Foster
2021-08-14 11:47 ` Vladimir Oltean
2021-08-14 18:40 ` Colin Foster
2021-08-14 19:08 ` Vladimir Oltean
2021-08-14 23:41 ` Colin Foster
2021-08-15 0:00 ` Vladimir Oltean
2021-08-15 1:08 ` Colin Foster
2021-08-17 22:08 ` Rob Herring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210815204149.GB3328995@euler \
--to=colin.foster@in-advantage.com \
--cc=UNGLinuxDriver@microchip.com \
--cc=alexandre.belloni@bootlin.com \
--cc=andrew@lunn.ch \
--cc=claudiu.manoil@nxp.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=f.fainelli@gmail.com \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=robh+dt@kernel.org \
--cc=vivien.didelot@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).