| Message ID | 2190629.1yaby32tsi@tool |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] bcm63xx_enet: fix internal phy IRQ assignment | expand |
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
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 --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 */
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(-)