diff mbox series

[V2,2/2] net: ks8851: Register MDIO bus and the internal PHY

Message ID 20210104123017.261452-2-marex@denx.de
State New
Headers show
Series None | expand

Commit Message

Marek Vasut Jan. 4, 2021, 12:30 p.m. UTC 
The KS8851 has a reduced internal PHY, which is accessible through its
registers at offset 0xe4. The PHY is compatible with KS886x PHY present
in Micrel switches, except the PHY ID Low/High registers are swapped.
Register MDIO bus so this PHY can be detected and probed by phylib.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Heiner Kallweit <hkallweit1@gmail.com>
Cc: Lukas Wunner <lukas@wunner.de>
To: netdev@vger.kernel.org
---
V2: - Cast the BIT(0) to u32 to avoid build warnings
    - Swap PHY ID Hi/Lo registers for MDIO bus access
      (retain old behavior for MII bus access)
    - Return -EOPNOTSUPP on read from nonexisting PHY registers
---
 drivers/net/ethernet/micrel/ks8851.h        |   1 +
 drivers/net/ethernet/micrel/ks8851_common.c | 113 +++++++++++++++++---
 2 files changed, 98 insertions(+), 16 deletions(-)

Comments

Jakub Kicinski Jan. 5, 2021, 1:13 a.m. UTC | #1 
On Mon,  4 Jan 2021 13:30:17 +0100 Marek Vasut wrote:
> The KS8851 has a reduced internal PHY, which is accessible through its

> registers at offset 0xe4. The PHY is compatible with KS886x PHY present

> in Micrel switches, except the PHY ID Low/High registers are swapped.

> Register MDIO bus so this PHY can be detected and probed by phylib.

> 

> Signed-off-by: Marek Vasut <marex@denx.de>

> Cc: Andrew Lunn <andrew@lunn.ch>

> Cc: Heiner Kallweit <hkallweit1@gmail.com>

> Cc: Lukas Wunner <lukas@wunner.de>

> To: netdev@vger.kernel.org


Please put this creative To: tag under --- so maintainers don't have to
stip it manually :)

There's a few warnings here:

drivers/net/ethernet/micrel/ks8851_common.c: In function ‘ks8851_mdio_read’:
drivers/net/ethernet/micrel/ks8851_common.c:1001:6: warning: unused variable ‘ret’ [-Wunused-variable]
 1001 |  int ret;
      |      ^~~

drivers/net/ethernet/micrel/ks8851.h:425: warning: Function parameter or member 'mii_bus' not described in 'ks8851_net'
kernel test robot Jan. 5, 2021, 5:26 a.m. UTC | #2 
Hi Marek,

I love your patch! Yet something to improve:

[auto build test ERROR on net/master]
[also build test ERROR on ipvs/master net-next/master linus/master v5.11-rc2 next-20210104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Marek-Vasut/net-phy-micrel-Add-KS8851-PHY-support/20210104-203217
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 4bfc4714849d005e6835bcffa3c29ebd6e5ee35d
config: i386-randconfig-a013-20210105 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/ab74850be61e7510a3304091c458b8aedfd51388
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Marek-Vasut/net-phy-micrel-Add-KS8851-PHY-support/20210104-203217
        git checkout ab74850be61e7510a3304091c458b8aedfd51388
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/net/ethernet/micrel/ks8851_common.o: in function `ks8851_register_mdiobus':
>> drivers/net/ethernet/micrel/ks8851_common.c:1103: undefined reference to `__this_module'



vim +1103 drivers/net/ethernet/micrel/ks8851_common.c

  1085	
  1086	static int ks8851_register_mdiobus(struct ks8851_net *ks, struct device *dev)
  1087	{
  1088		struct mii_bus *mii_bus;
  1089		int ret;
  1090	
  1091		mii_bus = mdiobus_alloc();
  1092		if (!mii_bus)
  1093			return -ENOMEM;
  1094	
  1095		mii_bus->name = "ks8851_eth_mii";
  1096		mii_bus->read = ks8851_mdio_read;
  1097		mii_bus->write = ks8851_mdio_write;
  1098		mii_bus->priv = ks;
  1099		mii_bus->parent = dev;
  1100		mii_bus->phy_mask = ~((u32)BIT(0));
  1101		snprintf(mii_bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
  1102	
> 1103		ret = mdiobus_register(mii_bus);

  1104		if (ret)
  1105			goto err_mdiobus_register;
  1106	
  1107		ks->mii_bus = mii_bus;
  1108	
  1109		return 0;
  1110	
  1111	err_mdiobus_register:
  1112		mdiobus_free(mii_bus);
  1113		return ret;
  1114	}
  1115	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/net/ethernet/micrel/ks8851.h b/drivers/net/ethernet/micrel/ks8851.h
index 2b319e451121..9bed9e024d81 100644
--- a/drivers/net/ethernet/micrel/ks8851.h
+++ b/drivers/net/ethernet/micrel/ks8851.h
@@ -403,6 +403,7 @@  struct ks8851_net {
 	struct regulator	*vdd_reg;
 	struct regulator	*vdd_io;
 	int			gpio;
+	struct mii_bus		*mii_bus;
 
 	void			(*lock)(struct ks8851_net *ks,
 					unsigned long *flags);
diff --git a/drivers/net/ethernet/micrel/ks8851_common.c b/drivers/net/ethernet/micrel/ks8851_common.c
index 6fc7483aea03..e0de9c4c98d1 100644
--- a/drivers/net/ethernet/micrel/ks8851_common.c
+++ b/drivers/net/ethernet/micrel/ks8851_common.c
@@ -23,6 +23,7 @@ 
 
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
+#include <linux/of_mdio.h>
 #include <linux/of_net.h>
 
 #include "ks8851.h"
@@ -932,7 +933,25 @@  static int ks8851_phy_reg(int reg)
 		return KS_P1ANLPR;
 	}
 
-	return 0x0;
+	return -EOPNOTSUPP;
+}
+
+static int ks8851_phy_read_common(struct net_device *dev, int phy_addr, int reg)
+{
+	struct ks8851_net *ks = netdev_priv(dev);
+	unsigned long flags;
+	int result;
+	int ksreg;
+
+	ksreg = ks8851_phy_reg(reg);
+	if (ksreg < 0)
+		return ksreg;
+
+	ks8851_lock(ks, &flags);
+	result = ks8851_rdreg16(ks, ksreg);
+	ks8851_unlock(ks, &flags);
+
+	return result;
 }
 
 /**
@@ -952,20 +971,13 @@  static int ks8851_phy_reg(int reg)
  */
 static int ks8851_phy_read(struct net_device *dev, int phy_addr, int reg)
 {
-	struct ks8851_net *ks = netdev_priv(dev);
-	unsigned long flags;
-	int ksreg;
-	int result;
+	int ret;
 
-	ksreg = ks8851_phy_reg(reg);
-	if (!ksreg)
+	ret = ks8851_phy_read_common(dev, phy_addr, reg);
+	if (ret < 0)
 		return 0x0;	/* no error return allowed, so use zero */
 
-	ks8851_lock(ks, &flags);
-	result = ks8851_rdreg16(ks, ksreg);
-	ks8851_unlock(ks, &flags);
-
-	return result;
+	return ret;
 }
 
 static void ks8851_phy_write(struct net_device *dev,
@@ -976,13 +988,38 @@  static void ks8851_phy_write(struct net_device *dev,
 	int ksreg;
 
 	ksreg = ks8851_phy_reg(reg);
-	if (ksreg) {
+	if (ksreg >= 0) {
 		ks8851_lock(ks, &flags);
 		ks8851_wrreg16(ks, ksreg, value);
 		ks8851_unlock(ks, &flags);
 	}
 }
 
+static int ks8851_mdio_read(struct mii_bus *bus, int phy_id, int reg)
+{
+	struct ks8851_net *ks = bus->priv;
+	int ret;
+
+	if (phy_id != 0)
+		return -EOPNOTSUPP;
+
+	/* KS8851 PHY ID registers are swapped in HW, swap them back. */
+	if (reg == MII_PHYSID1)
+		reg = MII_PHYSID2;
+	else if (reg == MII_PHYSID2)
+		reg = MII_PHYSID1;
+
+	return ks8851_phy_read_common(ks->netdev, phy_id, reg);
+}
+
+static int ks8851_mdio_write(struct mii_bus *bus, int phy_id, int reg, u16 val)
+{
+	struct ks8851_net *ks = bus->priv;
+
+	ks8851_phy_write(ks->netdev, phy_id, reg, val);
+	return 0;
+}
+
 /**
  * ks8851_read_selftest - read the selftest memory info.
  * @ks: The device state
@@ -1046,6 +1083,42 @@  int ks8851_resume(struct device *dev)
 }
 #endif
 
+static int ks8851_register_mdiobus(struct ks8851_net *ks, struct device *dev)
+{
+	struct mii_bus *mii_bus;
+	int ret;
+
+	mii_bus = mdiobus_alloc();
+	if (!mii_bus)
+		return -ENOMEM;
+
+	mii_bus->name = "ks8851_eth_mii";
+	mii_bus->read = ks8851_mdio_read;
+	mii_bus->write = ks8851_mdio_write;
+	mii_bus->priv = ks;
+	mii_bus->parent = dev;
+	mii_bus->phy_mask = ~((u32)BIT(0));
+	snprintf(mii_bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
+
+	ret = mdiobus_register(mii_bus);
+	if (ret)
+		goto err_mdiobus_register;
+
+	ks->mii_bus = mii_bus;
+
+	return 0;
+
+err_mdiobus_register:
+	mdiobus_free(mii_bus);
+	return ret;
+}
+
+static void ks8851_unregister_mdiobus(struct ks8851_net *ks)
+{
+	mdiobus_unregister(ks->mii_bus);
+	mdiobus_free(ks->mii_bus);
+}
+
 int ks8851_probe_common(struct net_device *netdev, struct device *dev,
 			int msg_en)
 {
@@ -1104,6 +1177,8 @@  int ks8851_probe_common(struct net_device *netdev, struct device *dev,
 
 	INIT_WORK(&ks->rxctrl_work, ks8851_rxctrl_work);
 
+	SET_NETDEV_DEV(netdev, dev);
+
 	/* setup EEPROM state */
 	ks->eeprom.data = ks;
 	ks->eeprom.width = PCI_EEPROM_WIDTH_93C46;
@@ -1120,6 +1195,10 @@  int ks8851_probe_common(struct net_device *netdev, struct device *dev,
 
 	dev_info(dev, "message enable is %d\n", msg_en);
 
+	ret = ks8851_register_mdiobus(ks, dev);
+	if (ret)
+		goto err_mdio;
+
 	/* set the default message enable */
 	ks->msg_enable = netif_msg_init(msg_en, NETIF_MSG_DRV |
 						NETIF_MSG_PROBE |
@@ -1128,7 +1207,6 @@  int ks8851_probe_common(struct net_device *netdev, struct device *dev,
 	skb_queue_head_init(&ks->txq);
 
 	netdev->ethtool_ops = &ks8851_ethtool_ops;
-	SET_NETDEV_DEV(netdev, dev);
 
 	dev_set_drvdata(dev, ks);
 
@@ -1156,7 +1234,7 @@  int ks8851_probe_common(struct net_device *netdev, struct device *dev,
 	ret = register_netdev(netdev);
 	if (ret) {
 		dev_err(dev, "failed to register network device\n");
-		goto err_netdev;
+		goto err_id;
 	}
 
 	netdev_info(netdev, "revision %d, MAC %pM, IRQ %d, %s EEPROM\n",
@@ -1165,8 +1243,9 @@  int ks8851_probe_common(struct net_device *netdev, struct device *dev,
 
 	return 0;
 
-err_netdev:
 err_id:
+	ks8851_unregister_mdiobus(ks);
+err_mdio:
 	if (gpio_is_valid(gpio))
 		gpio_set_value(gpio, 0);
 	regulator_disable(ks->vdd_reg);
@@ -1180,6 +1259,8 @@  int ks8851_remove_common(struct device *dev)
 {
 	struct ks8851_net *priv = dev_get_drvdata(dev);
 
+	ks8851_unregister_mdiobus(priv);
+
 	if (netif_msg_drv(priv))
 		dev_info(dev, "remove\n");