Re: [PATCH] MABC RTNet driver to work with GEM hardware

Re: [PATCH] MABC RTNet driver to work with GEM hardware

[Jan Kiszka]

Please keep this on the list - readding.

On 02.10.19 17:08, François Legal wrote:
> Le Mercredi, Octobre 02, 2019 16:59 CEST, Jan Kiszka <jan.kiszka@siemens.com> a écrit:
>   
>> On 02.10.19 15:44, François Legal via Xenomai wrote:
>>> From: François LEGAL <devel@thom.fr.eu.org>
>>>
>>> Make MACB driver compliant with GEM hardware (as mainline linux
>>> driver does). Main fix is about interrupt flag that is never
>>> cleared (write 1 to clear)
> 
> Do you mean I shall move these 3 lines right before "kernel/drivers/net/drivers/Kconfig  |  7 +++++++..." ?
> 

I mean something like this in your git commit:

"Make MACB driver compliant with GEM hardware

Main fix is about interrupt flag that is never
cleared (write 1 to clear)

Signed-off-by: ..."

That will generate a properly formatted e-mail when send that out.

>>>
>>> Signed-off-by: François LEGAL <devel@thom.fr.eu.org>
>>>
>>> Subject: [PATCH] Make MACB driver compliant with GEM hardware
>>
>> Hmm, two subject lines - which one should I use? Would be better get the first
>> one right already ;).
>>
>>>
>>> ---
>>>    kernel/drivers/net/drivers/Kconfig  |  7 +++++++
>>>    kernel/drivers/net/drivers/Makefile |  1 +
>>>    kernel/drivers/net/drivers/macb.c   | 23 ++++++++++++++++++++---
>>>    3 files changed, 28 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/kernel/drivers/net/drivers/Kconfig b/kernel/drivers/net/drivers/Kconfig
>>> index 65f0855..39c976f 100644
>>> --- a/kernel/drivers/net/drivers/Kconfig
>>> +++ b/kernel/drivers/net/drivers/Kconfig
>>> @@ -131,6 +131,13 @@ config XENO_DRIVERS_NET_DRV_MACB
>>>        Driver for internal MAC-controller on AT91SAM926x microcontrollers.
>>>        Porting by Cristiano Mantovani and Stefano Banzi (Marposs SpA).
>>>    
>>> +config XENO_DRIVERS_NET_DRV_GEM
>>> +    depends on XENO_DRIVERS_NET
>>> +    tristate "GEM devices"
>>> +    ---help---
>>> +    Driver for internal MAC-controller on Zynq 7000 SoCs.
>>> +    Porting by Cristiano Mantovani and Stefano Banzi (Marposs SpA).
>>> +
>>>    endif
>>>    
>>>    source "drivers/xenomai/net/drivers/experimental/Kconfig"
>>> diff --git a/kernel/drivers/net/drivers/Makefile b/kernel/drivers/net/drivers/Makefile
>>> index d95c5a8..9da1a56 100644
>>> --- a/kernel/drivers/net/drivers/Makefile
>>> +++ b/kernel/drivers/net/drivers/Makefile
>>> @@ -57,6 +57,7 @@ obj-$(CONFIG_XENO_DRIVERS_NET_DRV_SMC91111) += rt_smc91111.o
>>>    rt_smc91111-y := smc91111.o
>>>    
>>>    obj-$(CONFIG_XENO_DRIVERS_NET_DRV_MACB) += rt_macb.o
>>> +obj-$(CONFIG_XENO_DRIVERS_NET_DRV_GEM) += rt_macb.o
>>
>> Only saw this now: What's the idea behind adding XENO_DRIVERS_NET_DRV_GEM when
>> there is XENO_DRIVERS_NET_DRV_MACB already? If a selects both, the build will fail.
> 
> Yes, that's correct. However, the Kconfig selects automatically some AT91 specific stuff when CONFIG_XENO_DRIVERS_NET_DRV_MACB is selected.
> I'm not sure on the best way to handle those situations without breaking the existing function.
> 
> from Kconfig :
> config XENO_DRIVERS_NET_DRV_AT91_ETHER
>      depends on XENO_DRIVERS_NET && SOC_AT91RM9200
>      select XENO_DRIVERS_NET_DRV_MACB
>      tristate "AT91RM9200 Board Ethernet Driver"
> 
> config XENO_DRIVERS_NET_DRV_MACB
>      depends on XENO_DRIVERS_NET && ARCH_AT91
>      select AT91_PROGRAMMABLE_CLOCKS
>      tristate "MACB devices"
>      ---help---
>      Driver for internal MAC-controller on AT91SAM926x microcontrollers.
>      Porting by Cristiano Mantovani and Stefano Banzi (Marposs SpA).
> 
> Maybe I should replace the line select AT91_PROGRAMMABLE_CLOCKS with
> select AT91_PROGRAMMABLE_CLOCKS IF ARCH_AT91
> 
> What do you think ?

What does upstream do? Maybe we copied back than a suboptimal pattern that was 
improved there meanwhile.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] MABC RTNet driver to work with GEM hardware

[Jan Kiszka]

On 03.10.19 10:43, François Legal via Xenomai wrote:
> From: François LEGAL <devel@thom.fr.eu.org>
> Subject: [PATCH] Make MACB driver compliant with GEM hardware
> 
> Make MACB driver compliant with GEM hardware (as mainline linux
> driver does). Main fix is about interrupt flag that is never
> cleared (write 1 to clear)
> 
> Signed-off-by: François LEGAL <devel@thom.fr.eu.org>
> 
> ---
>   kernel/drivers/net/drivers/Kconfig |  6 +++---
>   kernel/drivers/net/drivers/macb.c  | 23 ++++++++++++++++++++---
>   2 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/drivers/net/drivers/Kconfig b/kernel/drivers/net/drivers/Kconfig
> index 65f0855..1aa1b3c 100644
> --- a/kernel/drivers/net/drivers/Kconfig
> +++ b/kernel/drivers/net/drivers/Kconfig
> @@ -124,9 +124,9 @@ config XENO_DRIVERS_NET_DRV_AT91_ETHER
>       tristate "AT91RM9200 Board Ethernet Driver"
>   
>   config XENO_DRIVERS_NET_DRV_MACB
> -    depends on XENO_DRIVERS_NET && ARCH_AT91
> -    select AT91_PROGRAMMABLE_CLOCKS
> -    tristate "MACB AT91SAM926x devices"
> +    depends on XENO_DRIVERS_NET
> +    select AT91_PROGRAMMABLE_CLOCKS if ARCH_AT91
> +    tristate "Cadence MACB/GEM devices"
>       ---help---
>       Driver for internal MAC-controller on AT91SAM926x microcontrollers.
>       Porting by Cristiano Mantovani and Stefano Banzi (Marposs SpA).
> diff --git a/kernel/drivers/net/drivers/macb.c b/kernel/drivers/net/drivers/macb.c
> index fc5b667..ccda6ae 100644
> --- a/kernel/drivers/net/drivers/macb.c
> +++ b/kernel/drivers/net/drivers/macb.c
> @@ -679,7 +679,13 @@ static int gem_rx(struct macb *bp, int budget, nanosecs_abs_t *time_stamp)
>   	unsigned int		entry;
>   	struct rtskb		*skb;
>   	struct macb_dma_desc	*desc;
> -	int			count = 0;
> +	int			count = 0, status;
> +
> +	status = macb_readl(bp, RSR);
> +	macb_writel(bp, RSR, status);
> +
> +	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +		macb_writel(bp, ISR, MACB_BIT(RCOMP));
>   
>   	while (count < budget) {
>   		u32 addr, ctrl;
> @@ -877,7 +883,7 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
>   	struct rtnet_device *dev = dev_id;
>   	struct macb *bp = rtnetdev_priv(dev);
>   	unsigned received = 0;
> -	u32 status;
> +	u32 status, ctrl;
>   
>   	status = macb_readl(bp, ISR);
>   
> @@ -895,9 +901,10 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
>   
>   		rtdev_vdbg(bp->dev, "isr = 0x%08lx\n", (unsigned long)status);
>   
> -		if (status & MACB_RX_INT_FLAGS)
> +		if (status & MACB_BIT(RCOMP)) {
>   			received += bp->macbgem_ops.mog_rx(bp, 100 - received,
>   							&time_stamp);
> +		}
>   
>   		if (unlikely(status & (MACB_TX_ERR_FLAGS))) {
>   			macb_writel(bp, IDR, MACB_TX_INT_FLAGS);
> @@ -917,6 +924,15 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
>   		 * add that if/when we get our hands on a full-blown MII PHY.
>   		 */
>   
> +		if (status & MACB_BIT(RXUBR)) {
> +			ctrl = macb_readl(bp, NCR);
> +			macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
> +			macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
> +
> +			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +				macb_writel(bp, ISR, MACB_BIT(RXUBR));
> +		}
> +
>   		if (status & MACB_BIT(ISR_ROVR)) {
>   			/* We missed at least one packet */
>   			if (macb_is_gem(bp))
> @@ -1320,6 +1336,7 @@ static void macb_configure_caps(struct macb *bp)
>   		if (GEM_BFEXT(IRQCOR, gem_readl(bp, DCFG1)) == 0)
>   			bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
>   	}
> +	rtdev_vdbg(bp->dev, "Capabilities : %X\n", bp->caps);
>   }
>   
>   static void macb_init_hw(struct macb *bp)
> 

Thanks, applied to next.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

[PATCH] MABC RTNet driver to work with GEM hardware

[François Legal]

From: François LEGAL <devel@thom.fr.eu.org>
Subject: [PATCH] Make MACB driver compliant with GEM hardware

Make MACB driver compliant with GEM hardware (as mainline linux
driver does). Main fix is about interrupt flag that is never
cleared (write 1 to clear)

Signed-off-by: François LEGAL <devel@thom.fr.eu.org>

---
 kernel/drivers/net/drivers/Kconfig |  6 +++---
 kernel/drivers/net/drivers/macb.c  | 23 ++++++++++++++++++++---
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/kernel/drivers/net/drivers/Kconfig b/kernel/drivers/net/drivers/Kconfig
index 65f0855..1aa1b3c 100644
--- a/kernel/drivers/net/drivers/Kconfig
+++ b/kernel/drivers/net/drivers/Kconfig
@@ -124,9 +124,9 @@ config XENO_DRIVERS_NET_DRV_AT91_ETHER
     tristate "AT91RM9200 Board Ethernet Driver"

 config XENO_DRIVERS_NET_DRV_MACB
-    depends on XENO_DRIVERS_NET && ARCH_AT91
-    select AT91_PROGRAMMABLE_CLOCKS
-    tristate "MACB AT91SAM926x devices"
+    depends on XENO_DRIVERS_NET
+    select AT91_PROGRAMMABLE_CLOCKS if ARCH_AT91
+    tristate "Cadence MACB/GEM devices"
     ---help---
     Driver for internal MAC-controller on AT91SAM926x microcontrollers.
     Porting by Cristiano Mantovani and Stefano Banzi (Marposs SpA).
diff --git a/kernel/drivers/net/drivers/macb.c b/kernel/drivers/net/drivers/macb.c
index fc5b667..ccda6ae 100644
--- a/kernel/drivers/net/drivers/macb.c
+++ b/kernel/drivers/net/drivers/macb.c
@@ -679,7 +679,13 @@ static int gem_rx(struct macb *bp, int budget, nanosecs_abs_t *time_stamp)
 	unsigned int		entry;
 	struct rtskb		*skb;
 	struct macb_dma_desc	*desc;
-	int			count = 0;
+	int			count = 0, status;
+
+	status = macb_readl(bp, RSR);
+	macb_writel(bp, RSR, status);
+
+	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+		macb_writel(bp, ISR, MACB_BIT(RCOMP));

 	while (count < budget) {
 		u32 addr, ctrl;
@@ -877,7 +883,7 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
 	struct rtnet_device *dev = dev_id;
 	struct macb *bp = rtnetdev_priv(dev);
 	unsigned received = 0;
-	u32 status;
+	u32 status, ctrl;

 	status = macb_readl(bp, ISR);

@@ -895,9 +901,10 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)

 		rtdev_vdbg(bp->dev, "isr = 0x%08lx\n", (unsigned long)status);

-		if (status & MACB_RX_INT_FLAGS)
+		if (status & MACB_BIT(RCOMP)) {
 			received += bp->macbgem_ops.mog_rx(bp, 100 - received,
 							&time_stamp);
+		}

 		if (unlikely(status & (MACB_TX_ERR_FLAGS))) {
 			macb_writel(bp, IDR, MACB_TX_INT_FLAGS);
@@ -917,6 +924,15 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
 		 * add that if/when we get our hands on a full-blown MII PHY.
 		 */

+		if (status & MACB_BIT(RXUBR)) {
+			ctrl = macb_readl(bp, NCR);
+			macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
+			macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
+
+			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+				macb_writel(bp, ISR, MACB_BIT(RXUBR));
+		}
+
 		if (status & MACB_BIT(ISR_ROVR)) {
 			/* We missed at least one packet */
 			if (macb_is_gem(bp))
@@ -1320,6 +1336,7 @@ static void macb_configure_caps(struct macb *bp)
 		if (GEM_BFEXT(IRQCOR, gem_readl(bp, DCFG1)) == 0)
 			bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
 	}
+	rtdev_vdbg(bp->dev, "Capabilities : %X\n", bp->caps);
 }

 static void macb_init_hw(struct macb *bp)
--
2.1.4

Re: [PATCH] MABC RTNet driver to work with GEM hardware

[Jan Kiszka]

On 02.10.19 15:44, François Legal via Xenomai wrote:
> From: François LEGAL <devel@thom.fr.eu.org>
> 
> Make MACB driver compliant with GEM hardware (as mainline linux
> driver does). Main fix is about interrupt flag that is never
> cleared (write 1 to clear)
> 
> Signed-off-by: François LEGAL <devel@thom.fr.eu.org>
> 
> Subject: [PATCH] Make MACB driver compliant with GEM hardware

Hmm, two subject lines - which one should I use? Would be better get the first 
one right already ;).

> 
> ---
>   kernel/drivers/net/drivers/Kconfig  |  7 +++++++
>   kernel/drivers/net/drivers/Makefile |  1 +
>   kernel/drivers/net/drivers/macb.c   | 23 ++++++++++++++++++++---
>   3 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/drivers/net/drivers/Kconfig b/kernel/drivers/net/drivers/Kconfig
> index 65f0855..39c976f 100644
> --- a/kernel/drivers/net/drivers/Kconfig
> +++ b/kernel/drivers/net/drivers/Kconfig
> @@ -131,6 +131,13 @@ config XENO_DRIVERS_NET_DRV_MACB
>       Driver for internal MAC-controller on AT91SAM926x microcontrollers.
>       Porting by Cristiano Mantovani and Stefano Banzi (Marposs SpA).
>   
> +config XENO_DRIVERS_NET_DRV_GEM
> +    depends on XENO_DRIVERS_NET
> +    tristate "GEM devices"
> +    ---help---
> +    Driver for internal MAC-controller on Zynq 7000 SoCs.
> +    Porting by Cristiano Mantovani and Stefano Banzi (Marposs SpA).
> +
>   endif
>   
>   source "drivers/xenomai/net/drivers/experimental/Kconfig"
> diff --git a/kernel/drivers/net/drivers/Makefile b/kernel/drivers/net/drivers/Makefile
> index d95c5a8..9da1a56 100644
> --- a/kernel/drivers/net/drivers/Makefile
> +++ b/kernel/drivers/net/drivers/Makefile
> @@ -57,6 +57,7 @@ obj-$(CONFIG_XENO_DRIVERS_NET_DRV_SMC91111) += rt_smc91111.o
>   rt_smc91111-y := smc91111.o
>   
>   obj-$(CONFIG_XENO_DRIVERS_NET_DRV_MACB) += rt_macb.o
> +obj-$(CONFIG_XENO_DRIVERS_NET_DRV_GEM) += rt_macb.o

Only saw this now: What's the idea behind adding XENO_DRIVERS_NET_DRV_GEM when 
there is XENO_DRIVERS_NET_DRV_MACB already? If a selects both, the build will fail.

>   
>   rt_macb-y := macb.o
>   
> diff --git a/kernel/drivers/net/drivers/macb.c b/kernel/drivers/net/drivers/macb.c
> index fc5b667..ccda6ae 100644
> --- a/kernel/drivers/net/drivers/macb.c
> +++ b/kernel/drivers/net/drivers/macb.c
> @@ -679,7 +679,13 @@ static int gem_rx(struct macb *bp, int budget, nanosecs_abs_t *time_stamp)
>   	unsigned int		entry;
>   	struct rtskb		*skb;
>   	struct macb_dma_desc	*desc;
> -	int			count = 0;
> +	int			count = 0, status;
> +
> +	status = macb_readl(bp, RSR);
> +	macb_writel(bp, RSR, status);
> +
> +	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +		macb_writel(bp, ISR, MACB_BIT(RCOMP));
>   
>   	while (count < budget) {
>   		u32 addr, ctrl;
> @@ -877,7 +883,7 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
>   	struct rtnet_device *dev = dev_id;
>   	struct macb *bp = rtnetdev_priv(dev);
>   	unsigned received = 0;
> -	u32 status;
> +	u32 status, ctrl;
>   
>   	status = macb_readl(bp, ISR);
>   
> @@ -895,9 +901,10 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
>   
>   		rtdev_vdbg(bp->dev, "isr = 0x%08lx\n", (unsigned long)status);
>   
> -		if (status & MACB_RX_INT_FLAGS)
> +		if (status & MACB_BIT(RCOMP)) {
>   			received += bp->macbgem_ops.mog_rx(bp, 100 - received,
>   							&time_stamp);
> +		}
>   
>   		if (unlikely(status & (MACB_TX_ERR_FLAGS))) {
>   			macb_writel(bp, IDR, MACB_TX_INT_FLAGS);
> @@ -917,6 +924,15 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
>   		 * add that if/when we get our hands on a full-blown MII PHY.
>   		 */
>   
> +		if (status & MACB_BIT(RXUBR)) {
> +			ctrl = macb_readl(bp, NCR);
> +			macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
> +			macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
> +
> +			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +				macb_writel(bp, ISR, MACB_BIT(RXUBR));
> +		}
> +
>   		if (status & MACB_BIT(ISR_ROVR)) {
>   			/* We missed at least one packet */
>   			if (macb_is_gem(bp))
> @@ -1320,6 +1336,7 @@ static void macb_configure_caps(struct macb *bp)
>   		if (GEM_BFEXT(IRQCOR, gem_readl(bp, DCFG1)) == 0)
>   			bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
>   	}
> +	rtdev_vdbg(bp->dev, "Capabilities : %X\n", bp->caps);
>   }
>   
>   static void macb_init_hw(struct macb *bp)
> 

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

[PATCH] MABC RTNet driver to work with GEM hardware

[François Legal]

From: François LEGAL <devel@thom.fr.eu.org>

Make MACB driver compliant with GEM hardware (as mainline linux
driver does). Main fix is about interrupt flag that is never
cleared (write 1 to clear)

Signed-off-by: François LEGAL <devel@thom.fr.eu.org>

Subject: [PATCH] Make MACB driver compliant with GEM hardware

---
 kernel/drivers/net/drivers/Kconfig  |  7 +++++++
 kernel/drivers/net/drivers/Makefile |  1 +
 kernel/drivers/net/drivers/macb.c   | 23 ++++++++++++++++++++---
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/kernel/drivers/net/drivers/Kconfig b/kernel/drivers/net/drivers/Kconfig
index 65f0855..39c976f 100644
--- a/kernel/drivers/net/drivers/Kconfig
+++ b/kernel/drivers/net/drivers/Kconfig
@@ -131,6 +131,13 @@ config XENO_DRIVERS_NET_DRV_MACB
     Driver for internal MAC-controller on AT91SAM926x microcontrollers.
     Porting by Cristiano Mantovani and Stefano Banzi (Marposs SpA).

+config XENO_DRIVERS_NET_DRV_GEM
+    depends on XENO_DRIVERS_NET
+    tristate "GEM devices"
+    ---help---
+    Driver for internal MAC-controller on Zynq 7000 SoCs.
+    Porting by Cristiano Mantovani and Stefano Banzi (Marposs SpA).
+
 endif

 source "drivers/xenomai/net/drivers/experimental/Kconfig"
diff --git a/kernel/drivers/net/drivers/Makefile b/kernel/drivers/net/drivers/Makefile
index d95c5a8..9da1a56 100644
--- a/kernel/drivers/net/drivers/Makefile
+++ b/kernel/drivers/net/drivers/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_XENO_DRIVERS_NET_DRV_SMC91111) += rt_smc91111.o
 rt_smc91111-y := smc91111.o

 obj-$(CONFIG_XENO_DRIVERS_NET_DRV_MACB) += rt_macb.o
+obj-$(CONFIG_XENO_DRIVERS_NET_DRV_GEM) += rt_macb.o

 rt_macb-y := macb.o

diff --git a/kernel/drivers/net/drivers/macb.c b/kernel/drivers/net/drivers/macb.c
index fc5b667..ccda6ae 100644
--- a/kernel/drivers/net/drivers/macb.c
+++ b/kernel/drivers/net/drivers/macb.c
@@ -679,7 +679,13 @@ static int gem_rx(struct macb *bp, int budget, nanosecs_abs_t *time_stamp)
 	unsigned int		entry;
 	struct rtskb		*skb;
 	struct macb_dma_desc	*desc;
-	int			count = 0;
+	int			count = 0, status;
+
+	status = macb_readl(bp, RSR);
+	macb_writel(bp, RSR, status);
+
+	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+		macb_writel(bp, ISR, MACB_BIT(RCOMP));

 	while (count < budget) {
 		u32 addr, ctrl;
@@ -877,7 +883,7 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
 	struct rtnet_device *dev = dev_id;
 	struct macb *bp = rtnetdev_priv(dev);
 	unsigned received = 0;
-	u32 status;
+	u32 status, ctrl;

 	status = macb_readl(bp, ISR);

@@ -895,9 +901,10 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)

 		rtdev_vdbg(bp->dev, "isr = 0x%08lx\n", (unsigned long)status);

-		if (status & MACB_RX_INT_FLAGS)
+		if (status & MACB_BIT(RCOMP)) {
 			received += bp->macbgem_ops.mog_rx(bp, 100 - received,
 							&time_stamp);
+		}

 		if (unlikely(status & (MACB_TX_ERR_FLAGS))) {
 			macb_writel(bp, IDR, MACB_TX_INT_FLAGS);
@@ -917,6 +924,15 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
 		 * add that if/when we get our hands on a full-blown MII PHY.
 		 */

+		if (status & MACB_BIT(RXUBR)) {
+			ctrl = macb_readl(bp, NCR);
+			macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
+			macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
+
+			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+				macb_writel(bp, ISR, MACB_BIT(RXUBR));
+		}
+
 		if (status & MACB_BIT(ISR_ROVR)) {
 			/* We missed at least one packet */
 			if (macb_is_gem(bp))
@@ -1320,6 +1336,7 @@ static void macb_configure_caps(struct macb *bp)
 		if (GEM_BFEXT(IRQCOR, gem_readl(bp, DCFG1)) == 0)
 			bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
 	}
+	rtdev_vdbg(bp->dev, "Capabilities : %X\n", bp->caps);
 }

 static void macb_init_hw(struct macb *bp)
--
2.1.4

[PATCH] MABC RTNet driver to work with GEM hardware

[François Legal]

From: François LEGAL <devel@thom.fr.eu.org>

Make MACB driver compliant with GEM hardware (as mainline linux
driver does). Main fix is about interrupt flag that is never
cleared (write 1 to clear)

Signed-off-by: François LEGAL <devel@thom.fr.eu.org>

Subject: [PATCH] Make MACB driver compliant with GEM hardware

---
 kernel/drivers/net/drivers/Kconfig  | 11 +++++++++--
 kernel/drivers/net/drivers/Makefile |  1 +
 kernel/drivers/net/drivers/macb.c   | 27 +++++++++++++++++++++++----
 3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/kernel/drivers/net/drivers/Kconfig b/kernel/drivers/net/drivers/Kconfig
index 65f0855..89ac237 100644
--- a/kernel/drivers/net/drivers/Kconfig
+++ b/kernel/drivers/net/drivers/Kconfig
@@ -77,7 +77,7 @@ config XENO_DRIVERS_NET_DRV_R8169
     tristate "Realtek 8169 (Gigabit)"


-if PPC
+if ARCH = ppc

 comment "Embedded MPC Drivers"
     depends on XENO_DRIVERS_NET
@@ -116,7 +116,7 @@ config XENO_DRIVERS_NET_DRV_SMC91111
     depends on XENO_DRIVERS_NET
     tristate "SMSC LAN91C111"

-if ARM
+if ARCH = arm

 config XENO_DRIVERS_NET_DRV_AT91_ETHER
     depends on XENO_DRIVERS_NET && SOC_AT91RM9200
@@ -131,6 +131,13 @@ config XENO_DRIVERS_NET_DRV_MACB
     Driver for internal MAC-controller on AT91SAM926x microcontrollers.
     Porting by Cristiano Mantovani and Stefano Banzi (Marposs SpA).

+config XENO_DRIVERS_NET_DRV_GEM
+    depends on XENO_DRIVERS_NET
+    tristate "GEM devices"
+    ---help---
+    Driver for internal MAC-controller on Zynq 7000 SoCs.
+    Porting by Cristiano Mantovani and Stefano Banzi (Marposs SpA).
+
 endif

 source "drivers/xenomai/net/drivers/experimental/Kconfig"
diff --git a/kernel/drivers/net/drivers/Makefile b/kernel/drivers/net/drivers/Makefile
index d95c5a8..9da1a56 100644
--- a/kernel/drivers/net/drivers/Makefile
+++ b/kernel/drivers/net/drivers/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_XENO_DRIVERS_NET_DRV_SMC91111) += rt_smc91111.o
 rt_smc91111-y := smc91111.o

 obj-$(CONFIG_XENO_DRIVERS_NET_DRV_MACB) += rt_macb.o
+obj-$(CONFIG_XENO_DRIVERS_NET_DRV_GEM) += rt_macb.o

 rt_macb-y := macb.o

diff --git a/kernel/drivers/net/drivers/macb.c b/kernel/drivers/net/drivers/macb.c
index fc5b667..e97a7d1 100644
--- a/kernel/drivers/net/drivers/macb.c
+++ b/kernel/drivers/net/drivers/macb.c
@@ -679,7 +679,14 @@ static int gem_rx(struct macb *bp, int budget, nanosecs_abs_t *time_stamp)
 	unsigned int		entry;
 	struct rtskb		*skb;
 	struct macb_dma_desc	*desc;
-	int			count = 0;
+	int			count = 0, status;
+
+
+	status = macb_readl(bp, RSR);
+	macb_writel(bp, RSR, status);
+
+	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+		macb_writel(bp, ISR, MACB_BIT(RCOMP));

 	while (count < budget) {
 		u32 addr, ctrl;
@@ -725,6 +732,7 @@ static int gem_rx(struct macb *bp, int budget, nanosecs_abs_t *time_stamp)
 				 bp->rx_buffer_size, DMA_FROM_DEVICE);

 		skb->protocol = rt_eth_type_trans(skb, bp->dev);
+		rtskb_checksum_none_assert(skb);

 		bp->stats.rx_packets++;
 		bp->stats.rx_bytes += skb->len;
@@ -791,6 +799,7 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
 	offset = 0;
 	len += NET_IP_ALIGN;
 	skb->time_stamp = *time_stamp;
+	rtskb_checksum_none_assert(skb);
 	rtskb_put(skb, len);

 	for (frag = first_frag; ; frag++) {
@@ -877,7 +886,7 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
 	struct rtnet_device *dev = dev_id;
 	struct macb *bp = rtnetdev_priv(dev);
 	unsigned received = 0;
-	u32 status;
+	u32 status, ctrl;

 	status = macb_readl(bp, ISR);

@@ -895,9 +904,10 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)

 		rtdev_vdbg(bp->dev, "isr = 0x%08lx\n", (unsigned long)status);

-		if (status & MACB_RX_INT_FLAGS)
+		if (status & MACB_BIT(RCOMP)) {
 			received += bp->macbgem_ops.mog_rx(bp, 100 - received,
 							&time_stamp);
+		}

 		if (unlikely(status & (MACB_TX_ERR_FLAGS))) {
 			macb_writel(bp, IDR, MACB_TX_INT_FLAGS);
@@ -917,6 +927,15 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
 		 * add that if/when we get our hands on a full-blown MII PHY.
 		 */

+		if (status & MACB_BIT(RXUBR)) {
+			ctrl = macb_readl(bp, NCR);
+			macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
+			macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
+
+			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+				macb_writel(bp, ISR, MACB_BIT(RXUBR));
+		}
+
 		if (status & MACB_BIT(ISR_ROVR)) {
 			/* We missed at least one packet */
 			if (macb_is_gem(bp))
@@ -1320,6 +1339,7 @@ static void macb_configure_caps(struct macb *bp)
 		if (GEM_BFEXT(IRQCOR, gem_readl(bp, DCFG1)) == 0)
 			bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
 	}
+	rtdev_dbg(bp->dev, "Capabilities : %X\n", bp->caps);
 }

 static void macb_init_hw(struct macb *bp)
@@ -1578,7 +1598,6 @@ static int __init macb_probe(struct platform_device *pdev)
 	rtdev_alloc_name(dev, "rteth%d");
 	rt_rtdev_connect(dev, &RTDEV_manager);
 	dev->vers = RTDEV_VERS_2_0;
-	dev->sysbind = &pdev->dev;

 	/* TODO: Actually, we have some interesting features... */
 	dev->features |= 0;
--
2.1.4

Re: [PATCH] MABC RTNet driver to work with GEM hardware

[Jan Kiszka]

On 02.10.19 13:41, François Legal via Xenomai wrote:
>  From 9703097ac23dfdea91bc688267e0d46ea20ceacc Mon Sep 17 00:00:00 2001
> From: dev <dev@DevCoreProcessing.assystem>
> Date: Wed, 2 Oct 2019 11:58:01 +0200
> Subject: [PATCH] Make MACB driver compliant with GEM hardware (as mainline
>   linux driver does). Main fix is about interrupt flag that is never cleared
>   (write 1 to clear)
> 

Almost: Subject should be brief, move the extended "why" into the commit message 
body. And then we need a signed-off, confirming [1].

> ---
>   kernel/drivers/net/drivers/Kconfig  | 11 +++++++++--
>   kernel/drivers/net/drivers/Makefile |  1 +
>   kernel/drivers/net/drivers/macb.c   | 27 +++++++++++++++++++++++----
>   3 files changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/drivers/net/drivers/Kconfig b/kernel/drivers/net/drivers/Kconfig
> index 65f0855..89ac237 100644
> --- a/kernel/drivers/net/drivers/Kconfig
> +++ b/kernel/drivers/net/drivers/Kconfig
> @@ -77,7 +77,7 @@ config XENO_DRIVERS_NET_DRV_R8169
>       tristate "Realtek 8169 (Gigabit)"
>   
>   
> -if PPC
> +if ARCH = ppc

Why that?

>   
>   comment "Embedded MPC Drivers"
>       depends on XENO_DRIVERS_NET
> @@ -116,7 +116,7 @@ config XENO_DRIVERS_NET_DRV_SMC91111
>       depends on XENO_DRIVERS_NET
>       tristate "SMSC LAN91C111"
>   
> -if ARM
> +if ARCH = arm

Same here.

>   
>   config XENO_DRIVERS_NET_DRV_AT91_ETHER
>       depends on XENO_DRIVERS_NET && SOC_AT91RM9200
> @@ -131,6 +131,13 @@ config XENO_DRIVERS_NET_DRV_MACB
>       Driver for internal MAC-controller on AT91SAM926x microcontrollers.
>       Porting by Cristiano Mantovani and Stefano Banzi (Marposs SpA).
>   
> +config XENO_DRIVERS_NET_DRV_GEM
> +    depends on XENO_DRIVERS_NET
> +    tristate "GEM devices"
> +    ---help---
> +    Driver for internal MAC-controller on Zynq 7000 SoCs.
> +    Porting by Cristiano Mantovani and Stefano Banzi (Marposs SpA).
> +
>   endif
>   
>   source "drivers/xenomai/net/drivers/experimental/Kconfig"
> diff --git a/kernel/drivers/net/drivers/Makefile b/kernel/drivers/net/drivers/Makefile
> index d95c5a8..9da1a56 100644
> --- a/kernel/drivers/net/drivers/Makefile
> +++ b/kernel/drivers/net/drivers/Makefile
> @@ -57,6 +57,7 @@ obj-$(CONFIG_XENO_DRIVERS_NET_DRV_SMC91111) += rt_smc91111.o
>   rt_smc91111-y := smc91111.o
>   
>   obj-$(CONFIG_XENO_DRIVERS_NET_DRV_MACB) += rt_macb.o
> +obj-$(CONFIG_XENO_DRIVERS_NET_DRV_GEM) += rt_macb.o
>   
>   rt_macb-y := macb.o
>   
> diff --git a/kernel/drivers/net/drivers/macb.c b/kernel/drivers/net/drivers/macb.c
> index fc5b667..e97a7d1 100644
> --- a/kernel/drivers/net/drivers/macb.c
> +++ b/kernel/drivers/net/drivers/macb.c
> @@ -679,7 +679,14 @@ static int gem_rx(struct macb *bp, int budget, nanosecs_abs_t *time_stamp)
>   	unsigned int		entry;
>   	struct rtskb		*skb;
>   	struct macb_dma_desc	*desc;
> -	int			count = 0;
> +	int			count = 0, status;
> +
> +

Likely one newline too much (unless that strictly aligns with upstream).

> +	status = macb_readl(bp, RSR);
> +	macb_writel(bp, RSR, status);
> +
> +	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +		macb_writel(bp, ISR, MACB_BIT(RCOMP));
>   
>   	while (count < budget) {
>   		u32 addr, ctrl;
> @@ -725,6 +732,7 @@ static int gem_rx(struct macb *bp, int budget, nanosecs_abs_t *time_stamp)
>   				 bp->rx_buffer_size, DMA_FROM_DEVICE);
>   
>   		skb->protocol = rt_eth_type_trans(skb, bp->dev);
> +		rtskb_checksum_none_assert(skb);

This was just removed (0ff501aa33f51f118b8339096f10bbea184141a4).

>   
>   		bp->stats.rx_packets++;
>   		bp->stats.rx_bytes += skb->len;
> @@ -791,6 +799,7 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
>   	offset = 0;
>   	len += NET_IP_ALIGN;
>   	skb->time_stamp = *time_stamp;
> +	rtskb_checksum_none_assert(skb);
>   	rtskb_put(skb, len);
>   
>   	for (frag = first_frag; ; frag++) {
> @@ -877,7 +886,7 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
>   	struct rtnet_device *dev = dev_id;
>   	struct macb *bp = rtnetdev_priv(dev);
>   	unsigned received = 0;
> -	u32 status;
> +	u32 status, ctrl;
>   
>   	status = macb_readl(bp, ISR);
>   
> @@ -895,9 +904,10 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
>   
>   		rtdev_vdbg(bp->dev, "isr = 0x%08lx\n", (unsigned long)status);
>   
> -		if (status & MACB_RX_INT_FLAGS)
> +		if (status & MACB_BIT(RCOMP)) {
>   			received += bp->macbgem_ops.mog_rx(bp, 100 - received,
>   							&time_stamp);
> +		}
>   
>   		if (unlikely(status & (MACB_TX_ERR_FLAGS))) {
>   			macb_writel(bp, IDR, MACB_TX_INT_FLAGS);
> @@ -917,6 +927,15 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
>   		 * add that if/when we get our hands on a full-blown MII PHY.
>   		 */
>   
> +		if (status & MACB_BIT(RXUBR)) {
> +			ctrl = macb_readl(bp, NCR);
> +			macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
> +			macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
> +
> +			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +				macb_writel(bp, ISR, MACB_BIT(RXUBR));
> +		}
> +
>   		if (status & MACB_BIT(ISR_ROVR)) {
>   			/* We missed at least one packet */
>   			if (macb_is_gem(bp))
> @@ -1320,6 +1339,7 @@ static void macb_configure_caps(struct macb *bp)
>   		if (GEM_BFEXT(IRQCOR, gem_readl(bp, DCFG1)) == 0)
>   			bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
>   	}
> +	rtdev_dbg(bp->dev, "Capabilities : %X\n", bp->caps);

Is that a left-over debug output?

>   }
>   
>   static void macb_init_hw(struct macb *bp)
> @@ -1578,7 +1598,6 @@ static int __init macb_probe(struct platform_device *pdev)
>   	rtdev_alloc_name(dev, "rteth%d");
>   	rt_rtdev_connect(dev, &RTDEV_manager);
>   	dev->vers = RTDEV_VERS_2_0;
> -	dev->sysbind = &pdev->dev;
>   
>   	/* TODO: Actually, we have some interesting features... */
>   	dev->features |= 0;
> 

Thanks,
Jan

[1] https://developercertificate.org/

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

[PATCH] MABC RTNet driver to work with GEM hardware

[François Legal]

>From 9703097ac23dfdea91bc688267e0d46ea20ceacc Mon Sep 17 00:00:00 2001
From: dev <dev@DevCoreProcessing.assystem>
Date: Wed, 2 Oct 2019 11:58:01 +0200
Subject: [PATCH] Make MACB driver compliant with GEM hardware (as mainline
 linux driver does). Main fix is about interrupt flag that is never cleared
 (write 1 to clear)

---
 kernel/drivers/net/drivers/Kconfig  | 11 +++++++++--
 kernel/drivers/net/drivers/Makefile |  1 +
 kernel/drivers/net/drivers/macb.c   | 27 +++++++++++++++++++++++----
 3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/kernel/drivers/net/drivers/Kconfig b/kernel/drivers/net/drivers/Kconfig
index 65f0855..89ac237 100644
--- a/kernel/drivers/net/drivers/Kconfig
+++ b/kernel/drivers/net/drivers/Kconfig
@@ -77,7 +77,7 @@ config XENO_DRIVERS_NET_DRV_R8169
     tristate "Realtek 8169 (Gigabit)"


-if PPC
+if ARCH = ppc

 comment "Embedded MPC Drivers"
     depends on XENO_DRIVERS_NET
@@ -116,7 +116,7 @@ config XENO_DRIVERS_NET_DRV_SMC91111
     depends on XENO_DRIVERS_NET
     tristate "SMSC LAN91C111"

-if ARM
+if ARCH = arm

 config XENO_DRIVERS_NET_DRV_AT91_ETHER
     depends on XENO_DRIVERS_NET && SOC_AT91RM9200
@@ -131,6 +131,13 @@ config XENO_DRIVERS_NET_DRV_MACB
     Driver for internal MAC-controller on AT91SAM926x microcontrollers.
     Porting by Cristiano Mantovani and Stefano Banzi (Marposs SpA).

+config XENO_DRIVERS_NET_DRV_GEM
+    depends on XENO_DRIVERS_NET
+    tristate "GEM devices"
+    ---help---
+    Driver for internal MAC-controller on Zynq 7000 SoCs.
+    Porting by Cristiano Mantovani and Stefano Banzi (Marposs SpA).
+
 endif

 source "drivers/xenomai/net/drivers/experimental/Kconfig"
diff --git a/kernel/drivers/net/drivers/Makefile b/kernel/drivers/net/drivers/Makefile
index d95c5a8..9da1a56 100644
--- a/kernel/drivers/net/drivers/Makefile
+++ b/kernel/drivers/net/drivers/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_XENO_DRIVERS_NET_DRV_SMC91111) += rt_smc91111.o
 rt_smc91111-y := smc91111.o

 obj-$(CONFIG_XENO_DRIVERS_NET_DRV_MACB) += rt_macb.o
+obj-$(CONFIG_XENO_DRIVERS_NET_DRV_GEM) += rt_macb.o

 rt_macb-y := macb.o

diff --git a/kernel/drivers/net/drivers/macb.c b/kernel/drivers/net/drivers/macb.c
index fc5b667..e97a7d1 100644
--- a/kernel/drivers/net/drivers/macb.c
+++ b/kernel/drivers/net/drivers/macb.c
@@ -679,7 +679,14 @@ static int gem_rx(struct macb *bp, int budget, nanosecs_abs_t *time_stamp)
 	unsigned int		entry;
 	struct rtskb		*skb;
 	struct macb_dma_desc	*desc;
-	int			count = 0;
+	int			count = 0, status;
+
+
+	status = macb_readl(bp, RSR);
+	macb_writel(bp, RSR, status);
+
+	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+		macb_writel(bp, ISR, MACB_BIT(RCOMP));

 	while (count < budget) {
 		u32 addr, ctrl;
@@ -725,6 +732,7 @@ static int gem_rx(struct macb *bp, int budget, nanosecs_abs_t *time_stamp)
 				 bp->rx_buffer_size, DMA_FROM_DEVICE);

 		skb->protocol = rt_eth_type_trans(skb, bp->dev);
+		rtskb_checksum_none_assert(skb);

 		bp->stats.rx_packets++;
 		bp->stats.rx_bytes += skb->len;
@@ -791,6 +799,7 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
 	offset = 0;
 	len += NET_IP_ALIGN;
 	skb->time_stamp = *time_stamp;
+	rtskb_checksum_none_assert(skb);
 	rtskb_put(skb, len);

 	for (frag = first_frag; ; frag++) {
@@ -877,7 +886,7 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
 	struct rtnet_device *dev = dev_id;
 	struct macb *bp = rtnetdev_priv(dev);
 	unsigned received = 0;
-	u32 status;
+	u32 status, ctrl;

 	status = macb_readl(bp, ISR);

@@ -895,9 +904,10 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)

 		rtdev_vdbg(bp->dev, "isr = 0x%08lx\n", (unsigned long)status);

-		if (status & MACB_RX_INT_FLAGS)
+		if (status & MACB_BIT(RCOMP)) {
 			received += bp->macbgem_ops.mog_rx(bp, 100 - received,
 							&time_stamp);
+		}

 		if (unlikely(status & (MACB_TX_ERR_FLAGS))) {
 			macb_writel(bp, IDR, MACB_TX_INT_FLAGS);
@@ -917,6 +927,15 @@ static int macb_interrupt(rtdm_irq_t *irq_handle)
 		 * add that if/when we get our hands on a full-blown MII PHY.
 		 */

+		if (status & MACB_BIT(RXUBR)) {
+			ctrl = macb_readl(bp, NCR);
+			macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
+			macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
+
+			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+				macb_writel(bp, ISR, MACB_BIT(RXUBR));
+		}
+
 		if (status & MACB_BIT(ISR_ROVR)) {
 			/* We missed at least one packet */
 			if (macb_is_gem(bp))
@@ -1320,6 +1339,7 @@ static void macb_configure_caps(struct macb *bp)
 		if (GEM_BFEXT(IRQCOR, gem_readl(bp, DCFG1)) == 0)
 			bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
 	}
+	rtdev_dbg(bp->dev, "Capabilities : %X\n", bp->caps);
 }

 static void macb_init_hw(struct macb *bp)
@@ -1578,7 +1598,6 @@ static int __init macb_probe(struct platform_device *pdev)
 	rtdev_alloc_name(dev, "rteth%d");
 	rt_rtdev_connect(dev, &RTDEV_manager);
 	dev->vers = RTDEV_VERS_2_0;
-	dev->sysbind = &pdev->dev;

 	/* TODO: Actually, we have some interesting features... */
 	dev->features |= 0;
--
2.1.4