[PATCH net-next 0/7] drivers/net/phy C=1 W=1 fixes

[PATCH net-next 0/7] drivers/net/phy C=1 W=1 fixes

[Andrew Lunn]

This fixed most of the Sparse and W=1 warnings in drivers/net/phy. The
Cavium code is still not fully clean, but it might actually be the
strange code is confusing Sparse.

GregKH: Is it O.K. to take the last patch via netdev, not staging?

Cc: Alexandru Ardelean <alexaundru.ardelean@analog.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Sunil Goutham <sgoutham@marvell.com>
Cc: Robert Richter <rrichter@marvell.com>
Cc: Sunil Goutham <sgoutham@marvell.com>
Cc: Robert Richter <rrichter@marvell.com>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Andrew Lunn (7):
  net: phy: at803x: Avoid comparison is always false warning
  net: phy: Fixup parameters in kerneldoc
  net: phy: Properly define genphy_c45_driver
  net: phy: Make  phy_10gbit_fec_features_array static
  net: phy: dp83640: Fixup cast to restricted __be16 warning
  net: phy: cavium: Improve __iomem mess
  net: phy: mdio-octeon: Cleanup module loading dependencies

 drivers/net/ethernet/cavium/octeon/octeon_mgmt.c |  6 +-----
 drivers/net/phy/adin.c                           | 12 ++++++------
 drivers/net/phy/at803x.c                         |  7 +++----
 drivers/net/phy/dp83640.c                        |  5 +++--
 drivers/net/phy/mdio-boardinfo.c                 |  3 ++-
 drivers/net/phy/mdio-cavium.h                    | 14 +++++++-------
 drivers/net/phy/mdio-octeon.c                    | 11 ++---------
 drivers/net/phy/mdio-thunder.c                   |  2 +-
 drivers/net/phy/mdio_device.c                    |  2 +-
 drivers/net/phy/phy_device.c                     |  4 +---
 drivers/staging/octeon/ethernet-mdio.c           |  2 +-
 drivers/staging/octeon/ethernet-mdio.h           |  2 --
 drivers/staging/octeon/ethernet.c                |  2 --
 include/linux/phy.h                              |  3 +++
 14 files changed, 31 insertions(+), 44 deletions(-)

-- 
2.27.0.rc2

[PATCH net-next 1/7] net: phy: at803x: Avoid comparison is always false warning

[Andrew Lunn]

By placing the GENMASK value into an unsigned int and then passing it
to PREF_FIELD, the type is reduces down from ULL. Given the reduced
size of the type, the range checks in PREP_FAIL() are always true, and
-Wtype-limits then gives a warning.

By skipping the intermediate variable, the warning can be avoided.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/at803x.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 97cbe593f0ea..bdd84f6f0214 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -400,7 +400,7 @@ static int at803x_parse_dt(struct phy_device *phydev)
 {
 	struct device_node *node = phydev->mdio.dev.of_node;
 	struct at803x_priv *priv = phydev->priv;
-	unsigned int sel, mask;
+	unsigned int sel;
 	u32 freq, strength;
 	int ret;
 
@@ -409,7 +409,6 @@ static int at803x_parse_dt(struct phy_device *phydev)
 
 	ret = of_property_read_u32(node, "qca,clk-out-frequency", &freq);
 	if (!ret) {
-		mask = AT803X_CLK_OUT_MASK;
 		switch (freq) {
 		case 25000000:
 			sel = AT803X_CLK_OUT_25MHZ_XTAL;
@@ -428,8 +427,8 @@ static int at803x_parse_dt(struct phy_device *phydev)
 			return -EINVAL;
 		}
 
-		priv->clk_25m_reg |= FIELD_PREP(mask, sel);
-		priv->clk_25m_mask |= mask;
+		priv->clk_25m_reg |= FIELD_PREP(AT803X_CLK_OUT_MASK, sel);
+		priv->clk_25m_mask |= AT803X_CLK_OUT_MASK;
 
 		/* Fixup for the AR8030/AR8035. This chip has another mask and
 		 * doesn't support the DSP reference. Eg. the lowest bit of the
-- 
2.27.0.rc2

Re: [PATCH net-next 1/7] net: phy: at803x: Avoid comparison is always false warning

[Jakub Kicinski]

On Sun,  5 Jul 2020 20:29:15 +0200 Andrew Lunn wrote:
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 97cbe593f0ea..bdd84f6f0214 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -400,7 +400,7 @@ static int at803x_parse_dt(struct phy_device *phydev)
>  {
>  	struct device_node *node = phydev->mdio.dev.of_node;
>  	struct at803x_priv *priv = phydev->priv;
> -	unsigned int sel, mask;
> +	unsigned int sel;

nit: ordering, otherwise feel free to add my rb on the series, thanks!

>  	u32 freq, strength;

[PATCH net-next 2/7] net: phy: Fixup parameters in kerneldoc

[Andrew Lunn]

Correct the kerneldoc for a few structure and function calls,
as reported by C=1 W=1.

Cc: Alexandru Ardelean <alexaundru.ardelean@analog.com>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/adin.c           | 12 ++++++------
 drivers/net/phy/mdio-boardinfo.c |  3 ++-
 drivers/net/phy/mdio_device.c    |  2 +-
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index c7eabe4382fb..7471a8b90873 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -106,8 +106,8 @@
 
 /**
  * struct adin_cfg_reg_map - map a config value to aregister value
- * @cfg		value in device configuration
- * @reg		value in the register
+ * @cfg:	value in device configuration
+ * @reg:	value in the register
  */
 struct adin_cfg_reg_map {
 	int cfg;
@@ -135,9 +135,9 @@ static const struct adin_cfg_reg_map adin_rmii_fifo_depths[] = {
 
 /**
  * struct adin_clause45_mmd_map - map to convert Clause 45 regs to Clause 22
- * @devad		device address used in Clause 45 access
- * @cl45_regnum		register address defined by Clause 45
- * @adin_regnum		equivalent register address accessible via Clause 22
+ * @devad:		device address used in Clause 45 access
+ * @cl45_regnum:	register address defined by Clause 45
+ * @adin_regnum:	equivalent register address accessible via Clause 22
  */
 struct adin_clause45_mmd_map {
 	int devad;
@@ -174,7 +174,7 @@ static const struct adin_hw_stat adin_hw_stats[] = {
 
 /**
  * struct adin_priv - ADIN PHY driver private data
- * stats		statistic counters for the PHY
+ * @stats:		statistic counters for the PHY
  */
 struct adin_priv {
 	u64			stats[ARRAY_SIZE(adin_hw_stats)];
diff --git a/drivers/net/phy/mdio-boardinfo.c b/drivers/net/phy/mdio-boardinfo.c
index d9b54c67ef9f..033df435f76c 100644
--- a/drivers/net/phy/mdio-boardinfo.c
+++ b/drivers/net/phy/mdio-boardinfo.c
@@ -17,7 +17,8 @@ static DEFINE_MUTEX(mdio_board_lock);
 /**
  * mdiobus_setup_mdiodev_from_board_info - create and setup MDIO devices
  * from pre-collected board specific MDIO information
- * @mdiodev: MDIO device pointer
+ * @bus: Bus the board_info belongs to
+ * @cb: Callback to create device on bus
  * Context: can sleep
  */
 void mdiobus_setup_mdiodev_from_board_info(struct mii_bus *bus,
diff --git a/drivers/net/phy/mdio_device.c b/drivers/net/phy/mdio_device.c
index c1d345c3cab3..5cb726a2bf1f 100644
--- a/drivers/net/phy/mdio_device.c
+++ b/drivers/net/phy/mdio_device.c
@@ -182,7 +182,7 @@ static int mdio_remove(struct device *dev)
 
 /**
  * mdio_driver_register - register an mdio_driver with the MDIO layer
- * @new_driver: new mdio_driver to register
+ * @drv: new mdio_driver to register
  */
 int mdio_driver_register(struct mdio_driver *drv)
 {
-- 
2.27.0.rc2

Re: [PATCH net-next 2/7] net: phy: Fixup parameters in kerneldoc

[Florian Fainelli]

On 7/5/2020 11:29 AM, Andrew Lunn wrote:
> Correct the kerneldoc for a few structure and function calls,
> as reported by C=1 W=1.
> 
> Cc: Alexandru Ardelean <alexaundru.ardelean@analog.com>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net-next 3/7] net: phy: Properly define genphy_c45_driver

[Andrew Lunn]

Avoid the W=1 warning that symbol 'genphy_c45_driver' was not
declared. Should it be static?

Declare it on the phy header file.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy_device.c | 1 -
 include/linux/phy.h          | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 29ef4456ac25..d2e1193b032c 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -226,7 +226,6 @@ static void phy_mdio_device_remove(struct mdio_device *mdiodev)
 }
 
 static struct phy_driver genphy_driver;
-extern struct phy_driver genphy_c45_driver;
 
 static LIST_HEAD(phy_fixup_list);
 static DEFINE_MUTEX(phy_fixup_lock);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 6fb8f302978d..eb3252474b97 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1398,6 +1398,9 @@ int genphy_c45_pma_read_abilities(struct phy_device *phydev);
 int genphy_c45_read_status(struct phy_device *phydev);
 int genphy_c45_config_aneg(struct phy_device *phydev);
 
+/* Generic C45 PHY driver */
+extern struct phy_driver genphy_c45_driver;
+
 /* The gen10g_* functions are the old Clause 45 stub */
 int gen10g_config_aneg(struct phy_device *phydev);
 
-- 
2.27.0.rc2

Re: [PATCH net-next 3/7] net: phy: Properly define genphy_c45_driver

[Florian Fainelli]

On 7/5/2020 11:29 AM, Andrew Lunn wrote:
> Avoid the W=1 warning that symbol 'genphy_c45_driver' was not
> declared. Should it be static?
> 
> Declare it on the phy header file.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net-next 4/7] net: phy: Make phy_10gbit_fec_features_array static

[Andrew Lunn]

This array is not used outside of phy_device.c, so make it static.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy_device.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index d2e1193b032c..03cada335ace 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -105,10 +105,9 @@ const int phy_10gbit_features_array[1] = {
 };
 EXPORT_SYMBOL_GPL(phy_10gbit_features_array);
 
-const int phy_10gbit_fec_features_array[1] = {
+static const int phy_10gbit_fec_features_array[1] = {
 	ETHTOOL_LINK_MODE_10000baseR_FEC_BIT,
 };
-EXPORT_SYMBOL_GPL(phy_10gbit_fec_features_array);
 
 __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_10gbit_full_features) __ro_after_init;
 EXPORT_SYMBOL_GPL(phy_10gbit_full_features);
-- 
2.27.0.rc2

Re: [PATCH net-next 4/7] net: phy: Make phy_10gbit_fec_features_array static

[Florian Fainelli]

On 7/5/2020 11:29 AM, Andrew Lunn wrote:
> This array is not used outside of phy_device.c, so make it static.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net-next 5/7] net: phy: dp83640: Fixup cast to restricted __be16 warning

[Andrew Lunn]

ntohs() expects to be passed a __be16. Correct the type of the
variable holding the sequence ID.

Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/dp83640.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index ecbd5e0d685c..da31756f5a70 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -803,9 +803,10 @@ static int decode_evnt(struct dp83640_private *dp83640,
 
 static int match(struct sk_buff *skb, unsigned int type, struct rxts *rxts)
 {
-	u16 *seqid, hash;
 	unsigned int offset = 0;
 	u8 *msgtype, *data = skb_mac_header(skb);
+	__be16 *seqid;
+	u16 hash;
 
 	/* check sequenceID, messageType, 12 bit hash of offset 20-29 */
 
@@ -836,7 +837,7 @@ static int match(struct sk_buff *skb, unsigned int type, struct rxts *rxts)
 	if (rxts->msgtype != (*msgtype & 0xf))
 		return 0;
 
-	seqid = (u16 *)(data + offset + OFF_PTP_SEQUENCE_ID);
+	seqid = (__be16 *)(data + offset + OFF_PTP_SEQUENCE_ID);
 	if (rxts->seqid != ntohs(*seqid))
 		return 0;
 
-- 
2.27.0.rc2

Re: [PATCH net-next 5/7] net: phy: dp83640: Fixup cast to restricted __be16 warning

[Florian Fainelli]

On 7/5/2020 11:29 AM, Andrew Lunn wrote:
> ntohs() expects to be passed a __be16. Correct the type of the
> variable holding the sequence ID.
> 
> Cc: Richard Cochran <richardcochran@gmail.com>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net-next 5/7] net: phy: dp83640: Fixup cast to restricted __be16 warning

[Richard Cochran]

On Sun, Jul 05, 2020 at 01:45:50PM -0700, Florian Fainelli wrote:
> 
> 
> On 7/5/2020 11:29 AM, Andrew Lunn wrote:
> > ntohs() expects to be passed a __be16. Correct the type of the
> > variable holding the sequence ID.
> > 
> > Cc: Richard Cochran <richardcochran@gmail.com>
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> 
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Acked-by: Richard Cochran <richardcochran@gmail.com>

[PATCH net-next 6/7] net: phy: cavium: Improve __iomem mess

[Andrew Lunn]

The MIPS low level register access functions seem to be missing
__iomem annotation. This cases lots of sparse warnings, when code
casts off the __iomem. Make the Cavium MDIO drivers cleaner by pushing
the casts lower down into the helpers, allow the drivers to work as
normal, with __iomem.

bus->register_base is now an void *, rather than a u64. So forming the
mii_bus->id string cannot use %llx any more. Use %px, so this kernel
address is still exposed to user space, as it was before.

Cc: Sunil Goutham <sgoutham@marvell.com>
Cc: Robert Richter <rrichter@marvell.com>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/mdio-cavium.h  | 14 +++++++-------
 drivers/net/phy/mdio-octeon.c  |  5 ++---
 drivers/net/phy/mdio-thunder.c |  2 +-
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/mdio-cavium.h b/drivers/net/phy/mdio-cavium.h
index e33d3ea9a907..a2245d436f5d 100644
--- a/drivers/net/phy/mdio-cavium.h
+++ b/drivers/net/phy/mdio-cavium.h
@@ -90,7 +90,7 @@ union cvmx_smix_wr_dat {
 
 struct cavium_mdiobus {
 	struct mii_bus *mii_bus;
-	u64 register_base;
+	void __iomem *register_base;
 	enum cavium_mdiobus_mode mode;
 };
 
@@ -98,20 +98,20 @@ struct cavium_mdiobus {
 
 #include <asm/octeon/octeon.h>
 
-static inline void oct_mdio_writeq(u64 val, u64 addr)
+static inline void oct_mdio_writeq(u64 val, void __iomem *addr)
 {
-	cvmx_write_csr(addr, val);
+	cvmx_write_csr((u64 __force)addr, val);
 }
 
-static inline u64 oct_mdio_readq(u64 addr)
+static inline u64 oct_mdio_readq(void __iomem *addr)
 {
-	return cvmx_read_csr(addr);
+	return cvmx_read_csr((u64 __force)addr);
 }
 #else
 #include <linux/io-64-nonatomic-lo-hi.h>
 
-#define oct_mdio_writeq(val, addr)	writeq(val, (void *)addr)
-#define oct_mdio_readq(addr)		readq((void *)addr)
+#define oct_mdio_writeq(val, addr)	writeq(val, addr)
+#define oct_mdio_readq(addr)		readq(addr)
 #endif
 
 int cavium_mdiobus_read(struct mii_bus *bus, int phy_id, int regnum);
diff --git a/drivers/net/phy/mdio-octeon.c b/drivers/net/phy/mdio-octeon.c
index 8327382aa568..a2f93948db97 100644
--- a/drivers/net/phy/mdio-octeon.c
+++ b/drivers/net/phy/mdio-octeon.c
@@ -44,8 +44,7 @@ static int octeon_mdiobus_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
-	bus->register_base =
-		(u64)devm_ioremap(&pdev->dev, mdio_phys, regsize);
+	bus->register_base = devm_ioremap(&pdev->dev, mdio_phys, regsize);
 	if (!bus->register_base) {
 		dev_err(&pdev->dev, "dev_ioremap failed\n");
 		return -ENOMEM;
@@ -56,7 +55,7 @@ static int octeon_mdiobus_probe(struct platform_device *pdev)
 	oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN);
 
 	bus->mii_bus->name = KBUILD_MODNAME;
-	snprintf(bus->mii_bus->id, MII_BUS_ID_SIZE, "%llx", bus->register_base);
+	snprintf(bus->mii_bus->id, MII_BUS_ID_SIZE, "%px", bus->register_base);
 	bus->mii_bus->parent = &pdev->dev;
 
 	bus->mii_bus->read = cavium_mdiobus_read;
diff --git a/drivers/net/phy/mdio-thunder.c b/drivers/net/phy/mdio-thunder.c
index 2a97938d1972..3d7eda99d34e 100644
--- a/drivers/net/phy/mdio-thunder.c
+++ b/drivers/net/phy/mdio-thunder.c
@@ -84,7 +84,7 @@ static int thunder_mdiobus_pci_probe(struct pci_dev *pdev,
 		nexus->buses[i] = bus;
 		i++;
 
-		bus->register_base = (u64)nexus->bar0 +
+		bus->register_base = nexus->bar0 +
 			r.start - pci_resource_start(pdev, 0);
 
 		smi_en.u64 = 0;
-- 
2.27.0.rc2

Re: [PATCH net-next 6/7] net: phy: cavium: Improve __iomem mess

[Florian Fainelli]

On 7/5/2020 11:29 AM, Andrew Lunn wrote:
> The MIPS low level register access functions seem to be missing
> __iomem annotation. This cases lots of sparse warnings, when code

s/cases/causes/g

> casts off the __iomem. Make the Cavium MDIO drivers cleaner by pushing
> the casts lower down into the helpers, allow the drivers to work as
> normal, with __iomem.
> 
> bus->register_base is now an void *, rather than a u64. So forming the
> mii_bus->id string cannot use %llx any more. Use %px, so this kernel
> address is still exposed to user space, as it was before.
> 
> Cc: Sunil Goutham <sgoutham@marvell.com>
> Cc: Robert Richter <rrichter@marvell.com>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net-next 7/7] net: phy: mdio-octeon: Cleanup module loading dependencies

[Andrew Lunn]

To ensure that the octoen MDIO driver has been loaded, the Cavium
ethernet drivers reference a dummy symbol in the MDIO driver. This
forces it to be loaded first. And this symbol has not been cleanly
implemented, resulting in warnings when build W=1 C=1.

Since device tree is being used, and a phandle points to the PHY on
the MDIO bus, we can make use of deferred probing. If the PHY fails to
connect, it should be because the MDIO bus driver has not loaded
yet. Return -EPROBE_DEFER so it will be tried again later.

Cc: Sunil Goutham <sgoutham@marvell.com>
Cc: Robert Richter <rrichter@marvell.com>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/cavium/octeon/octeon_mgmt.c | 6 +-----
 drivers/net/phy/mdio-octeon.c                    | 6 ------
 drivers/staging/octeon/ethernet-mdio.c           | 2 +-
 drivers/staging/octeon/ethernet-mdio.h           | 2 --
 drivers/staging/octeon/ethernet.c                | 2 --
 5 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
index cbaa1924afbe..4bf0237ee52e 100644
--- a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
+++ b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
@@ -961,7 +961,7 @@ static int octeon_mgmt_init_phy(struct net_device *netdev)
 				PHY_INTERFACE_MODE_MII);
 
 	if (!phydev)
-		return -ENODEV;
+		return -EPROBE_DEFER;
 
 	return 0;
 }
@@ -1554,12 +1554,8 @@ static struct platform_driver octeon_mgmt_driver = {
 	.remove		= octeon_mgmt_remove,
 };
 
-extern void octeon_mdiobus_force_mod_depencency(void);
-
 static int __init octeon_mgmt_mod_init(void)
 {
-	/* Force our mdiobus driver module to be loaded first. */
-	octeon_mdiobus_force_mod_depencency();
 	return platform_driver_register(&octeon_mgmt_driver);
 }
 
diff --git a/drivers/net/phy/mdio-octeon.c b/drivers/net/phy/mdio-octeon.c
index a2f93948db97..d1e1009d51af 100644
--- a/drivers/net/phy/mdio-octeon.c
+++ b/drivers/net/phy/mdio-octeon.c
@@ -108,12 +108,6 @@ static struct platform_driver octeon_mdiobus_driver = {
 	.remove		= octeon_mdiobus_remove,
 };
 
-void octeon_mdiobus_force_mod_depencency(void)
-{
-	/* Let ethernet drivers force us to be loaded.  */
-}
-EXPORT_SYMBOL(octeon_mdiobus_force_mod_depencency);
-
 module_platform_driver(octeon_mdiobus_driver);
 
 MODULE_DESCRIPTION("Cavium OCTEON MDIO bus driver");
diff --git a/drivers/staging/octeon/ethernet-mdio.c b/drivers/staging/octeon/ethernet-mdio.c
index c798672d61b2..cfb673a52b25 100644
--- a/drivers/staging/octeon/ethernet-mdio.c
+++ b/drivers/staging/octeon/ethernet-mdio.c
@@ -163,7 +163,7 @@ int cvm_oct_phy_setup_device(struct net_device *dev)
 	of_node_put(phy_node);
 
 	if (!phydev)
-		return -ENODEV;
+		return -EPROBE_DEFER;
 
 	priv->last_link = 0;
 	phy_start(phydev);
diff --git a/drivers/staging/octeon/ethernet-mdio.h b/drivers/staging/octeon/ethernet-mdio.h
index e3771d48c49b..7f6716e3fad4 100644
--- a/drivers/staging/octeon/ethernet-mdio.h
+++ b/drivers/staging/octeon/ethernet-mdio.h
@@ -22,7 +22,5 @@
 
 extern const struct ethtool_ops cvm_oct_ethtool_ops;
 
-void octeon_mdiobus_force_mod_depencency(void);
-
 int cvm_oct_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
 int cvm_oct_phy_setup_device(struct net_device *dev);
diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
index f42c3816ce49..920ab08e4311 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -689,8 +689,6 @@ static int cvm_oct_probe(struct platform_device *pdev)
 	mtu_overhead += VLAN_HLEN;
 #endif
 
-	octeon_mdiobus_force_mod_depencency();
-
 	pip = pdev->dev.of_node;
 	if (!pip) {
 		pr_err("Error: No 'pip' in /aliases\n");
-- 
2.27.0.rc2

Re: [PATCH net-next 7/7] net: phy: mdio-octeon: Cleanup module loading dependencies

[Florian Fainelli]

On 7/5/2020 11:29 AM, Andrew Lunn wrote:
> To ensure that the octoen MDIO driver has been loaded, the Cavium

s/octoen/octeon/

> ethernet drivers reference a dummy symbol in the MDIO driver. This
> forces it to be loaded first. And this symbol has not been cleanly
> implemented, resulting in warnings when build W=1 C=1.
> 
> Since device tree is being used, and a phandle points to the PHY on
> the MDIO bus, we can make use of deferred probing. If the PHY fails to
> connect, it should be because the MDIO bus driver has not loaded
> yet. Return -EPROBE_DEFER so it will be tried again later.
Do you also want to add a MODULE_SOFTDEP() to ensure that mdio-octeon is
loaded prior those those modules? This is a hint for modprobe and
libkmod but it would be nicer to have. With that:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net-next 7/7] net: phy: mdio-octeon: Cleanup module loading dependencies

[Greg Kroah-Hartman]

On Sun, Jul 05, 2020 at 08:29:21PM +0200, Andrew Lunn wrote:
> To ensure that the octoen MDIO driver has been loaded, the Cavium
> ethernet drivers reference a dummy symbol in the MDIO driver. This
> forces it to be loaded first. And this symbol has not been cleanly
> implemented, resulting in warnings when build W=1 C=1.
> 
> Since device tree is being used, and a phandle points to the PHY on
> the MDIO bus, we can make use of deferred probing. If the PHY fails to
> connect, it should be because the MDIO bus driver has not loaded
> yet. Return -EPROBE_DEFER so it will be tried again later.
> 
> Cc: Sunil Goutham <sgoutham@marvell.com>
> Cc: Robert Richter <rrichter@marvell.com>
> Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH net-next 7/7] net: phy: mdio-octeon: Cleanup module loading dependencies

[Chris Packham]

Hi Andrew

On 6/07/20 6:29 am, Andrew Lunn wrote:
> To ensure that the octoen MDIO driver has been loaded, the Cavium
> ethernet drivers reference a dummy symbol in the MDIO driver. This
> forces it to be loaded first. And this symbol has not been cleanly
> implemented, resulting in warnings when build W=1 C=1.
>
> Since device tree is being used, and a phandle points to the PHY on
> the MDIO bus, we can make use of deferred probing. If the PHY fails to
> connect, it should be because the MDIO bus driver has not loaded
> yet. Return -EPROBE_DEFER so it will be tried again later.
>
> Cc: Sunil Goutham <sgoutham@marvell.com>
> Cc: Robert Richter <rrichter@marvell.com>
> Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Gave this a quick spin on one of our boards using the Octeon SoC. Looks 
good to me

Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

> ---
>   drivers/net/ethernet/cavium/octeon/octeon_mgmt.c | 6 +-----
>   drivers/net/phy/mdio-octeon.c                    | 6 ------
>   drivers/staging/octeon/ethernet-mdio.c           | 2 +-
>   drivers/staging/octeon/ethernet-mdio.h           | 2 --
>   drivers/staging/octeon/ethernet.c                | 2 --
>   5 files changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
> index cbaa1924afbe..4bf0237ee52e 100644
> --- a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
> +++ b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
> @@ -961,7 +961,7 @@ static int octeon_mgmt_init_phy(struct net_device *netdev)
>   				PHY_INTERFACE_MODE_MII);
>   
>   	if (!phydev)
> -		return -ENODEV;
> +		return -EPROBE_DEFER;
>   
>   	return 0;
>   }
> @@ -1554,12 +1554,8 @@ static struct platform_driver octeon_mgmt_driver = {
>   	.remove		= octeon_mgmt_remove,
>   };
>   
> -extern void octeon_mdiobus_force_mod_depencency(void);
> -
>   static int __init octeon_mgmt_mod_init(void)
>   {
> -	/* Force our mdiobus driver module to be loaded first. */
> -	octeon_mdiobus_force_mod_depencency();
>   	return platform_driver_register(&octeon_mgmt_driver);
>   }
>   
> diff --git a/drivers/net/phy/mdio-octeon.c b/drivers/net/phy/mdio-octeon.c
> index a2f93948db97..d1e1009d51af 100644
> --- a/drivers/net/phy/mdio-octeon.c
> +++ b/drivers/net/phy/mdio-octeon.c
> @@ -108,12 +108,6 @@ static struct platform_driver octeon_mdiobus_driver = {
>   	.remove		= octeon_mdiobus_remove,
>   };
>   
> -void octeon_mdiobus_force_mod_depencency(void)
> -{
> -	/* Let ethernet drivers force us to be loaded.  */
> -}
> -EXPORT_SYMBOL(octeon_mdiobus_force_mod_depencency);
> -
>   module_platform_driver(octeon_mdiobus_driver);
>   
>   MODULE_DESCRIPTION("Cavium OCTEON MDIO bus driver");
> diff --git a/drivers/staging/octeon/ethernet-mdio.c b/drivers/staging/octeon/ethernet-mdio.c
> index c798672d61b2..cfb673a52b25 100644
> --- a/drivers/staging/octeon/ethernet-mdio.c
> +++ b/drivers/staging/octeon/ethernet-mdio.c
> @@ -163,7 +163,7 @@ int cvm_oct_phy_setup_device(struct net_device *dev)
>   	of_node_put(phy_node);
>   
>   	if (!phydev)
> -		return -ENODEV;
> +		return -EPROBE_DEFER;
>   
>   	priv->last_link = 0;
>   	phy_start(phydev);
> diff --git a/drivers/staging/octeon/ethernet-mdio.h b/drivers/staging/octeon/ethernet-mdio.h
> index e3771d48c49b..7f6716e3fad4 100644
> --- a/drivers/staging/octeon/ethernet-mdio.h
> +++ b/drivers/staging/octeon/ethernet-mdio.h
> @@ -22,7 +22,5 @@
>   
>   extern const struct ethtool_ops cvm_oct_ethtool_ops;
>   
> -void octeon_mdiobus_force_mod_depencency(void);
> -
>   int cvm_oct_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
>   int cvm_oct_phy_setup_device(struct net_device *dev);
> diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
> index f42c3816ce49..920ab08e4311 100644
> --- a/drivers/staging/octeon/ethernet.c
> +++ b/drivers/staging/octeon/ethernet.c
> @@ -689,8 +689,6 @@ static int cvm_oct_probe(struct platform_device *pdev)
>   	mtu_overhead += VLAN_HLEN;
>   #endif
>   
> -	octeon_mdiobus_force_mod_depencency();
> -
>   	pip = pdev->dev.of_node;
>   	if (!pip) {
>   		pr_err("Error: No 'pip' in /aliases\n");