Re: [PATCH v3 12/15] fpga: altera: use ARCH_INTEL_SOCFPGA also for 32-bit ARM SoCs
Hi Krzysztof, On Thu, Mar 11, 2021 at 04:27:35PM +0100, Krzysztof Kozlowski wrote: > ARCH_SOCFPGA is being renamed to ARCH_INTEL_SOCFPGA so adjust the > 32-bit ARM drivers to rely on new symbol. > > Signed-off-by: Krzysztof Kozlowski Acked-by: Moritz Fischer > --- > drivers/fpga/Kconfig | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig > index fd325e9c5ce6..b1026c6fb119 100644 > --- a/drivers/fpga/Kconfig > +++ b/drivers/fpga/Kconfig > @@ -14,13 +14,13 @@ if FPGA > > config FPGA_MGR_SOCFPGA > tristate "Altera SOCFPGA FPGA Manager" > - depends on ARCH_SOCFPGA || COMPILE_TEST > + depends on ARCH_INTEL_SOCFPGA || COMPILE_TEST > help > FPGA manager driver support for Altera SOCFPGA. > > config FPGA_MGR_SOCFPGA_A10 > tristate "Altera SoCFPGA Arria10" > - depends on ARCH_SOCFPGA || COMPILE_TEST > + depends on ARCH_INTEL_SOCFPGA || COMPILE_TEST > select REGMAP_MMIO > help > FPGA manager driver support for Altera Arria10 SoCFPGA. > @@ -99,7 +99,7 @@ config FPGA_BRIDGE > > config SOCFPGA_FPGA_BRIDGE > tristate "Altera SoCFPGA FPGA Bridges" > - depends on ARCH_SOCFPGA && FPGA_BRIDGE > + depends on ARCH_INTEL_SOCFPGA && FPGA_BRIDGE > help > Say Y to enable drivers for FPGA bridges for Altera SOCFPGA > devices. > -- > 2.25.1 > Thanks, Moritz
[PATCH net-next v4] net: dec: tulip: de2104x: Add shutdown handler to stop NIC
The driver does not implement a shutdown handler which leads to issues when using kexec in certain scenarios. The NIC keeps on fetching descriptors which gets flagged by the IOMMU with errors like this: DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 Signed-off-by: Moritz Fischer --- Changes from v3: - Added rtnl_lock()/unlock() as suggested by Jakub and use dev_close() Changes from v2: - Changed to net-next - Removed extra whitespace Changes from v1: - Replace call to de_remove_one with de_shutdown() function as suggested by James. --- drivers/net/ethernet/dec/tulip/de2104x.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c index d9f6c19940ef..c3cbe55205a7 100644 --- a/drivers/net/ethernet/dec/tulip/de2104x.c +++ b/drivers/net/ethernet/dec/tulip/de2104x.c @@ -2175,11 +2175,21 @@ static int __maybe_unused de_resume(struct device *dev_d) static SIMPLE_DEV_PM_OPS(de_pm_ops, de_suspend, de_resume); +static void de_shutdown(struct pci_dev *pdev) +{ + struct net_device *dev = pci_get_drvdata(pdev); + + rtnl_lock(); + dev_close(dev); + rtnl_unlock(); +} + static struct pci_driver de_driver = { .name = DRV_NAME, .id_table = de_pci_tbl, .probe = de_init_one, .remove = de_remove_one, + .shutdown = de_shutdown, .driver.pm = &de_pm_ops, }; -- 2.29.1
Re: [PATCH/RFC net-next v3] net: dec: tulip: de2104x: Add shutdown handler to stop NIC
Hi Jakub, On Tue, Oct 27, 2020 at 04:16:06PM -0700, Jakub Kicinski wrote: > On Fri, 23 Oct 2020 13:28:34 -0700 Moritz Fischer wrote: > > diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c > > b/drivers/net/ethernet/dec/tulip/de2104x.c > > index d9f6c19940ef..ea7442cc8e75 100644 > > --- a/drivers/net/ethernet/dec/tulip/de2104x.c > > +++ b/drivers/net/ethernet/dec/tulip/de2104x.c > > @@ -2175,11 +2175,19 @@ static int __maybe_unused de_resume(struct device > > *dev_d) > > > > static SIMPLE_DEV_PM_OPS(de_pm_ops, de_suspend, de_resume); > > > > +static void de_shutdown(struct pci_dev *pdev) > > +{ > > + struct net_device *dev = pci_get_drvdata(pdev); > > + > > + de_close(dev); > > Apparently I get all the best ideas when I'm about to apply something.. Better now than after =) > I don't think you can just call de_close() like that, because > (a) it may expect rtnl_lock() to be held, and (b) it may not be open. how about: rtnl_lock(); if (netif_running(dev)) dev_close(dev); rtnl_unlock(); > > Perhaps call unregister_netdev(dev) - that'll close the device. > Or rtnl_lock(); dev_close(dev); rtnl_unlock(); > > > +} > > + > > static struct pci_driver de_driver = { > > .name = DRV_NAME, > > .id_table = de_pci_tbl, > > .probe = de_init_one, > > .remove = de_remove_one, > > + .shutdown = de_shutdown, > > .driver.pm = &de_pm_ops, > > }; > > > Cheers, Moritz
[PATCH/RFC net-next v3] net: dec: tulip: de2104x: Add shutdown handler to stop NIC
The driver does not implement a shutdown handler which leads to issues when using kexec in certain scenarios. The NIC keeps on fetching descriptors which gets flagged by the IOMMU with errors like this: DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 Signed-off-by: Moritz Fischer --- I'd consider it a bug-fix but Jakub said it's a feature, so net-next it is. I don't have a strong preference either way. Changes from v2: - Changed to net-next - Removed extra whitespace Changes from v1: - Replace call to de_remove_one with de_shutdown() function as suggested by James. --- drivers/net/ethernet/dec/tulip/de2104x.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c index d9f6c19940ef..ea7442cc8e75 100644 --- a/drivers/net/ethernet/dec/tulip/de2104x.c +++ b/drivers/net/ethernet/dec/tulip/de2104x.c @@ -2175,11 +2175,19 @@ static int __maybe_unused de_resume(struct device *dev_d) static SIMPLE_DEV_PM_OPS(de_pm_ops, de_suspend, de_resume); +static void de_shutdown(struct pci_dev *pdev) +{ + struct net_device *dev = pci_get_drvdata(pdev); + + de_close(dev); +} + static struct pci_driver de_driver = { .name = DRV_NAME, .id_table = de_pci_tbl, .probe = de_init_one, .remove = de_remove_one, + .shutdown = de_shutdown, .driver.pm = &de_pm_ops, }; -- 2.28.0
[PATCH/RFC net v2] net: dec: tulip: de2104x: Add shutdown handler to stop NIC
The driver does not implement a shutdown handler which leads to issues when using kexec in certain scenarios. The NIC keeps on fetching descriptors which gets flagged by the IOMMU with errors like this: DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 Signed-off-by: Moritz Fischer --- Changes from v1: - Replace call to de_remove_one with de_shutdown() function as suggested by James. --- drivers/net/ethernet/dec/tulip/de2104x.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c index f1a2da15dd0a..6de0cd6cf4ca 100644 --- a/drivers/net/ethernet/dec/tulip/de2104x.c +++ b/drivers/net/ethernet/dec/tulip/de2104x.c @@ -2180,11 +2180,19 @@ static int de_resume (struct pci_dev *pdev) #endif /* CONFIG_PM */ +static void de_shutdown(struct pci_dev *pdev) +{ + struct net_device *dev = pci_get_drvdata (pdev); + + de_close(dev); +} + static struct pci_driver de_driver = { .name = DRV_NAME, .id_table = de_pci_tbl, .probe = de_init_one, .remove = de_remove_one, + .shutdown = de_shutdown, #ifdef CONFIG_PM .suspend= de_suspend, .resume = de_resume, -- 2.28.0
Re: [PATCH/RFC net] net: dec: tulip: de2104x: Add shutdown handler to stop NIC
On Thu, Oct 22, 2020 at 04:04:16PM -0700, James Bottomley wrote: > On Thu, 2020-10-22 at 15:06 -0700, Moritz Fischer wrote: > > The driver does not implement a shutdown handler which leads to > > issues > > when using kexec in certain scenarios. The NIC keeps on fetching > > descriptors which gets flagged by the IOMMU with errors like this: > > > > DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 > > DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 > > DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 > > DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 > > DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 > > > > Signed-off-by: Moritz Fischer > > --- > > > > Hi all, > > > > I'm not sure if this is the proper way for a shutdown handler, > > I've tried to look at a bunch of examples and couldn't find a > > specific > > solution, in my tests on hardware this works, though. > > > > Open to suggestions. > > > > Thanks, > > Moritz > > > > --- > > drivers/net/ethernet/dec/tulip/de2104x.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c > > b/drivers/net/ethernet/dec/tulip/de2104x.c > > index f1a2da15dd0a..372c62c7e60f 100644 > > --- a/drivers/net/ethernet/dec/tulip/de2104x.c > > +++ b/drivers/net/ethernet/dec/tulip/de2104x.c > > @@ -2185,6 +2185,7 @@ static struct pci_driver de_driver = { > > .id_table = de_pci_tbl, > > .probe = de_init_one, > > .remove = de_remove_one, > > + .shutdown = de_remove_one, > > This doesn't look right: shutdown is supposed to turn off the device > without disturbing the tree or causing any knock on effects (I think > that rule is mostly because you don't want anything in userspace > triggering since it's likely to be nearly dead). Remove removes the > device from the tree and cleans up everything. I think the function > you want that's closest to what shutdown needs is de_close(). That > basically just turns off the chip and frees the interrupt ... you'll > have to wrapper it to call it from the pci_driver, though. Thanks for the suggestion, I like that better. I'll send a v2 after testing. I think anything that hits on de_stop_hw() will keep the NIC from fetching further descriptors. Cheers, Moritz
[PATCH/RFC net] net: dec: tulip: de2104x: Add shutdown handler to stop NIC
The driver does not implement a shutdown handler which leads to issues when using kexec in certain scenarios. The NIC keeps on fetching descriptors which gets flagged by the IOMMU with errors like this: DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 Signed-off-by: Moritz Fischer --- Hi all, I'm not sure if this is the proper way for a shutdown handler, I've tried to look at a bunch of examples and couldn't find a specific solution, in my tests on hardware this works, though. Open to suggestions. Thanks, Moritz --- drivers/net/ethernet/dec/tulip/de2104x.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c index f1a2da15dd0a..372c62c7e60f 100644 --- a/drivers/net/ethernet/dec/tulip/de2104x.c +++ b/drivers/net/ethernet/dec/tulip/de2104x.c @@ -2185,6 +2185,7 @@ static struct pci_driver de_driver = { .id_table = de_pci_tbl, .probe = de_init_one, .remove = de_remove_one, + .shutdown = de_remove_one, #ifdef CONFIG_PM .suspend= de_suspend, .resume = de_resume, -- 2.28.0
[PATCH net-next v2 3/3] net: dec: tulip: de2104x: Replace kmemdup() with devm_kmempdup()
Replace an instance of kmemdup() with the devres counted version instead. Signed-off-by: Moritz Fischer --- drivers/net/ethernet/dec/tulip/de2104x.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c index 698d79bc4784..a3a002c8e9ac 100644 --- a/drivers/net/ethernet/dec/tulip/de2104x.c +++ b/drivers/net/ethernet/dec/tulip/de2104x.c @@ -1940,7 +1940,8 @@ static void de21041_get_srom_info(struct de_private *de) de->media[i].csr15 = t21041_csr15[i]; } - de->ee_data = kmemdup(&ee_data[0], DE_EEPROM_SIZE, GFP_KERNEL); + de->ee_data = devm_kmemdup(&de->pdev->dev, &ee_data[0], DE_EEPROM_SIZE, + GFP_KERNEL); return; @@ -2092,7 +2093,6 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) return 0; err_out_iomap: - kfree(de->ee_data); iounmap(regs); err_out_res: pci_release_regions(pdev); @@ -2106,7 +2106,6 @@ static void de_remove_one(struct pci_dev *pdev) BUG_ON(!dev); unregister_netdev(dev); - kfree(de->ee_data); iounmap(de->regs); pci_release_regions(pdev); } -- 2.28.0
[PATCH net-next v2 2/3] net: dec: tulip: de2104x: Replace pci_enable_device with devres version
Replace pci_enable_device() with its devres counterpart pcim_enable_device(). Signed-off-by: Moritz Fischer --- Note: Please check my logic on this, it would seem to me calling pci_disable_device() on devices enabled with pcim_enable_device() *should* be fine. Changes from v1: - Fixed missing replace for resume function --- drivers/net/ethernet/dec/tulip/de2104x.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c index 9bcfc82b71d1..698d79bc4784 100644 --- a/drivers/net/ethernet/dec/tulip/de2104x.c +++ b/drivers/net/ethernet/dec/tulip/de2104x.c @@ -2009,14 +2009,14 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) netif_carrier_off(dev); /* wake up device, assign resources */ - rc = pci_enable_device(pdev); + rc = pcim_enable_device(pdev); if (rc) return rc; /* reserve PCI resources to ensure driver atomicity */ rc = pci_request_regions(pdev, DRV_NAME); if (rc) - goto err_out_disable; + return rc; /* check for invalid IRQ value */ if (pdev->irq < 2) { @@ -2096,8 +2096,6 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) iounmap(regs); err_out_res: pci_release_regions(pdev); -err_out_disable: - pci_disable_device(pdev); return rc; } @@ -2111,7 +2109,6 @@ static void de_remove_one(struct pci_dev *pdev) kfree(de->ee_data); iounmap(de->regs); pci_release_regions(pdev); - pci_disable_device(pdev); } #ifdef CONFIG_PM @@ -2164,7 +2161,7 @@ static int de_resume (struct pci_dev *pdev) goto out; if (!netif_running(dev)) goto out_attach; - if ((retval = pci_enable_device(pdev))) { + if ((retval = pcim_enable_device(pdev))) { netdev_err(dev, "pci_enable_device failed in resume\n"); goto out; } -- 2.28.0
[PATCH net-next v2 0/3] First bunch of Tulip cleanups
This series is the first bunch of minor cleanups for the de2104x driver to make it look and behave more like a modern driver. These changes replace some of the non-devres versions with devres versions of functions to simplify the error paths. Next up after this will be the ioremap part. Changes from v1: - Fix issue with the pci_enable_device patch. Moritz Fischer (3): net: dec: tulip: de2104x: Replace alloc_etherdev by devm_alloc_etherdev net: dec: tulip: de2104x: Replace pci_enable_device with devres version net: dec: tulip: de2104x: Replace kmemdup() with devm_kmempdup() drivers/net/ethernet/dec/tulip/de2104x.c | 21 +++-- 1 file changed, 7 insertions(+), 14 deletions(-) -- 2.28.0
[PATCH net-next v2 1/3] net: dec: tulip: de2104x: Replace alloc_etherdev by devm_alloc_etherdev
Replace devm_alloc_etherdev() with its devres version. Signed-off-by: Moritz Fischer --- drivers/net/ethernet/dec/tulip/de2104x.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c index 42b798a3fad4..9bcfc82b71d1 100644 --- a/drivers/net/ethernet/dec/tulip/de2104x.c +++ b/drivers/net/ethernet/dec/tulip/de2104x.c @@ -1986,7 +1986,7 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) #endif /* allocate a new ethernet device structure, and fill in defaults */ - dev = alloc_etherdev(sizeof(struct de_private)); + dev = devm_alloc_etherdev(&pdev->dev, sizeof(struct de_private)); if (!dev) return -ENOMEM; @@ -2011,7 +2011,7 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) /* wake up device, assign resources */ rc = pci_enable_device(pdev); if (rc) - goto err_out_free; + return rc; /* reserve PCI resources to ensure driver atomicity */ rc = pci_request_regions(pdev, DRV_NAME); @@ -2098,8 +2098,6 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) pci_release_regions(pdev); err_out_disable: pci_disable_device(pdev); -err_out_free: - free_netdev(dev); return rc; } @@ -2114,7 +2112,6 @@ static void de_remove_one(struct pci_dev *pdev) iounmap(de->regs); pci_release_regions(pdev); pci_disable_device(pdev); - free_netdev(dev); } #ifdef CONFIG_PM -- 2.28.0
Re: [PATCH net-next 2/3] net: dec: tulip: de2104x: Replace pci_enable_device with devres version
On Sun, Sep 13, 2020 at 05:10:01PM -0700, Moritz Fischer wrote: > Replace pci_enable_device() with its devres counterpart > pcim_enable_device(). > > Signed-off-by: Moritz Fischer > --- > drivers/net/ethernet/dec/tulip/de2104x.c | 7 ++- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c > b/drivers/net/ethernet/dec/tulip/de2104x.c > index 9bcfc82b71d1..e4189c45c2ba 100644 > --- a/drivers/net/ethernet/dec/tulip/de2104x.c > +++ b/drivers/net/ethernet/dec/tulip/de2104x.c > @@ -2009,14 +2009,14 @@ static int de_init_one(struct pci_dev *pdev, const > struct pci_device_id *ent) > netif_carrier_off(dev); > > /* wake up device, assign resources */ > - rc = pci_enable_device(pdev); > + rc = pcim_enable_device(pdev); > if (rc) > return rc; > > /* reserve PCI resources to ensure driver atomicity */ > rc = pci_request_regions(pdev, DRV_NAME); > if (rc) > - goto err_out_disable; > + return rc; > > /* check for invalid IRQ value */ > if (pdev->irq < 2) { > @@ -2096,8 +2096,6 @@ static int de_init_one(struct pci_dev *pdev, const > struct pci_device_id *ent) > iounmap(regs); > err_out_res: > pci_release_regions(pdev); > -err_out_disable: > - pci_disable_device(pdev); > return rc; > } > > @@ -2111,7 +2109,6 @@ static void de_remove_one(struct pci_dev *pdev) > kfree(de->ee_data); > iounmap(de->regs); > pci_release_regions(pdev); > - pci_disable_device(pdev); > } > > #ifdef CONFIG_PM > -- > 2.28.0 > Ugh, sorry for the noise, I missed the instances in the suspend/resume. Let me resend a v2 of this ... Thanks, Moritz
[PATCH net-next 1/3] net: dec: tulip: de2104x: Replace alloc_etherdev by devm_alloc_etherdev
Replace devm_alloc_etherdev() with its devres version. Signed-off-by: Moritz Fischer --- drivers/net/ethernet/dec/tulip/de2104x.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c index 42b798a3fad4..9bcfc82b71d1 100644 --- a/drivers/net/ethernet/dec/tulip/de2104x.c +++ b/drivers/net/ethernet/dec/tulip/de2104x.c @@ -1986,7 +1986,7 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) #endif /* allocate a new ethernet device structure, and fill in defaults */ - dev = alloc_etherdev(sizeof(struct de_private)); + dev = devm_alloc_etherdev(&pdev->dev, sizeof(struct de_private)); if (!dev) return -ENOMEM; @@ -2011,7 +2011,7 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) /* wake up device, assign resources */ rc = pci_enable_device(pdev); if (rc) - goto err_out_free; + return rc; /* reserve PCI resources to ensure driver atomicity */ rc = pci_request_regions(pdev, DRV_NAME); @@ -2098,8 +2098,6 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) pci_release_regions(pdev); err_out_disable: pci_disable_device(pdev); -err_out_free: - free_netdev(dev); return rc; } @@ -2114,7 +2112,6 @@ static void de_remove_one(struct pci_dev *pdev) iounmap(de->regs); pci_release_regions(pdev); pci_disable_device(pdev); - free_netdev(dev); } #ifdef CONFIG_PM -- 2.28.0
[PATCH net-next 3/3] net: dec: tulip: de2104x: Replace kmemdup() with devm_kmempdup()
Replace an instance of kmemdup() with the devres counted version instead. Signed-off-by: Moritz Fischer --- drivers/net/ethernet/dec/tulip/de2104x.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c index e4189c45c2ba..4933799c6a15 100644 --- a/drivers/net/ethernet/dec/tulip/de2104x.c +++ b/drivers/net/ethernet/dec/tulip/de2104x.c @@ -1940,7 +1940,8 @@ static void de21041_get_srom_info(struct de_private *de) de->media[i].csr15 = t21041_csr15[i]; } - de->ee_data = kmemdup(&ee_data[0], DE_EEPROM_SIZE, GFP_KERNEL); + de->ee_data = devm_kmemdup(&de->pdev->dev, &ee_data[0], DE_EEPROM_SIZE, + GFP_KERNEL); return; @@ -2092,7 +2093,6 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) return 0; err_out_iomap: - kfree(de->ee_data); iounmap(regs); err_out_res: pci_release_regions(pdev); @@ -2106,7 +2106,6 @@ static void de_remove_one(struct pci_dev *pdev) BUG_ON(!dev); unregister_netdev(dev); - kfree(de->ee_data); iounmap(de->regs); pci_release_regions(pdev); } -- 2.28.0
[PATCH net-next 2/3] net: dec: tulip: de2104x: Replace pci_enable_device with devres version
Replace pci_enable_device() with its devres counterpart pcim_enable_device(). Signed-off-by: Moritz Fischer --- drivers/net/ethernet/dec/tulip/de2104x.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c index 9bcfc82b71d1..e4189c45c2ba 100644 --- a/drivers/net/ethernet/dec/tulip/de2104x.c +++ b/drivers/net/ethernet/dec/tulip/de2104x.c @@ -2009,14 +2009,14 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) netif_carrier_off(dev); /* wake up device, assign resources */ - rc = pci_enable_device(pdev); + rc = pcim_enable_device(pdev); if (rc) return rc; /* reserve PCI resources to ensure driver atomicity */ rc = pci_request_regions(pdev, DRV_NAME); if (rc) - goto err_out_disable; + return rc; /* check for invalid IRQ value */ if (pdev->irq < 2) { @@ -2096,8 +2096,6 @@ static int de_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) iounmap(regs); err_out_res: pci_release_regions(pdev); -err_out_disable: - pci_disable_device(pdev); return rc; } @@ -2111,7 +2109,6 @@ static void de_remove_one(struct pci_dev *pdev) kfree(de->ee_data); iounmap(de->regs); pci_release_regions(pdev); - pci_disable_device(pdev); } #ifdef CONFIG_PM -- 2.28.0
[PATCH net-next 0/3] First bunch of cleanups
This series is the first bunch of minor cleanups for the de2104x driver to make it look and behave more like a modern driver. These changes replace some of the non-devres versions with devres versions of functions to simplify the error paths. Next up after this will be the ioremap part. Moritz Fischer (3): net: dec: tulip: de2104x: Replace alloc_etherdev by devm_alloc_etherdev net: dec: tulip: de2104x: Replace pci_enable_device with devres version net: dec: tulip: de2104x: Replace kmemdup() with devm_kmempdup() drivers/net/ethernet/dec/tulip/de2104x.c | 19 ++- 1 file changed, 6 insertions(+), 13 deletions(-) -- 2.28.0
Re: [PATCH] tulip: de2104x: switch from 'pci_' to 'dma_' API
On Sun, Sep 13, 2020 at 02:44:53PM +0200, Christophe JAILLET wrote: > The wrappers in include/linux/pci-dma-compat.h should go away. > > The patch has been generated with the coccinelle script below and has been > hand modified to replace GFP_ with a correct flag. > It has been compile tested. > > When memory is allocated in 'de_alloc_rings()' GFP_KERNEL can be used > because it is only called from 'de_open()' which is a '.ndo_open' > function. Such functions are synchronized using the rtnl_lock() semaphore > and no lock is taken in the between. > > > @@ > @@ > -PCI_DMA_BIDIRECTIONAL > +DMA_BIDIRECTIONAL > > @@ > @@ > -PCI_DMA_TODEVICE > +DMA_TO_DEVICE > > @@ > @@ > -PCI_DMA_FROMDEVICE > +DMA_FROM_DEVICE > > @@ > @@ > -PCI_DMA_NONE > +DMA_NONE > > @@ > expression e1, e2, e3; > @@ > -pci_alloc_consistent(e1, e2, e3) > +dma_alloc_coherent(&e1->dev, e2, e3, GFP_) > > @@ > expression e1, e2, e3; > @@ > -pci_zalloc_consistent(e1, e2, e3) > +dma_alloc_coherent(&e1->dev, e2, e3, GFP_) > > @@ > expression e1, e2, e3, e4; > @@ > -pci_free_consistent(e1, e2, e3, e4) > +dma_free_coherent(&e1->dev, e2, e3, e4) > > @@ > expression e1, e2, e3, e4; > @@ > -pci_map_single(e1, e2, e3, e4) > +dma_map_single(&e1->dev, e2, e3, e4) > > @@ > expression e1, e2, e3, e4; > @@ > -pci_unmap_single(e1, e2, e3, e4) > +dma_unmap_single(&e1->dev, e2, e3, e4) > > @@ > expression e1, e2, e3, e4, e5; > @@ > -pci_map_page(e1, e2, e3, e4, e5) > +dma_map_page(&e1->dev, e2, e3, e4, e5) > > @@ > expression e1, e2, e3, e4; > @@ > -pci_unmap_page(e1, e2, e3, e4) > +dma_unmap_page(&e1->dev, e2, e3, e4) > > @@ > expression e1, e2, e3, e4; > @@ > -pci_map_sg(e1, e2, e3, e4) > +dma_map_sg(&e1->dev, e2, e3, e4) > > @@ > expression e1, e2, e3, e4; > @@ > -pci_unmap_sg(e1, e2, e3, e4) > +dma_unmap_sg(&e1->dev, e2, e3, e4) > > @@ > expression e1, e2, e3, e4; > @@ > -pci_dma_sync_single_for_cpu(e1, e2, e3, e4) > +dma_sync_single_for_cpu(&e1->dev, e2, e3, e4) > > @@ > expression e1, e2, e3, e4; > @@ > -pci_dma_sync_single_for_device(e1, e2, e3, e4) > +dma_sync_single_for_device(&e1->dev, e2, e3, e4) > > @@ > expression e1, e2, e3, e4; > @@ > -pci_dma_sync_sg_for_cpu(e1, e2, e3, e4) > +dma_sync_sg_for_cpu(&e1->dev, e2, e3, e4) > > @@ > expression e1, e2, e3, e4; > @@ > -pci_dma_sync_sg_for_device(e1, e2, e3, e4) > + dma_sync_sg_for_device(&e1->dev, e2, e3, e4) > > @@ > expression e1, e2; > @@ > -pci_dma_mapping_error(e1, e2) > +dma_mapping_error(&e1->dev, e2) > > @@ > expression e1, e2; > @@ > -pci_set_dma_mask(e1, e2) > +dma_set_mask(&e1->dev, e2) > > @@ > expression e1, e2; > @@ > -pci_set_consistent_dma_mask(e1, e2) > +dma_set_coherent_mask(&e1->dev, e2) > > Signed-off-by: Christophe JAILLET Acked-by: Moritz Fischer > --- > If needed, see post from Christoph Hellwig on the kernel-janitors ML: >https://marc.info/?l=kernel-janitors&m=158745678307186&w=4 > --- > drivers/net/ethernet/dec/tulip/de2104x.c | 62 ++-- > 1 file changed, 36 insertions(+), 26 deletions(-) > > diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c > b/drivers/net/ethernet/dec/tulip/de2104x.c > index cb116b530f5e..0931881403b6 100644 > --- a/drivers/net/ethernet/dec/tulip/de2104x.c > +++ b/drivers/net/ethernet/dec/tulip/de2104x.c > @@ -443,21 +443,23 @@ static void de_rx (struct de_private *de) > } > > if (!copying_skb) { > - pci_unmap_single(de->pdev, mapping, > - buflen, PCI_DMA_FROMDEVICE); > + dma_unmap_single(&de->pdev->dev, mapping, buflen, > + DMA_FROM_DEVICE); > skb_put(skb, len); > > mapping = > de->rx_skb[rx_tail].mapping = > - pci_map_single(de->pdev, copy_skb->data, > -buflen, PCI_DMA_FROMDEVICE); > + dma_map_single(&de->pdev->dev, copy_skb->data, > +buflen, DMA_FROM_DEVICE); > de->rx_skb[rx
Re: [PATCH net] net: dec: de2104x: Increase receive ring size for Tulip
On Thu, Sep 10, 2020 at 12:05:09PM -0700, Lucy Yan wrote: > Increase Rx ring size to address issue where hardware is reaching > the receive work limit. > > Before: > > [ 102.223342] de2104x :17:00.0 eth0: rx work limit reached > [ 102.245695] de2104x :17:00.0 eth0: rx work limit reached > [ 102.251387] de2104x :17:00.0 eth0: rx work limit reached > [ 102.267444] de2104x :17:00.0 eth0: rx work limit reached > > Signed-off-by: Lucy Yan Reviewed-by: Moritz Fischer > --- > drivers/net/ethernet/dec/tulip/de2104x.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c > b/drivers/net/ethernet/dec/tulip/de2104x.c > index cb116b530f5e..2610efe4f873 100644 > --- a/drivers/net/ethernet/dec/tulip/de2104x.c > +++ b/drivers/net/ethernet/dec/tulip/de2104x.c > @@ -85,7 +85,7 @@ MODULE_PARM_DESC (rx_copybreak, "de2104x Breakpoint at > which Rx packets are copi > #define DSL CONFIG_DE2104X_DSL > #endif > > -#define DE_RX_RING_SIZE 64 > +#define DE_RX_RING_SIZE 128 > #define DE_TX_RING_SIZE 64 > #define DE_RING_BYTES\ > ((sizeof(struct de_desc) * DE_RX_RING_SIZE) + \ > -- > 2.28.0.526.ge36021eeef-goog > Thanks, Moritz
Re: [PATCH V1 net 2/2] net: ena: update driver version from 2.0.2 to 2.0.3
Hi Arthur, On Mon, Feb 11, 2019 at 9:19 AM wrote: > > From: Arthur Kiyanovski > > Update driver version due to bug fix. Wouldn't you want to do this atomically with the actual fix in one commit? Thanks, Moritz
Re: [PATCH v2 net-next] net: fixed-phy: Add fixed_phy_register_with_gpiod() API
Hi David, On Thu, Feb 7, 2019 at 6:15 PM David Miller wrote: > > From: Moritz Fischer > Date: Thu, 7 Feb 2019 12:14:55 -0800 > > > Add fixed_phy_register_with_gpiod() API. It lets users create a > > fixed_phy instance that uses a GPIO descriptor which was obtained > > externally e.g. through platform data. > > This enables platform devices (non-DT based) to use GPIOs for link > > status. > > > > Reviewed-by: Florian Fainelli > > Signed-off-by: Moritz Fischer > > This doesn't apply cleanly to net-next, please respin. I think 5468e82f7034f ("net: phy: fixed-phy: Drop GPIO from fixed_phy_add()") might've been missing when you tried. I just tried with net-next and next, and it applied fine. It also seems to be in net next as 71bd106d25676 ("net: fixed-phy: Add fixed_phy_register_with_gpiod() API"), do you still want me to resend? Sorry for the confusion, Moritz
[PATCH v2 net-next] net: fixed-phy: Add fixed_phy_register_with_gpiod() API
Add fixed_phy_register_with_gpiod() API. It lets users create a fixed_phy instance that uses a GPIO descriptor which was obtained externally e.g. through platform data. This enables platform devices (non-DT based) to use GPIOs for link status. Reviewed-by: Florian Fainelli Signed-off-by: Moritz Fischer --- Changes from v1: - Added Florian's Reviewed-by: tag Changes from RFC: - Implemented Andrew's/Florians suggestion to add new API instead of changing old API --- drivers/net/phy/fixed_phy.c | 32 +--- include/linux/phy_fixed.h | 15 +++ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c index d810f914aaa4..b0d1368c3400 100644 --- a/drivers/net/phy/fixed_phy.c +++ b/drivers/net/phy/fixed_phy.c @@ -229,12 +229,12 @@ static struct gpio_desc *fixed_phy_get_gpiod(struct device_node *np) } #endif -struct phy_device *fixed_phy_register(unsigned int irq, - struct fixed_phy_status *status, - struct device_node *np) +static struct phy_device *__fixed_phy_register(unsigned int irq, + struct fixed_phy_status *status, + struct device_node *np, + struct gpio_desc *gpiod) { struct fixed_mdio_bus *fmb = &platform_fmb; - struct gpio_desc *gpiod = NULL; struct phy_device *phy; int phy_addr; int ret; @@ -243,9 +243,11 @@ struct phy_device *fixed_phy_register(unsigned int irq, return ERR_PTR(-EPROBE_DEFER); /* Check if we have a GPIO associated with this fixed phy */ - gpiod = fixed_phy_get_gpiod(np); - if (IS_ERR(gpiod)) - return ERR_CAST(gpiod); + if (!gpiod) { + gpiod = fixed_phy_get_gpiod(np); + if (IS_ERR(gpiod)) + return ERR_CAST(gpiod); + } /* Get the next available PHY address, up to PHY_MAX_ADDR */ phy_addr = ida_simple_get(&phy_fixed_ida, 0, PHY_MAX_ADDR, GFP_KERNEL); @@ -308,8 +310,24 @@ struct phy_device *fixed_phy_register(unsigned int irq, return phy; } + +struct phy_device *fixed_phy_register(unsigned int irq, + struct fixed_phy_status *status, + struct device_node *np) +{ + return __fixed_phy_register(irq, status, np, NULL); +} EXPORT_SYMBOL_GPL(fixed_phy_register); +struct phy_device * +fixed_phy_register_with_gpiod(unsigned int irq, + struct fixed_phy_status *status, + struct gpio_desc *gpiod) +{ + return __fixed_phy_register(irq, status, NULL, gpiod); +} +EXPORT_SYMBOL_GPL(fixed_phy_register_with_gpiod); + void fixed_phy_unregister(struct phy_device *phy) { phy_device_remove(phy); diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h index c78fc203db43..1e5d86ebdaeb 100644 --- a/include/linux/phy_fixed.h +++ b/include/linux/phy_fixed.h @@ -19,6 +19,12 @@ extern int fixed_phy_add(unsigned int irq, int phy_id, extern struct phy_device *fixed_phy_register(unsigned int irq, struct fixed_phy_status *status, struct device_node *np); + +extern struct phy_device * +fixed_phy_register_with_gpiod(unsigned int irq, + struct fixed_phy_status *status, + struct gpio_desc *gpiod); + extern void fixed_phy_unregister(struct phy_device *phydev); extern int fixed_phy_set_link_update(struct phy_device *phydev, int (*link_update)(struct net_device *, @@ -35,6 +41,15 @@ static inline struct phy_device *fixed_phy_register(unsigned int irq, { return ERR_PTR(-ENODEV); } + +static inline struct phy_device * +fixed_phy_register_with_gpiod(unsigned int irq, + struct fixed_phy_status *status, + struct gpio_desc *gpiod) +{ + return ERR_PTR(-ENODEV); +} + static inline void fixed_phy_unregister(struct phy_device *phydev) { } -- 2.20.1
[PATCH net-next] net: fixed-phy: Add fixed_phy_register_with_gpiod() API
Add fixed_phy_register_with_gpiod() API. It lets users create a fixed_phy instance that uses a GPIO descriptor which was obtained externally e.g. through platform data. This enables platform devices (non-DT based) to use GPIOs for link status. Signed-off-by: Moritz Fischer --- Changes from RFC: - Implemented Andrew's/Florians suggestion to add new API instead of changing old API --- drivers/net/phy/fixed_phy.c | 32 +--- include/linux/phy_fixed.h | 15 +++ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c index d810f914aaa4..b0d1368c3400 100644 --- a/drivers/net/phy/fixed_phy.c +++ b/drivers/net/phy/fixed_phy.c @@ -229,12 +229,12 @@ static struct gpio_desc *fixed_phy_get_gpiod(struct device_node *np) } #endif -struct phy_device *fixed_phy_register(unsigned int irq, - struct fixed_phy_status *status, - struct device_node *np) +static struct phy_device *__fixed_phy_register(unsigned int irq, + struct fixed_phy_status *status, + struct device_node *np, + struct gpio_desc *gpiod) { struct fixed_mdio_bus *fmb = &platform_fmb; - struct gpio_desc *gpiod = NULL; struct phy_device *phy; int phy_addr; int ret; @@ -243,9 +243,11 @@ struct phy_device *fixed_phy_register(unsigned int irq, return ERR_PTR(-EPROBE_DEFER); /* Check if we have a GPIO associated with this fixed phy */ - gpiod = fixed_phy_get_gpiod(np); - if (IS_ERR(gpiod)) - return ERR_CAST(gpiod); + if (!gpiod) { + gpiod = fixed_phy_get_gpiod(np); + if (IS_ERR(gpiod)) + return ERR_CAST(gpiod); + } /* Get the next available PHY address, up to PHY_MAX_ADDR */ phy_addr = ida_simple_get(&phy_fixed_ida, 0, PHY_MAX_ADDR, GFP_KERNEL); @@ -308,8 +310,24 @@ struct phy_device *fixed_phy_register(unsigned int irq, return phy; } + +struct phy_device *fixed_phy_register(unsigned int irq, + struct fixed_phy_status *status, + struct device_node *np) +{ + return __fixed_phy_register(irq, status, np, NULL); +} EXPORT_SYMBOL_GPL(fixed_phy_register); +struct phy_device * +fixed_phy_register_with_gpiod(unsigned int irq, + struct fixed_phy_status *status, + struct gpio_desc *gpiod) +{ + return __fixed_phy_register(irq, status, NULL, gpiod); +} +EXPORT_SYMBOL_GPL(fixed_phy_register_with_gpiod); + void fixed_phy_unregister(struct phy_device *phy) { phy_device_remove(phy); diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h index c78fc203db43..1e5d86ebdaeb 100644 --- a/include/linux/phy_fixed.h +++ b/include/linux/phy_fixed.h @@ -19,6 +19,12 @@ extern int fixed_phy_add(unsigned int irq, int phy_id, extern struct phy_device *fixed_phy_register(unsigned int irq, struct fixed_phy_status *status, struct device_node *np); + +extern struct phy_device * +fixed_phy_register_with_gpiod(unsigned int irq, + struct fixed_phy_status *status, + struct gpio_desc *gpiod); + extern void fixed_phy_unregister(struct phy_device *phydev); extern int fixed_phy_set_link_update(struct phy_device *phydev, int (*link_update)(struct net_device *, @@ -35,6 +41,15 @@ static inline struct phy_device *fixed_phy_register(unsigned int irq, { return ERR_PTR(-ENODEV); } + +static inline struct phy_device * +fixed_phy_register_with_gpiod(unsigned int irq, + struct fixed_phy_status *status, + struct gpio_desc *gpiod) +{ + return ERR_PTR(-ENODEV); +} + static inline void fixed_phy_unregister(struct phy_device *phydev) { } -- 2.20.1
[PATCH v2 net-next] net: phy: fixed_phy: Fix fixed_phy not checking GPIO
Fix fixed_phy not checking GPIO if no link_update callback is registered. In the original version all users registered a link_update callback so the issue was masked. Fixes: a5597008dbc2 ("phy: fixed_phy: Add gpio to determine link up/down.") Reviewed-by: Andrew Lunn Signed-off-by: Moritz Fischer --- Changes from v1: - Added Andrew's Reviewed-by: tag - Added Fixes: tag (Thanks for digging, Andrew!) --- drivers/net/phy/fixed_phy.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c index f136a23c1a35..d810f914aaa4 100644 --- a/drivers/net/phy/fixed_phy.c +++ b/drivers/net/phy/fixed_phy.c @@ -85,11 +85,11 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num) s = read_seqcount_begin(&fp->seqcount); fp->status.link = !fp->no_carrier; /* Issue callback if user registered it. */ - if (fp->link_update) { + if (fp->link_update) fp->link_update(fp->phydev->attached_dev, &fp->status); - fixed_phy_update(fp); - } + /* Check the GPIO for change in status */ + fixed_phy_update(fp); state = fp->status; } while (read_seqcount_retry(&fp->seqcount, s)); -- 2.20.1
Re: [RFC net-next] net: fixed_phy: Move the DT based link GPIO parsing to of_mdio.c
Hi Andrew, thanks for your feedback. On Wed, Feb 06, 2019 at 10:53:22PM +0100, Andrew Lunn wrote: > On Wed, Feb 06, 2019 at 12:51:06PM -0800, Moritz Fischer wrote: > > Move the DT based link GPIO parsing to of_mdio and let the places > > that register a fixed_phy pass in a GPIO descriptor or NULL. > > > > This allows fixed_phy on non-DT platforms to have link GPIOs, too. > > > > Signed-off-by: Moritz Fischer > > --- > > drivers/net/dsa/dsa_loop.c | 2 +- > > drivers/net/ethernet/broadcom/bgmac.c| 2 +- > > drivers/net/ethernet/broadcom/genet/bcmmii.c | 2 +- > > drivers/net/phy/fixed_phy.c | 48 ++-- > > drivers/net/usb/lan78xx.c| 2 +- > > drivers/of/of_mdio.c | 15 +- > > include/linux/phy_fixed.h| 3 +- > > 7 files changed, 23 insertions(+), 51 deletions(-) > > > > diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c > > index 17482ae09aa5..7f124c620092 100644 > > --- a/drivers/net/dsa/dsa_loop.c > > +++ b/drivers/net/dsa/dsa_loop.c > > @@ -343,7 +343,7 @@ static int __init dsa_loop_init(void) > > unsigned int i; > > > > for (i = 0; i < NUM_FIXED_PHYS; i++) > > - phydevs[i] = fixed_phy_register(PHY_POLL, &status, NULL); > > + phydevs[i] = fixed_phy_register(PHY_POLL, &status, NULL, NULL); > > > > return mdio_driver_register(&dsa_loop_drv); > > } > > diff --git a/drivers/net/ethernet/broadcom/bgmac.c > > b/drivers/net/ethernet/broadcom/bgmac.c > > index 4632dd5dbad1..bce644dec5c2 100644 > > --- a/drivers/net/ethernet/broadcom/bgmac.c > > +++ b/drivers/net/ethernet/broadcom/bgmac.c > > @@ -1446,7 +1446,7 @@ int bgmac_phy_connect_direct(struct bgmac *bgmac) > > struct phy_device *phy_dev; > > int err; > > > > - phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, NULL); > > + phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, NULL, NULL); > > if (!phy_dev || IS_ERR(phy_dev)) { > > dev_err(bgmac->dev, "Failed to register fixed PHY device\n"); > > return -ENODEV; > > diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c > > b/drivers/net/ethernet/broadcom/genet/bcmmii.c > > index 51880d83131a..7cbd737aba80 100644 > > --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c > > +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c > > @@ -525,7 +525,7 @@ static int bcmgenet_mii_pd_init(struct bcmgenet_priv > > *priv) > > .asym_pause = 0, > > }; > > > > - phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL); > > + phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL, NULL); > > if (!phydev || IS_ERR(phydev)) { > > dev_err(kdev, "failed to register fixed PHY device\n"); > > return -ENODEV; > > diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c > > index d810f914aaa4..845bd7c2065a 100644 > > --- a/drivers/net/phy/fixed_phy.c > > +++ b/drivers/net/phy/fixed_phy.c > > @@ -18,6 +18,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -191,50 +192,12 @@ static void fixed_phy_del(int phy_addr) > > } > > } > > > > -#ifdef CONFIG_OF_GPIO > > -static struct gpio_desc *fixed_phy_get_gpiod(struct device_node *np) > > -{ > > - struct device_node *fixed_link_node; > > - struct gpio_desc *gpiod; > > - > > - if (!np) > > - return NULL; > > - > > - fixed_link_node = of_get_child_by_name(np, "fixed-link"); > > - if (!fixed_link_node) > > - return NULL; > > - > > - /* > > -* As the fixed link is just a device tree node without any > > -* Linux device associated with it, we simply have obtain > > -* the GPIO descriptor from the device tree like this. > > -*/ > > - gpiod = gpiod_get_from_of_node(fixed_link_node, "link-gpios", 0, > > - GPIOD_IN, "mdio"); > > - of_node_put(fixed_link_node); > > - if (IS_ERR(gpiod)) { > > - if (PTR_ERR(gpiod) == -EPROBE_DEFER) > > - return gpiod; > > - pr_err("error getting GPIO for fixed link %pOF, proceed > > without\n", > > -
Re: [PATCH net-next] net: phy: fixed_phy: Fix fixed_phy not checking GPIO
Hi Andrew, On Wed, Feb 06, 2019 at 10:59:05PM +0100, Andrew Lunn wrote: > On Wed, Feb 06, 2019 at 10:10:40AM -0800, Moritz Fischer wrote: > > Fix fixed_phy not checking GPIO if no link_update callback > > is registered. > > > > Signed-off-by: Moritz Fischer > > --- > > > > Hi all, > > > > I've been trying to figure out where exactly this broke, > > it must've been somewhere when the file was refactored > > in connection with phylink? > > Hi Moritz > > With a quick inspection, i also cannot see where it broken. > > I think part of the issue is that all the current users have moved > onto using phylink, and phylink polls the GPIO, rather than having > fixed_link do it. Yeah, I suspected at much :) I still feel we should fix fixed_phy as long as there are still users for it. > > I would prefer to understand exactly which change broke it. Without > knowing how it broke, it is hard to say if this is the correct fix. It might've been always this way. That being said I don't see why one should've to implement an empty function (link_update) in my driver just to read back the GPIO pin. Looking at the code it seems clear that nothing else polls the GPIO, which doesn't seem right. In my current understanding (correct me if I'm wrong), the link_update callback would give the MAC a chance to update link parameters that since we are a fixed phy we cannot read back from a PHY. That seems conceptually independent from obtaining a link up/down info from a GPIO pin. Wouldn't you agree? > > What is your use-case? You Cc: the usb list. So a USB-Ethernet dongle? > But then why fixed-link? Not for this patch, did I ? I ran into this when testing nixge, but it seemed unrelated with my changes. Thanks, Moritz
[RFC net-next] net: fixed_phy: Move the DT based link GPIO parsing to of_mdio.c
Move the DT based link GPIO parsing to of_mdio and let the places that register a fixed_phy pass in a GPIO descriptor or NULL. This allows fixed_phy on non-DT platforms to have link GPIOs, too. Signed-off-by: Moritz Fischer --- drivers/net/dsa/dsa_loop.c | 2 +- drivers/net/ethernet/broadcom/bgmac.c| 2 +- drivers/net/ethernet/broadcom/genet/bcmmii.c | 2 +- drivers/net/phy/fixed_phy.c | 48 ++-- drivers/net/usb/lan78xx.c| 2 +- drivers/of/of_mdio.c | 15 +- include/linux/phy_fixed.h| 3 +- 7 files changed, 23 insertions(+), 51 deletions(-) diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c index 17482ae09aa5..7f124c620092 100644 --- a/drivers/net/dsa/dsa_loop.c +++ b/drivers/net/dsa/dsa_loop.c @@ -343,7 +343,7 @@ static int __init dsa_loop_init(void) unsigned int i; for (i = 0; i < NUM_FIXED_PHYS; i++) - phydevs[i] = fixed_phy_register(PHY_POLL, &status, NULL); + phydevs[i] = fixed_phy_register(PHY_POLL, &status, NULL, NULL); return mdio_driver_register(&dsa_loop_drv); } diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c index 4632dd5dbad1..bce644dec5c2 100644 --- a/drivers/net/ethernet/broadcom/bgmac.c +++ b/drivers/net/ethernet/broadcom/bgmac.c @@ -1446,7 +1446,7 @@ int bgmac_phy_connect_direct(struct bgmac *bgmac) struct phy_device *phy_dev; int err; - phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, NULL); + phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, NULL, NULL); if (!phy_dev || IS_ERR(phy_dev)) { dev_err(bgmac->dev, "Failed to register fixed PHY device\n"); return -ENODEV; diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c index 51880d83131a..7cbd737aba80 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c @@ -525,7 +525,7 @@ static int bcmgenet_mii_pd_init(struct bcmgenet_priv *priv) .asym_pause = 0, }; - phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL); + phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL, NULL); if (!phydev || IS_ERR(phydev)) { dev_err(kdev, "failed to register fixed PHY device\n"); return -ENODEV; diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c index d810f914aaa4..845bd7c2065a 100644 --- a/drivers/net/phy/fixed_phy.c +++ b/drivers/net/phy/fixed_phy.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -191,50 +192,12 @@ static void fixed_phy_del(int phy_addr) } } -#ifdef CONFIG_OF_GPIO -static struct gpio_desc *fixed_phy_get_gpiod(struct device_node *np) -{ - struct device_node *fixed_link_node; - struct gpio_desc *gpiod; - - if (!np) - return NULL; - - fixed_link_node = of_get_child_by_name(np, "fixed-link"); - if (!fixed_link_node) - return NULL; - - /* -* As the fixed link is just a device tree node without any -* Linux device associated with it, we simply have obtain -* the GPIO descriptor from the device tree like this. -*/ - gpiod = gpiod_get_from_of_node(fixed_link_node, "link-gpios", 0, - GPIOD_IN, "mdio"); - of_node_put(fixed_link_node); - if (IS_ERR(gpiod)) { - if (PTR_ERR(gpiod) == -EPROBE_DEFER) - return gpiod; - pr_err("error getting GPIO for fixed link %pOF, proceed without\n", - fixed_link_node); - gpiod = NULL; - } - - return gpiod; -} -#else -static struct gpio_desc *fixed_phy_get_gpiod(struct device_node *np) -{ - return NULL; -} -#endif - struct phy_device *fixed_phy_register(unsigned int irq, struct fixed_phy_status *status, - struct device_node *np) + struct device_node *np, + struct gpio_desc *gpiod) { struct fixed_mdio_bus *fmb = &platform_fmb; - struct gpio_desc *gpiod = NULL; struct phy_device *phy; int phy_addr; int ret; @@ -242,11 +205,6 @@ struct phy_device *fixed_phy_register(unsigned int irq, if (!fmb->mii_bus || fmb->mii_bus->state != MDIOBUS_REGISTERED) return ERR_PTR(-EPROBE_DEFER); - /* Check if we have a GPIO associated with this fixed phy */ - gpiod = fixed_phy_get_gpiod(np); - if (IS_ERR(gpiod))
[PATCH net-next] net: phy: fixed_phy: Fix fixed_phy not checking GPIO
Fix fixed_phy not checking GPIO if no link_update callback is registered. Signed-off-by: Moritz Fischer --- Hi all, I've been trying to figure out where exactly this broke, it must've been somewhere when the file was refactored in connection with phylink? Unfortunately I couldn't tell exactly so I don't have a 'Fixes' tag. Should this also go be queued up for stable/5.0 if it is indeed a bug? Thanks, Moritz --- drivers/net/phy/fixed_phy.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c index f136a23c1a35..d810f914aaa4 100644 --- a/drivers/net/phy/fixed_phy.c +++ b/drivers/net/phy/fixed_phy.c @@ -85,11 +85,11 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num) s = read_seqcount_begin(&fp->seqcount); fp->status.link = !fp->no_carrier; /* Issue callback if user registered it. */ - if (fp->link_update) { + if (fp->link_update) fp->link_update(fp->phydev->attached_dev, &fp->status); - fixed_phy_update(fp); - } + /* Check the GPIO for change in status */ + fixed_phy_update(fp); state = fp->status; } while (read_seqcount_retry(&fp->seqcount, s)); -- 2.20.1
[PATCH net-next 1/3] net: nixge: Make mdio child node optional
From: Moritz Fischer Make MDIO child optional and only instantiate the MDIO bus if the child is actually present. There are currently no (in-tree) users of this binding; all (out-of-tree) users use overlays that get shipped together with the FPGA images that contain the IP. This will significantly increase maintainabilty of future revisions of this IP. Reviewed-by: Andrew Lunn Signed-off-by: Moritz Fischer --- Changes from RFC: - Add Andrew's Reviewed-by tag - Rebased on top of net-next --- .../devicetree/bindings/net/nixge.txt | 27 --- drivers/net/ethernet/ni/nixge.c | 19 - 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/Documentation/devicetree/bindings/net/nixge.txt b/Documentation/devicetree/bindings/net/nixge.txt index 44a7358b4399..bb2929f9c64f 100644 --- a/Documentation/devicetree/bindings/net/nixge.txt +++ b/Documentation/devicetree/bindings/net/nixge.txt @@ -16,6 +16,9 @@ Required properties: - nvmem-cells: Phandle of nvmem cell containing the MAC address - nvmem-cell-names: Should be "address" +Optional properties: +- mdio subnode to indicate presence of MDIO controller + Examples (10G generic PHY): nixge0: ethernet@4000 { compatible = "ni,xge-enet-3.00"; @@ -33,8 +36,26 @@ Examples (10G generic PHY): phy-mode = "xgmii"; phy-handle = <ðernet_phy1>; - ethernet_phy1: ethernet-phy@4 { - compatible = "ethernet-phy-ieee802.3-c45"; - reg = <4>; + mdio { + ethernet_phy1: ethernet-phy@4 { + compatible = "ethernet-phy-ieee802.3-c45"; + reg = <4>; + }; }; }; + +Examples (10G generic PHY, no MDIO): + nixge0: ethernet@4000 { + compatible = "ni,xge-enet-2.00"; + reg = <0x4000 0x6000>; + + nvmem-cells = <ð1_addr>; + nvmem-cell-names = "address"; + + interrupts = <0 29 IRQ_TYPE_LEVEL_HIGH>, <0 30 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "rx", "tx"; + interrupt-parent = <&intc>; + + phy-mode = "xgmii"; + phy-handle = <ðernet_phy1>; + }; diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c index 73a98bd2fcd2..c8dd1e4c759d 100644 --- a/drivers/net/ethernet/ni/nixge.c +++ b/drivers/net/ethernet/ni/nixge.c @@ -1284,6 +1284,7 @@ static int nixge_probe(struct platform_device *pdev) { struct nixge_priv *priv; struct net_device *ndev; + struct device_node *mn; const u8 *mac_addr; int err; @@ -1335,10 +1336,14 @@ static int nixge_probe(struct platform_device *pdev) priv->coalesce_count_rx = XAXIDMA_DFT_RX_THRESHOLD; priv->coalesce_count_tx = XAXIDMA_DFT_TX_THRESHOLD; - err = nixge_mdio_setup(priv, pdev->dev.of_node); - if (err) { - netdev_err(ndev, "error registering mdio bus"); - goto free_netdev; + mn = of_get_child_by_name(pdev->dev.of_node, "mdio"); + if (mn) { + err = nixge_mdio_setup(priv, mn); + of_node_put(mn); + if (err) { + netdev_err(ndev, "error registering mdio bus"); + goto free_netdev; + } } priv->phy_mode = of_get_phy_mode(pdev->dev.of_node); @@ -1364,7 +1369,8 @@ static int nixge_probe(struct platform_device *pdev) return 0; unregister_mdio: - mdiobus_unregister(priv->mii_bus); + if (priv->mii_bus) + mdiobus_unregister(priv->mii_bus); free_netdev: free_netdev(ndev); @@ -1379,7 +1385,8 @@ static int nixge_remove(struct platform_device *pdev) unregister_netdev(ndev); - mdiobus_unregister(priv->mii_bus); + if (priv->mii_bus) + mdiobus_unregister(priv->mii_bus); free_netdev(ndev); -- 2.20.1
[PATCH net-next 0/3] nixge: Fixed-link support
From: Moritz Fischer This series adds fixed-link support to nixge. The first patch corrects the binding to correctly reflect hardware that does not come with MDIO cores instantiated. The second patch adds fixed link support to the driver. The third patch updates the binding document with the now optional (formerly required) phy-handle property and references the fixed-link docs. Thanks, Moritz Moritz Fischer (3): net: nixge: Make mdio child node optional net: nixge: Add support for fixed-link configurations dt-bindings: net: Add fixed-link support .../devicetree/bindings/net/nixge.txt | 60 +-- drivers/net/ethernet/ni/nixge.c | 44 ++ 2 files changed, 88 insertions(+), 16 deletions(-) -- 2.20.1
[PATCH net-next 2/3] net: nixge: Add support for fixed-link configurations
From: Moritz Fischer Add support for fixed-link configurations to nixge driver. Signed-off-by: Moritz Fischer --- drivers/net/ethernet/ni/nixge.c | 27 --- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c index c8dd1e4c759d..96f7a9818294 100644 --- a/drivers/net/ethernet/ni/nixge.c +++ b/drivers/net/ethernet/ni/nixge.c @@ -1282,9 +1282,9 @@ static int nixge_of_get_resources(struct platform_device *pdev) static int nixge_probe(struct platform_device *pdev) { + struct device_node *mn, *phy_node; struct nixge_priv *priv; struct net_device *ndev; - struct device_node *mn; const u8 *mac_addr; int err; @@ -1353,21 +1353,30 @@ static int nixge_probe(struct platform_device *pdev) goto unregister_mdio; } - priv->phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0); - if (!priv->phy_node) { - netdev_err(ndev, "not find \"phy-handle\" property\n"); - err = -EINVAL; - goto unregister_mdio; + phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0); + if (!phy_node && of_phy_is_fixed_link(pdev->dev.of_node)) { + err = of_phy_register_fixed_link(pdev->dev.of_node); + if (err < 0) { + netdev_err(ndev, "broken fixed-link specification\n"); + goto unregister_mdio; + } + phy_node = of_node_get(pdev->dev.of_node); } + priv->phy_node = phy_node; err = register_netdev(priv->ndev); if (err) { netdev_err(ndev, "register_netdev() error (%i)\n", err); - goto unregister_mdio; + goto free_phy; } return 0; +free_phy: + if (of_phy_is_fixed_link(pdev->dev.of_node)) + of_phy_deregister_fixed_link(pdev->dev.of_node); + of_node_put(phy_node); + unregister_mdio: if (priv->mii_bus) mdiobus_unregister(priv->mii_bus); @@ -1385,6 +1394,10 @@ static int nixge_remove(struct platform_device *pdev) unregister_netdev(ndev); + if (of_phy_is_fixed_link(pdev->dev.of_node)) + of_phy_deregister_fixed_link(pdev->dev.of_node); + of_node_put(priv->phy_node); + if (priv->mii_bus) mdiobus_unregister(priv->mii_bus); -- 2.20.1
[PATCH net-next 3/3] dt-bindings: net: Add fixed-link support
From: Moritz Fischer Update device-tree binding with fixed-link support. With fixed-link support the formerly required property 'phy-handle' is now optional if 'fixed-link' child is present. Signed-off-by: Moritz Fischer --- .../devicetree/bindings/net/nixge.txt | 33 ++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/net/nixge.txt b/Documentation/devicetree/bindings/net/nixge.txt index bb2929f9c64f..85d7240a9b20 100644 --- a/Documentation/devicetree/bindings/net/nixge.txt +++ b/Documentation/devicetree/bindings/net/nixge.txt @@ -12,12 +12,14 @@ Required properties: - interrupts: Should contain tx and rx interrupt - interrupt-names: Should be "rx" and "tx" - phy-mode: See ethernet.txt file in the same directory. -- phy-handle: See ethernet.txt file in the same directory. - nvmem-cells: Phandle of nvmem cell containing the MAC address - nvmem-cell-names: Should be "address" Optional properties: - mdio subnode to indicate presence of MDIO controller +- fixed-link : Assume a fixed link. See fixed-link.txt in the same directory. + Use instead of phy-handle. +- phy-handle: See ethernet.txt file in the same directory. Examples (10G generic PHY): nixge0: ethernet@4000 { @@ -59,3 +61,32 @@ Examples (10G generic PHY, no MDIO): phy-mode = "xgmii"; phy-handle = <ðernet_phy1>; }; + +Examples (1G generic fixed-link + MDIO): + nixge0: ethernet@4000 { + compatible = "ni,xge-enet-2.00"; + reg = <0x4000 0x6000>; + + nvmem-cells = <ð1_addr>; + nvmem-cell-names = "address"; + + interrupts = <0 29 IRQ_TYPE_LEVEL_HIGH>, <0 30 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "rx", "tx"; + interrupt-parent = <&intc>; + + phy-mode = "xgmii"; + + fixed-link { + speed = <1000>; + pause; + link-gpios = <&gpio0 63 GPIO_ACTIVE_HIGH>; + }; + + mdio { + ethernet_phy1: ethernet-phy@4 { + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <4>; + }; + }; + + }; -- 2.20.1
Re: [net-next RFC/PATCH] net: nixge: Make mdio child node optional
On Mon, Feb 04, 2019 at 03:58:54PM +0100, Andrew Lunn wrote: > On Fri, Feb 01, 2019 at 06:50:48PM -0800, Moritz Fischer wrote: > > Make MDIO child optional and only instantiate the > > MDIO bus if the child is actually present. > > > > There are currently no (in-tree) users of this > > binding; all (out-of-tree) users use overlays that > > get shipped together with the FPGA images that contain > > the IP. > > > > This will significantly increase maintainabilty > > of future revisions of this IP. > > > > Signed-off-by: Moritz Fischer > > Cc: Andrew Lunn > > Cc: Rob Herring > > Reviewed-by: Andrew Lunn Thanks. > > Andrew I'll resubmit as a series together with the fixed-link change. Since Dave has applied Alex' patches I'll need to rebase anyways, otherwise it doesn't apply anymore to net-next. Cheers, Moritz
[net-next RFC/PATCH] net: nixge: Make mdio child node optional
Make MDIO child optional and only instantiate the MDIO bus if the child is actually present. There are currently no (in-tree) users of this binding; all (out-of-tree) users use overlays that get shipped together with the FPGA images that contain the IP. This will significantly increase maintainabilty of future revisions of this IP. Signed-off-by: Moritz Fischer Cc: Andrew Lunn Cc: Rob Herring --- Hi Rob, Andrew, I know generally changing bindings is a no-no. Ultimately I'm working on adding fixed-link support to this driver, during the review of that Andrew suggested to drop the wrong binding if I don't need legacy behavior. In my case I can pretty much guarantee there's no users, even out of tree due to how we ship the software with our devices (overlays & fpga images always go together). I think in the long this change will make the code easier to maintain and allow to easier support the different variants. Thanks, Moritz --- .../devicetree/bindings/net/nixge.txt | 27 --- drivers/net/ethernet/ni/nixge.c | 19 - 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/Documentation/devicetree/bindings/net/nixge.txt b/Documentation/devicetree/bindings/net/nixge.txt index e55af7f0881a..5a0072b69ce7 100644 --- a/Documentation/devicetree/bindings/net/nixge.txt +++ b/Documentation/devicetree/bindings/net/nixge.txt @@ -10,6 +10,9 @@ Required properties: - nvmem-cells: Phandle of nvmem cell containing the MAC address - nvmem-cell-names: Should be "address" +Optional properties: +- mdio subnode to indicate presence of MDIO controller + Examples (10G generic PHY): nixge0: ethernet@4000 { compatible = "ni,xge-enet-2.00"; @@ -25,8 +28,26 @@ Examples (10G generic PHY): phy-mode = "xgmii"; phy-handle = <ðernet_phy1>; - ethernet_phy1: ethernet-phy@4 { - compatible = "ethernet-phy-ieee802.3-c45"; - reg = <4>; + mdio { + ethernet_phy1: ethernet-phy@4 { + compatible = "ethernet-phy-ieee802.3-c45"; + reg = <4>; + }; }; }; + +Examples (10G generic PHY, no MDIO): + nixge0: ethernet@4000 { + compatible = "ni,xge-enet-2.00"; + reg = <0x4000 0x6000>; + + nvmem-cells = <ð1_addr>; + nvmem-cell-names = "address"; + + interrupts = <0 29 IRQ_TYPE_LEVEL_HIGH>, <0 30 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "rx", "tx"; + interrupt-parent = <&intc>; + + phy-mode = "xgmii"; + phy-handle = <ðernet_phy1>; + }; diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c index 1e408d1a9b5f..93d5263cc57e 100644 --- a/drivers/net/ethernet/ni/nixge.c +++ b/drivers/net/ethernet/ni/nixge.c @@ -1230,6 +1230,7 @@ static int nixge_probe(struct platform_device *pdev) struct nixge_priv *priv; struct net_device *ndev; struct resource *dmares; + struct device_node *mn; const u8 *mac_addr; int err; @@ -1286,10 +1287,14 @@ static int nixge_probe(struct platform_device *pdev) priv->coalesce_count_rx = XAXIDMA_DFT_RX_THRESHOLD; priv->coalesce_count_tx = XAXIDMA_DFT_TX_THRESHOLD; - err = nixge_mdio_setup(priv, pdev->dev.of_node); - if (err) { - netdev_err(ndev, "error registering mdio bus"); - goto free_netdev; + mn = of_get_child_by_name(pdev->dev.of_node, "mdio"); + if (mn) { + err = nixge_mdio_setup(priv, mn); + of_node_put(mn); + if (err) { + netdev_err(ndev, "error registering mdio bus"); + goto free_netdev; + } } priv->phy_mode = of_get_phy_mode(pdev->dev.of_node); @@ -1315,7 +1320,8 @@ static int nixge_probe(struct platform_device *pdev) return 0; unregister_mdio: - mdiobus_unregister(priv->mii_bus); + if (priv->mii_bus) + mdiobus_unregister(priv->mii_bus); free_netdev: free_netdev(ndev); @@ -1330,7 +1336,8 @@ static int nixge_remove(struct platform_device *pdev) unregister_netdev(ndev); - mdiobus_unregister(priv->mii_bus); + if (priv->mii_bus) + mdiobus_unregister(priv->mii_bus); free_netdev(ndev); -- 2.20.1
[PATCH net-next] net: nixge: Address compiler warnings when building for i386
Address compiler warning reported by kbuild autobuilders when building for i386 as a result of dma_addr_t size on different architectures. warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] Fixes: 7e8d5755be0e ("net: nixge: Add support for 64-bit platforms") Signed-off-by: Moritz Fischer Cc: Arnd Bergmann --- drivers/net/ethernet/ni/nixge.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c index 74cf52e3fb09..0611f2335b4a 100644 --- a/drivers/net/ethernet/ni/nixge.c +++ b/drivers/net/ethernet/ni/nixge.c @@ -127,8 +127,8 @@ struct nixge_hw_dma_bd { #ifdef CONFIG_PHYS_ADDR_T_64BIT #define nixge_hw_dma_bd_set_addr(bd, field, addr) \ do { \ - (bd)->field##_lo = lower_32_bits(((u64)addr)); \ - (bd)->field##_hi = upper_32_bits(((u64)addr)); \ + (bd)->field##_lo = lower_32_bits((addr)); \ + (bd)->field##_hi = upper_32_bits((addr)); \ } while (0) #else #define nixge_hw_dma_bd_set_addr(bd, field, addr) \ @@ -251,7 +251,7 @@ static void nixge_hw_dma_bd_release(struct net_device *ndev) NIXGE_MAX_JUMBO_FRAME_SIZE, DMA_FROM_DEVICE); - skb = (struct sk_buff *) + skb = (struct sk_buff *)(uintptr_t) nixge_hw_dma_bd_get_addr(&priv->rx_bd_v[i], sw_id_offset); dev_kfree_skb(skb); @@ -323,7 +323,7 @@ static int nixge_hw_dma_bd_init(struct net_device *ndev) if (!skb) goto out; - nixge_hw_dma_bd_set_offset(&priv->rx_bd_v[i], skb); + nixge_hw_dma_bd_set_offset(&priv->rx_bd_v[i], (uintptr_t)skb); phys = dma_map_single(ndev->dev.parent, skb->data, NIXGE_MAX_JUMBO_FRAME_SIZE, DMA_FROM_DEVICE); @@ -601,8 +601,8 @@ static int nixge_recv(struct net_device *ndev, int budget) tail_p = priv->rx_bd_p + sizeof(*priv->rx_bd_v) * priv->rx_bd_ci; - skb = (struct sk_buff *)nixge_hw_dma_bd_get_addr(cur_p, -sw_id_offset); + skb = (struct sk_buff *)(uintptr_t) + nixge_hw_dma_bd_get_addr(cur_p, sw_id_offset); length = cur_p->status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK; if (length > NIXGE_MAX_JUMBO_FRAME_SIZE) @@ -643,7 +643,7 @@ static int nixge_recv(struct net_device *ndev, int budget) nixge_hw_dma_bd_set_phys(cur_p, cur_phys); cur_p->cntrl = NIXGE_MAX_JUMBO_FRAME_SIZE; cur_p->status = 0; - nixge_hw_dma_bd_set_offset(cur_p, new_skb); + nixge_hw_dma_bd_set_offset(cur_p, (uintptr_t)new_skb); ++priv->rx_bd_ci; priv->rx_bd_ci %= RX_BD_NUM; -- 2.19.0
[PATCH] net: nixge: Add support for 64-bit platforms
Add support for 64-bit platforms to driver. The hardware only supports 32-bit register accesses so the accesses need to be split up into two writes when setting the current and tail descriptor values. Cc: Florian Fainelli Signed-off-by: Moritz Fischer --- Changes from RFC: - Work around warning when building by casting dma_addr to (u64) when netdev_err() printing it - Use nixge_hw_dma_bd_set_{offset,phys} to zero out descs - change KConfig from depends on ARCH_ZYNQ to "depends on HAS_IOMEM && HAS_DMA" --- drivers/net/ethernet/ni/Kconfig | 3 +- drivers/net/ethernet/ni/nixge.c | 168 ++-- 2 files changed, 116 insertions(+), 55 deletions(-) diff --git a/drivers/net/ethernet/ni/Kconfig b/drivers/net/ethernet/ni/Kconfig index aa41e5f6e437..04e315704f71 100644 --- a/drivers/net/ethernet/ni/Kconfig +++ b/drivers/net/ethernet/ni/Kconfig @@ -18,8 +18,9 @@ if NET_VENDOR_NI config NI_XGE_MANAGEMENT_ENET tristate "National Instruments XGE management enet support" - depends on ARCH_ZYNQ + depends on HAS_IOMEM && HAS_DMA select PHYLIB + select OF_MDIO help Simple LAN device for debug or management purposes. Can support either 10G or 1G PHYs via SFP+ ports. diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c index 76efed058f33..74cf52e3fb09 100644 --- a/drivers/net/ethernet/ni/nixge.c +++ b/drivers/net/ethernet/ni/nixge.c @@ -106,10 +106,10 @@ (NIXGE_JUMBO_MTU + NIXGE_HDR_SIZE + NIXGE_TRL_SIZE) struct nixge_hw_dma_bd { - u32 next; - u32 reserved1; - u32 phys; - u32 reserved2; + u32 next_lo; + u32 next_hi; + u32 phys_lo; + u32 phys_hi; u32 reserved3; u32 reserved4; u32 cntrl; @@ -119,11 +119,39 @@ struct nixge_hw_dma_bd { u32 app2; u32 app3; u32 app4; - u32 sw_id_offset; - u32 reserved5; + u32 sw_id_offset_lo; + u32 sw_id_offset_hi; u32 reserved6; }; +#ifdef CONFIG_PHYS_ADDR_T_64BIT +#define nixge_hw_dma_bd_set_addr(bd, field, addr) \ + do { \ + (bd)->field##_lo = lower_32_bits(((u64)addr)); \ + (bd)->field##_hi = upper_32_bits(((u64)addr)); \ + } while (0) +#else +#define nixge_hw_dma_bd_set_addr(bd, field, addr) \ + ((bd)->field##_lo = lower_32_bits((addr))) +#endif + +#define nixge_hw_dma_bd_set_phys(bd, addr) \ + nixge_hw_dma_bd_set_addr((bd), phys, (addr)) + +#define nixge_hw_dma_bd_set_next(bd, addr) \ + nixge_hw_dma_bd_set_addr((bd), next, (addr)) + +#define nixge_hw_dma_bd_set_offset(bd, addr) \ + nixge_hw_dma_bd_set_addr((bd), sw_id_offset, (addr)) + +#ifdef CONFIG_PHYS_ADDR_T_64BIT +#define nixge_hw_dma_bd_get_addr(bd, field) \ + (dma_addr_t)u64)(bd)->field##_hi) << 32) | ((bd)->field##_lo)) +#else +#define nixge_hw_dma_bd_get_addr(bd, field) \ + (dma_addr_t)((bd)->field##_lo) +#endif + struct nixge_tx_skb { struct sk_buff *skb; dma_addr_t mapping; @@ -176,6 +204,15 @@ static void nixge_dma_write_reg(struct nixge_priv *priv, off_t offset, u32 val) writel(val, priv->dma_regs + offset); } +static void nixge_dma_write_desc_reg(struct nixge_priv *priv, off_t offset, +dma_addr_t addr) +{ + writel(lower_32_bits(addr), priv->dma_regs + offset); +#ifdef CONFIG_PHYS_ADDR_T_64BIT + writel(upper_32_bits(addr), priv->dma_regs + offset + 4); +#endif +} + static u32 nixge_dma_read_reg(const struct nixge_priv *priv, off_t offset) { return readl(priv->dma_regs + offset); @@ -202,13 +239,22 @@ static u32 nixge_ctrl_read_reg(struct nixge_priv *priv, off_t offset) static void nixge_hw_dma_bd_release(struct net_device *ndev) { struct nixge_priv *priv = netdev_priv(ndev); + dma_addr_t phys_addr; + struct sk_buff *skb; int i; for (i = 0; i < RX_BD_NUM; i++) { - dma_unmap_single(ndev->dev.parent, priv->rx_bd_v[i].phys, -NIXGE_MAX_JUMBO_FRAME_SIZE, DMA_FROM_DEVICE); - dev_kfree_skb((struct sk_buff *) - (priv->rx_bd_v[i].sw_id_offset)); + phys_addr = nixge_hw_dma_bd_get_addr(&priv->rx_bd_v[i], +phys); + + dma_unmap_single(ndev->dev.parent, phys_addr, +NIXGE_MAX_JUMBO_FRAME_SIZE, +DMA_FROM_DEVICE); + + skb = (struct sk_buff *) + nixge_hw_dma_bd_get_addr(&priv->rx_bd_v[i], +sw_id_offset); + dev_kfree_skb(skb); } if (priv->rx_bd_v) @@ -231,6 +277,7 @@ static int nixge_hw_dma_bd_init(struct net_device *ndev) {
[RFC/PATCH]] net: nixge: Add support for 64-bit ZynqMP platform
Add support for 64-bit (ZynqMP) platform to driver. The hardware only supports 32-bit register accesses so the accesses need to be split up into two writes when setting the current and tail descriptor values. Signed-off-by: Moritz Fischer Cc: Florian Fainelli --- Hi all, I'm working on making this work for X86_64, too (in fact I tested a version where I got rid of the phy/mdio code on X86_64 over PCIe) which will come as a follow up patch once I got the details worked out. in the PCIe case this will be platform_device subdevice of another device. I digress ... For the generic case would my depends on look like depends on HAS_MMIO && HAS_DMA ? Am I lacking a depends on OF_MDIO? This patch also intoduces a warning where with netdev_err() in the IRQ handlers I print the address which for the 64bit case is %llx vs %x for the 32-bit case. I know %pad is the right format string to print dma_addr_t, but that seemed to generate a warning, too. Ideas? Thanks for your feedback, Moritz --- drivers/net/ethernet/ni/Kconfig | 2 +- drivers/net/ethernet/ni/nixge.c | 170 ++-- 2 files changed, 117 insertions(+), 55 deletions(-) diff --git a/drivers/net/ethernet/ni/Kconfig b/drivers/net/ethernet/ni/Kconfig index aa41e5f6e437..bc6b29c7b0c1 100644 --- a/drivers/net/ethernet/ni/Kconfig +++ b/drivers/net/ethernet/ni/Kconfig @@ -18,7 +18,7 @@ if NET_VENDOR_NI config NI_XGE_MANAGEMENT_ENET tristate "National Instruments XGE management enet support" - depends on ARCH_ZYNQ + depends on ARCH_ZYNQ || ARCH_ZYNQMP select PHYLIB help Simple LAN device for debug or management purposes. Can diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c index 76efed058f33..352d4a52191a 100644 --- a/drivers/net/ethernet/ni/nixge.c +++ b/drivers/net/ethernet/ni/nixge.c @@ -106,10 +106,10 @@ (NIXGE_JUMBO_MTU + NIXGE_HDR_SIZE + NIXGE_TRL_SIZE) struct nixge_hw_dma_bd { - u32 next; - u32 reserved1; - u32 phys; - u32 reserved2; + u32 next_lo; + u32 next_hi; + u32 phys_lo; + u32 phys_hi; u32 reserved3; u32 reserved4; u32 cntrl; @@ -119,11 +119,39 @@ struct nixge_hw_dma_bd { u32 app2; u32 app3; u32 app4; - u32 sw_id_offset; - u32 reserved5; + u32 sw_id_offset_lo; + u32 sw_id_offset_hi; u32 reserved6; }; +#ifdef CONFIG_PHYS_ADDR_T_64BIT +#define nixge_hw_dma_bd_set_addr(bd, field, addr) \ + do { \ + (bd)->field##_lo = lower_32_bits(((u64)addr)); \ + (bd)->field##_hi = upper_32_bits(((u64)addr)); \ + } while (0) +#else +#define nixge_hw_dma_bd_set_addr(bd, field, addr) \ + ((bd)->field##_lo = lower_32_bits((addr))) +#endif + +#define nixge_hw_dma_bd_set_phys(bd, addr) \ + nixge_hw_dma_bd_set_addr((bd), phys, (addr)) + +#define nixge_hw_dma_bd_set_next(bd, addr) \ + nixge_hw_dma_bd_set_addr((bd), next, (addr)) + +#define nixge_hw_dma_bd_set_offset(bd, addr) \ + nixge_hw_dma_bd_set_addr((bd), sw_id_offset, (addr)) + +#ifdef CONFIG_PHYS_ADDR_T_64BIT +#define nixge_hw_dma_bd_get_addr(bd, field) \ + (dma_addr_t)u64)(bd)->field##_hi) << 32) | ((bd)->field##_lo)) +#else +#define nixge_hw_dma_bd_get_addr(bd, field) \ + (dma_addr_t)((bd)->field##_lo) +#endif + struct nixge_tx_skb { struct sk_buff *skb; dma_addr_t mapping; @@ -176,6 +204,15 @@ static void nixge_dma_write_reg(struct nixge_priv *priv, off_t offset, u32 val) writel(val, priv->dma_regs + offset); } +static void nixge_dma_write_desc_reg(struct nixge_priv *priv, off_t offset, +dma_addr_t addr) +{ + writel(lower_32_bits(addr), priv->dma_regs + offset); +#ifdef CONFIG_PHYS_ADDR_T_64BIT + writel(upper_32_bits(addr), priv->dma_regs + offset + 4); +#endif +} + static u32 nixge_dma_read_reg(const struct nixge_priv *priv, off_t offset) { return readl(priv->dma_regs + offset); @@ -202,13 +239,22 @@ static u32 nixge_ctrl_read_reg(struct nixge_priv *priv, off_t offset) static void nixge_hw_dma_bd_release(struct net_device *ndev) { struct nixge_priv *priv = netdev_priv(ndev); + dma_addr_t phys_addr; + struct sk_buff *skb; int i; for (i = 0; i < RX_BD_NUM; i++) { - dma_unmap_single(ndev->dev.parent, priv->rx_bd_v[i].phys, -NIXGE_MAX_JUMBO_FRAME_SIZE, DMA_FROM_DEVICE); - dev_kfree_skb((struct sk_buff *) - (priv->rx_bd_v[i].sw_id_offset)); + phys_addr = nixge_hw_dma_bd_get_addr(&priv->rx_bd_v[i], +phys); + + dma_unmap_single(ndev->dev.parent, phys_addr, +NIXGE_MAX_JUMBO_FRAME_SIZE, +
[PATCH 1/2] net: nixge: Fix error path for obtaining mac address
Fix issue where nixge_get_nvmem_address() returns a non-NULL return value on a failed nvmem_cell_get() that causes an invalid access when error value encoded in pointer is dereferenced. Furthermore ensure that buffer allocated by nvmem_cell_read() actually gets kfreed() if the function succeeds. Fixes commit 492caffa8a1a ("net: ethernet: nixge: Add support for National Instruments XGE netdev") Reported-by: Alex Williams Signed-off-by: Moritz Fischer --- drivers/net/ethernet/ni/nixge.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c index 27364b7572fc..c41fea9253e3 100644 --- a/drivers/net/ethernet/ni/nixge.c +++ b/drivers/net/ethernet/ni/nixge.c @@ -1170,7 +1170,7 @@ static void *nixge_get_nvmem_address(struct device *dev) cell = nvmem_cell_get(dev, "address"); if (IS_ERR(cell)) - return cell; + return NULL; mac = nvmem_cell_read(cell, &cell_size); nvmem_cell_put(cell); @@ -1202,10 +1202,12 @@ static int nixge_probe(struct platform_device *pdev) ndev->max_mtu = NIXGE_JUMBO_MTU; mac_addr = nixge_get_nvmem_address(&pdev->dev); - if (mac_addr && is_valid_ether_addr(mac_addr)) + if (mac_addr && is_valid_ether_addr(mac_addr)) { ether_addr_copy(ndev->dev_addr, mac_addr); - else + kfree(mac_addr); + } else { eth_hw_addr_random(ndev); + } priv = netdev_priv(ndev); priv->ndev = ndev; -- 2.17.0
[PATCH 2/2] net: nixge: Address compiler warnings about signedness
Fixes the following warnings: warning: pointer targets in passing argument 1 of ‘is_valid_ether_addr’ differ in signedness [-Wpointer-sign] if (mac_addr && is_valid_ether_addr(mac_addr)) { ^~~~ expected ‘const u8 * {aka const unsigned char *}’ but argument is of type ‘const char *’ static inline bool is_valid_ether_addr(const u8 *addr) ^~~ warning: pointer targets in passing argument 2 of ‘ether_addr_copy’ differ in signedness [-Wpointer-sign] ether_addr_copy(ndev->dev_addr, mac_addr); ^~~~ expected ‘const u8 * {aka const unsigned char *}’ but argument is of type ‘const char *’ static inline void ether_addr_copy(u8 *dst, const u8 *src) Signed-off-by: Moritz Fischer --- drivers/net/ethernet/ni/nixge.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c index c41fea9253e3..b092894dd128 100644 --- a/drivers/net/ethernet/ni/nixge.c +++ b/drivers/net/ethernet/ni/nixge.c @@ -1183,7 +1183,7 @@ static int nixge_probe(struct platform_device *pdev) struct nixge_priv *priv; struct net_device *ndev; struct resource *dmares; - const char *mac_addr; + const u8 *mac_addr; int err; ndev = alloc_etherdev(sizeof(*priv)); -- 2.17.0
Re: [RFC/PATCH] net: ethernet: nixge: Use of_get_mac_address()
On Thu, Apr 26, 2018 at 02:57:42PM -0700, Moritz Fischer wrote: > Make nixge driver work with 'mac-address' property instead of > 'address' property. There are currently no in-tree users and > the only users of this driver are devices that use overlays > we control to instantiate the device together with the corresponding > FPGA images. > > Signed-off-by: Moritz Fischer > --- > > Hi David, Rob, > > with Mike's change that enable the generic 'mac-address' > binding that I barely missed with the submission of this > driver I was wondering if we can still change the binding. > > I'm aware that this generally is a nonono case, since the binding > is considered API, but since there are no users outside of our > devicetree overlays that we ship with our devices I thought I'd ask. > > If you don't think that's a good idea do you think supporting both > would be worthwhile? > > Thanks, > > Moritz > > --- > .../devicetree/bindings/net/nixge.txt | 4 ++-- > drivers/net/ethernet/ni/nixge.c | 20 ++- > 2 files changed, 4 insertions(+), 20 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/nixge.txt > b/Documentation/devicetree/bindings/net/nixge.txt > index e55af7f0881a..9bc1ecfb6762 100644 > --- a/Documentation/devicetree/bindings/net/nixge.txt > +++ b/Documentation/devicetree/bindings/net/nixge.txt > @@ -8,7 +8,7 @@ Required properties: > - phy-mode: See ethernet.txt file in the same directory. > - phy-handle: See ethernet.txt file in the same directory. > - nvmem-cells: Phandle of nvmem cell containing the MAC address > -- nvmem-cell-names: Should be "address" > +- nvmem-cell-names: Should be "mac-address" > > Examples (10G generic PHY): > nixge0: ethernet@4000 { > @@ -16,7 +16,7 @@ Examples (10G generic PHY): > reg = <0x4000 0x6000>; > > nvmem-cells = <ð1_addr>; > - nvmem-cell-names = "address"; > + nvmem-cell-names = "mac-address"; > > interrupts = <0 29 IRQ_TYPE_LEVEL_HIGH>, <0 30 > IRQ_TYPE_LEVEL_HIGH>; > interrupt-names = "rx", "tx"; > diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c > index 27364b7572fc..7918c7b7273b 100644 > --- a/drivers/net/ethernet/ni/nixge.c > +++ b/drivers/net/ethernet/ni/nixge.c > @@ -1162,22 +1162,6 @@ static int nixge_mdio_setup(struct nixge_priv *priv, > struct device_node *np) > return of_mdiobus_register(bus, np); > } > > -static void *nixge_get_nvmem_address(struct device *dev) > -{ > - struct nvmem_cell *cell; > - size_t cell_size; > - char *mac; > - > - cell = nvmem_cell_get(dev, "address"); > - if (IS_ERR(cell)) > - return cell; > - > - mac = nvmem_cell_read(cell, &cell_size); > - nvmem_cell_put(cell); > - > - return mac; > -} > - > static int nixge_probe(struct platform_device *pdev) > { > struct nixge_priv *priv; > @@ -1201,8 +1185,8 @@ static int nixge_probe(struct platform_device *pdev) > ndev->min_mtu = 64; > ndev->max_mtu = NIXGE_JUMBO_MTU; > > - mac_addr = nixge_get_nvmem_address(&pdev->dev); > - if (mac_addr && is_valid_ether_addr(mac_addr)) > + mac_addr = of_get_mac_address(np); Sorry, that should be &pdev->dev.of_node here ... I'll resubmit if general idea ok. > + if (mac_addr) > ether_addr_copy(ndev->dev_addr, mac_addr); > else > eth_hw_addr_random(ndev); > -- > 2.17.0 > - Moritz
[RFC/PATCH] net: ethernet: nixge: Use of_get_mac_address()
Make nixge driver work with 'mac-address' property instead of 'address' property. There are currently no in-tree users and the only users of this driver are devices that use overlays we control to instantiate the device together with the corresponding FPGA images. Signed-off-by: Moritz Fischer --- Hi David, Rob, with Mike's change that enable the generic 'mac-address' binding that I barely missed with the submission of this driver I was wondering if we can still change the binding. I'm aware that this generally is a nonono case, since the binding is considered API, but since there are no users outside of our devicetree overlays that we ship with our devices I thought I'd ask. If you don't think that's a good idea do you think supporting both would be worthwhile? Thanks, Moritz --- .../devicetree/bindings/net/nixge.txt | 4 ++-- drivers/net/ethernet/ni/nixge.c | 20 ++- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/Documentation/devicetree/bindings/net/nixge.txt b/Documentation/devicetree/bindings/net/nixge.txt index e55af7f0881a..9bc1ecfb6762 100644 --- a/Documentation/devicetree/bindings/net/nixge.txt +++ b/Documentation/devicetree/bindings/net/nixge.txt @@ -8,7 +8,7 @@ Required properties: - phy-mode: See ethernet.txt file in the same directory. - phy-handle: See ethernet.txt file in the same directory. - nvmem-cells: Phandle of nvmem cell containing the MAC address -- nvmem-cell-names: Should be "address" +- nvmem-cell-names: Should be "mac-address" Examples (10G generic PHY): nixge0: ethernet@4000 { @@ -16,7 +16,7 @@ Examples (10G generic PHY): reg = <0x4000 0x6000>; nvmem-cells = <ð1_addr>; - nvmem-cell-names = "address"; + nvmem-cell-names = "mac-address"; interrupts = <0 29 IRQ_TYPE_LEVEL_HIGH>, <0 30 IRQ_TYPE_LEVEL_HIGH>; interrupt-names = "rx", "tx"; diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c index 27364b7572fc..7918c7b7273b 100644 --- a/drivers/net/ethernet/ni/nixge.c +++ b/drivers/net/ethernet/ni/nixge.c @@ -1162,22 +1162,6 @@ static int nixge_mdio_setup(struct nixge_priv *priv, struct device_node *np) return of_mdiobus_register(bus, np); } -static void *nixge_get_nvmem_address(struct device *dev) -{ - struct nvmem_cell *cell; - size_t cell_size; - char *mac; - - cell = nvmem_cell_get(dev, "address"); - if (IS_ERR(cell)) - return cell; - - mac = nvmem_cell_read(cell, &cell_size); - nvmem_cell_put(cell); - - return mac; -} - static int nixge_probe(struct platform_device *pdev) { struct nixge_priv *priv; @@ -1201,8 +1185,8 @@ static int nixge_probe(struct platform_device *pdev) ndev->min_mtu = 64; ndev->max_mtu = NIXGE_JUMBO_MTU; - mac_addr = nixge_get_nvmem_address(&pdev->dev); - if (mac_addr && is_valid_ether_addr(mac_addr)) + mac_addr = of_get_mac_address(np); + if (mac_addr) ether_addr_copy(ndev->dev_addr, mac_addr); else eth_hw_addr_random(ndev); -- 2.17.0
[PATCH v6 1/2] dt-bindings: net: Add bindings for National Instruments XGE netdev
This adds bindings for the NI XGE 1G/10G network device. Reviewed-by: Rob Herring Signed-off-by: Moritz Fischer --- Changes from v5: - None Changes from v4: - None Changes from v3: - Added Rob's Reviewed-by Changes from v2: - Addressed Rob's comments w.r.t to IRQ names and typo Changes from v1: - Corrected from nixge -> nixge.txt --- Documentation/devicetree/bindings/net/nixge.txt | 32 + 1 file changed, 32 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/nixge.txt diff --git a/Documentation/devicetree/bindings/net/nixge.txt b/Documentation/devicetree/bindings/net/nixge.txt new file mode 100644 index ..e55af7f0881a --- /dev/null +++ b/Documentation/devicetree/bindings/net/nixge.txt @@ -0,0 +1,32 @@ +* NI XGE Ethernet controller + +Required properties: +- compatible: Should be "ni,xge-enet-2.00" +- reg: Address and length of the register set for the device +- interrupts: Should contain tx and rx interrupt +- interrupt-names: Should be "rx" and "tx" +- phy-mode: See ethernet.txt file in the same directory. +- phy-handle: See ethernet.txt file in the same directory. +- nvmem-cells: Phandle of nvmem cell containing the MAC address +- nvmem-cell-names: Should be "address" + +Examples (10G generic PHY): + nixge0: ethernet@4000 { + compatible = "ni,xge-enet-2.00"; + reg = <0x4000 0x6000>; + + nvmem-cells = <ð1_addr>; + nvmem-cell-names = "address"; + + interrupts = <0 29 IRQ_TYPE_LEVEL_HIGH>, <0 30 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "rx", "tx"; + interrupt-parent = <&intc>; + + phy-mode = "xgmii"; + phy-handle = <ðernet_phy1>; + + ethernet_phy1: ethernet-phy@4 { + compatible = "ethernet-phy-ieee802.3-c45"; + reg = <4>; + }; + }; -- 2.16.2
[PATCH v6 2/2] net: ethernet: nixge: Add support for National Instruments XGE netdev
Add support for the National Instruments XGE 1/10G network device. It uses the EEPROM on the board via NVMEM. Signed-off-by: Moritz Fischer --- Changes from v5: - Fixed up indents according to David's feedback - Fixed KConfig ---help--- -> help Changes from v4: - Worked on consistency for constants - Removed unused constants - Removed unused includes Changes from v3: - Added NIXGE prefix to MDIO constants - Removed NIXGE_MAX_PHY_ADDR (unused) - Consistency for NIXGE_MDIO_CXX_READ constants - Use devm_mdiobus_alloc() - Fixed order of netdev_unregister() and mdio_bus_unregister() - Addressed feedback w.r.t. disconnecting the PHY - Removed now superfluous max_frm_size member - Fix SPDX vs module license to be 'GPL v2' SPDX-License-Identifier: GPL-2.0 Changes from v2: - Implement recv side NAPI - Improved error handling - Implemented C45 writes - Added ethtool callbacks & blink functionality - Improved nixge_ctrl_poll_timeout() macro - Removed dev_dbg() for mdio accesses - Added businfo to ethtool drvinfo Changes from v1: - Added dependency on ARCH_ZYNQ (Kbuild) - Removed unused variables - Use of_phy_connect as suggested - Removed masking of (un)supported modes - Added #define for some constants - Removed empty pm functions - Reworked mac_address handling - Made nixge_mdio_*() static (sparse) - Removed driver version - Addressed timeout loop - Adressed return values on timeout --- drivers/net/ethernet/Kconfig |1 + drivers/net/ethernet/Makefile|1 + drivers/net/ethernet/ni/Kconfig | 27 + drivers/net/ethernet/ni/Makefile |1 + drivers/net/ethernet/ni/nixge.c | 1310 ++ 5 files changed, 1340 insertions(+) create mode 100644 drivers/net/ethernet/ni/Kconfig create mode 100644 drivers/net/ethernet/ni/Makefile create mode 100644 drivers/net/ethernet/ni/nixge.c diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig index 074d760a568b..603a5704dab8 100644 --- a/drivers/net/ethernet/Kconfig +++ b/drivers/net/ethernet/Kconfig @@ -128,6 +128,7 @@ config FEALNX source "drivers/net/ethernet/natsemi/Kconfig" source "drivers/net/ethernet/netronome/Kconfig" +source "drivers/net/ethernet/ni/Kconfig" source "drivers/net/ethernet/8390/Kconfig" config NET_NETX diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile index 135dae67d671..2bfd2eea50bf 100644 --- a/drivers/net/ethernet/Makefile +++ b/drivers/net/ethernet/Makefile @@ -60,6 +60,7 @@ obj-$(CONFIG_NET_VENDOR_MYRI) += myricom/ obj-$(CONFIG_FEALNX) += fealnx.o obj-$(CONFIG_NET_VENDOR_NATSEMI) += natsemi/ obj-$(CONFIG_NET_VENDOR_NETRONOME) += netronome/ +obj-$(CONFIG_NET_VENDOR_NI) += ni/ obj-$(CONFIG_NET_NETX) += netx-eth.o obj-$(CONFIG_NET_VENDOR_NUVOTON) += nuvoton/ obj-$(CONFIG_NET_VENDOR_NVIDIA) += nvidia/ diff --git a/drivers/net/ethernet/ni/Kconfig b/drivers/net/ethernet/ni/Kconfig new file mode 100644 index ..cd30f7de16de --- /dev/null +++ b/drivers/net/ethernet/ni/Kconfig @@ -0,0 +1,27 @@ +# +# National Instuments network device configuration +# + +config NET_VENDOR_NI + bool "National Instruments Devices" + default y + help + If you have a network (Ethernet) device belonging to this class, say Y. + + Note that the answer to this question doesn't directly affect the + kernel: saying N will just cause the configurator to skip all + the questions about National Instrument devices. + If you say Y, you will be asked for your specific device in the + following questions. + +if NET_VENDOR_NI + +config NI_XGE_MANAGEMENT_ENET + tristate "National Instruments XGE management enet support" + depends on ARCH_ZYNQ + select PHYLIB + help + Simple LAN device for debug or management purposes. Can + support either 10G or 1G PHYs via SFP+ ports. + +endif diff --git a/drivers/net/ethernet/ni/Makefile b/drivers/net/ethernet/ni/Makefile new file mode 100644 index ..99c664651c51 --- /dev/null +++ b/drivers/net/ethernet/ni/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_NI_XGE_MANAGEMENT_ENET) += nixge.o diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c new file mode 100644 index ..27364b7572fc --- /dev/null +++ b/drivers/net/ethernet/ni/nixge.c @@ -0,0 +1,1310 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2016-2017, National Instruments Corp. + * + * Author: Moritz Fischer + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define TX_BD_NUM 64 +#define RX_BD_NUM 128 + +/* Axi DMA Register definitions */ +#define XAXIDMA_TX_CR_OFFSET 0x00 /* Channel control */ +#define XAXIDMA_TX_SR_OFFSET 0x04 /* Status */ +#define XAXIDMA_TX_CDESC_OFFSET0x08 /* Curren
Re: [PATCH v5 2/2] net: ethernet: nixge: Add support for National Instruments XGE netdev
Hi David, On Mon, Mar 26, 2018 at 11:38:30AM -0400, David Miller wrote: > From: Moritz Fischer > Date: Fri, 23 Mar 2018 13:41:28 -0700 > > > +static void nixge_hw_dma_bd_release(struct net_device *ndev) > > +{ > > + int i; > > + struct nixge_priv *priv = netdev_priv(ndev); > > Please order local variables from longest to shortest line (ie. reverse > christmas tree layout). Sure. > > > +static int nixge_hw_dma_bd_init(struct net_device *ndev) > > +{ > > + u32 cr; > > + int i; > > + struct sk_buff *skb; > > + struct nixge_priv *priv = netdev_priv(ndev); > > Likewise. Sure. > > > +static void __nixge_device_reset(struct nixge_priv *priv, off_t offset) > > +{ > > + u32 status; > > + int err; > > + /* Reset Axi DMA. This would reset NIXGE Ethernet core as well. > > +* The reset process of Axi DMA takes a while to complete as all > > +* pending commands/transfers will be flushed or completed during > > +* this reset process. > > +*/ > > Please put an empty line between the local variable declarations > and this comment. Will do in v6 Thanks for your review! Cheers Moritz
[PATCH v5 1/2] dt-bindings: net: Add bindings for National Instruments XGE netdev
This adds bindings for the NI XGE 1G/10G network device. Reviewed-by: Rob Herring Signed-off-by: Moritz Fischer --- Changes from v4: - None Changes from v3: - Added Rob's Reviewed-by Changes from v2: - Addressed Rob's comments w.r.t to IRQ names and typo Changes from v1: - Corrected from nixge -> nixge.txt --- Documentation/devicetree/bindings/net/nixge.txt | 32 + 1 file changed, 32 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/nixge.txt diff --git a/Documentation/devicetree/bindings/net/nixge.txt b/Documentation/devicetree/bindings/net/nixge.txt new file mode 100644 index ..e55af7f0881a --- /dev/null +++ b/Documentation/devicetree/bindings/net/nixge.txt @@ -0,0 +1,32 @@ +* NI XGE Ethernet controller + +Required properties: +- compatible: Should be "ni,xge-enet-2.00" +- reg: Address and length of the register set for the device +- interrupts: Should contain tx and rx interrupt +- interrupt-names: Should be "rx" and "tx" +- phy-mode: See ethernet.txt file in the same directory. +- phy-handle: See ethernet.txt file in the same directory. +- nvmem-cells: Phandle of nvmem cell containing the MAC address +- nvmem-cell-names: Should be "address" + +Examples (10G generic PHY): + nixge0: ethernet@4000 { + compatible = "ni,xge-enet-2.00"; + reg = <0x4000 0x6000>; + + nvmem-cells = <ð1_addr>; + nvmem-cell-names = "address"; + + interrupts = <0 29 IRQ_TYPE_LEVEL_HIGH>, <0 30 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "rx", "tx"; + interrupt-parent = <&intc>; + + phy-mode = "xgmii"; + phy-handle = <ðernet_phy1>; + + ethernet_phy1: ethernet-phy@4 { + compatible = "ethernet-phy-ieee802.3-c45"; + reg = <4>; + }; + }; -- 2.16.2
[PATCH v5 2/2] net: ethernet: nixge: Add support for National Instruments XGE netdev
Add support for the National Instruments XGE 1/10G network device. It uses the EEPROM on the board via NVMEM. Signed-off-by: Moritz Fischer --- Changes from v4: - Worked on consistency for constants - Removed unused constants - Removed unused includes Changes from v3: - Added NIXGE prefix to MDIO constants - Removed NIXGE_MAX_PHY_ADDR (unused) - Consistency for NIXGE_MDIO_CXX_READ constants - Use devm_mdiobus_alloc() - Fixed order of netdev_unregister() and mdio_bus_unregister() - Addressed feedback w.r.t. disconnecting the PHY - Removed now superfluous max_frm_size member - Fix SPDX vs module license to be 'GPL v2' SPDX-License-Identifier: GPL-2.0 Changes from v2: - Implement recv side NAPI - Improved error handling - Implemented C45 writes - Added ethtool callbacks & blink functionality - Improved nixge_ctrl_poll_timeout() macro - Removed dev_dbg() for mdio accesses - Added businfo to ethtool drvinfo Changes from v1: - Added dependency on ARCH_ZYNQ (Kbuild) - Removed unused variables - Use of_phy_connect as suggested - Removed masking of (un)supported modes - Added #define for some constants - Removed empty pm functions - Reworked mac_address handling - Made nixge_mdio_*() static (sparse) - Removed driver version - Addressed timeout loop - Adressed return values on timeout --- drivers/net/ethernet/Kconfig |1 + drivers/net/ethernet/Makefile|1 + drivers/net/ethernet/ni/Kconfig | 27 + drivers/net/ethernet/ni/Makefile |1 + drivers/net/ethernet/ni/nixge.c | 1309 ++ 5 files changed, 1339 insertions(+) create mode 100644 drivers/net/ethernet/ni/Kconfig create mode 100644 drivers/net/ethernet/ni/Makefile create mode 100644 drivers/net/ethernet/ni/nixge.c diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig index 074d760a568b..603a5704dab8 100644 --- a/drivers/net/ethernet/Kconfig +++ b/drivers/net/ethernet/Kconfig @@ -128,6 +128,7 @@ config FEALNX source "drivers/net/ethernet/natsemi/Kconfig" source "drivers/net/ethernet/netronome/Kconfig" +source "drivers/net/ethernet/ni/Kconfig" source "drivers/net/ethernet/8390/Kconfig" config NET_NETX diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile index 135dae67d671..2bfd2eea50bf 100644 --- a/drivers/net/ethernet/Makefile +++ b/drivers/net/ethernet/Makefile @@ -60,6 +60,7 @@ obj-$(CONFIG_NET_VENDOR_MYRI) += myricom/ obj-$(CONFIG_FEALNX) += fealnx.o obj-$(CONFIG_NET_VENDOR_NATSEMI) += natsemi/ obj-$(CONFIG_NET_VENDOR_NETRONOME) += netronome/ +obj-$(CONFIG_NET_VENDOR_NI) += ni/ obj-$(CONFIG_NET_NETX) += netx-eth.o obj-$(CONFIG_NET_VENDOR_NUVOTON) += nuvoton/ obj-$(CONFIG_NET_VENDOR_NVIDIA) += nvidia/ diff --git a/drivers/net/ethernet/ni/Kconfig b/drivers/net/ethernet/ni/Kconfig new file mode 100644 index ..cd30f7de16de --- /dev/null +++ b/drivers/net/ethernet/ni/Kconfig @@ -0,0 +1,27 @@ +# +# National Instuments network device configuration +# + +config NET_VENDOR_NI + bool "National Instruments Devices" + default y + ---help--- + If you have a network (Ethernet) device belonging to this class, say Y. + + Note that the answer to this question doesn't directly affect the + kernel: saying N will just cause the configurator to skip all + the questions about National Instrument devices. + If you say Y, you will be asked for your specific device in the + following questions. + +if NET_VENDOR_NI + +config NI_XGE_MANAGEMENT_ENET + tristate "National Instruments XGE management enet support" + depends on ARCH_ZYNQ + select PHYLIB + ---help--- + Simple LAN device for debug or management purposes. Can + support either 10G or 1G PHYs via SFP+ ports. + +endif diff --git a/drivers/net/ethernet/ni/Makefile b/drivers/net/ethernet/ni/Makefile new file mode 100644 index ..99c664651c51 --- /dev/null +++ b/drivers/net/ethernet/ni/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_NI_XGE_MANAGEMENT_ENET) += nixge.o diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c new file mode 100644 index ..c71e59b10340 --- /dev/null +++ b/drivers/net/ethernet/ni/nixge.c @@ -0,0 +1,1309 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2016-2017, National Instruments Corp. + * + * Author: Moritz Fischer + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define TX_BD_NUM 64 +#define RX_BD_NUM 128 + +/* Axi DMA Register definitions */ +#define XAXIDMA_TX_CR_OFFSET 0x00 /* Channel control */ +#define XAXIDMA_TX_SR_OFFSET 0x04 /* Status */ +#define XAXIDMA_TX_CDESC_OFFSET0x08 /* Current descriptor pointer */ +#define XAXIDMA_TX_TDESC_OFFSET0x10 /* Tail descriptor p
[RESEND PATCH v4 2/2] net: ethernet: nixge: Add support for National Instruments XGE netdev
Add support for the National Instruments XGE 1/10G network device. It uses the EEPROM on the board via NVMEM. Signed-off-by: Moritz Fischer --- Changes from v3: - Added NIXGE prefix to MDIO constants - Removed NIXGE_MAX_PHY_ADDR (unused) - Consistency for NIXGE_MDIO_CXX_READ constants - Use devm_mdiobus_alloc() - Fixed order of netdev_unregister() and mdio_bus_unregister() - Addressed feedback w.r.t. disconnecting the PHY - Removed now superfluous max_frm_size member - Fix SPDX vs module license to be 'GPL v2' SPDX-License-Identifier: GPL-2.0 Changes from v2: - Implement recv side NAPI - Improved error handling - Implemented C45 writes - Added ethtool callbacks & blink functionality - Improved nixge_ctrl_poll_timeout() macro - Removed dev_dbg() for mdio accesses - Added businfo to ethtool drvinfo Changes from v1: - Added dependency on ARCH_ZYNQ (Kbuild) - Removed unused variables - Use of_phy_connect as suggested - Removed masking of (un)supported modes - Added #define for some constants - Removed empty pm functions - Reworked mac_address handling - Made nixge_mdio_*() static (sparse) - Removed driver version - Addressed timeout loop - Adressed return values on timeout --- drivers/net/ethernet/Kconfig |1 + drivers/net/ethernet/Makefile|1 + drivers/net/ethernet/ni/Kconfig | 27 + drivers/net/ethernet/ni/Makefile |1 + drivers/net/ethernet/ni/nixge.c | 1337 ++ 5 files changed, 1367 insertions(+) create mode 100644 drivers/net/ethernet/ni/Kconfig create mode 100644 drivers/net/ethernet/ni/Makefile create mode 100644 drivers/net/ethernet/ni/nixge.c diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig index b6cf4b6962f5..908218561fdd 100644 --- a/drivers/net/ethernet/Kconfig +++ b/drivers/net/ethernet/Kconfig @@ -129,6 +129,7 @@ config FEALNX source "drivers/net/ethernet/natsemi/Kconfig" source "drivers/net/ethernet/netronome/Kconfig" +source "drivers/net/ethernet/ni/Kconfig" source "drivers/net/ethernet/8390/Kconfig" config NET_NETX diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile index 3cdf01e96e0b..d732e9522b76 100644 --- a/drivers/net/ethernet/Makefile +++ b/drivers/net/ethernet/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_NET_VENDOR_MYRI) += myricom/ obj-$(CONFIG_FEALNX) += fealnx.o obj-$(CONFIG_NET_VENDOR_NATSEMI) += natsemi/ obj-$(CONFIG_NET_VENDOR_NETRONOME) += netronome/ +obj-$(CONFIG_NET_VENDOR_NI) += ni/ obj-$(CONFIG_NET_NETX) += netx-eth.o obj-$(CONFIG_NET_VENDOR_NUVOTON) += nuvoton/ obj-$(CONFIG_NET_VENDOR_NVIDIA) += nvidia/ diff --git a/drivers/net/ethernet/ni/Kconfig b/drivers/net/ethernet/ni/Kconfig new file mode 100644 index ..cd30f7de16de --- /dev/null +++ b/drivers/net/ethernet/ni/Kconfig @@ -0,0 +1,27 @@ +# +# National Instuments network device configuration +# + +config NET_VENDOR_NI + bool "National Instruments Devices" + default y + ---help--- + If you have a network (Ethernet) device belonging to this class, say Y. + + Note that the answer to this question doesn't directly affect the + kernel: saying N will just cause the configurator to skip all + the questions about National Instrument devices. + If you say Y, you will be asked for your specific device in the + following questions. + +if NET_VENDOR_NI + +config NI_XGE_MANAGEMENT_ENET + tristate "National Instruments XGE management enet support" + depends on ARCH_ZYNQ + select PHYLIB + ---help--- + Simple LAN device for debug or management purposes. Can + support either 10G or 1G PHYs via SFP+ ports. + +endif diff --git a/drivers/net/ethernet/ni/Makefile b/drivers/net/ethernet/ni/Makefile new file mode 100644 index ..99c664651c51 --- /dev/null +++ b/drivers/net/ethernet/ni/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_NI_XGE_MANAGEMENT_ENET) += nixge.o diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c new file mode 100644 index ..69b5198b3784 --- /dev/null +++ b/drivers/net/ethernet/ni/nixge.c @@ -0,0 +1,1337 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2016-2017, National Instruments Corp. + * + * Author: Moritz Fischer + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define TX_BD_NUM 64 +#define RX_BD_NUM 128 + +/* Axi DMA Register definitions */ + +#define XAXIDMA_TX_CR_OFFSET 0x /* Channel control */ +#define XAXIDMA_TX_SR_OFFSET 0x0004 /* Status */ +#define XAXIDMA_TX_CDESC_OFFSET0x0008 /* Current descriptor pointer */ +#define XAXIDMA_TX_TDESC_OFFSET0x0010 /* Tail descriptor pointer */ + +#define XAXIDMA_RX_CR_OFFSET 0x
[RESEND PATCH v4 1/2] dt-bindings: net: Add bindings for National Instruments XGE netdev
This adds bindings for the NI XGE 1G/10G network device. Reviewed-by: Rob Herring Signed-off-by: Moritz Fischer --- Changes from v3: - Added Rob's Reviewed-by Changes from v2: - Addressed Rob's comments w.r.t to IRQ names and typo Changes from v1: - Corrected from nixge -> nixge.txt --- Documentation/devicetree/bindings/net/nixge.txt | 32 + 1 file changed, 32 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/nixge.txt diff --git a/Documentation/devicetree/bindings/net/nixge.txt b/Documentation/devicetree/bindings/net/nixge.txt new file mode 100644 index ..e55af7f0881a --- /dev/null +++ b/Documentation/devicetree/bindings/net/nixge.txt @@ -0,0 +1,32 @@ +* NI XGE Ethernet controller + +Required properties: +- compatible: Should be "ni,xge-enet-2.00" +- reg: Address and length of the register set for the device +- interrupts: Should contain tx and rx interrupt +- interrupt-names: Should be "rx" and "tx" +- phy-mode: See ethernet.txt file in the same directory. +- phy-handle: See ethernet.txt file in the same directory. +- nvmem-cells: Phandle of nvmem cell containing the MAC address +- nvmem-cell-names: Should be "address" + +Examples (10G generic PHY): + nixge0: ethernet@4000 { + compatible = "ni,xge-enet-2.00"; + reg = <0x4000 0x6000>; + + nvmem-cells = <ð1_addr>; + nvmem-cell-names = "address"; + + interrupts = <0 29 IRQ_TYPE_LEVEL_HIGH>, <0 30 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "rx", "tx"; + interrupt-parent = <&intc>; + + phy-mode = "xgmii"; + phy-handle = <ðernet_phy1>; + + ethernet_phy1: ethernet-phy@4 { + compatible = "ethernet-phy-ieee802.3-c45"; + reg = <4>; + }; + }; -- 2.16.1
Re: [PATCH net-next 5/5] net: phy: marvell10g: Utilize gen10g_soft_reset()
Florian, On Wed, Feb 28, 2018 at 11:44 AM, Russell King wrote: > On Wed, Feb 28, 2018 at 11:36:12AM -0800, Florian Fainelli wrote: >> We do the same thing as the generic function: nothing, so utilize it. >> >> Signed-off-by: Florian Fainelli >> --- >> drivers/net/phy/marvell10g.c | 7 +-- >> 1 file changed, 1 insertion(+), 6 deletions(-) >> >> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c >> index 8a0bd98fdec7..da014eae1476 100644 >> --- a/drivers/net/phy/marvell10g.c >> +++ b/drivers/net/phy/marvell10g.c >> @@ -75,11 +75,6 @@ static int mv3310_probe(struct phy_device *phydev) >> * Resetting the MV88X3310 causes it to become non-responsive. Avoid >> * setting the reset bit(s). >> */ >> -static int mv3310_soft_reset(struct phy_device *phydev) >> -{ >> - return 0; >> -} >> - > > You do realise that getting rid of that function makes a nonsense of the > comment above it - and removing the comment along with the function gets > rid of the very important reason _why_ we have en empty reset method? > >> static int mv3310_config_init(struct phy_device *phydev) >> { >> __ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, }; >> @@ -377,7 +372,7 @@ static struct phy_driver mv3310_drivers[] = { >> SUPPORTED_1baseT_Full | >> SUPPORTED_Backplane, >> .probe = mv3310_probe, >> - .soft_reset = mv3310_soft_reset, >> + .soft_reset = gen10g_soft_reset, >> .config_init= mv3310_config_init, >> .config_aneg= mv3310_config_aneg, >> .aneg_done = mv3310_aneg_done, >> -- >> 2.14.1 >> > > -- > Russell King > ARM architecture Linux Kernel maintainer FWIW I have a local patch that goes something like that, which I meant to send at one point and forgot Something like that: static int gen10g_soft_reset(struct phy_device *phydev) { + int val; + + val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_CTRL1); + if (val < 0) + return val; + + val |= MDIO_CTRL1_RESET; + phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1, val); + - /* Do nothing for now */ return 0; } If that looks reasonable I can properly submit a patch, Moritz
[PATCH v3 1/2] dt-bindings: net: Add bindings for National Instruments XGE netdev
This adds bindings for the NI XGE 1G/10G network device. Signed-off-by: Moritz Fischer --- Changes from v2: - Addressed Rob's comments w.r.t to IRQ names and typo Changes from v1: - Corrected from nixge -> nixge.txt --- Documentation/devicetree/bindings/net/nixge.txt | 32 + 1 file changed, 32 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/nixge.txt diff --git a/Documentation/devicetree/bindings/net/nixge.txt b/Documentation/devicetree/bindings/net/nixge.txt new file mode 100644 index ..e55af7f0881a --- /dev/null +++ b/Documentation/devicetree/bindings/net/nixge.txt @@ -0,0 +1,32 @@ +* NI XGE Ethernet controller + +Required properties: +- compatible: Should be "ni,xge-enet-2.00" +- reg: Address and length of the register set for the device +- interrupts: Should contain tx and rx interrupt +- interrupt-names: Should be "rx" and "tx" +- phy-mode: See ethernet.txt file in the same directory. +- phy-handle: See ethernet.txt file in the same directory. +- nvmem-cells: Phandle of nvmem cell containing the MAC address +- nvmem-cell-names: Should be "address" + +Examples (10G generic PHY): + nixge0: ethernet@4000 { + compatible = "ni,xge-enet-2.00"; + reg = <0x4000 0x6000>; + + nvmem-cells = <ð1_addr>; + nvmem-cell-names = "address"; + + interrupts = <0 29 IRQ_TYPE_LEVEL_HIGH>, <0 30 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "rx", "tx"; + interrupt-parent = <&intc>; + + phy-mode = "xgmii"; + phy-handle = <ðernet_phy1>; + + ethernet_phy1: ethernet-phy@4 { + compatible = "ethernet-phy-ieee802.3-c45"; + reg = <4>; + }; + }; -- 2.16.1
[PATCH v3 2/2] net: ethernet: nixge: Add support for National Instruments XGE netdev
Add support for the National Instruments XGE 1/10G network device. It uses the EEPROM on the board via NVMEM. Signed-off-by: Moritz Fischer --- Changes from v2: - Implement recv side NAPI - Improved error handling - Implemented C45 writes - Added ethtool callbacks & blink functionality - Improved nixge_ctrl_poll_timeout() macro - Removed dev_dbg() for mdio accesses - Added businfo to ethtool drvinfo Changes from v1: - Added dependency on ARCH_ZYNQ (Kbuild) - Removed unused variables - Use of_phy_connect as suggested - Removed masking of (un)supported modes - Added #define for some constants - Removed empty pm functions - Reworked mac_address handling - Made nixge_mdio_*() static (sparse) - Removed driver version - Addressed timeout loop - Adressed return values on timeout --- drivers/net/ethernet/Kconfig |1 + drivers/net/ethernet/Makefile|1 + drivers/net/ethernet/ni/Kconfig | 27 + drivers/net/ethernet/ni/Makefile |1 + drivers/net/ethernet/ni/nixge.c | 1352 ++ 5 files changed, 1382 insertions(+) create mode 100644 drivers/net/ethernet/ni/Kconfig create mode 100644 drivers/net/ethernet/ni/Makefile create mode 100644 drivers/net/ethernet/ni/nixge.c diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig index b6cf4b6962f5..908218561fdd 100644 --- a/drivers/net/ethernet/Kconfig +++ b/drivers/net/ethernet/Kconfig @@ -129,6 +129,7 @@ config FEALNX source "drivers/net/ethernet/natsemi/Kconfig" source "drivers/net/ethernet/netronome/Kconfig" +source "drivers/net/ethernet/ni/Kconfig" source "drivers/net/ethernet/8390/Kconfig" config NET_NETX diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile index 3cdf01e96e0b..d732e9522b76 100644 --- a/drivers/net/ethernet/Makefile +++ b/drivers/net/ethernet/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_NET_VENDOR_MYRI) += myricom/ obj-$(CONFIG_FEALNX) += fealnx.o obj-$(CONFIG_NET_VENDOR_NATSEMI) += natsemi/ obj-$(CONFIG_NET_VENDOR_NETRONOME) += netronome/ +obj-$(CONFIG_NET_VENDOR_NI) += ni/ obj-$(CONFIG_NET_NETX) += netx-eth.o obj-$(CONFIG_NET_VENDOR_NUVOTON) += nuvoton/ obj-$(CONFIG_NET_VENDOR_NVIDIA) += nvidia/ diff --git a/drivers/net/ethernet/ni/Kconfig b/drivers/net/ethernet/ni/Kconfig new file mode 100644 index ..cd30f7de16de --- /dev/null +++ b/drivers/net/ethernet/ni/Kconfig @@ -0,0 +1,27 @@ +# +# National Instuments network device configuration +# + +config NET_VENDOR_NI + bool "National Instruments Devices" + default y + ---help--- + If you have a network (Ethernet) device belonging to this class, say Y. + + Note that the answer to this question doesn't directly affect the + kernel: saying N will just cause the configurator to skip all + the questions about National Instrument devices. + If you say Y, you will be asked for your specific device in the + following questions. + +if NET_VENDOR_NI + +config NI_XGE_MANAGEMENT_ENET + tristate "National Instruments XGE management enet support" + depends on ARCH_ZYNQ + select PHYLIB + ---help--- + Simple LAN device for debug or management purposes. Can + support either 10G or 1G PHYs via SFP+ ports. + +endif diff --git a/drivers/net/ethernet/ni/Makefile b/drivers/net/ethernet/ni/Makefile new file mode 100644 index ..99c664651c51 --- /dev/null +++ b/drivers/net/ethernet/ni/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_NI_XGE_MANAGEMENT_ENET) += nixge.o diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c new file mode 100644 index ..9b255c23d7cd --- /dev/null +++ b/drivers/net/ethernet/ni/nixge.c @@ -0,0 +1,1352 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2016-2017, National Instruments Corp. + * + * Author: Moritz Fischer + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define TX_BD_NUM 64 +#define RX_BD_NUM 128 + +/* Axi DMA Register definitions */ + +#define XAXIDMA_TX_CR_OFFSET 0x /* Channel control */ +#define XAXIDMA_TX_SR_OFFSET 0x0004 /* Status */ +#define XAXIDMA_TX_CDESC_OFFSET0x0008 /* Current descriptor pointer */ +#define XAXIDMA_TX_TDESC_OFFSET0x0010 /* Tail descriptor pointer */ + +#define XAXIDMA_RX_CR_OFFSET 0x0030 /* Channel control */ +#define XAXIDMA_RX_SR_OFFSET 0x0034 /* Status */ +#define XAXIDMA_RX_CDESC_OFFSET0x0038 /* Current descriptor pointer */ +#define XAXIDMA_RX_TDESC_OFFSET0x0040 /* Tail descriptor pointer */ + +#define XAXIDMA_CR_RUNSTOP_MASK0x0001 /* Start/stop DMA channel */ +#define XAXIDMA_CR_RESET_MASK 0x0004 /* Reset DMA engine */ + +#define XAXIDMA_BD_NDE
Re: [PATCH v2 1/2] dt-bindings: net: Add bindings for National Instruments XGE netdev
On Sat, Jul 15, 2017 at 09:48:32PM +0200, Andrew Lunn wrote: > > > > + ethernet_phy1: ethernet-phy@4 { > > > > + compatible = "ethernet-phy-ieee802.3-c45"; > > > > + reg = <4>; > > > > + devices = <0xa>; > > > > + }; > > > > > > Since you don't fully implement c45, does this example actually work? > > > > Yeah, I've tested this continuously. But for v3 I anyways implmented c45 > > writes. > > Hi Moritz > > Just out of interest, what PHY are you using? Depending on whether the FPGA image is configured either: - Xilinx 10G PCS/PMA LogiCore IP (C45) - Xilinx LogiCORE IP Ethernet 1000Base-X PCS/PMA (C22) in between that and the DMA engine there's a bunch of custom stuff (will be open source once the product ships). Moritz signature.asc Description: PGP signature
Re: [PATCH v2 1/2] dt-bindings: net: Add bindings for National Instruments XGE netdev
Hi Andrew, On Sat, Jul 15, 2017 at 08:37:45PM +0200, Andrew Lunn wrote: > On Fri, Jul 14, 2017 at 01:48:45PM -0700, Moritz Fischer wrote: > > This adds bindings for the NI XGE 1G/10G network device. > > > > Signed-off-by: Moritz Fischer > > --- > > Documentation/devicetree/bindings/net/nixge.txt | 32 > > + > > 1 file changed, 32 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/net/nixge.txt > > > > diff --git a/Documentation/devicetree/bindings/net/nixge.txt > > b/Documentation/devicetree/bindings/net/nixge.txt > > new file mode 100644 > > index 000..9fff5a7 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/nixge.txt > > @@ -0,0 +1,32 @@ > > +* NI XGE Ethernet controller > > + > > +Required properties: > > +- compatible: Should be "ni,xge-enet-2.00" > > +- reg: Address and length of the register set for the device > > +- interrupts: Should contain tx and rx interrupt > > +- interrupt-names: Should be "rx-irq" and "tx-irq" > > +- phy-mode: See ethernet.txt file in the same directory. > > Hi Moritz > > phy-handle is now required. Good catch, thanks. > > > +Examples (10G generic PHY): > > + nixge0: ethernet@4000 { > > + compatible = "ni,xge-enet-2.00"; > > + reg = <0x4000 0x6000>; > > + > > + nvmem-cells = <ð1_addr>; > > + nvmem-cell-names = "address"; > > + > > + interrupts = <0 29 4>, <0 30 4>; > > IRQ_TYPE_LEVEL_HIGH Sure, will do. > > > + interrupt-names = "rx-irq", "tx-irq"; > > + interrupt-parent = <&intc>; > > + > > + phy-mode = "xgmii"; > > + phy-handle = <ðernet_phy1>; > > + > > + ethernet_phy1: ethernet-phy@4 { > > + compatible = "ethernet-phy-ieee802.3-c45"; > > + reg = <4>; > > + devices = <0xa>; > > + }; > > Since you don't fully implement c45, does this example actually work? Yeah, I've tested this continuously. But for v3 I anyways implmented c45 writes. > And devices is not a standard phy property. > Will fix. > Andrew Cheers, Moritz
[PATCH v2 1/2] dt-bindings: net: Add bindings for National Instruments XGE netdev
This adds bindings for the NI XGE 1G/10G network device. Signed-off-by: Moritz Fischer --- Documentation/devicetree/bindings/net/nixge.txt | 32 + 1 file changed, 32 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/nixge.txt diff --git a/Documentation/devicetree/bindings/net/nixge.txt b/Documentation/devicetree/bindings/net/nixge.txt new file mode 100644 index 000..9fff5a7 --- /dev/null +++ b/Documentation/devicetree/bindings/net/nixge.txt @@ -0,0 +1,32 @@ +* NI XGE Ethernet controller + +Required properties: +- compatible: Should be "ni,xge-enet-2.00" +- reg: Address and length of the register set for the device +- interrupts: Should contain tx and rx interrupt +- interrupt-names: Should be "rx-irq" and "tx-irq" +- phy-mode: See ethernet.txt file in the same directory. +- nvmem-cells: Phandle of nvmem cell containing the mac address +- nvmem-cell-names: Should be "address" + +Examples (10G generic PHY): + nixge0: ethernet@4000 { + compatible = "ni,xge-enet-2.00"; + reg = <0x4000 0x6000>; + + nvmem-cells = <ð1_addr>; + nvmem-cell-names = "address"; + + interrupts = <0 29 4>, <0 30 4>; + interrupt-names = "rx-irq", "tx-irq"; + interrupt-parent = <&intc>; + + phy-mode = "xgmii"; + phy-handle = <ðernet_phy1>; + + ethernet_phy1: ethernet-phy@4 { + compatible = "ethernet-phy-ieee802.3-c45"; + reg = <4>; + devices = <0xa>; + }; + }; -- 2.7.4
[PATCH v2 2/2] net: ethernet: nixge: Add support for National Instruments XGE netdev
Add support for the National Instruments XGE 1/10G network device. It uses the EEPROM on the board via NVMEM. Signed-off-by: Moritz Fischer --- Changes from v1: - Added dependency on ARCH_ZYNQ (Kbuild) - Removed unused variables - Use of_phy_connect as suggested - Removed masking of (un)supported modes - Added #define for some constants - Removed empty pm functions - Reworked mac_address handling - Made nixge_mdio_*() static (sparse) - Removed driver version - Addressed timeout loop - Adressed return values on timeout --- drivers/net/ethernet/Kconfig |1 + drivers/net/ethernet/Makefile|1 + drivers/net/ethernet/ni/Kconfig | 27 + drivers/net/ethernet/ni/Makefile |1 + drivers/net/ethernet/ni/nixge.c | 1190 ++ 5 files changed, 1220 insertions(+) create mode 100644 drivers/net/ethernet/ni/Kconfig create mode 100644 drivers/net/ethernet/ni/Makefile create mode 100644 drivers/net/ethernet/ni/nixge.c diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig index edae15ac..2021806 100644 --- a/drivers/net/ethernet/Kconfig +++ b/drivers/net/ethernet/Kconfig @@ -127,6 +127,7 @@ config FEALNX source "drivers/net/ethernet/natsemi/Kconfig" source "drivers/net/ethernet/netronome/Kconfig" +source "drivers/net/ethernet/ni/Kconfig" source "drivers/net/ethernet/8390/Kconfig" config NET_NETX diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile index bf7f450..68f49f7 100644 --- a/drivers/net/ethernet/Makefile +++ b/drivers/net/ethernet/Makefile @@ -58,6 +58,7 @@ obj-$(CONFIG_NET_VENDOR_MYRI) += myricom/ obj-$(CONFIG_FEALNX) += fealnx.o obj-$(CONFIG_NET_VENDOR_NATSEMI) += natsemi/ obj-$(CONFIG_NET_VENDOR_NETRONOME) += netronome/ +obj-$(CONFIG_NET_VENDOR_NI) += ni/ obj-$(CONFIG_NET_NETX) += netx-eth.o obj-$(CONFIG_NET_VENDOR_NUVOTON) += nuvoton/ obj-$(CONFIG_NET_VENDOR_NVIDIA) += nvidia/ diff --git a/drivers/net/ethernet/ni/Kconfig b/drivers/net/ethernet/ni/Kconfig new file mode 100644 index 000..cd30f7d --- /dev/null +++ b/drivers/net/ethernet/ni/Kconfig @@ -0,0 +1,27 @@ +# +# National Instuments network device configuration +# + +config NET_VENDOR_NI + bool "National Instruments Devices" + default y + ---help--- + If you have a network (Ethernet) device belonging to this class, say Y. + + Note that the answer to this question doesn't directly affect the + kernel: saying N will just cause the configurator to skip all + the questions about National Instrument devices. + If you say Y, you will be asked for your specific device in the + following questions. + +if NET_VENDOR_NI + +config NI_XGE_MANAGEMENT_ENET + tristate "National Instruments XGE management enet support" + depends on ARCH_ZYNQ + select PHYLIB + ---help--- + Simple LAN device for debug or management purposes. Can + support either 10G or 1G PHYs via SFP+ ports. + +endif diff --git a/drivers/net/ethernet/ni/Makefile b/drivers/net/ethernet/ni/Makefile new file mode 100644 index 000..99c6646 --- /dev/null +++ b/drivers/net/ethernet/ni/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_NI_XGE_MANAGEMENT_ENET) += nixge.o diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c new file mode 100644 index 000..6b52683 --- /dev/null +++ b/drivers/net/ethernet/ni/nixge.c @@ -0,0 +1,1190 @@ +/* + * Copyright (c) 2016-2017, National Instruments Corp. + * + * Network Driver for Ettus Research XGE MAC + * + * This is largely based on the Xilinx AXI Ethernet Driver, + * and uses the same DMA engine in the FPGA + * + * SPDX-License-Identifier: GPL-2.0 + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define TX_BD_NUM 64 +#define RX_BD_NUM 128 + +/* Axi DMA Register definitions */ + +#define XAXIDMA_TX_CR_OFFSET 0x /* Channel control */ +#define XAXIDMA_TX_SR_OFFSET 0x0004 /* Status */ +#define XAXIDMA_TX_CDESC_OFFSET0x0008 /* Current descriptor pointer */ +#define XAXIDMA_TX_TDESC_OFFSET0x0010 /* Tail descriptor pointer */ + +#define XAXIDMA_RX_CR_OFFSET 0x0030 /* Channel control */ +#define XAXIDMA_RX_SR_OFFSET 0x0034 /* Status */ +#define XAXIDMA_RX_CDESC_OFFSET0x0038 /* Current descriptor pointer */ +#define XAXIDMA_RX_TDESC_OFFSET0x0040 /* Tail descriptor pointer */ + +#define XAXIDMA_CR_RUNSTOP_MASK0x0001 /* Start/stop DMA channel */ +#define XAXIDMA_CR_RESET_MASK 0x0004 /* Reset DMA engine */ + +#define XAXIDMA_BD_NDESC_OFFSET0x00 /* Next descriptor pointer */ +#define XAXIDMA_BD_BUFA_OFFSET 0x08 /* Buffer address */ +#define XAXIDMA_BD_CTRL_LEN_OFFSET 0x18 /* Cont
Re: [PATCH 2/2] net: ethernet: nixge: Add support for National Instruments XGE netdev
Hi Andrew, On Thu, Jul 13, 2017 at 6:34 PM, Andrew Lunn wrote: >> > > + /* not sure if this is the correct way of dealing with this ... */ >> > > + ndev->phydev->supported &= ~(SUPPORTED_Autoneg); >> > > + ndev->phydev->advertising = ndev->phydev->supported; >> > > + ndev->phydev->autoneg = AUTONEG_DISABLE; >> > >> > What are you trying to achieve? >> >> Basically can't do Autoneg, I'll need to take a closer look. > > Hi Moritz > > What i actually think you mean, is it can only do 1Gbps. So you could > autoneg, but only advertise 1Gbps. Look at masking out > PHY_10BT_FEATURES and PHY_100BT_FEATURES. It does either 1Gbps or 10Gbps (over SFP+), depending which bitstream is loaded into the FPGA. In the current setup I could also just have two different compatible strings, since neither setup supports the other rate, but that might change. It seems getting rid of that part (the default values) now works, too. I'll need to take a closer look tomorrow (and I need to retest with 1Gbps) > > Take a look at: > > http://elixir.free-electrons.com/linux/latest/source/drivers/net/ethernet/renesas/ravb_main.c#L1045 Will do. Thanks for feedback, Moritz
Re: [PATCH 1/2] dt-bindings: net: Add bindings for National Instruments XGE netdev
Hi Yuan, On Thu, Jul 13, 2017 at 5:33 PM, YUAN Linyu wrote: > > >> -Original Message- >> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] >> On Behalf Of Moritz Fischer >> Sent: Friday, July 14, 2017 5:22 AM >> To: netdev@vger.kernel.org >> Cc: devicet...@vger.kernel.org; linux-ker...@vger.kernel.org; >> da...@davemloft.net; mark.rutl...@arm.com; robh...@kernel.org; Moritz >> Fischer >> Subject: [PATCH 1/2] dt-bindings: net: Add bindings for National Instruments >> XGE netdev >> >> This adds bindings for the NI XGE 1G/10G network device. >> >> Signed-off-by: Moritz Fischer >> --- >> Documentation/devicetree/bindings/net/nixge.c | 32 >> +++ >> 1 file changed, 32 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/net/nixge.c > > It should be a text file, nixge.txt You are absolutely right ... I need to have my head examined. > >> >> diff --git a/Documentation/devicetree/bindings/net/nixge.c >> b/Documentation/devicetree/bindings/net/nixge.c >> new file mode 100644 >> index 000..9fff5a7
Re: [PATCH 2/2] net: ethernet: nixge: Add support for National Instruments XGE netdev
Hi Andrew, thanks for the quick response. On Fri, Jul 14, 2017 at 12:36:36AM +0200, Andrew Lunn wrote: > > +++ b/drivers/net/ethernet/ni/nixge.c > > @@ -0,0 +1,1246 @@ > > +/* > > + * Copyright (c) 2016-2017, National Instruments Corp. > > + * > > + * Network Driver for Ettus Research XGE MAC > > + * > > + * This is largely based on the Xilinx AXI Ethernet Driver, > > + * and uses the same DMA engine in the FPGA > > Hi Moritz > > Is the DMA code the same as in the AXI driver? Should it be pulled out > into a library and shared? Mostly, I'll see what I can do. At least the register definitions and common structures can be pulled out into a common header file. > > > +struct nixge_priv { > > + struct net_device *ndev; > > + struct device *dev; > > + > > + /* Connection to PHY device */ > > + struct phy_device *phy_dev; > > + phy_interface_t phy_interface; > > > + /* protecting link parameters */ > > + spinlock_t lock; > > + int link; > > + int speed; > > + int duplex; > > All these seem to be pointless. They are set, but never used. Will fix. > > > + > > +static inline void nixge_dma_write_reg(struct nixge_priv *priv, off_t > > offset, > > + u32 val) > > Please leave it up to the compile to inline. Will fix. > > > +static void __nixge_device_reset(struct nixge_priv *priv, off_t offset) > > +{ > > + u32 timeout; > > + /* Reset Axi DMA. This would reset NIXGE Ethernet core as well. > > +* The reset process of Axi DMA takes a while to complete as all > > +* pending commands/transfers will be flushed or completed during > > +* this reset process. > > +*/ > > + nixge_dma_write_reg(priv, offset, XAXIDMA_CR_RESET_MASK); > > + timeout = DELAY_OF_ONE_MILLISEC; > > + while (nixge_dma_read_reg(priv, offset) & XAXIDMA_CR_RESET_MASK) { > > + udelay(1); > > There is a link between the 1 and the value of DELAY_OF_ONE_MILLISEC. > It would be good to try to link these two together. D'oh ... Seems like a good candidate for iopoll ... > > > + if (--timeout == 0) { > > + netdev_err(priv->ndev, "%s: DMA reset timeout!\n", > > + __func__); > > + break; > > + } > > + } > > +} > > + > > > +static void nixge_handle_link_change(struct net_device *ndev) > > +{ > > + struct nixge_priv *priv = netdev_priv(ndev); > > + struct phy_device *phydev = ndev->phydev; > > + unsigned long flags; > > + int status_change = 0; > > + > > + spin_lock_irqsave(&priv->lock, flags); > > + > > + if (phydev->link != priv->link) { > > + if (!phydev->link) { > > + priv->speed = 0; > > + priv->duplex = -1; > > + } > > + priv->link = phydev->link; > > + > > + status_change = 1; > > + } > > + > > + spin_unlock_irqrestore(&priv->lock, flags); > > + > > + if (status_change) { > > + if (phydev->link) { > > + netif_carrier_on(ndev); > > + netdev_info(ndev, "link up (%d/%s)\n", > > + phydev->speed, > > + phydev->duplex == DUPLEX_FULL ? > > + "Full" : "Half"); > > + } else { > > + netif_carrier_off(ndev); > > + netdev_info(ndev, "link down\n"); > > + } > > phy_print_status() should be used. Will do. > > Also, the phylib will handle netif_carrier_off/on for you. Good to know :) > > > +static int nixge_open(struct net_device *ndev) > > +{ > > + struct nixge_priv *priv = netdev_priv(ndev); > > + int ret; > > + > > + nixge_device_reset(ndev); > > + > > + /* start netif carrier down */ > > + netif_carrier_off(ndev); > > + > > + if (!ndev->phydev) > > + netdev_err(ndev, "no phy, phy_start() failed\n"); > > Not really correct. You don't call phy_start(). And phy_start() cannot > indicate a failure, it is a void function. Will fix. > > It would be a lot better to bail out if there is no phy. Probably > during probe. Yeah. > > > +static s32 __nixge_set_mac_address(struct net_device *ndev, const void > > *addr) > > +{ > > + struct nixge_priv *priv = netdev_priv(ndev); > > + > > + if (addr) > > + memcpy(ndev->dev_addr, addr, ETH_ALEN); > > + if (!is_valid_ether_addr(ndev->dev_addr)) > > + eth_random_addr(ndev->dev_addr); > > Messy. I would change this. Make addr mandatory. If it is invalid, > return an error. That will make nixge_net_set_mac_address() do the > right thing. When called from nixge_probe() should verify what it gets > from the nvmem, and if it is invalid, pass a random MAC address. Will fix. > > > + > > + nixge_ctrl_write_reg(priv, NIXGE_REG_MAC_LSB, > > +(ndev->dev_addr[2]) << 24 | > > +(ndev->dev_addr[3] << 16) | > > +(ndev->dev_addr[4]
[PATCH 1/2] dt-bindings: net: Add bindings for National Instruments XGE netdev
This adds bindings for the NI XGE 1G/10G network device. Signed-off-by: Moritz Fischer --- Documentation/devicetree/bindings/net/nixge.c | 32 +++ 1 file changed, 32 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/nixge.c diff --git a/Documentation/devicetree/bindings/net/nixge.c b/Documentation/devicetree/bindings/net/nixge.c new file mode 100644 index 000..9fff5a7 --- /dev/null +++ b/Documentation/devicetree/bindings/net/nixge.c @@ -0,0 +1,32 @@ +* NI XGE Ethernet controller + +Required properties: +- compatible: Should be "ni,xge-enet-2.00" +- reg: Address and length of the register set for the device +- interrupts: Should contain tx and rx interrupt +- interrupt-names: Should be "rx-irq" and "tx-irq" +- phy-mode: See ethernet.txt file in the same directory. +- nvmem-cells: Phandle of nvmem cell containing the mac address +- nvmem-cell-names: Should be "address" + +Examples (10G generic PHY): + nixge0: ethernet@4000 { + compatible = "ni,xge-enet-2.00"; + reg = <0x4000 0x6000>; + + nvmem-cells = <ð1_addr>; + nvmem-cell-names = "address"; + + interrupts = <0 29 4>, <0 30 4>; + interrupt-names = "rx-irq", "tx-irq"; + interrupt-parent = <&intc>; + + phy-mode = "xgmii"; + phy-handle = <ðernet_phy1>; + + ethernet_phy1: ethernet-phy@4 { + compatible = "ethernet-phy-ieee802.3-c45"; + reg = <4>; + devices = <0xa>; + }; + }; -- 2.7.4
[PATCH 2/2] net: ethernet: nixge: Add support for National Instruments XGE netdev
Add support for the National Instruments XGE 1/10G network device. It uses the EEPROM on the board via NVMEM. Signed-off-by: Moritz Fischer --- drivers/net/ethernet/Kconfig |1 + drivers/net/ethernet/Makefile|1 + drivers/net/ethernet/ni/Kconfig | 26 + drivers/net/ethernet/ni/Makefile |1 + drivers/net/ethernet/ni/nixge.c | 1246 ++ 5 files changed, 1275 insertions(+) create mode 100644 drivers/net/ethernet/ni/Kconfig create mode 100644 drivers/net/ethernet/ni/Makefile create mode 100644 drivers/net/ethernet/ni/nixge.c diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig index edae15ac..2021806 100644 --- a/drivers/net/ethernet/Kconfig +++ b/drivers/net/ethernet/Kconfig @@ -127,6 +127,7 @@ config FEALNX source "drivers/net/ethernet/natsemi/Kconfig" source "drivers/net/ethernet/netronome/Kconfig" +source "drivers/net/ethernet/ni/Kconfig" source "drivers/net/ethernet/8390/Kconfig" config NET_NETX diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile index bf7f450..68f49f7 100644 --- a/drivers/net/ethernet/Makefile +++ b/drivers/net/ethernet/Makefile @@ -58,6 +58,7 @@ obj-$(CONFIG_NET_VENDOR_MYRI) += myricom/ obj-$(CONFIG_FEALNX) += fealnx.o obj-$(CONFIG_NET_VENDOR_NATSEMI) += natsemi/ obj-$(CONFIG_NET_VENDOR_NETRONOME) += netronome/ +obj-$(CONFIG_NET_VENDOR_NI) += ni/ obj-$(CONFIG_NET_NETX) += netx-eth.o obj-$(CONFIG_NET_VENDOR_NUVOTON) += nuvoton/ obj-$(CONFIG_NET_VENDOR_NVIDIA) += nvidia/ diff --git a/drivers/net/ethernet/ni/Kconfig b/drivers/net/ethernet/ni/Kconfig new file mode 100644 index 000..a74ffeb --- /dev/null +++ b/drivers/net/ethernet/ni/Kconfig @@ -0,0 +1,26 @@ +# +# National Instuments network device configuration +# + +config NET_VENDOR_NI + bool "National Instruments Devices" + default y + ---help--- + If you have a network (Ethernet) device belonging to this class, say Y. + + Note that the answer to this question doesn't directly affect the + kernel: saying N will just cause the configurator to skip all + the questions about National Instrument devices. + If you say Y, you will be asked for your specific device in the + following questions. + +if NET_VENDOR_NI + +config NI_XGE_MANAGEMENT_ENET + tristate "National Instruments XGE management enet support" + select PHYLIB + ---help--- + Simple LAN device for debug or management purposes. Can + support either 10G or 1G PHYs via SFP+ ports. + +endif diff --git a/drivers/net/ethernet/ni/Makefile b/drivers/net/ethernet/ni/Makefile new file mode 100644 index 000..99c6646 --- /dev/null +++ b/drivers/net/ethernet/ni/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_NI_XGE_MANAGEMENT_ENET) += nixge.o diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c new file mode 100644 index 000..85b213c --- /dev/null +++ b/drivers/net/ethernet/ni/nixge.c @@ -0,0 +1,1246 @@ +/* + * Copyright (c) 2016-2017, National Instruments Corp. + * + * Network Driver for Ettus Research XGE MAC + * + * This is largely based on the Xilinx AXI Ethernet Driver, + * and uses the same DMA engine in the FPGA + * + * SPDX-License-Identifier: GPL-2.0 + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define TX_BD_NUM 64 +#define RX_BD_NUM 128 + +#define DELAY_OF_ONE_MILLISEC 1000 + +/* Axi DMA Register definitions */ + +#define XAXIDMA_TX_CR_OFFSET 0x /* Channel control */ +#define XAXIDMA_TX_SR_OFFSET 0x0004 /* Status */ +#define XAXIDMA_TX_CDESC_OFFSET0x0008 /* Current descriptor pointer */ +#define XAXIDMA_TX_TDESC_OFFSET0x0010 /* Tail descriptor pointer */ + +#define XAXIDMA_RX_CR_OFFSET 0x0030 /* Channel control */ +#define XAXIDMA_RX_SR_OFFSET 0x0034 /* Status */ +#define XAXIDMA_RX_CDESC_OFFSET0x0038 /* Current descriptor pointer */ +#define XAXIDMA_RX_TDESC_OFFSET0x0040 /* Tail descriptor pointer */ + +#define XAXIDMA_CR_RUNSTOP_MASK0x0001 /* Start/stop DMA channel */ +#define XAXIDMA_CR_RESET_MASK 0x0004 /* Reset DMA engine */ + +#define XAXIDMA_BD_NDESC_OFFSET0x00 /* Next descriptor pointer */ +#define XAXIDMA_BD_BUFA_OFFSET 0x08 /* Buffer address */ +#define XAXIDMA_BD_CTRL_LEN_OFFSET 0x18 /* Control/buffer length */ +#define XAXIDMA_BD_STS_OFFSET 0x1C /* Status */ +#define XAXIDMA_BD_USR0_OFFSET 0x20 /* User IP specific word0 */ +#define XAXIDMA_BD_USR1_OFFSET 0x24 /* User IP specific word1 */ +#define XAXIDMA_BD_USR2_OFFSET 0x28 /* User IP specific word2 */ +#define XAXIDMA_BD_USR3_OFFSET 0x2C /* User IP specific
Re: [PATCH] net: ethernet: cadence: Add fixed-link functionality
Andrew, On Wed, Feb 15, 2017 at 2:12 PM, Andrew Lunn wrote: >> @@ -3342,8 +3371,18 @@ static int macb_probe(struct platform_device *pdev) >> macb_get_hwaddr(bp); >> >> /* Power up the PHY if there is a GPIO reset */ >> - phy_node = of_get_next_available_child(np, NULL); >> - if (phy_node) { >> + phy_node = of_parse_phandle(np, "phy-handle", 0); >> + if (!phy_node && of_phy_is_fixed_link(np)) { >> + err = of_phy_register_fixed_link(np); > > Hi Moritz > > I don't see any calls to of_phy_deregister_fixed_link(), either in the > error path, or the remove code. Whoops, yeah I rebased it from like a year ago. Must've not survived my merge conflict resolution. I'll rework it. Thanks, Moritz
Re: [PATCH] net: ethernet: cadence: Add fixed-link functionality
Hi Florian, thanks for the quick reply. On Wed, Feb 15, 2017 at 12:57 PM, Florian Fainelli wrote: > On 02/15/2017 12:44 PM, m...@kernel.org wrote: >> From: Moritz Fischer >> >> This allows 'fixed-link' direct MAC connections to be declared >> in devicetree. >> >> Signed-off-by: Moritz Fischer >> Cc: Nicolas Ferre >> --- >> drivers/net/ethernet/cadence/macb.c | 61 >> ++--- >> drivers/net/ethernet/cadence/macb.h | 1 + >> 2 files changed, 57 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/net/ethernet/cadence/macb.c >> b/drivers/net/ethernet/cadence/macb.c >> index 30606b1..af443a8 100644 >> --- a/drivers/net/ethernet/cadence/macb.c >> +++ b/drivers/net/ethernet/cadence/macb.c >> @@ -469,6 +469,34 @@ static int macb_mii_probe(struct net_device *dev) >> return 0; >> } >> >> +static int macb_fixed_init(struct macb *bp) >> +{ >> + struct phy_device *phydev; >> + >> + phydev = of_phy_connect(bp->dev, bp->phy_node, >> + &macb_handle_link_change, 0, >> + bp->phy_interface); >> + if (!phydev) >> + return -ENODEV; >> + >> + /* mask with MAC supported features */ >> + if (macb_is_gem(bp) && bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) >> + phydev->supported &= PHY_GBIT_FEATURES; >> + else >> + phydev->supported &= PHY_BASIC_FEATURES; >> + >> + if (bp->caps & MACB_CAPS_NO_GIGABIT_HALF) >> + phydev->supported &= ~SUPPORTED_1000baseT_Half; >> + >> + phydev->advertising = phydev->supported; >> + >> + bp->link = 0; >> + bp->speed = 0; >> + bp->duplex = -1; >> + >> + return 0; >> +} > > This is nearly identical to macb_mii_probe(), can you try to re-use what > is done there and just extract the phy_find_first() part which is > different here? Yeah. It probably still needs some work. > >> + >> static int macb_mii_init(struct macb *bp) >> { >> struct macb_platform_data *pdata; >> @@ -3245,6 +3273,7 @@ static int macb_probe(struct platform_device *pdev) >> const char *mac; >> struct macb *bp; >> int err; >> + bool fixed_link = false; >> >> regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> mem = devm_ioremap_resource(&pdev->dev, regs); >> @@ -3342,8 +3371,18 @@ static int macb_probe(struct platform_device *pdev) >> macb_get_hwaddr(bp); >> >> /* Power up the PHY if there is a GPIO reset */ >> - phy_node = of_get_next_available_child(np, NULL); >> - if (phy_node) { >> + phy_node = of_parse_phandle(np, "phy-handle", 0); >> + if (!phy_node && of_phy_is_fixed_link(np)) { >> + err = of_phy_register_fixed_link(np); >> + if (err < 0) { >> + dev_err(&pdev->dev, "broken fixed-link specification"); >> + goto failed_phy; >> + } >> + /* in case of a fixed PHY, the DT node is the ethernet MAC */ >> + phy_node = of_node_get(np); >> + bp->phy_node = phy_node; >> + fixed_link = true; >> + } else { >> int gpio = of_get_named_gpio(phy_node, "reset-gpios", 0); >> >> if (gpio_is_valid(gpio)) { >> @@ -3369,7 +3408,10 @@ static int macb_probe(struct platform_device *pdev) >> if (err) >> goto err_out_free_netdev; >> >> - err = macb_mii_init(bp); >> + if (!fixed_link) >> + err = macb_mii_init(bp); >> + else >> + err = macb_fixed_init(bp); >> if (err) >> goto err_out_free_netdev; >> >> @@ -3400,6 +3442,9 @@ static int macb_probe(struct platform_device *pdev) >> if (bp->reset_gpio) >> gpiod_set_value(bp->reset_gpio, 0); >> >> +failed_phy: >> + of_node_put(phy_node); >> + >> err_out_free_netdev: >> free_netdev(dev); >> >> @@ -3423,9 +3468,14 @@ static int macb_remove(struct platform_device *pdev) >> bp = netdev_priv(dev); >> if (dev->phydev) >> phy_disconnect(dev->phydev); >> - mdiobus_unregister(bp->mii_bus); >> + >
Re: [PATCH] net: macb: NULL out phydev after removing mdio bus
On Fri, Oct 7, 2016 at 8:13 AM, Xander Huff wrote: > From: Nathan Sullivan > > To ensure the dev->phydev pointer is not used after becoming invalid in > mdiobus_unregister, set it to NULL. This happens when removing the macb > driver without first taking its interface down, since unregister_netdev > will end up calling macb_close. > > Signed-off-by: Xander Huff > Signed-off-by: Nathan Sullivan > Signed-off-by: Brad Mouring Reviewed-by: Moritz Fischer
Re: Supporting C45 PHY without ID registers
Hi Andrew, On Mon, Jun 27, 2016 at 5:56 PM, Andrew Lunn wrote: > Does it have any ID registers at all? There is a vendor specific (to my knowledge) register at device 1 register 65535 ([1]) that could be read back. I haven't seen anyone else do that. Thanks, Moritz [1] http://www.xilinx.com/support/documentation/ip_documentation/ten_gig_eth_pcs_pma/v6_0/pg068-ten-gig-eth-pcs-pma.pdf
Supporting C45 PHY without ID registers
Hi all, I have a 10GigE PHY that I'm working with that has most of it's functionality availabile via MDIO in a clause 45 compliant fashion, however the usual probe method fails since the id registers are not implemented. I hacked up drivers/of/of_mdio.c to include something similar to of_get_phy_id() for c45 phys but I was wondering if someone else has a better idea than this in my dt: ethernet_phy1: ethernet-phy@4 { compatible = "ethernet-phy-id.", "ethernet-phy-id4242.4242", "ethernet-phy-id.", "ethernet-phy-id4343.4343", "ethernet-phy-ieee802.3-c45"; reg = <4>; }; Where I made up 42424242 and 43434343 as ids for my PCS / PMA. Ideas? Cheers, Moritz
[PATCH v3 1/5] net: macb: Fix coding style error message
checkpatch.pl gave the following error: ERROR: space required before the open parenthesis '(' + for(; p < end; p++, offset += 4) Acked-by: Nicolas Ferre Acked-by: Michal Simek Signed-off-by: Moritz Fischer --- drivers/net/ethernet/cadence/macb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index 48a7d7d..b5aa96e 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -499,7 +499,7 @@ static void macb_update_stats(struct macb *bp) WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4); - for(; p < end; p++, offset += 4) + for (; p < end; p++, offset += 4) *p += bp->macb_reg_readl(bp, offset); } -- 2.7.4
[PATCH v3 5/5] net: macb: Fix simple typo
Acked-by: Michal Simek Acked-by: Nicolas Ferre Signed-off-by: Moritz Fischer --- drivers/net/ethernet/cadence/macb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index 01a8ffb..eec3200 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -130,7 +130,7 @@ static void hw_writel(struct macb *bp, int offset, u32 value) } /* Find the CPU endianness by using the loopback bit of NCR register. When the - * CPU is in big endian we need to program swaped mode for management + * CPU is in big endian we need to program swapped mode for management * descriptor access. */ static bool hw_is_native_io(void __iomem *addr) -- 2.7.4
[PATCH v3 3/5] net: macb: Fix coding style suggestions
This commit deals with a bunch of checkpatch suggestions that without changing behavior make checkpatch happier. Acked-by: Michal Simek Acked-by: Nicolas Ferre Signed-off-by: Moritz Fischer --- drivers/net/ethernet/cadence/macb.c | 46 +++-- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index f25681b..2ba0934 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -187,7 +187,7 @@ static void macb_get_hwaddr(struct macb *bp) pdata = dev_get_platdata(&bp->pdev->dev); - /* Check all 4 address register for vaild address */ + /* Check all 4 address register for valid address */ for (i = 0; i < 4; i++) { bottom = macb_or_gem_readl(bp, SA1B + i * 8); top = macb_or_gem_readl(bp, SA1T + i * 8); @@ -295,7 +295,7 @@ static void macb_set_tx_clk(struct clk *clk, int speed, struct net_device *dev) ferr = DIV_ROUND_UP(ferr, rate / 10); if (ferr > 5) netdev_warn(dev, "unable to generate target frequency: %ld Hz\n", - rate); + rate); if (clk_set_rate(clk, rate_rounded)) netdev_err(dev, "adjusting tx_clk failed.\n"); @@ -429,7 +429,7 @@ static int macb_mii_init(struct macb *bp) macb_writel(bp, NCR, MACB_BIT(MPE)); bp->mii_bus = mdiobus_alloc(); - if (bp->mii_bus == NULL) { + if (!bp->mii_bus) { err = -ENOMEM; goto err_out; } @@ -438,7 +438,7 @@ static int macb_mii_init(struct macb *bp) bp->mii_bus->read = &macb_mdio_read; bp->mii_bus->write = &macb_mdio_write; snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x", - bp->pdev->name, bp->pdev->id); +bp->pdev->name, bp->pdev->id); bp->mii_bus->priv = bp; bp->mii_bus->parent = &bp->dev->dev; pdata = dev_get_platdata(&bp->pdev->dev); @@ -659,7 +659,7 @@ static void macb_tx_interrupt(struct macb_queue *queue) queue_writel(queue, ISR, MACB_BIT(TCOMP)); netdev_vdbg(bp->dev, "macb_tx_interrupt status = 0x%03lx\n", - (unsigned long)status); + (unsigned long)status); head = queue->tx_head; for (tail = queue->tx_tail; tail != head; tail++) { @@ -728,10 +728,10 @@ static void gem_rx_refill(struct macb *bp) bp->rx_prepared_head++; - if (bp->rx_skbuff[entry] == NULL) { + if (!bp->rx_skbuff[entry]) { /* allocate sk_buff for this free entry in ring */ skb = netdev_alloc_skb(bp->dev, bp->rx_buffer_size); - if (unlikely(skb == NULL)) { + if (unlikely(!skb)) { netdev_err(bp->dev, "Unable to allocate sk_buff\n"); break; @@ -765,7 +765,7 @@ static void gem_rx_refill(struct macb *bp) wmb(); netdev_vdbg(bp->dev, "rx ring: prepared head %d, tail %d\n", - bp->rx_prepared_head, bp->rx_tail); + bp->rx_prepared_head, bp->rx_tail); } /* Mark DMA descriptors from begin up to and not including end as unused */ @@ -879,8 +879,8 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag, len = desc->ctrl & bp->rx_frm_len_mask; netdev_vdbg(bp->dev, "macb_rx_frame frags %u - %u (len %u)\n", - macb_rx_ring_wrap(first_frag), - macb_rx_ring_wrap(last_frag), len); + macb_rx_ring_wrap(first_frag), + macb_rx_ring_wrap(last_frag), len); /* The ethernet header starts NET_IP_ALIGN bytes into the * first buffer. Since the header is 14 bytes, this makes the @@ -922,7 +922,8 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag, frag_len = len - offset; } skb_copy_to_linear_data_offset(skb, offset, - macb_rx_buffer(bp, frag), frag_len); + macb_rx_buffer(bp, frag), + frag_len); offset += bp->rx_buffer_size; desc = macb_rx_desc(bp, frag); desc->addr &= ~MACB_BIT(RX_USED); @@ -940,7 +941,7 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag, bp->stats.rx_packets++; bp->stats.rx_bytes += skb->len; netdev_vdbg(bp->dev, &qu
[PATCH v3 0/5] macb: Codingstyle cleanups
Hi all, resending almost unchanged v2 here: Changes from v2: * Rebased onto net-next * Changed 5th patches commit message * Added Nicholas' and Michal's Acked-Bys Changes from v1: * Backed out variable scope changes * Separated out ether_addr_copy into it's own commit * Fixed typo in comments as suggested by Joe Cheers, Moritz Moritz Fischer (5): net: macb: Fix coding style error message net: macb: Fix coding style warnings net: macb: Fix coding style suggestions net: macb: Use ether_addr_copy over memcpy net: macb: Fix simple typo drivers/net/ethernet/cadence/macb.c | 152 1 file changed, 69 insertions(+), 83 deletions(-) -- 2.7.4
[PATCH v3 2/5] net: macb: Fix coding style warnings
This commit takes care of the coding style warnings that are mostly due to a different comment style and lines over 80 chars, as well as a dangling else. Acked-by: Nicolas Ferre Signed-off-by: Moritz Fischer --- drivers/net/ethernet/cadence/macb.c | 100 +++- 1 file changed, 42 insertions(+), 58 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index b5aa96e..f25681b 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -61,8 +61,7 @@ #define MACB_WOL_HAS_MAGIC_PACKET (0x1 << 0) #define MACB_WOL_ENABLED (0x1 << 1) -/* - * Graceful stop timeouts in us. We should allow up to +/* Graceful stop timeouts in us. We should allow up to * 1 frame time (10 Mbits/s, full-duplex, ignoring collisions) */ #define MACB_HALT_TIMEOUT 1230 @@ -130,8 +129,7 @@ static void hw_writel(struct macb *bp, int offset, u32 value) writel_relaxed(value, bp->regs + offset); } -/* - * Find the CPU endianness by using the loopback bit of NCR register. When the +/* Find the CPU endianness by using the loopback bit of NCR register. When the * CPU is in big endian we need to program swaped mode for management * descriptor access. */ @@ -386,7 +384,8 @@ static int macb_mii_probe(struct net_device *dev) pdata = dev_get_platdata(&bp->pdev->dev); if (pdata && gpio_is_valid(pdata->phy_irq_pin)) { - ret = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin, "phy int"); + ret = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin, + "phy int"); if (!ret) { phy_irq = gpio_to_irq(pdata->phy_irq_pin); phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq; @@ -452,7 +451,8 @@ static int macb_mii_init(struct macb *bp) err = of_mdiobus_register(bp->mii_bus, np); /* fallback to standard phy registration if no phy were - found during dt phy registration */ +* found during dt phy registration +*/ if (!err && !phy_find_first(bp->mii_bus)) { for (i = 0; i < PHY_MAX_ADDR; i++) { struct phy_device *phydev; @@ -567,8 +567,7 @@ static void macb_tx_error_task(struct work_struct *work) /* Make sure nobody is trying to queue up new packets */ netif_tx_stop_all_queues(bp->dev); - /* -* Stop transmission now + /* Stop transmission now * (in case we have just queued new packets) * macb/gem must be halted to write TBQP register */ @@ -576,8 +575,7 @@ static void macb_tx_error_task(struct work_struct *work) /* Just complain for now, reinitializing TX path can be good */ netdev_err(bp->dev, "BUG: halt tx timed out\n"); - /* -* Treat frames in TX queue including the ones that caused the error. + /* Treat frames in TX queue including the ones that caused the error. * Free transmit buffers in upper layer. */ for (tail = queue->tx_tail; tail != queue->tx_head; tail++) { @@ -607,10 +605,9 @@ static void macb_tx_error_task(struct work_struct *work) bp->stats.tx_bytes += skb->len; } } else { - /* -* "Buffers exhausted mid-frame" errors may only happen -* if the driver is buggy, so complain loudly about those. -* Statistics are updated by hardware. + /* "Buffers exhausted mid-frame" errors may only happen +* if the driver is buggy, so complain loudly about +* those. Statistics are updated by hardware. */ if (ctrl & MACB_BIT(TX_BUF_EXHAUSTED)) netdev_err(bp->dev, @@ -722,7 +719,8 @@ static void gem_rx_refill(struct macb *bp) struct sk_buff *skb; dma_addr_t paddr; - while (CIRC_SPACE(bp->rx_prepared_head, bp->rx_tail, RX_RING_SIZE) > 0) { + while (CIRC_SPACE(bp->rx_prepared_head, bp->rx_tail, + RX_RING_SIZE) > 0) { entry = macb_rx_ring_wrap(bp->rx_prepared_head); /* Make hw descriptor updates visible to CPU */ @@ -741,7 +739,8 @@ static void gem_rx_refill(struct macb *bp) /* now fill corresponding descriptor entry */ paddr = dma_map_single(&bp->pdev->dev, skb->data, -
[PATCH v3 4/5] net: macb: Use ether_addr_copy over memcpy
Checkpatch suggests using ether_addr_copy over memcpy to copy the mac address. Acked-by: Michal Simek Acked-by: Nicolas Ferre Signed-off-by: Moritz Fischer --- drivers/net/ethernet/cadence/macb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index 2ba0934..01a8ffb 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -2973,7 +2973,7 @@ static int macb_probe(struct platform_device *pdev) mac = of_get_mac_address(np); if (mac) - memcpy(bp->dev->dev_addr, mac, ETH_ALEN); + ether_addr_copy(bp->dev->dev_addr, mac); else macb_get_hwaddr(bp); -- 2.7.4
Re: [PATCH 0/5] net: macb: Checkpatch cleanups
Nicolas, On Wed, Mar 16, 2016 at 6:39 AM, Nicolas Ferre wrote: > Le 13/03/2016 20:10, Moritz Fischer a écrit : >> Hi all, >> >> I backed out the variable scope changes and made a separate >> patch for the ether_addr_copy change. >> >> Changes from v1: > > As it's v2, it's better to add it in each subject of the patch series like: > "[PATCH v2 0/5] net: macb: Checkpatch cleanups" Yeah, I fat-fingered that. Figured it is less annoying than resending the series immediately. Do you want me to resend the series as v3 with Acked-bys and fixed patch 5? Through which tree will this go? Merci, Moritz
[PATCH 2/5] net: macb: Fix coding style warnings
This commit takes care of the coding style warnings that are mostly due to a different comment style and lines over 80 chars, as well as a dangling else. Signed-off-by: Moritz Fischer --- drivers/net/ethernet/cadence/macb.c | 101 +++- 1 file changed, 43 insertions(+), 58 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index 4370f37..c2d31c5 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -58,8 +58,7 @@ #define GEM_MTU_MIN_SIZE 68 -/* - * Graceful stop timeouts in us. We should allow up to +/* Graceful stop timeouts in us. We should allow up to * 1 frame time (10 Mbits/s, full-duplex, ignoring collisions) */ #define MACB_HALT_TIMEOUT 1230 @@ -127,8 +126,7 @@ static void hw_writel(struct macb *bp, int offset, u32 value) writel_relaxed(value, bp->regs + offset); } -/* - * Find the CPU endianness by using the loopback bit of NCR register. When the +/* Find the CPU endianness by using the loopback bit of NCR register. When the * CPU is in big endian we need to program swaped mode for management * descriptor access. */ @@ -383,7 +381,8 @@ static int macb_mii_probe(struct net_device *dev) pdata = dev_get_platdata(&bp->pdev->dev); if (pdata && gpio_is_valid(pdata->phy_irq_pin)) { - ret = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin, "phy int"); + ret = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin, + "phy int"); if (!ret) { phy_irq = gpio_to_irq(pdata->phy_irq_pin); phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq; @@ -449,7 +448,8 @@ static int macb_mii_init(struct macb *bp) err = of_mdiobus_register(bp->mii_bus, np); /* fallback to standard phy registration if no phy were - found during dt phy registration */ +* found during dt phy registration +*/ if (!err && !phy_find_first(bp->mii_bus)) { for (i = 0; i < PHY_MAX_ADDR; i++) { struct phy_device *phydev; @@ -564,8 +564,7 @@ static void macb_tx_error_task(struct work_struct *work) /* Make sure nobody is trying to queue up new packets */ netif_tx_stop_all_queues(bp->dev); - /* -* Stop transmission now + /* Stop transmission now * (in case we have just queued new packets) * macb/gem must be halted to write TBQP register */ @@ -573,8 +572,7 @@ static void macb_tx_error_task(struct work_struct *work) /* Just complain for now, reinitializing TX path can be good */ netdev_err(bp->dev, "BUG: halt tx timed out\n"); - /* -* Treat frames in TX queue including the ones that caused the error. + /* Treat frames in TX queue including the ones that caused the error. * Free transmit buffers in upper layer. */ for (tail = queue->tx_tail; tail != queue->tx_head; tail++) { @@ -604,10 +602,9 @@ static void macb_tx_error_task(struct work_struct *work) bp->stats.tx_bytes += skb->len; } } else { - /* -* "Buffers exhausted mid-frame" errors may only happen -* if the driver is buggy, so complain loudly about those. -* Statistics are updated by hardware. + /* "Buffers exhausted mid-frame" errors may only happen +* if the driver is buggy, so complain loudly about +* those. Statistics are updated by hardware. */ if (ctrl & MACB_BIT(TX_BUF_EXHAUSTED)) netdev_err(bp->dev, @@ -719,7 +716,8 @@ static void gem_rx_refill(struct macb *bp) struct sk_buff *skb; dma_addr_t paddr; - while (CIRC_SPACE(bp->rx_prepared_head, bp->rx_tail, RX_RING_SIZE) > 0) { + while (CIRC_SPACE(bp->rx_prepared_head, bp->rx_tail, + RX_RING_SIZE) > 0) { entry = macb_rx_ring_wrap(bp->rx_prepared_head); /* Make hw descriptor updates visible to CPU */ @@ -738,7 +736,8 @@ static void gem_rx_refill(struct macb *bp) /* now fill corresponding descriptor entry */ paddr = dma_map_single(&bp->pdev->dev, skb->data, - bp->rx_buffer_size, DMA_FROM_DEVICE); +
[PATCH 0/5] net: macb: Checkpatch cleanups
Hi all, I backed out the variable scope changes and made a separate patch for the ether_addr_copy change. Changes from v1: * Backed out variable scope changes * Separated out ether_addr_copy into it's own commit * Fixed typo in comments as suggested by Joe Cheers, Moritz Moritz Fischer (5): net: macb: Fix coding style error message net: macb: Fix coding style warnings net: macb: Address checkpatch 'check' suggestions net: macb: Use ether_addr_copy over memcpy net: macb: Fix simple typo. drivers/net/ethernet/cadence/macb.c | 153 +--- 1 file changed, 70 insertions(+), 83 deletions(-) -- 2.4.3
[PATCH 5/5] net: macb: Fix simple typo.
Signed-off-by: Moritz Fischer --- drivers/net/ethernet/cadence/macb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index a0c01e5..681e5bf 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -127,7 +127,7 @@ static void hw_writel(struct macb *bp, int offset, u32 value) } /* Find the CPU endianness by using the loopback bit of NCR register. When the - * CPU is in big endian we need to program swaped mode for management + * CPU is in big endian we need to program swapped mode for management * descriptor access. */ static bool hw_is_native_io(void __iomem *addr) -- 2.4.3
[PATCH 3/5] net: macb: Address checkpatch 'check' suggestions
This commit deals with a bunch of checkpatch suggestions that without changing behavior make checkpatch happier. Signed-off-by: Moritz Fischer --- drivers/net/ethernet/cadence/macb.c | 46 +++-- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index c2d31c5..53400f6 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -184,7 +184,7 @@ static void macb_get_hwaddr(struct macb *bp) pdata = dev_get_platdata(&bp->pdev->dev); - /* Check all 4 address register for vaild address */ + /* Check all 4 address register for valid address */ for (i = 0; i < 4; i++) { bottom = macb_or_gem_readl(bp, SA1B + i * 8); top = macb_or_gem_readl(bp, SA1T + i * 8); @@ -292,7 +292,7 @@ static void macb_set_tx_clk(struct clk *clk, int speed, struct net_device *dev) ferr = DIV_ROUND_UP(ferr, rate / 10); if (ferr > 5) netdev_warn(dev, "unable to generate target frequency: %ld Hz\n", - rate); + rate); if (clk_set_rate(clk, rate_rounded)) netdev_err(dev, "adjusting tx_clk failed.\n"); @@ -426,7 +426,7 @@ static int macb_mii_init(struct macb *bp) macb_writel(bp, NCR, MACB_BIT(MPE)); bp->mii_bus = mdiobus_alloc(); - if (bp->mii_bus == NULL) { + if (!bp->mii_bus) { err = -ENOMEM; goto err_out; } @@ -435,7 +435,7 @@ static int macb_mii_init(struct macb *bp) bp->mii_bus->read = &macb_mdio_read; bp->mii_bus->write = &macb_mdio_write; snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x", - bp->pdev->name, bp->pdev->id); +bp->pdev->name, bp->pdev->id); bp->mii_bus->priv = bp; bp->mii_bus->parent = &bp->dev->dev; pdata = dev_get_platdata(&bp->pdev->dev); @@ -656,7 +656,7 @@ static void macb_tx_interrupt(struct macb_queue *queue) queue_writel(queue, ISR, MACB_BIT(TCOMP)); netdev_vdbg(bp->dev, "macb_tx_interrupt status = 0x%03lx\n", - (unsigned long)status); + (unsigned long)status); head = queue->tx_head; for (tail = queue->tx_tail; tail != head; tail++) { @@ -725,10 +725,10 @@ static void gem_rx_refill(struct macb *bp) bp->rx_prepared_head++; - if (bp->rx_skbuff[entry] == NULL) { + if (!bp->rx_skbuff[entry]) { /* allocate sk_buff for this free entry in ring */ skb = netdev_alloc_skb(bp->dev, bp->rx_buffer_size); - if (unlikely(skb == NULL)) { + if (unlikely(!skb)) { netdev_err(bp->dev, "Unable to allocate sk_buff\n"); break; @@ -762,7 +762,7 @@ static void gem_rx_refill(struct macb *bp) wmb(); netdev_vdbg(bp->dev, "rx ring: prepared head %d, tail %d\n", - bp->rx_prepared_head, bp->rx_tail); + bp->rx_prepared_head, bp->rx_tail); } /* Mark DMA descriptors from begin up to and not including end as unused */ @@ -876,8 +876,8 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag, len = desc->ctrl & bp->rx_frm_len_mask; netdev_vdbg(bp->dev, "macb_rx_frame frags %u - %u (len %u)\n", - macb_rx_ring_wrap(first_frag), - macb_rx_ring_wrap(last_frag), len); + macb_rx_ring_wrap(first_frag), + macb_rx_ring_wrap(last_frag), len); /* The ethernet header starts NET_IP_ALIGN bytes into the * first buffer. Since the header is 14 bytes, this makes the @@ -916,7 +916,8 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag, frag_len = len - offset; } skb_copy_to_linear_data_offset(skb, offset, - macb_rx_buffer(bp, frag), frag_len); + macb_rx_buffer(bp, frag), + frag_len); offset += bp->rx_buffer_size; desc = macb_rx_desc(bp, frag); desc->addr &= ~MACB_BIT(RX_USED); @@ -934,7 +935,7 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag, bp->stats.rx_packets++; bp->stats.rx_bytes += skb->len; netdev_vdbg(bp->dev, "received skb of length %u, csum: %08x\n", -
[PATCH 4/5] net: macb: Use ether_addr_copy over memcpy
Checkpatch suggests using ether_addr_copy over memcpy to copy the mac address. Signed-off-by: Moritz Fischer --- drivers/net/ethernet/cadence/macb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index 53400f6..a0c01e5 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -2891,7 +2891,7 @@ static int macb_probe(struct platform_device *pdev) mac = of_get_mac_address(np); if (mac) - memcpy(bp->dev->dev_addr, mac, ETH_ALEN); + ether_addr_copy(bp->dev->dev_addr, mac); else macb_get_hwaddr(bp); -- 2.4.3
[PATCH 1/5] net: macb: Fix coding style error message
checkpatch.pl gave the following error: ERROR: space required before the open parenthesis '(' + for(; p < end; p++, offset += 4) Signed-off-by: Moritz Fischer --- drivers/net/ethernet/cadence/macb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index 50c9410..4370f37 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -496,7 +496,7 @@ static void macb_update_stats(struct macb *bp) WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4); - for(; p < end; p++, offset += 4) + for (; p < end; p++, offset += 4) *p += bp->macb_reg_readl(bp, offset); } -- 2.4.3
Re: [PATCH 0/3] net: macb: Fix coding style issues
Hi all, thanks for the feedback. On Wed, Mar 9, 2016 at 8:29 AM, Michal Simek wrote: > On 7.3.2016 18:13, Nicolas Ferre wrote: >> I'm not usually fond of this type of patches, but I must admit that this >> series corrects some style issues. While I was playing around with my fixed-link for macb RFC I was flooded with warnings, so I figured I'll fix it while I'm at it. Just makes it easier for other people to work on it afterwards I thought. > > mac = of_get_mac_address(np); > if (mac) > - memcpy(bp->dev->dev_addr, mac, ETH_ALEN); > + ether_addr_copy(bp->dev->dev_addr, mac); > else So don't you like this in general or just would like to have it split out in a separate patch? > Also extending scope of variables is not the right way to go. Especially > when some automation tools are reporting that you should reduce scope of > use for them. Wolfram is checking it for example. Alright, understood, I'll address this. Cheers, Moritz
Re: [PATCH 2/3] net: macb: Fix more coding style issues
Hi Joe, David, On Mon, Mar 7, 2016 at 10:49 AM, David Miller wrote: > From: Moritz Fischer > Date: Mon, 7 Mar 2016 08:17:38 -0800 > >> @@ -945,6 +943,7 @@ static int macb_rx_frame(struct macb *bp, unsigned int >> first_frag, >> static int macb_rx(struct macb *bp, int budget) >> { >> int received = 0; >> + int dropped; >> unsigned int tail; >> int first_frag = -1; >> >> @@ -968,7 +967,6 @@ static int macb_rx(struct macb *bp, int budget) >> } >> >> if (ctrl & MACB_BIT(RX_EOF)) { >> - int dropped; >> BUG_ON(first_frag == -1); >> >> dropped = macb_rx_frame(bp, first_frag, tail); > > I totally disagree with moving local variable declarations up to the > top-most scope. > > It is always best to keep them in the inner-most scope. Alright, I can back these changes out. Didn't mean to sneak anything in. Cheers, Moritz
[PATCH 3/3] net: macb: Address checkpatch 'check' suggestions
This commit deals with a bunch of checkpatch suggestions that without changing behavior make checkpatch happier. Signed-off-by: Moritz Fischer --- drivers/net/ethernet/cadence/macb.c | 46 +++-- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index fba4239..75c19a2 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -184,7 +184,7 @@ static void macb_get_hwaddr(struct macb *bp) pdata = dev_get_platdata(&bp->pdev->dev); - /* Check all 4 address register for vaild address */ + /* Check all 4 address register for valid address */ for (i = 0; i < 4; i++) { bottom = macb_or_gem_readl(bp, SA1B + i * 8); top = macb_or_gem_readl(bp, SA1T + i * 8); @@ -292,7 +292,7 @@ static void macb_set_tx_clk(struct clk *clk, int speed, struct net_device *dev) ferr = DIV_ROUND_UP(ferr, rate / 10); if (ferr > 5) netdev_warn(dev, "unable to generate target frequency: %ld Hz\n", - rate); + rate); if (clk_set_rate(clk, rate_rounded)) netdev_err(dev, "adjusting tx_clk failed.\n"); @@ -426,7 +426,7 @@ static int macb_mii_init(struct macb *bp) macb_writel(bp, NCR, MACB_BIT(MPE)); bp->mii_bus = mdiobus_alloc(); - if (bp->mii_bus == NULL) { + if (!bp->mii_bus) { err = -ENOMEM; goto err_out; } @@ -435,7 +435,7 @@ static int macb_mii_init(struct macb *bp) bp->mii_bus->read = &macb_mdio_read; bp->mii_bus->write = &macb_mdio_write; snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x", - bp->pdev->name, bp->pdev->id); +bp->pdev->name, bp->pdev->id); bp->mii_bus->priv = bp; bp->mii_bus->parent = &bp->dev->dev; pdata = dev_get_platdata(&bp->pdev->dev); @@ -656,7 +656,7 @@ static void macb_tx_interrupt(struct macb_queue *queue) queue_writel(queue, ISR, MACB_BIT(TCOMP)); netdev_vdbg(bp->dev, "macb_tx_interrupt status = 0x%03lx\n", - (unsigned long)status); + (unsigned long)status); head = queue->tx_head; for (tail = queue->tx_tail; tail != head; tail++) { @@ -725,10 +725,10 @@ static void gem_rx_refill(struct macb *bp) bp->rx_prepared_head++; - if (bp->rx_skbuff[entry] == NULL) { + if (!bp->rx_skbuff[entry]) { /* allocate sk_buff for this free entry in ring */ skb = netdev_alloc_skb(bp->dev, bp->rx_buffer_size); - if (unlikely(skb == NULL)) { + if (unlikely(!skb)) { netdev_err(bp->dev, "Unable to allocate sk_buff\n"); break; @@ -762,7 +762,7 @@ static void gem_rx_refill(struct macb *bp) wmb(); netdev_vdbg(bp->dev, "rx ring: prepared head %d, tail %d\n", - bp->rx_prepared_head, bp->rx_tail); + bp->rx_prepared_head, bp->rx_tail); } /* Mark DMA descriptors from begin up to and not including end as unused */ @@ -876,8 +876,8 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag, len = desc->ctrl & bp->rx_frm_len_mask; netdev_vdbg(bp->dev, "macb_rx_frame frags %u - %u (len %u)\n", - macb_rx_ring_wrap(first_frag), - macb_rx_ring_wrap(last_frag), len); + macb_rx_ring_wrap(first_frag), + macb_rx_ring_wrap(last_frag), len); /* The ethernet header starts NET_IP_ALIGN bytes into the * first buffer. Since the header is 14 bytes, this makes the @@ -916,7 +916,8 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag, frag_len = len - offset; } skb_copy_to_linear_data_offset(skb, offset, - macb_rx_buffer(bp, frag), frag_len); + macb_rx_buffer(bp, frag), + frag_len); offset += bp->rx_buffer_size; desc = macb_rx_desc(bp, frag); desc->addr &= ~MACB_BIT(RX_USED); @@ -934,7 +935,7 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag, bp->stats.rx_packets++; bp->stats.rx_bytes += skb->len; netdev_vdbg(bp->dev, "received skb of length %u, csum: %08x\n", -
[PATCH 1/3] net: macb: Fix coding style error message
checkpatch.pl gave the following error: ERROR: space required before the open parenthesis '(' + for(; p < end; p++, offset += 4) Signed-off-by: Moritz Fischer --- drivers/net/ethernet/cadence/macb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index 50c9410..4370f37 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -496,7 +496,7 @@ static void macb_update_stats(struct macb *bp) WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4); - for(; p < end; p++, offset += 4) + for (; p < end; p++, offset += 4) *p += bp->macb_reg_readl(bp, offset); } -- 2.4.3
Re: [PATCH 3/3] net: macb: Cleanup checkpatch checks
Derp, it's monday morning. Ignore the second [3/3] patch... Sorry for the noise. Moritz
[PATCH 2/3] net: macb: Fix more coding style issues
This commit takes care of the coding style warnings that are mostly due to a different comment style and lines over 80 chars. Notable exceptions are ether_addr_copy vs memcpy, as well as a dangling else after a return. Signed-off-by: Moritz Fischer --- drivers/net/ethernet/cadence/macb.c | 109 +++- 1 file changed, 46 insertions(+), 63 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index 4370f37..fba4239 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -58,8 +58,7 @@ #define GEM_MTU_MIN_SIZE 68 -/* - * Graceful stop timeouts in us. We should allow up to +/* Graceful stop timeouts in us. We should allow up to * 1 frame time (10 Mbits/s, full-duplex, ignoring collisions) */ #define MACB_HALT_TIMEOUT 1230 @@ -127,8 +126,7 @@ static void hw_writel(struct macb *bp, int offset, u32 value) writel_relaxed(value, bp->regs + offset); } -/* - * Find the CPU endianness by using the loopback bit of NCR register. When the +/* Find the CPU endianness by using the loopback bit of NCR register. When the * CPU is in big endian we need to program swaped mode for management * descriptor access. */ @@ -383,7 +381,8 @@ static int macb_mii_probe(struct net_device *dev) pdata = dev_get_platdata(&bp->pdev->dev); if (pdata && gpio_is_valid(pdata->phy_irq_pin)) { - ret = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin, "phy int"); + ret = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin, + "phy int"); if (!ret) { phy_irq = gpio_to_irq(pdata->phy_irq_pin); phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq; @@ -449,7 +448,8 @@ static int macb_mii_init(struct macb *bp) err = of_mdiobus_register(bp->mii_bus, np); /* fallback to standard phy registration if no phy were - found during dt phy registration */ +* found during dt phy registration +*/ if (!err && !phy_find_first(bp->mii_bus)) { for (i = 0; i < PHY_MAX_ADDR; i++) { struct phy_device *phydev; @@ -564,8 +564,7 @@ static void macb_tx_error_task(struct work_struct *work) /* Make sure nobody is trying to queue up new packets */ netif_tx_stop_all_queues(bp->dev); - /* -* Stop transmission now + /* Stop transmission now * (in case we have just queued new packets) * macb/gem must be halted to write TBQP register */ @@ -573,8 +572,7 @@ static void macb_tx_error_task(struct work_struct *work) /* Just complain for now, reinitializing TX path can be good */ netdev_err(bp->dev, "BUG: halt tx timed out\n"); - /* -* Treat frames in TX queue including the ones that caused the error. + /* Treat frames in TX queue including the ones that caused the error. * Free transmit buffers in upper layer. */ for (tail = queue->tx_tail; tail != queue->tx_head; tail++) { @@ -604,10 +602,9 @@ static void macb_tx_error_task(struct work_struct *work) bp->stats.tx_bytes += skb->len; } } else { - /* -* "Buffers exhausted mid-frame" errors may only happen -* if the driver is buggy, so complain loudly about those. -* Statistics are updated by hardware. + /* "Buffers exhausted mid-frame" errors may only happen +* if the driver is buggy, so complain loudly about +* those. Statistics are updated by hardware. */ if (ctrl & MACB_BIT(TX_BUF_EXHAUSTED)) netdev_err(bp->dev, @@ -719,7 +716,8 @@ static void gem_rx_refill(struct macb *bp) struct sk_buff *skb; dma_addr_t paddr; - while (CIRC_SPACE(bp->rx_prepared_head, bp->rx_tail, RX_RING_SIZE) > 0) { + while (CIRC_SPACE(bp->rx_prepared_head, bp->rx_tail, + RX_RING_SIZE) > 0) { entry = macb_rx_ring_wrap(bp->rx_prepared_head); /* Make hw descriptor updates visible to CPU */ @@ -738,7 +736,8 @@ static void gem_rx_refill(struct macb *bp) /* now fill corresponding descriptor entry */ paddr = dma_map_single(&bp->pdev->dev, skb->data, - bp->rx_bu
[PATCH 0/3] net: macb: Fix coding style issues
Hi Nicolas, this series deals with most of the checkpatch warnings generated for macb. There are two BUG_ON()'s that I didn't touch, yet, that were suggested by checkpatch, that I can address in a follow up commit if needed. Let me know if you want me to split the fixes differently or squash them into one commit. Cheers, Moritz Moritz Fischer (3): net: macb: Fix coding style error message net: macb: Fix more coding style issues net: macb: Address checkpatch 'check' suggestions drivers/net/ethernet/cadence/macb.c | 157 1 file changed, 71 insertions(+), 86 deletions(-) -- 2.4.3
[PATCH 3/3] net: macb: Cleanup checkpatch checks
This commit deals with a bunch of check suggestions that without changing behavior make checkpatch hapy. Signed-off-by: Moritz Fischer --- drivers/net/ethernet/cadence/macb.c | 46 +++-- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index fba4239..75c19a2 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -184,7 +184,7 @@ static void macb_get_hwaddr(struct macb *bp) pdata = dev_get_platdata(&bp->pdev->dev); - /* Check all 4 address register for vaild address */ + /* Check all 4 address register for valid address */ for (i = 0; i < 4; i++) { bottom = macb_or_gem_readl(bp, SA1B + i * 8); top = macb_or_gem_readl(bp, SA1T + i * 8); @@ -292,7 +292,7 @@ static void macb_set_tx_clk(struct clk *clk, int speed, struct net_device *dev) ferr = DIV_ROUND_UP(ferr, rate / 10); if (ferr > 5) netdev_warn(dev, "unable to generate target frequency: %ld Hz\n", - rate); + rate); if (clk_set_rate(clk, rate_rounded)) netdev_err(dev, "adjusting tx_clk failed.\n"); @@ -426,7 +426,7 @@ static int macb_mii_init(struct macb *bp) macb_writel(bp, NCR, MACB_BIT(MPE)); bp->mii_bus = mdiobus_alloc(); - if (bp->mii_bus == NULL) { + if (!bp->mii_bus) { err = -ENOMEM; goto err_out; } @@ -435,7 +435,7 @@ static int macb_mii_init(struct macb *bp) bp->mii_bus->read = &macb_mdio_read; bp->mii_bus->write = &macb_mdio_write; snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x", - bp->pdev->name, bp->pdev->id); +bp->pdev->name, bp->pdev->id); bp->mii_bus->priv = bp; bp->mii_bus->parent = &bp->dev->dev; pdata = dev_get_platdata(&bp->pdev->dev); @@ -656,7 +656,7 @@ static void macb_tx_interrupt(struct macb_queue *queue) queue_writel(queue, ISR, MACB_BIT(TCOMP)); netdev_vdbg(bp->dev, "macb_tx_interrupt status = 0x%03lx\n", - (unsigned long)status); + (unsigned long)status); head = queue->tx_head; for (tail = queue->tx_tail; tail != head; tail++) { @@ -725,10 +725,10 @@ static void gem_rx_refill(struct macb *bp) bp->rx_prepared_head++; - if (bp->rx_skbuff[entry] == NULL) { + if (!bp->rx_skbuff[entry]) { /* allocate sk_buff for this free entry in ring */ skb = netdev_alloc_skb(bp->dev, bp->rx_buffer_size); - if (unlikely(skb == NULL)) { + if (unlikely(!skb)) { netdev_err(bp->dev, "Unable to allocate sk_buff\n"); break; @@ -762,7 +762,7 @@ static void gem_rx_refill(struct macb *bp) wmb(); netdev_vdbg(bp->dev, "rx ring: prepared head %d, tail %d\n", - bp->rx_prepared_head, bp->rx_tail); + bp->rx_prepared_head, bp->rx_tail); } /* Mark DMA descriptors from begin up to and not including end as unused */ @@ -876,8 +876,8 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag, len = desc->ctrl & bp->rx_frm_len_mask; netdev_vdbg(bp->dev, "macb_rx_frame frags %u - %u (len %u)\n", - macb_rx_ring_wrap(first_frag), - macb_rx_ring_wrap(last_frag), len); + macb_rx_ring_wrap(first_frag), + macb_rx_ring_wrap(last_frag), len); /* The ethernet header starts NET_IP_ALIGN bytes into the * first buffer. Since the header is 14 bytes, this makes the @@ -916,7 +916,8 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag, frag_len = len - offset; } skb_copy_to_linear_data_offset(skb, offset, - macb_rx_buffer(bp, frag), frag_len); + macb_rx_buffer(bp, frag), + frag_len); offset += bp->rx_buffer_size; desc = macb_rx_desc(bp, frag); desc->addr &= ~MACB_BIT(RX_USED); @@ -934,7 +935,7 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag, bp->stats.rx_packets++; bp->stats.rx_bytes += skb->len; netdev_vdbg(bp->dev, "received skb of length %u, csum: %08x\n", -
[RFC] net: macb: Attempt to make fixed link working on macb
Signed-off-by: Moritz Fischer --- drivers/net/ethernet/cadence/macb.c | 63 - drivers/net/ethernet/cadence/macb.h | 1 + 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index 50c9410..4a3d45d 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -375,27 +375,37 @@ static int macb_mii_probe(struct net_device *dev) int phy_irq; int ret; - phydev = phy_find_first(bp->mii_bus); - if (!phydev) { - netdev_err(dev, "no PHY found\n"); - return -ENXIO; - } + if (bp->phy_node) { + phydev = of_phy_connect(dev, bp->phy_node, + &macb_handle_link_change, 0, + bp->phy_interface); + if (!phydev) + return -ENODEV; + } else { + phydev = phy_find_first(bp->mii_bus); + if (!phydev) { + netdev_err(dev, "no PHY found\n"); + return -ENXIO; + } - pdata = dev_get_platdata(&bp->pdev->dev); - if (pdata && gpio_is_valid(pdata->phy_irq_pin)) { - ret = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin, "phy int"); - if (!ret) { - phy_irq = gpio_to_irq(pdata->phy_irq_pin); - phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq; + pdata = dev_get_platdata(&bp->pdev->dev); + if (pdata && gpio_is_valid(pdata->phy_irq_pin)) { + ret = devm_gpio_request(&bp->pdev->dev, + pdata->phy_irq_pin, "phy int"); + if (!ret) { + phy_irq = gpio_to_irq(pdata->phy_irq_pin); + phydev->irq = (phy_irq < 0) + ? PHY_POLL : phy_irq; + } } - } - /* attach the mac to the phy */ - ret = phy_connect_direct(dev, phydev, &macb_handle_link_change, -bp->phy_interface); - if (ret) { - netdev_err(dev, "Could not attach to PHY\n"); - return ret; + /* attach the mac to the phy */ + ret = phy_connect_direct(dev, phydev, &macb_handle_link_change, +bp->phy_interface); + if (ret) { + netdev_err(dev, "Could not attach to PHY\n"); + return ret; + } } /* mask with MAC supported features */ @@ -2910,14 +2920,21 @@ static int macb_probe(struct platform_device *pdev) macb_get_hwaddr(bp); /* Power up the PHY if there is a GPIO reset */ - phy_node = of_get_next_available_child(np, NULL); - if (phy_node) { + phy_node = of_parse_phandle(np, "phy-handle", 0); + if (!phy_node && of_phy_is_fixed_link(np)) { + err = of_phy_register_fixed_link(np); + if (err < 0) { + dev_err(&pdev->dev, "broken fixed-link specification"); + goto failed_phy; + } + phy_node = of_node_get(np); + bp->phy_node = phy_node; + } else { int gpio = of_get_named_gpio(phy_node, "reset-gpios", 0); if (gpio_is_valid(gpio)) bp->reset_gpio = gpio_to_desc(gpio); gpiod_set_value(bp->reset_gpio, GPIOD_OUT_HIGH); } - of_node_put(phy_node); err = of_get_phy_mode(np); if (err < 0) { @@ -2959,6 +2976,9 @@ static int macb_probe(struct platform_device *pdev) err_out_unregister_netdev: unregister_netdev(dev); +failed_phy: + of_node_put(phy_node); + err_out_free_netdev: free_netdev(dev); @@ -2991,6 +3011,7 @@ static int macb_remove(struct platform_device *pdev) clk_disable_unprepare(bp->tx_clk); clk_disable_unprepare(bp->hclk); clk_disable_unprepare(bp->pclk); + of_node_put(bp->phy_node); free_netdev(dev); } diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h index 0d4ecfc..0373aa47 100644 --- a/drivers/net/ethernet/cadence/macb.h +++ b/drivers/net/ethernet/cadence/macb.h @@ -822,6 +822,7 @@ struct macb { struct mii_bus *mii_bus; struct phy_device *phy_dev; + struct device_node *phy_node; int link; int speed; int duplex; -- 2.7.2
[RFC] Attempt to add fixed link functionality to macb
Hi all, I'm trying to make the dt 'fixed-link' work with the Zynq's GEM on my board. The endgoal will be to hook it up to an ethernet switch in the PL (FPGA) logic. Does the attached patch look halfway reasonable? Note the code is pretty hacky, and it's not 100% working, so I'm more looking for pointers on obvious mistakes than for a thorough code review. Thanks in advance, Moritz Moritz Fischer (1): net: macb: Attempt to make fixed link working on macb drivers/net/ethernet/cadence/macb.c | 63 - drivers/net/ethernet/cadence/macb.h | 1 + 2 files changed, 43 insertions(+), 21 deletions(-) -- 2.7.2
Re: [PATCH] net/macb: add support for resetting PHY using GPIO
Hi Gregory, so far dealt with this in u-boot. The power down the PHY part makes sense, though. Minor nit down inline. Will need to test on hardware (Zynq). On Wed, Dec 9, 2015 at 9:49 AM, Gregory CLEMENT wrote: > With device tree it is no more possible to reset the PHY at board > level. Furthermore, doing in the driver allow to power down the PHY when > the network interface is no more used. > > The patch introduces a new optional property "phy-reset-gpio" inspired > from the one use for the FEC. > > Signed-off-by: Gregory CLEMENT > --- > Documentation/devicetree/bindings/net/macb.txt | 3 +++ > drivers/net/ethernet/cadence/macb.c| 26 > ++ > drivers/net/ethernet/cadence/macb.h| 1 + > 3 files changed, 30 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/macb.txt > b/Documentation/devicetree/bindings/net/macb.txt > index b5d7976..546d34d 100644 > --- a/Documentation/devicetree/bindings/net/macb.txt > +++ b/Documentation/devicetree/bindings/net/macb.txt > @@ -19,6 +19,9 @@ Required properties: > Optional elements: 'tx_clk' > - clocks: Phandles to input clocks. > > +Optional properties: > +- phy-reset-gpio : Should specify the gpio for phy reset > + > Examples: > > macb0: ethernet@fffc4000 { > diff --git a/drivers/net/ethernet/cadence/macb.c > b/drivers/net/ethernet/cadence/macb.c > index 88c1e1a..e630c56 100644 > --- a/drivers/net/ethernet/cadence/macb.c > +++ b/drivers/net/ethernet/cadence/macb.c > @@ -30,6 +30,7 @@ > #include > #include > #include > +#include > > #include "macb.h" > > @@ -2733,6 +2734,28 @@ static int at91ether_init(struct platform_device *pdev) > return 0; > } > > +#ifdef CONFIG_OF > +static void macb_reset_phy(struct macb *bp, struct device_node *np, int > state) > +{ > + if (!np) > + return; > + > + bp->reset_gpio = of_get_named_gpio(np, "phy-reset-gpio", 0); Any reason to do of_get_named() all the time instead of using the one you stored in bp->reset_gpio? You could move it to probe and then just use the stored value here ... not sure if it buys much ;-) Cheers, Moritz -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html