Re: [PATCH v3 12/15] fpga: altera: use ARCH_INTEL_SOCFPGA also for 32-bit ARM SoCs

2021-03-11 Thread Moritz Fischer
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

2020-10-28 Thread Moritz Fischer
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

2020-10-28 Thread Moritz Fischer
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

2020-10-23 Thread Moritz Fischer
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

2020-10-22 Thread Moritz Fischer
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

2020-10-22 Thread Moritz Fischer
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

2020-10-22 Thread Moritz Fischer
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()

2020-09-14 Thread Moritz Fischer
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

2020-09-14 Thread Moritz Fischer
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

2020-09-14 Thread Moritz Fischer
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

2020-09-14 Thread Moritz Fischer
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

2020-09-13 Thread Moritz Fischer
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

2020-09-13 Thread Moritz Fischer
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()

2020-09-13 Thread Moritz Fischer
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

2020-09-13 Thread Moritz Fischer
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

2020-09-13 Thread Moritz Fischer
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

2020-09-13 Thread Moritz Fischer
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

2020-09-10 Thread Moritz Fischer
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

2019-02-12 Thread Moritz Fischer
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

2019-02-08 Thread Moritz Fischer
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

2019-02-07 Thread Moritz Fischer
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

2019-02-07 Thread Moritz Fischer
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

2019-02-06 Thread Moritz Fischer
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

2019-02-06 Thread Moritz Fischer
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

2019-02-06 Thread Moritz Fischer
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

2019-02-06 Thread Moritz Fischer
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

2019-02-06 Thread Moritz Fischer
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

2019-02-04 Thread Moritz Fischer
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

2019-02-04 Thread Moritz Fischer
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

2019-02-04 Thread Moritz Fischer
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

2019-02-04 Thread Moritz Fischer
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

2019-02-04 Thread Moritz Fischer
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

2019-02-01 Thread 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.

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

2018-09-27 Thread Moritz Fischer
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

2018-08-16 Thread Moritz Fischer
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

2018-08-13 Thread Moritz Fischer
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

2018-05-04 Thread Moritz Fischer
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

2018-05-04 Thread Moritz Fischer
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()

2018-04-26 Thread Moritz Fischer
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()

2018-04-26 Thread Moritz Fischer
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

2018-03-27 Thread Moritz Fischer
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

2018-03-27 Thread Moritz Fischer
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

2018-03-26 Thread Moritz Fischer
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

2018-03-23 Thread Moritz Fischer
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

2018-03-23 Thread Moritz Fischer
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

2018-03-01 Thread Moritz Fischer
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

2018-03-01 Thread Moritz Fischer
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()

2018-02-28 Thread Moritz Fischer
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

2018-02-16 Thread Moritz Fischer
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

2018-02-16 Thread Moritz Fischer
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

2017-07-17 Thread Moritz Fischer
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

2017-07-15 Thread Moritz Fischer
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

2017-07-14 Thread Moritz Fischer
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

2017-07-14 Thread Moritz Fischer
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

2017-07-13 Thread Moritz Fischer
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

2017-07-13 Thread Moritz Fischer
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

2017-07-13 Thread Moritz Fischer
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

2017-07-13 Thread Moritz Fischer
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

2017-07-13 Thread Moritz Fischer
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

2017-02-15 Thread Moritz Fischer
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

2017-02-15 Thread Moritz Fischer
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

2016-10-07 Thread Moritz Fischer
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

2016-06-27 Thread Moritz Fischer
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

2016-06-27 Thread Moritz Fischer
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

2016-04-03 Thread Moritz Fischer
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

2016-04-03 Thread Moritz Fischer
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

2016-04-03 Thread Moritz Fischer
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

2016-04-03 Thread Moritz Fischer
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

2016-04-03 Thread Moritz Fischer
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

2016-04-03 Thread Moritz Fischer
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

2016-03-19 Thread Moritz Fischer
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

2016-03-13 Thread Moritz Fischer
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

2016-03-13 Thread Moritz Fischer
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.

2016-03-13 Thread Moritz Fischer
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

2016-03-13 Thread Moritz Fischer
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

2016-03-13 Thread Moritz Fischer
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

2016-03-13 Thread Moritz Fischer
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

2016-03-09 Thread Moritz Fischer
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

2016-03-07 Thread Moritz Fischer
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

2016-03-07 Thread Moritz Fischer
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

2016-03-07 Thread Moritz Fischer
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

2016-03-07 Thread Moritz Fischer
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

2016-03-07 Thread Moritz Fischer
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

2016-03-07 Thread Moritz Fischer
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

2016-03-07 Thread Moritz Fischer
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

2016-03-06 Thread Moritz Fischer
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

2016-03-06 Thread Moritz Fischer
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

2015-12-09 Thread Moritz Fischer
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