* Re: [PATCH v2] net: ftgmac100: Request clock and set speed
2017-10-12 3:32 [PATCH v2] net: ftgmac100: Request clock and set speed Joel Stanley
@ 2017-10-12 18:20 ` Benjamin Herrenschmidt
2017-10-14 14:53 ` kbuild test robot
1 sibling, 0 replies; 3+ messages in thread
From: Benjamin Herrenschmidt @ 2017-10-12 18:20 UTC (permalink / raw)
To: Joel Stanley, David S . Miller; +Cc: netdev, linux-kernel, Andrew Jeffery
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.
> */
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] net: ftgmac100: Request clock and set speed
2017-10-12 3:32 [PATCH v2] net: ftgmac100: Request clock and set speed Joel Stanley
2017-10-12 18:20 ` Benjamin Herrenschmidt
@ 2017-10-14 14:53 ` kbuild test robot
1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2017-10-14 14:53 UTC (permalink / raw)
To: Joel Stanley
Cc: kbuild-all, David S . Miller, Benjamin Herrenschmidt, netdev,
linux-kernel, Andrew Jeffery
[-- Attachment #1: Type: text/plain, Size: 7465 bytes --]
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
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 64016 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread