Hello everyone,
This is the V3 of a series that follows-up on the work [1] aiming to drop the
altera TSE PCS driver, as it turns out to be a version of the Lynx PCS exposed
as a memory-mapped block, instead of living on an MDIO bus.
One step of this removal involved creating a regmap-based mdio driver
that translates MDIO accesses into the actual underlying bus that
exposes the register. The register layout must of course match the
standard MDIO layout, but we can now account for differences in stride
with recent work on the regmap subsystem [2].
Mark, Net maintainers, this series depends on the patch e12ff2876493 that was
recently merged into the regmap tree [3].
For this series to be usable in net-next, this patch must be applied
beforehand. Should Mark create a tag that would then be merged into
net-next ?
This series introduces a new MDIO driver, and uses it to convert Altera
TSE from the actual TSE PCS driver to Lynx PCS.
Since it turns out dwmac_socfpga also uses a TSE PCS block, port that
driver to Lynx as well.
Changes in V3 :
- Use a dedicated struct for the mii bus's priv data, to avoid
duplicating the whole struct mdio_regmap_config, from which 2 fields
only are necessary after init, as suggested by Russell
- Use ~0 instead of ~0UL for the no-scan bitmask, following Simon's
review.
Changes in V2 :
- Use phy_mask to avoid unnecessarily scanning the whole mdio bus
- Go one step further and completely disable scanning if users
set the .autoscan flag to false, in case the mdiodevice isn't an
actual PHY (a PCS for example).
Thanks,
Maxime
[1] : https://lore.kernel.org/all/[email protected]/
[2] : https://lore.kernel.org/all/[email protected]/#t
[3] : https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git/commit/?id=e12ff28764937dd58c8613f16065da60da149048
Maxime Chevallier (4):
net: mdio: Introduce a regmap-based mdio driver
net: ethernet: altera-tse: Convert to mdio-regmap and use PCS Lynx
net: pcs: Drop the TSE PCS driver
net: stmmac: dwmac-sogfpga: use the lynx pcs driver
MAINTAINERS | 14 +-
drivers/net/ethernet/altera/Kconfig | 2 +
drivers/net/ethernet/altera/altera_tse.h | 1 +
drivers/net/ethernet/altera/altera_tse_main.c | 57 +++-
drivers/net/ethernet/stmicro/stmmac/Kconfig | 1 +
drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +-
.../ethernet/stmicro/stmmac/altr_tse_pcs.c | 257 ------------------
.../ethernet/stmicro/stmmac/altr_tse_pcs.h | 29 --
drivers/net/ethernet/stmicro/stmmac/common.h | 1 +
.../ethernet/stmicro/stmmac/dwmac-socfpga.c | 90 ++++--
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 12 +-
drivers/net/mdio/Kconfig | 10 +
drivers/net/mdio/Makefile | 1 +
drivers/net/mdio/mdio-regmap.c | 93 +++++++
drivers/net/pcs/Kconfig | 6 -
drivers/net/pcs/Makefile | 1 -
drivers/net/pcs/pcs-altera-tse.c | 160 -----------
include/linux/mdio/mdio-regmap.h | 26 ++
include/linux/pcs-altera-tse.h | 17 --
19 files changed, 267 insertions(+), 513 deletions(-)
delete mode 100644 drivers/net/ethernet/stmicro/stmmac/altr_tse_pcs.c
delete mode 100644 drivers/net/ethernet/stmicro/stmmac/altr_tse_pcs.h
create mode 100644 drivers/net/mdio/mdio-regmap.c
delete mode 100644 drivers/net/pcs/pcs-altera-tse.c
create mode 100644 include/linux/mdio/mdio-regmap.h
delete mode 100644 include/linux/pcs-altera-tse.h
--
2.40.1
The newly introduced regmap-based MDIO driver allows for an easy mapping
of an mdiodevice onto the memory-mapped TSE PCS, which is actually a
Lynx PCS.
Convert Altera TSE to use this PCS instead of the pcs-altera-tse, which
is nothing more than a memory-mapped Lynx PCS.
Signed-off-by: Maxime Chevallier <[email protected]>
---
V2->V3 : No changes
V1->V2 : No changes
drivers/net/ethernet/altera/altera_tse.h | 1 +
drivers/net/ethernet/altera/altera_tse_main.c | 57 +++++++++++++++++--
include/linux/mdio/mdio-regmap.h | 2 +
3 files changed, 54 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/altera/altera_tse.h b/drivers/net/ethernet/altera/altera_tse.h
index db5eed06e92d..d50cf440d01b 100644
--- a/drivers/net/ethernet/altera/altera_tse.h
+++ b/drivers/net/ethernet/altera/altera_tse.h
@@ -477,6 +477,7 @@ struct altera_tse_private {
struct phylink *phylink;
struct phylink_config phylink_config;
struct phylink_pcs *pcs;
+ struct mdio_device *pcs_mdiodev;
};
/* Function prototypes
diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index 190ff1bcd94e..66db6a7d0b22 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -27,14 +27,16 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/mii.h>
+#include <linux/mdio/mdio-regmap.h>
#include <linux/netdevice.h>
#include <linux/of_device.h>
#include <linux/of_mdio.h>
#include <linux/of_net.h>
#include <linux/of_platform.h>
-#include <linux/pcs-altera-tse.h>
+#include <linux/pcs-lynx.h>
#include <linux/phy.h>
#include <linux/platform_device.h>
+#include <linux/regmap.h>
#include <linux/skbuff.h>
#include <asm/cacheflush.h>
@@ -1134,13 +1136,21 @@ static int altera_tse_probe(struct platform_device *pdev)
const struct of_device_id *of_id = NULL;
struct altera_tse_private *priv;
struct resource *control_port;
+ struct regmap *pcs_regmap;
struct resource *dma_res;
struct resource *pcs_res;
+ struct mii_bus *pcs_bus;
struct net_device *ndev;
void __iomem *descmap;
- int pcs_reg_width = 2;
int ret = -ENODEV;
+ struct regmap_config pcs_regmap_cfg;
+
+ struct mdio_regmap_config mrc = {
+ .parent = &pdev->dev,
+ .valid_addr = 0x0,
+ };
+
ndev = alloc_etherdev(sizeof(struct altera_tse_private));
if (!ndev) {
dev_err(&pdev->dev, "Could not allocate network device\n");
@@ -1258,10 +1268,29 @@ static int altera_tse_probe(struct platform_device *pdev)
ret = request_and_map(pdev, "pcs", &pcs_res,
&priv->pcs_base);
if (ret) {
+ /* If we can't find a dedicated resource for the PCS, fallback
+ * to the internal PCS, that has a different address stride
+ */
priv->pcs_base = priv->mac_dev + tse_csroffs(mdio_phy0);
- pcs_reg_width = 4;
+ pcs_regmap_cfg.reg_bits = 32;
+ /* Values are MDIO-like values, on 16 bits */
+ pcs_regmap_cfg.val_bits = 16;
+ pcs_regmap_cfg.reg_shift = REGMAP_UPSHIFT(2);
+ } else {
+ pcs_regmap_cfg.reg_bits = 16;
+ pcs_regmap_cfg.val_bits = 16;
+ pcs_regmap_cfg.reg_shift = REGMAP_UPSHIFT(1);
}
+ /* Create a regmap for the PCS so that it can be used by the PCS driver */
+ pcs_regmap = devm_regmap_init_mmio(&pdev->dev, priv->pcs_base,
+ &pcs_regmap_cfg);
+ if (IS_ERR(pcs_regmap)) {
+ ret = PTR_ERR(pcs_regmap);
+ goto err_free_netdev;
+ }
+ mrc.regmap = pcs_regmap;
+
/* Rx IRQ */
priv->rx_irq = platform_get_irq_byname(pdev, "rx_irq");
if (priv->rx_irq == -ENXIO) {
@@ -1384,7 +1413,20 @@ static int altera_tse_probe(struct platform_device *pdev)
(unsigned long) control_port->start, priv->rx_irq,
priv->tx_irq);
- priv->pcs = alt_tse_pcs_create(ndev, priv->pcs_base, pcs_reg_width);
+ snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii", ndev->name);
+ pcs_bus = devm_mdio_regmap_register(&pdev->dev, &mrc);
+ if (IS_ERR(pcs_bus)) {
+ ret = PTR_ERR(pcs_bus);
+ goto err_init_phy;
+ }
+
+ priv->pcs_mdiodev = mdio_device_create(pcs_bus, 0);
+
+ priv->pcs = lynx_pcs_create(priv->pcs_mdiodev);
+ if (!priv->pcs) {
+ ret = -ENODEV;
+ goto err_init_phy;
+ }
priv->phylink_config.dev = &ndev->dev;
priv->phylink_config.type = PHYLINK_NETDEV;
@@ -1407,11 +1449,12 @@ static int altera_tse_probe(struct platform_device *pdev)
if (IS_ERR(priv->phylink)) {
dev_err(&pdev->dev, "failed to create phylink\n");
ret = PTR_ERR(priv->phylink);
- goto err_init_phy;
+ goto err_pcs;
}
return 0;
-
+err_pcs:
+ mdio_device_free(priv->pcs_mdiodev);
err_init_phy:
unregister_netdev(ndev);
err_register_netdev:
@@ -1433,6 +1476,8 @@ static int altera_tse_remove(struct platform_device *pdev)
altera_tse_mdio_destroy(ndev);
unregister_netdev(ndev);
phylink_destroy(priv->phylink);
+ mdio_device_free(priv->pcs_mdiodev);
+
free_netdev(ndev);
return 0;
diff --git a/include/linux/mdio/mdio-regmap.h b/include/linux/mdio/mdio-regmap.h
index b8508f152552..679d9069846b 100644
--- a/include/linux/mdio/mdio-regmap.h
+++ b/include/linux/mdio/mdio-regmap.h
@@ -7,6 +7,8 @@
#ifndef MDIO_REGMAP_H
#define MDIO_REGMAP_H
+#include <linux/phy.h>
+
struct device;
struct regmap;
--
2.40.1
On Fri, May 26, 2023 at 09:42:50AM +0200, Maxime Chevallier wrote:
> The newly introduced regmap-based MDIO driver allows for an easy mapping
> of an mdiodevice onto the memory-mapped TSE PCS, which is actually a
> Lynx PCS.
>
> Convert Altera TSE to use this PCS instead of the pcs-altera-tse, which
> is nothing more than a memory-mapped Lynx PCS.
>
> Signed-off-by: Maxime Chevallier <[email protected]>
Hi Maxime,
I have some concerns about the error paths in this patch.
...
> @@ -1134,13 +1136,21 @@ static int altera_tse_probe(struct platform_device *pdev)
> const struct of_device_id *of_id = NULL;
> struct altera_tse_private *priv;
> struct resource *control_port;
> + struct regmap *pcs_regmap;
> struct resource *dma_res;
> struct resource *pcs_res;
> + struct mii_bus *pcs_bus;
> struct net_device *ndev;
> void __iomem *descmap;
> - int pcs_reg_width = 2;
> int ret = -ENODEV;
>
> + struct regmap_config pcs_regmap_cfg;
nit: this probably belongs in with the bunch of declarations above it.
> +
> + struct mdio_regmap_config mrc = {
> + .parent = &pdev->dev,
> + .valid_addr = 0x0,
> + };
nit: maybe this too.
> +
> ndev = alloc_etherdev(sizeof(struct altera_tse_private));
> if (!ndev) {
> dev_err(&pdev->dev, "Could not allocate network device\n");
> @@ -1258,10 +1268,29 @@ static int altera_tse_probe(struct platform_device *pdev)
> ret = request_and_map(pdev, "pcs", &pcs_res,
> &priv->pcs_base);
> if (ret) {
> + /* If we can't find a dedicated resource for the PCS, fallback
> + * to the internal PCS, that has a different address stride
> + */
> priv->pcs_base = priv->mac_dev + tse_csroffs(mdio_phy0);
> - pcs_reg_width = 4;
> + pcs_regmap_cfg.reg_bits = 32;
> + /* Values are MDIO-like values, on 16 bits */
> + pcs_regmap_cfg.val_bits = 16;
> + pcs_regmap_cfg.reg_shift = REGMAP_UPSHIFT(2);
> + } else {
> + pcs_regmap_cfg.reg_bits = 16;
> + pcs_regmap_cfg.val_bits = 16;
> + pcs_regmap_cfg.reg_shift = REGMAP_UPSHIFT(1);
> }
>
> + /* Create a regmap for the PCS so that it can be used by the PCS driver */
> + pcs_regmap = devm_regmap_init_mmio(&pdev->dev, priv->pcs_base,
> + &pcs_regmap_cfg);
> + if (IS_ERR(pcs_regmap)) {
> + ret = PTR_ERR(pcs_regmap);
> + goto err_free_netdev;
> + }
> + mrc.regmap = pcs_regmap;
> +
> /* Rx IRQ */
> priv->rx_irq = platform_get_irq_byname(pdev, "rx_irq");
> if (priv->rx_irq == -ENXIO) {
> @@ -1384,7 +1413,20 @@ static int altera_tse_probe(struct platform_device *pdev)
> (unsigned long) control_port->start, priv->rx_irq,
> priv->tx_irq);
>
> - priv->pcs = alt_tse_pcs_create(ndev, priv->pcs_base, pcs_reg_width);
> + snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii", ndev->name);
> + pcs_bus = devm_mdio_regmap_register(&pdev->dev, &mrc);
> + if (IS_ERR(pcs_bus)) {
> + ret = PTR_ERR(pcs_bus);
> + goto err_init_phy;
> + }
> +
> + priv->pcs_mdiodev = mdio_device_create(pcs_bus, 0);
mdio_device_create() can fail. Should that be handled here?
> +
> + priv->pcs = lynx_pcs_create(priv->pcs_mdiodev);
> + if (!priv->pcs) {
> + ret = -ENODEV;
> + goto err_init_phy;
Does this leak priv->pcs_mdiodev?
> + }
>
> priv->phylink_config.dev = &ndev->dev;
> priv->phylink_config.type = PHYLINK_NETDEV;
> @@ -1407,11 +1449,12 @@ static int altera_tse_probe(struct platform_device *pdev)
> if (IS_ERR(priv->phylink)) {
> dev_err(&pdev->dev, "failed to create phylink\n");
> ret = PTR_ERR(priv->phylink);
> - goto err_init_phy;
> + goto err_pcs;
Does this leak priv->pcs ?
> }
>
> return 0;
> -
> +err_pcs:
> + mdio_device_free(priv->pcs_mdiodev);
> err_init_phy:
> unregister_netdev(ndev);
> err_register_netdev:
> @@ -1433,6 +1476,8 @@ static int altera_tse_remove(struct platform_device *pdev)
> altera_tse_mdio_destroy(ndev);
> unregister_netdev(ndev);
> phylink_destroy(priv->phylink);
> + mdio_device_free(priv->pcs_mdiodev);
> +
> free_netdev(ndev);
>
> return 0;
> diff --git a/include/linux/mdio/mdio-regmap.h b/include/linux/mdio/mdio-regmap.h
> index b8508f152552..679d9069846b 100644
> --- a/include/linux/mdio/mdio-regmap.h
> +++ b/include/linux/mdio/mdio-regmap.h
> @@ -7,6 +7,8 @@
> #ifndef MDIO_REGMAP_H
> #define MDIO_REGMAP_H
>
> +#include <linux/phy.h>
> +
> struct device;
> struct regmap;
>
This hunk doesn't seem strictly related to the patch.
Perhaps the include belongs elsewhere.
Or the hunk belongs in another patch.
On Fri, May 26, 2023 at 10:39:08AM +0200, Simon Horman wrote:
> On Fri, May 26, 2023 at 09:42:50AM +0200, Maxime Chevallier wrote:
> > The newly introduced regmap-based MDIO driver allows for an easy mapping
> > of an mdiodevice onto the memory-mapped TSE PCS, which is actually a
> > Lynx PCS.
> >
> > Convert Altera TSE to use this PCS instead of the pcs-altera-tse, which
> > is nothing more than a memory-mapped Lynx PCS.
> >
> > Signed-off-by: Maxime Chevallier <[email protected]>
>
> Hi Maxime,
>
> I have some concerns about the error paths in this patch.
We've had similar problems with mdio_device_create() vs the XPCS
driver.
I think it's time that we made this easier for users.
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index b87c69c4cdd7..802222581feb 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -1240,6 +1240,7 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
if (!xpcs)
return ERR_PTR(-ENOMEM);
+ mdio_device_get(mdiodev);
xpcs->mdiodev = mdiodev;
xpcs_id = xpcs_get_id(xpcs);
@@ -1272,6 +1273,7 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
ret = -ENODEV;
out:
+ mdio_device_put(mdiodev);
kfree(xpcs);
return ERR_PTR(ret);
@@ -1280,8 +1282,33 @@ EXPORT_SYMBOL_GPL(xpcs_create);
void xpcs_destroy(struct dw_xpcs *xpcs)
{
+ mdio_device_put(mdiodev);
kfree(xpcs);
}
EXPORT_SYMBOL_GPL(xpcs_destroy);
+struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
+ phy_interface_t interface)
+{
+ struct mdio_device *mdiodev;
+ struct dw_xpcs *xpcs;
+
+ mdiodev = mdio_device_create(bus, addr);
+ if (IS_ERR(mdiodev))
+ return ERR_CAST(mdiodev);
+
+ xpcs = xpcs_create(mdiodev, interface);
+
+ /* xpcs_create() has taken a refcount on the mdiodev if it was
+ * successful. If xpcs_create() fails, this will free the mdio
+ * device here. In any case, we don't need to hold our reference
+ * anymore, and putting it here will allow mdio_device_put() in
+ * xpcs_destroy() to automatically free the mdio device.
+ */
+ mdio_device_put(mdiodev);
+
+ return xpcs;
+}
+EXPORT_SYMBOL_GPL(xpcs_create_mdiodev);
+
MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index 1d7d550bbf1a..537b62330c90 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -108,6 +108,16 @@ int mdio_driver_register(struct mdio_driver *drv);
void mdio_driver_unregister(struct mdio_driver *drv);
int mdio_device_bus_match(struct device *dev, struct device_driver *drv);
+static inline void mdio_device_get(struct mdio_device *mdiodev)
+{
+ get_device(&mdiodev->dev);
+}
+
+static inline void mdio_device_put(struct mdio_device *mdiodev)
+{
+ mdio_device_free(mdiodev);
+}
+
static inline bool mdio_phy_id_is_c45(int phy_id)
{
return (phy_id & MDIO_PHY_ID_C45) && !(phy_id & ~MDIO_PHY_ID_C45_MASK);
The same for pcs-lynx. That way we remove the need for driver authors
to get the creation and destruction of the mdio device correct
without messing up the existing users - they only have to worry about
creating the PCS via the xxx_create_mdiodev() function and destroying
it via xxx_destroy().
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Fri, May 26, 2023 at 10:05:17AM +0100, Russell King (Oracle) wrote:
> On Fri, May 26, 2023 at 10:39:08AM +0200, Simon Horman wrote:
> > On Fri, May 26, 2023 at 09:42:50AM +0200, Maxime Chevallier wrote:
> > > The newly introduced regmap-based MDIO driver allows for an easy mapping
> > > of an mdiodevice onto the memory-mapped TSE PCS, which is actually a
> > > Lynx PCS.
> > >
> > > Convert Altera TSE to use this PCS instead of the pcs-altera-tse, which
> > > is nothing more than a memory-mapped Lynx PCS.
> > >
> > > Signed-off-by: Maxime Chevallier <[email protected]>
> >
> > Hi Maxime,
> >
> > I have some concerns about the error paths in this patch.
>
> We've had similar problems with mdio_device_create() vs the XPCS
> driver.
>
> I think it's time that we made this easier for users.
Patch series here:
https://lore.kernel.org/all/[email protected]/
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hello Simon,
On Fri, 26 May 2023 10:39:08 +0200
Simon Horman <[email protected]> wrote:
> On Fri, May 26, 2023 at 09:42:50AM +0200, Maxime Chevallier wrote:
> > The newly introduced regmap-based MDIO driver allows for an easy
> > mapping of an mdiodevice onto the memory-mapped TSE PCS, which is
> > actually a Lynx PCS.
> >
> > Convert Altera TSE to use this PCS instead of the pcs-altera-tse,
> > which is nothing more than a memory-mapped Lynx PCS.
> >
> > Signed-off-by: Maxime Chevallier <[email protected]>
>
> Hi Maxime,
>
> I have some concerns about the error paths in this patch.
Heh I didn't do a very good job in that regard indeed :/ I'll address
these, but I'll wait for Russell series to make it through though.
Thanks for spotting this.
> ...
>
> > @@ -1134,13 +1136,21 @@ static int altera_tse_probe(struct
> > platform_device *pdev) const struct of_device_id *of_id = NULL;
> > struct altera_tse_private *priv;
> > struct resource *control_port;
> > + struct regmap *pcs_regmap;
> > struct resource *dma_res;
> > struct resource *pcs_res;
> > + struct mii_bus *pcs_bus;
> > struct net_device *ndev;
> > void __iomem *descmap;
> > - int pcs_reg_width = 2;
> > int ret = -ENODEV;
> >
> > + struct regmap_config pcs_regmap_cfg;
>
> nit: this probably belongs in with the bunch of declarations above it.
>
> > +
> > + struct mdio_regmap_config mrc = {
> > + .parent = &pdev->dev,
> > + .valid_addr = 0x0,
> > + };
>
> nit: maybe this too.
>
> > +
> > ndev = alloc_etherdev(sizeof(struct altera_tse_private));
> > if (!ndev) {
> > dev_err(&pdev->dev, "Could not allocate network
> > device\n"); @@ -1258,10 +1268,29 @@ static int
> > altera_tse_probe(struct platform_device *pdev) ret =
> > request_and_map(pdev, "pcs", &pcs_res, &priv->pcs_base);
> > if (ret) {
> > + /* If we can't find a dedicated resource for the
> > PCS, fallback
> > + * to the internal PCS, that has a different
> > address stride
> > + */
> > priv->pcs_base = priv->mac_dev +
> > tse_csroffs(mdio_phy0);
> > - pcs_reg_width = 4;
> > + pcs_regmap_cfg.reg_bits = 32;
> > + /* Values are MDIO-like values, on 16 bits */
> > + pcs_regmap_cfg.val_bits = 16;
> > + pcs_regmap_cfg.reg_shift = REGMAP_UPSHIFT(2);
> > + } else {
> > + pcs_regmap_cfg.reg_bits = 16;
> > + pcs_regmap_cfg.val_bits = 16;
> > + pcs_regmap_cfg.reg_shift = REGMAP_UPSHIFT(1);
> > }
> >
> > + /* Create a regmap for the PCS so that it can be used by
> > the PCS driver */
> > + pcs_regmap = devm_regmap_init_mmio(&pdev->dev,
> > priv->pcs_base,
> > + &pcs_regmap_cfg);
> > + if (IS_ERR(pcs_regmap)) {
> > + ret = PTR_ERR(pcs_regmap);
> > + goto err_free_netdev;
> > + }
> > + mrc.regmap = pcs_regmap;
> > +
> > /* Rx IRQ */
> > priv->rx_irq = platform_get_irq_byname(pdev, "rx_irq");
> > if (priv->rx_irq == -ENXIO) {
> > @@ -1384,7 +1413,20 @@ static int altera_tse_probe(struct
> > platform_device *pdev) (unsigned long) control_port->start,
> > priv->rx_irq, priv->tx_irq);
> >
> > - priv->pcs = alt_tse_pcs_create(ndev, priv->pcs_base,
> > pcs_reg_width);
> > + snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii",
> > ndev->name);
> > + pcs_bus = devm_mdio_regmap_register(&pdev->dev, &mrc);
> > + if (IS_ERR(pcs_bus)) {
> > + ret = PTR_ERR(pcs_bus);
> > + goto err_init_phy;
> > + }
> > +
> > + priv->pcs_mdiodev = mdio_device_create(pcs_bus, 0);
>
> mdio_device_create() can fail. Should that be handled here?
>
> > +
> > + priv->pcs = lynx_pcs_create(priv->pcs_mdiodev);
> > + if (!priv->pcs) {
> > + ret = -ENODEV;
> > + goto err_init_phy;
>
> Does this leak priv->pcs_mdiodev?
>
> > + }
> >
> > priv->phylink_config.dev = &ndev->dev;
> > priv->phylink_config.type = PHYLINK_NETDEV;
> > @@ -1407,11 +1449,12 @@ static int altera_tse_probe(struct
> > platform_device *pdev) if (IS_ERR(priv->phylink)) {
> > dev_err(&pdev->dev, "failed to create phylink\n");
> > ret = PTR_ERR(priv->phylink);
> > - goto err_init_phy;
> > + goto err_pcs;
>
> Does this leak priv->pcs ?
>
> > }
> >
> > return 0;
> > -
> > +err_pcs:
> > + mdio_device_free(priv->pcs_mdiodev);
> > err_init_phy:
> > unregister_netdev(ndev);
> > err_register_netdev:
> > @@ -1433,6 +1476,8 @@ static int altera_tse_remove(struct
> > platform_device *pdev) altera_tse_mdio_destroy(ndev);
> > unregister_netdev(ndev);
> > phylink_destroy(priv->phylink);
> > + mdio_device_free(priv->pcs_mdiodev);
> > +
> > free_netdev(ndev);
> >
> > return 0;
> > diff --git a/include/linux/mdio/mdio-regmap.h
> > b/include/linux/mdio/mdio-regmap.h index b8508f152552..679d9069846b
> > 100644 --- a/include/linux/mdio/mdio-regmap.h
> > +++ b/include/linux/mdio/mdio-regmap.h
> > @@ -7,6 +7,8 @@
> > #ifndef MDIO_REGMAP_H
> > #define MDIO_REGMAP_H
> >
> > +#include <linux/phy.h>
> > +
> > struct device;
> > struct regmap;
> >
>
> This hunk doesn't seem strictly related to the patch.
> Perhaps the include belongs elsewhere.
> Or the hunk belongs in another patch.
Indeed, I had the same issue in another patch in this series. I'll
re-split everything correctly.
Thank you for the review,
Maxime