diff mbox series

[v2] net: ftgmac100: Request clock and set speed

Message ID 20171012033201.12845-1-joel@jms.id.au
State Superseded, archived
Delegated to: David Miller
Headers show
Series [v2] net: ftgmac100: Request clock and set speed | expand

Commit Message

Joel Stanley Oct. 12, 2017, 3:32 a.m. UTC 
According to the ASPEED datasheet, gigabit speeds require a clock of
100MHz or higher. Other speeds require 25MHz or higher. This patch
configures a 100MHz clock if the system has a direct-attached
PHY, or 25MHz if the system is running NC-SI which is limited to 100MHz.

There appear to be no other upstream users of the FTGMAC100 driver so it
is hard to know the clocking requirements of other platforms. Therefore
a conservative approach was taken with enabling clocks. If the platform
is not ASPEED, both requesting the clock and configuring the speed is
skipped.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
Andrew, as I'm travelling can you please test this on the evb and a
palmetto? Use my wip/aspeed-v4.14-clk branch, or OpenBMC's dev-4.13.

David, please wait for Andrew's tested-by before applying.

Cheers!

v2:
 - only touch the clocks on Aspeed platforms
 - unconditionally call clk_unprepare_disable

 drivers/net/ethernet/faraday/ftgmac100.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

Comments

Benjamin Herrenschmidt Oct. 12, 2017, 6:20 p.m. UTC | #1 
On Thu, 2017-10-12 at 11:32 +0800, Joel Stanley wrote:
> According to the ASPEED datasheet, gigabit speeds require a clock of
> 100MHz or higher. Other speeds require 25MHz or higher. This patch
> configures a 100MHz clock if the system has a direct-attached
> PHY, or 25MHz if the system is running NC-SI which is limited to 100MHz.
> 
> There appear to be no other upstream users of the FTGMAC100 driver so it
> is hard to know the clocking requirements of other platforms. Therefore
> a conservative approach was taken with enabling clocks. If the platform
> is not ASPEED, both requesting the clock and configuring the speed is
> skipped.

We might still be able to check the PHY capabilities and it might also
be possible to do the live change as you were doing previously but it
needs testing. So I'm ok with this patch for now, and later I might be
able to try the live change option on the eval board (provided I still
have one) when I come back to Australia.

> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> Andrew, as I'm travelling can you please test this on the evb and a
> palmetto? Use my wip/aspeed-v4.14-clk branch, or OpenBMC's dev-4.13.
> 
> David, please wait for Andrew's tested-by before applying.
> 
> Cheers!
> 
> v2:
>  - only touch the clocks on Aspeed platforms
>  - unconditionally call clk_unprepare_disable
> 
>  drivers/net/ethernet/faraday/ftgmac100.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 9ed8e4b81530..cd352bf41da1 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -21,6 +21,7 @@
>  
>  #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
>  
> +#include <linux/clk.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/etherdevice.h>
>  #include <linux/ethtool.h>
> @@ -59,6 +60,9 @@
>  /* Min number of tx ring entries before stopping queue */
>  #define TX_THRESHOLD		(MAX_SKB_FRAGS + 1)
>  
> +#define FTGMAC_100MHZ		100000000
> +#define FTGMAC_25MHZ		25000000
> +
>  struct ftgmac100 {
>  	/* Registers */
>  	struct resource *res;
> @@ -96,6 +100,7 @@ struct ftgmac100 {
>  	struct napi_struct napi;
>  	struct work_struct reset_task;
>  	struct mii_bus *mii_bus;
> +	struct clk *clk;
>  
>  	/* Link management */
>  	int cur_speed;
> @@ -1734,6 +1739,22 @@ static void ftgmac100_ncsi_handler(struct ncsi_dev *nd)
>  		    nd->link_up ? "up" : "down");
>  }
>  
> +static void ftgmac100_setup_clk(struct ftgmac100_priv *priv)
> +{
> +	priv->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(priv->clk))
> +		return;
> +
> +	clk_prepare_enable(priv->clk);
> +
> +	/* Aspeed specifies a 100MHz clock is required for up to
> +	 * 1000Mbit link speeds. As NCSI is limited to 100Mbit, 25MHz
> +	 * is sufficient
> +	 */
> +	clk_set_rate(priv->clk, priv->is_ncsi ? FTGMAC_25MHZ :
> +			FTGMAC_100MHZ);
> +}
> +
>  static int ftgmac100_probe(struct platform_device *pdev)
>  {
>  	struct resource *res;
> @@ -1830,6 +1851,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
>  			goto err_setup_mdio;
>  	}
>  
> +	if (priv->is_aspeed)
> +		ftgmac100_setup_clk(priv);
> +
>  	/* Default ring sizes */
>  	priv->rx_q_entries = priv->new_rx_q_entries = DEF_RX_QUEUE_ENTRIES;
>  	priv->tx_q_entries = priv->new_tx_q_entries = DEF_TX_QUEUE_ENTRIES;
> @@ -1883,6 +1907,8 @@ static int ftgmac100_remove(struct platform_device *pdev)
>  
>  	unregister_netdev(netdev);
>  
> +	clk_disable_unprepare(priv->clk);
> +
>  	/* There's a small chance the reset task will have been re-queued,
>  	 * during stop, make sure it's gone before we free the structure.
>  	 */
kernel test robot Oct. 14, 2017, 2:53 p.m. UTC | #2 
Hi Joel,

[auto build test ERROR on net-next/master]
[also build test ERROR on v4.14-rc4 next-20171013]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Joel-Stanley/net-ftgmac100-Request-clock-and-set-speed/20171014-195836
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All error/warnings (new ones prefixed by >>):

>> drivers/net//ethernet/faraday/ftgmac100.c:1742:40: warning: 'struct ftgmac100_priv' declared inside parameter list will not be visible outside of this definition or declaration
    static void ftgmac100_setup_clk(struct ftgmac100_priv *priv)
                                           ^~~~~~~~~~~~~~
   drivers/net//ethernet/faraday/ftgmac100.c: In function 'ftgmac100_setup_clk':
>> drivers/net//ethernet/faraday/ftgmac100.c:1744:6: error: dereferencing pointer to incomplete type 'struct ftgmac100_priv'
     priv->clk = devm_clk_get(&pdev->dev, NULL);
         ^~
>> drivers/net//ethernet/faraday/ftgmac100.c:1744:28: error: 'pdev' undeclared (first use in this function)
     priv->clk = devm_clk_get(&pdev->dev, NULL);
                               ^~~~
   drivers/net//ethernet/faraday/ftgmac100.c:1744:28: note: each undeclared identifier is reported only once for each function it appears in
   drivers/net//ethernet/faraday/ftgmac100.c: In function 'ftgmac100_probe':
>> drivers/net//ethernet/faraday/ftgmac100.c:1855:23: error: passing argument 1 of 'ftgmac100_setup_clk' from incompatible pointer type [-Werror=incompatible-pointer-types]
      ftgmac100_setup_clk(priv);
                          ^~~~
   drivers/net//ethernet/faraday/ftgmac100.c:1742:13: note: expected 'struct ftgmac100_priv *' but argument is of type 'struct ftgmac100 *'
    static void ftgmac100_setup_clk(struct ftgmac100_priv *priv)
                ^~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +1744 drivers/net//ethernet/faraday/ftgmac100.c

  1741	
> 1742	static void ftgmac100_setup_clk(struct ftgmac100_priv *priv)
  1743	{
> 1744		priv->clk = devm_clk_get(&pdev->dev, NULL);
  1745		if (IS_ERR(priv->clk))
  1746			return;
  1747	
  1748		clk_prepare_enable(priv->clk);
  1749	
  1750		/* Aspeed specifies a 100MHz clock is required for up to
  1751		 * 1000Mbit link speeds. As NCSI is limited to 100Mbit, 25MHz
  1752		 * is sufficient
  1753		 */
  1754		clk_set_rate(priv->clk, priv->is_ncsi ? FTGMAC_25MHZ :
  1755				FTGMAC_100MHZ);
  1756	}
  1757	
  1758	static int ftgmac100_probe(struct platform_device *pdev)
  1759	{
  1760		struct resource *res;
  1761		int irq;
  1762		struct net_device *netdev;
  1763		struct ftgmac100 *priv;
  1764		struct device_node *np;
  1765		int err = 0;
  1766	
  1767		if (!pdev)
  1768			return -ENODEV;
  1769	
  1770		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  1771		if (!res)
  1772			return -ENXIO;
  1773	
  1774		irq = platform_get_irq(pdev, 0);
  1775		if (irq < 0)
  1776			return irq;
  1777	
  1778		/* setup net_device */
  1779		netdev = alloc_etherdev(sizeof(*priv));
  1780		if (!netdev) {
  1781			err = -ENOMEM;
  1782			goto err_alloc_etherdev;
  1783		}
  1784	
  1785		SET_NETDEV_DEV(netdev, &pdev->dev);
  1786	
  1787		netdev->ethtool_ops = &ftgmac100_ethtool_ops;
  1788		netdev->netdev_ops = &ftgmac100_netdev_ops;
  1789		netdev->watchdog_timeo = 5 * HZ;
  1790	
  1791		platform_set_drvdata(pdev, netdev);
  1792	
  1793		/* setup private data */
  1794		priv = netdev_priv(netdev);
  1795		priv->netdev = netdev;
  1796		priv->dev = &pdev->dev;
  1797		INIT_WORK(&priv->reset_task, ftgmac100_reset_task);
  1798	
  1799		/* map io memory */
  1800		priv->res = request_mem_region(res->start, resource_size(res),
  1801					       dev_name(&pdev->dev));
  1802		if (!priv->res) {
  1803			dev_err(&pdev->dev, "Could not reserve memory region\n");
  1804			err = -ENOMEM;
  1805			goto err_req_mem;
  1806		}
  1807	
  1808		priv->base = ioremap(res->start, resource_size(res));
  1809		if (!priv->base) {
  1810			dev_err(&pdev->dev, "Failed to ioremap ethernet registers\n");
  1811			err = -EIO;
  1812			goto err_ioremap;
  1813		}
  1814	
  1815		netdev->irq = irq;
  1816	
  1817		/* Enable pause */
  1818		priv->tx_pause = true;
  1819		priv->rx_pause = true;
  1820		priv->aneg_pause = true;
  1821	
  1822		/* MAC address from chip or random one */
  1823		ftgmac100_initial_mac(priv);
  1824	
  1825		np = pdev->dev.of_node;
  1826		if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
  1827			   of_device_is_compatible(np, "aspeed,ast2500-mac"))) {
  1828			priv->rxdes0_edorr_mask = BIT(30);
  1829			priv->txdes0_edotr_mask = BIT(30);
  1830			priv->is_aspeed = true;
  1831		} else {
  1832			priv->rxdes0_edorr_mask = BIT(15);
  1833			priv->txdes0_edotr_mask = BIT(15);
  1834		}
  1835	
  1836		if (np && of_get_property(np, "use-ncsi", NULL)) {
  1837			if (!IS_ENABLED(CONFIG_NET_NCSI)) {
  1838				dev_err(&pdev->dev, "NCSI stack not enabled\n");
  1839				goto err_ncsi_dev;
  1840			}
  1841	
  1842			dev_info(&pdev->dev, "Using NCSI interface\n");
  1843			priv->use_ncsi = true;
  1844			priv->ndev = ncsi_register_dev(netdev, ftgmac100_ncsi_handler);
  1845			if (!priv->ndev)
  1846				goto err_ncsi_dev;
  1847		} else {
  1848			priv->use_ncsi = false;
  1849			err = ftgmac100_setup_mdio(netdev);
  1850			if (err)
  1851				goto err_setup_mdio;
  1852		}
  1853	
  1854		if (priv->is_aspeed)
> 1855			ftgmac100_setup_clk(priv);
  1856	
  1857		/* Default ring sizes */
  1858		priv->rx_q_entries = priv->new_rx_q_entries = DEF_RX_QUEUE_ENTRIES;
  1859		priv->tx_q_entries = priv->new_tx_q_entries = DEF_TX_QUEUE_ENTRIES;
  1860	
  1861		/* Base feature set */
  1862		netdev->hw_features = NETIF_F_RXCSUM | NETIF_F_HW_CSUM |
  1863			NETIF_F_GRO | NETIF_F_SG | NETIF_F_HW_VLAN_CTAG_RX |
  1864			NETIF_F_HW_VLAN_CTAG_TX;
  1865	
  1866		if (priv->use_ncsi)
  1867			netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
  1868	
  1869		/* AST2400  doesn't have working HW checksum generation */
  1870		if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
  1871			netdev->hw_features &= ~NETIF_F_HW_CSUM;
  1872		if (np && of_get_property(np, "no-hw-checksum", NULL))
  1873			netdev->hw_features &= ~(NETIF_F_HW_CSUM | NETIF_F_RXCSUM);
  1874		netdev->features |= netdev->hw_features;
  1875	
  1876		/* register network device */
  1877		err = register_netdev(netdev);
  1878		if (err) {
  1879			dev_err(&pdev->dev, "Failed to register netdev\n");
  1880			goto err_register_netdev;
  1881		}
  1882	
  1883		netdev_info(netdev, "irq %d, mapped at %p\n", netdev->irq, priv->base);
  1884	
  1885		return 0;
  1886	
  1887	err_ncsi_dev:
  1888	err_register_netdev:
  1889		ftgmac100_destroy_mdio(netdev);
  1890	err_setup_mdio:
  1891		iounmap(priv->base);
  1892	err_ioremap:
  1893		release_resource(priv->res);
  1894	err_req_mem:
  1895		free_netdev(netdev);
  1896	err_alloc_etherdev:
  1897		return err;
  1898	}
  1899	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox series

Patch

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 9ed8e4b81530..cd352bf41da1 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -21,6 +21,7 @@ 
 
 #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
 
+#include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
@@ -59,6 +60,9 @@ 
 /* Min number of tx ring entries before stopping queue */
 #define TX_THRESHOLD		(MAX_SKB_FRAGS + 1)
 
+#define FTGMAC_100MHZ		100000000
+#define FTGMAC_25MHZ		25000000
+
 struct ftgmac100 {
 	/* Registers */
 	struct resource *res;
@@ -96,6 +100,7 @@  struct ftgmac100 {
 	struct napi_struct napi;
 	struct work_struct reset_task;
 	struct mii_bus *mii_bus;
+	struct clk *clk;
 
 	/* Link management */
 	int cur_speed;
@@ -1734,6 +1739,22 @@  static void ftgmac100_ncsi_handler(struct ncsi_dev *nd)
 		    nd->link_up ? "up" : "down");
 }
 
+static void ftgmac100_setup_clk(struct ftgmac100_priv *priv)
+{
+	priv->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(priv->clk))
+		return;
+
+	clk_prepare_enable(priv->clk);
+
+	/* Aspeed specifies a 100MHz clock is required for up to
+	 * 1000Mbit link speeds. As NCSI is limited to 100Mbit, 25MHz
+	 * is sufficient
+	 */
+	clk_set_rate(priv->clk, priv->is_ncsi ? FTGMAC_25MHZ :
+			FTGMAC_100MHZ);
+}
+
 static int ftgmac100_probe(struct platform_device *pdev)
 {
 	struct resource *res;
@@ -1830,6 +1851,9 @@  static int ftgmac100_probe(struct platform_device *pdev)
 			goto err_setup_mdio;
 	}
 
+	if (priv->is_aspeed)
+		ftgmac100_setup_clk(priv);
+
 	/* Default ring sizes */
 	priv->rx_q_entries = priv->new_rx_q_entries = DEF_RX_QUEUE_ENTRIES;
 	priv->tx_q_entries = priv->new_tx_q_entries = DEF_TX_QUEUE_ENTRIES;
@@ -1883,6 +1907,8 @@  static int ftgmac100_remove(struct platform_device *pdev)
 
 	unregister_netdev(netdev);
 
+	clk_disable_unprepare(priv->clk);
+
 	/* There's a small chance the reset task will have been re-queued,
 	 * during stop, make sure it's gone before we free the structure.
 	 */