diff mbox series

[v3] bcm63xx_enet: fix internal phy IRQ assignment

Message ID 2190629.1yaby32tsi@tool (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [v3] bcm63xx_enet: fix internal phy IRQ assignment | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cc_maintainers warning 5 maintainers not CCed: liew.s.piaw@gmail.com bcm-kernel-feedback-list@broadcom.com leon@kernel.org linux-arm-kernel@lists.infradead.org tangbin@cmss.chinamobile.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: 'asign' may be misspelled - perhaps 'assign'?
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Daniel González Cabanelas Feb. 24, 2021, 4:11 p.m. UTC 
The current bcm63xx_enet driver doesn't asign the internal phy IRQ. As a
result of this it works in polling mode.

Fix it using the phy_device structure to assign the platform IRQ.

Tested under a BCM6348 board. Kernel dmesg before the patch:
   Broadcom BCM63XX (1) bcm63xx_enet-0:01: attached PHY driver [Broadcom
              BCM63XX (1)] (mii_bus:phy_addr=bcm63xx_enet-0:01, irq=POLL)

After the patch:
   Broadcom BCM63XX (1) bcm63xx_enet-0:01: attached PHY driver [Broadcom
              BCM63XX (1)] (mii_bus:phy_addr=bcm63xx_enet-0:01, irq=17)

Pluging and uplugging the ethernet cable now generates interrupts and the
PHY goes up and down as expected.

Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
---
changes in V3: 
  - snippet moved after the mdiobus err check
  - snippet moved after the mdiobus registration
  - added missing brackets

 drivers/net/ethernet/broadcom/bcm63xx_enet.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Florian Fainelli Feb. 24, 2021, 5:08 p.m. UTC | #1 
On 2/24/2021 8:11 AM, Daniel González Cabanelas wrote:
> The current bcm63xx_enet driver doesn't asign the internal phy IRQ. As a
> result of this it works in polling mode.
> 
> Fix it using the phy_device structure to assign the platform IRQ.
> 
> Tested under a BCM6348 board. Kernel dmesg before the patch:
>    Broadcom BCM63XX (1) bcm63xx_enet-0:01: attached PHY driver [Broadcom
>               BCM63XX (1)] (mii_bus:phy_addr=bcm63xx_enet-0:01, irq=POLL)
> 
> After the patch:
>    Broadcom BCM63XX (1) bcm63xx_enet-0:01: attached PHY driver [Broadcom
>               BCM63XX (1)] (mii_bus:phy_addr=bcm63xx_enet-0:01, irq=17)
> 
> Pluging and uplugging the ethernet cable now generates interrupts and the
> PHY goes up and down as expected.
> 
> Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Andrew Lunn Feb. 25, 2021, 10:14 p.m. UTC | #2 
On Wed, Feb 24, 2021 at 05:11:33PM +0100, Daniel González Cabanelas wrote:
> The current bcm63xx_enet driver doesn't asign the internal phy IRQ. As a
> result of this it works in polling mode.
> 
> Fix it using the phy_device structure to assign the platform IRQ.

Hi Daniel

We still don't have a root cause analysis. I really would like to
understand what is wrong here, since there should be no need to assign
phydev->irq. I want to be sure we don't have the same problem with
other drivers.

      Andrew
kernel test robot March 10, 2021, 4:55 a.m. UTC | #3 
Hi "Daniel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on ipvs/master]
[also build test ERROR on linus/master sparc-next/master v5.12-rc2 next-20210309]
[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/Daniel-Gonz-lez-Cabanelas/bcm63xx_enet-fix-internal-phy-IRQ-assignment/20210225-001952
base:   https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
config: mips-bcm63xx_defconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/65174cd38e1739216cc40e3d285f16391d79d6e5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Daniel-Gonz-lez-Cabanelas/bcm63xx_enet-fix-internal-phy-IRQ-assignment/20210225-001952
        git checkout 65174cd38e1739216cc40e3d285f16391d79d6e5
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips 

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 >>):

   drivers/net/ethernet/broadcom/bcm63xx_enet.c: In function 'bcm_enet_start_xmit':
   drivers/net/ethernet/broadcom/bcm63xx_enet.c:583:9: warning: variable 'data' set but not used [-Wunused-but-set-variable]
     583 |   char *data;
         |         ^~~~
   drivers/net/ethernet/broadcom/bcm63xx_enet.c: In function 'bcm_enet_probe':
>> drivers/net/ethernet/broadcom/bcm63xx_enet.c:1807:4: error: 'phydev' undeclared (first use in this function); did you mean 'pdev'?
    1807 |    phydev = mdiobus_get_phy(bus, priv->phy_id);
         |    ^~~~~~
         |    pdev
   drivers/net/ethernet/broadcom/bcm63xx_enet.c:1807:4: note: each undeclared identifier is reported only once for each function it appears in


vim +1807 drivers/net/ethernet/broadcom/bcm63xx_enet.c

  1683	
  1684	/*
  1685	 * allocate netdevice, request register memory and register device.
  1686	 */
  1687	static int bcm_enet_probe(struct platform_device *pdev)
  1688	{
  1689		struct bcm_enet_priv *priv;
  1690		struct net_device *dev;
  1691		struct bcm63xx_enet_platform_data *pd;
  1692		struct resource *res_irq, *res_irq_rx, *res_irq_tx;
  1693		struct mii_bus *bus;
  1694		int i, ret;
  1695	
  1696		if (!bcm_enet_shared_base[0])
  1697			return -EPROBE_DEFER;
  1698	
  1699		res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
  1700		res_irq_rx = platform_get_resource(pdev, IORESOURCE_IRQ, 1);
  1701		res_irq_tx = platform_get_resource(pdev, IORESOURCE_IRQ, 2);
  1702		if (!res_irq || !res_irq_rx || !res_irq_tx)
  1703			return -ENODEV;
  1704	
  1705		dev = alloc_etherdev(sizeof(*priv));
  1706		if (!dev)
  1707			return -ENOMEM;
  1708		priv = netdev_priv(dev);
  1709	
  1710		priv->enet_is_sw = false;
  1711		priv->dma_maxburst = BCMENET_DMA_MAXBURST;
  1712	
  1713		ret = bcm_enet_change_mtu(dev, dev->mtu);
  1714		if (ret)
  1715			goto out;
  1716	
  1717		priv->base = devm_platform_ioremap_resource(pdev, 0);
  1718		if (IS_ERR(priv->base)) {
  1719			ret = PTR_ERR(priv->base);
  1720			goto out;
  1721		}
  1722	
  1723		dev->irq = priv->irq = res_irq->start;
  1724		priv->irq_rx = res_irq_rx->start;
  1725		priv->irq_tx = res_irq_tx->start;
  1726	
  1727		priv->mac_clk = devm_clk_get(&pdev->dev, "enet");
  1728		if (IS_ERR(priv->mac_clk)) {
  1729			ret = PTR_ERR(priv->mac_clk);
  1730			goto out;
  1731		}
  1732		ret = clk_prepare_enable(priv->mac_clk);
  1733		if (ret)
  1734			goto out;
  1735	
  1736		/* initialize default and fetch platform data */
  1737		priv->rx_ring_size = BCMENET_DEF_RX_DESC;
  1738		priv->tx_ring_size = BCMENET_DEF_TX_DESC;
  1739	
  1740		pd = dev_get_platdata(&pdev->dev);
  1741		if (pd) {
  1742			memcpy(dev->dev_addr, pd->mac_addr, ETH_ALEN);
  1743			priv->has_phy = pd->has_phy;
  1744			priv->phy_id = pd->phy_id;
  1745			priv->has_phy_interrupt = pd->has_phy_interrupt;
  1746			priv->phy_interrupt = pd->phy_interrupt;
  1747			priv->use_external_mii = !pd->use_internal_phy;
  1748			priv->pause_auto = pd->pause_auto;
  1749			priv->pause_rx = pd->pause_rx;
  1750			priv->pause_tx = pd->pause_tx;
  1751			priv->force_duplex_full = pd->force_duplex_full;
  1752			priv->force_speed_100 = pd->force_speed_100;
  1753			priv->dma_chan_en_mask = pd->dma_chan_en_mask;
  1754			priv->dma_chan_int_mask = pd->dma_chan_int_mask;
  1755			priv->dma_chan_width = pd->dma_chan_width;
  1756			priv->dma_has_sram = pd->dma_has_sram;
  1757			priv->dma_desc_shift = pd->dma_desc_shift;
  1758			priv->rx_chan = pd->rx_chan;
  1759			priv->tx_chan = pd->tx_chan;
  1760		}
  1761	
  1762		if (priv->has_phy && !priv->use_external_mii) {
  1763			/* using internal PHY, enable clock */
  1764			priv->phy_clk = devm_clk_get(&pdev->dev, "ephy");
  1765			if (IS_ERR(priv->phy_clk)) {
  1766				ret = PTR_ERR(priv->phy_clk);
  1767				priv->phy_clk = NULL;
  1768				goto out_disable_clk_mac;
  1769			}
  1770			ret = clk_prepare_enable(priv->phy_clk);
  1771			if (ret)
  1772				goto out_disable_clk_mac;
  1773		}
  1774	
  1775		/* do minimal hardware init to be able to probe mii bus */
  1776		bcm_enet_hw_preinit(priv);
  1777	
  1778		/* MII bus registration */
  1779		if (priv->has_phy) {
  1780	
  1781			priv->mii_bus = mdiobus_alloc();
  1782			if (!priv->mii_bus) {
  1783				ret = -ENOMEM;
  1784				goto out_uninit_hw;
  1785			}
  1786	
  1787			bus = priv->mii_bus;
  1788			bus->name = "bcm63xx_enet MII bus";
  1789			bus->parent = &pdev->dev;
  1790			bus->priv = priv;
  1791			bus->read = bcm_enet_mdio_read_phylib;
  1792			bus->write = bcm_enet_mdio_write_phylib;
  1793			sprintf(bus->id, "%s-%d", pdev->name, pdev->id);
  1794	
  1795			/* only probe bus where we think the PHY is, because
  1796			 * the mdio read operation return 0 instead of 0xffff
  1797			 * if a slave is not present on hw */
  1798			bus->phy_mask = ~(1 << priv->phy_id);
  1799	
  1800			ret = mdiobus_register(bus);
  1801			if (ret) {
  1802				dev_err(&pdev->dev, "unable to register mdio bus\n");
  1803				goto out_free_mdio;
  1804			}
  1805	
  1806			if (priv->has_phy_interrupt) {
> 1807				phydev = mdiobus_get_phy(bus, priv->phy_id);
  1808				if (!phydev) {
  1809					dev_err(&dev->dev, "no PHY found\n");
  1810					goto out_unregister_mdio;
  1811				}
  1812	
  1813				bus->irq[priv->phy_id] = priv->phy_interrupt;
  1814				phydev->irq = priv->phy_interrupt;
  1815			}
  1816		} else {
  1817	
  1818			/* run platform code to initialize PHY device */
  1819			if (pd && pd->mii_config &&
  1820			    pd->mii_config(dev, 1, bcm_enet_mdio_read_mii,
  1821					   bcm_enet_mdio_write_mii)) {
  1822				dev_err(&pdev->dev, "unable to configure mdio bus\n");
  1823				goto out_uninit_hw;
  1824			}
  1825		}
  1826	
  1827		spin_lock_init(&priv->rx_lock);
  1828	
  1829		/* init rx timeout (used for oom) */
  1830		timer_setup(&priv->rx_timeout, bcm_enet_refill_rx_timer, 0);
  1831	
  1832		/* init the mib update lock&work */
  1833		mutex_init(&priv->mib_update_lock);
  1834		INIT_WORK(&priv->mib_update_task, bcm_enet_update_mib_counters_defer);
  1835	
  1836		/* zero mib counters */
  1837		for (i = 0; i < ENET_MIB_REG_COUNT; i++)
  1838			enet_writel(priv, 0, ENET_MIB_REG(i));
  1839	
  1840		/* register netdevice */
  1841		dev->netdev_ops = &bcm_enet_ops;
  1842		netif_napi_add(dev, &priv->napi, bcm_enet_poll, 16);
  1843	
  1844		dev->ethtool_ops = &bcm_enet_ethtool_ops;
  1845		/* MTU range: 46 - 2028 */
  1846		dev->min_mtu = ETH_ZLEN - ETH_HLEN;
  1847		dev->max_mtu = BCMENET_MAX_MTU - VLAN_ETH_HLEN;
  1848		SET_NETDEV_DEV(dev, &pdev->dev);
  1849	
  1850		ret = register_netdev(dev);
  1851		if (ret)
  1852			goto out_unregister_mdio;
  1853	
  1854		netif_carrier_off(dev);
  1855		platform_set_drvdata(pdev, dev);
  1856		priv->pdev = pdev;
  1857		priv->net_dev = dev;
  1858	
  1859		return 0;
  1860	
  1861	out_unregister_mdio:
  1862		if (priv->mii_bus)
  1863			mdiobus_unregister(priv->mii_bus);
  1864	
  1865	out_free_mdio:
  1866		if (priv->mii_bus)
  1867			mdiobus_free(priv->mii_bus);
  1868	
  1869	out_uninit_hw:
  1870		/* turn off mdc clock */
  1871		enet_writel(priv, 0, ENET_MIISC_REG);
  1872		clk_disable_unprepare(priv->phy_clk);
  1873	
  1874	out_disable_clk_mac:
  1875		clk_disable_unprepare(priv->mac_clk);
  1876	out:
  1877		free_netdev(dev);
  1878		return ret;
  1879	}
  1880	

---
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/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index fd876721316..22c782ed76a 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -1818,14 +1818,22 @@  static int bcm_enet_probe(struct platform_device *pdev)
 		 * if a slave is not present on hw */
 		bus->phy_mask = ~(1 << priv->phy_id);
 
-		if (priv->has_phy_interrupt)
-			bus->irq[priv->phy_id] = priv->phy_interrupt;
-
 		ret = mdiobus_register(bus);
 		if (ret) {
 			dev_err(&pdev->dev, "unable to register mdio bus\n");
 			goto out_free_mdio;
 		}
+
+		if (priv->has_phy_interrupt) {
+			phydev = mdiobus_get_phy(bus, priv->phy_id);
+			if (!phydev) {
+				dev_err(&dev->dev, "no PHY found\n");
+				goto out_unregister_mdio;
+			}
+
+			bus->irq[priv->phy_id] = priv->phy_interrupt;
+			phydev->irq = priv->phy_interrupt;
+		}
 	} else {
 
 		/* run platform code to initialize PHY device */