netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
@ 2013-05-13 16:05 Nicolas Ferre
  2013-05-13 16:05 ` Jean-Christophe PLAGNIOL-VILLARD
                   ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Nicolas Ferre @ 2013-05-13 16:05 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD, hein_tibosch
  Cc: michal.simek, Ludovic Desroches, s.trumtrar, linux-arm-kernel,
	linux-kernel, netdev, Nicolas Ferre

Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
introduces clear-on-write on ISR register. This behavior is not always
implemented when using Cadence MACB/GEM and is breaking other platforms.
We are using a new Device Tree compatibility string and a capability
property to actually activate this clear-on-write behavior on ISR.

Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 Documentation/devicetree/bindings/net/macb.txt |  2 ++
 drivers/net/ethernet/cadence/macb.c            | 15 +++++++++++++--
 drivers/net/ethernet/cadence/macb.h            |  5 +++++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 44afa0e..13ec4f6 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -6,6 +6,8 @@ Required properties:
   Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb".
   Use "cnds,pc302-gem" for Picochip picoXcell pc302 and later devices based on
   the Cadence GEM, or the generic form: "cdns,gem".
+  Use "cdns,zynq-7000-gem" for devices based on Cadence GEM with alternative
+  options enabled (ISR clear on write).
 - reg: Address and length of the register set for the device
 - interrupts: Should contain macb interrupt
 - phy-mode: String, operation mode of the PHY interface.
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 6be513d..628f2b0 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -485,7 +485,8 @@ static void macb_tx_interrupt(struct macb *bp)
 	status = macb_readl(bp, TSR);
 	macb_writel(bp, TSR, status);
 
-	macb_writel(bp, ISR, MACB_BIT(TCOMP));
+	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+		macb_writel(bp, ISR, MACB_BIT(TCOMP));
 
 	netdev_vdbg(bp->dev, "macb_tx_interrupt status = 0x%03lx\n",
 		(unsigned long)status);
@@ -738,7 +739,8 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 			 * now.
 			 */
 			macb_writel(bp, IDR, MACB_RX_INT_FLAGS);
-			macb_writel(bp, ISR, MACB_BIT(RCOMP));
+			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+				macb_writel(bp, ISR, MACB_BIT(RCOMP));
 
 			if (napi_schedule_prep(&bp->napi)) {
 				netdev_vdbg(bp->dev, "scheduling RX softirq\n");
@@ -1474,6 +1476,10 @@ static const struct of_device_id macb_dt_ids[] = {
 	{ .compatible = "cdns,macb" },
 	{ .compatible = "cdns,pc302-gem" },
 	{ .compatible = "cdns,gem" },
+	{
+		.compatible = "cdns,zynq-7000-gem",
+		.data = (void *)MACB_CAPS_ISR_CLEAR_ON_WRITE,
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, macb_dt_ids);
@@ -1484,6 +1490,7 @@ static int __init macb_probe(struct platform_device *pdev)
 	struct macb_platform_data *pdata;
 	struct resource *regs;
 	struct net_device *dev;
+	const struct of_device_id *dev_id;
 	struct macb *bp;
 	struct phy_device *phydev;
 	u32 config;
@@ -1558,6 +1565,10 @@ static int __init macb_probe(struct platform_device *pdev)
 
 	dev->base_addr = regs->start;
 
+	dev_id = of_match_device(macb_dt_ids, &pdev->dev);
+	if (dev_id)
+		bp->caps = (u32)dev_id->data;
+
 	/* Set MII management clock divider */
 	config = macb_mdc_clk_div(bp);
 	config |= macb_dbw(bp);
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 993d703..5d622fe 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -323,6 +323,9 @@
 #define MACB_MAN_READ				2
 #define MACB_MAN_CODE				2
 
+/* Capability mask bits */
+#define MACB_CAPS_ISR_CLEAR_ON_WRITE		0x1
+
 /* Bit manipulation macros */
 #define MACB_BIT(name)					\
 	(1 << MACB_##name##_OFFSET)
@@ -574,6 +577,8 @@ struct macb {
 	unsigned int 		speed;
 	unsigned int 		duplex;
 
+	u32			caps;
+
 	phy_interface_t		phy_interface;
 
 	/* AT91RM9200 transmit */
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-13 16:05 [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC Nicolas Ferre
@ 2013-05-13 16:05 ` Jean-Christophe PLAGNIOL-VILLARD
  2013-05-14  0:58   ` Hein Tibosch
  2013-05-14  9:16   ` Nicolas Ferre
  2013-05-14  7:01 ` Steffen Trumtrar
  2013-05-14 13:00 ` [PATCH v2] " Nicolas Ferre
  2 siblings, 2 replies; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-13 16:05 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Jean-Christophe PLAGNIOL-VILLARD, hein_tibosch, michal.simek,
	Ludovic Desroches, s.trumtrar, linux-arm-kernel, linux-kernel,
	netdev


On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:

> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
> introduces clear-on-write on ISR register. This behavior is not always
> implemented when using Cadence MACB/GEM and is breaking other platforms.
> We are using a new Device Tree compatibility string and a capability
> property to actually activate this clear-on-write behavior on ISR.
> 
> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>

can we detect it via the IP?
> ---
> Documentation/devicetree/bindings/net/macb.txt |  2 ++
> drivers/net/ethernet/cadence/macb.c            | 15 +++++++++++++--
> drivers/net/ethernet/cadence/macb.h            |  5 +++++
> 3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
> index 44afa0e..13ec4f6 100644
> --- a/Documentation/devicetree/bindings/net/macb.txt
> +++ b/Documentation/devicetree/bindings/net/macb.txt
> @@ -6,6 +6,8 @@ Required properties:
>   Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb".
>   Use "cnds,pc302-gem" for Picochip picoXcell pc302 and later devices based on
>   the Cadence GEM, or the generic form: "cdns,gem".
> +  Use "cdns,zynq-7000-gem" for devices based on Cadence GEM with alternative
> +  options enabled (ISR clear on write).
> - reg: Address and length of the register set for the device
> - interrupts: Should contain macb interrupt
> - phy-mode: String, operation mode of the PHY interface.
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 6be513d..628f2b0 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -485,7 +485,8 @@ static void macb_tx_interrupt(struct macb *bp)
> 	status = macb_readl(bp, TSR);
> 	macb_writel(bp, TSR, status);
> 
> -	macb_writel(bp, ISR, MACB_BIT(TCOMP));
> +	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +		macb_writel(bp, ISR, MACB_BIT(TCOMP));
> 
> 	netdev_vdbg(bp->dev, "macb_tx_interrupt status = 0x%03lx\n",
> 		(unsigned long)status);
> @@ -738,7 +739,8 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
> 			 * now.
> 			 */
> 			macb_writel(bp, IDR, MACB_RX_INT_FLAGS);
> -			macb_writel(bp, ISR, MACB_BIT(RCOMP));
> +			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +				macb_writel(bp, ISR, MACB_BIT(RCOMP));
> 
> 			if (napi_schedule_prep(&bp->napi)) {
> 				netdev_vdbg(bp->dev, "scheduling RX softirq\n");
> @@ -1474,6 +1476,10 @@ static const struct of_device_id macb_dt_ids[] = {
> 	{ .compatible = "cdns,macb" },
> 	{ .compatible = "cdns,pc302-gem" },
> 	{ .compatible = "cdns,gem" },
> +	{
> +		.compatible = "cdns,zynq-7000-gem",
> +		.data = (void *)MACB_CAPS_ISR_CLEAR_ON_WRITE,
> +	},
> 	{ /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, macb_dt_ids);
> @@ -1484,6 +1490,7 @@ static int __init macb_probe(struct platform_device *pdev)
> 	struct macb_platform_data *pdata;
> 	struct resource *regs;
> 	struct net_device *dev;
> +	const struct of_device_id *dev_id;
> 	struct macb *bp;
> 	struct phy_device *phydev;
> 	u32 config;
> @@ -1558,6 +1565,10 @@ static int __init macb_probe(struct platform_device *pdev)
> 
> 	dev->base_addr = regs->start;
> 
> +	dev_id = of_match_device(macb_dt_ids, &pdev->dev);
> +	if (dev_id)
> +		bp->caps = (u32)dev_id->data;
> +
> 	/* Set MII management clock divider */
> 	config = macb_mdc_clk_div(bp);
> 	config |= macb_dbw(bp);
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 993d703..5d622fe 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -323,6 +323,9 @@
> #define MACB_MAN_READ				2
> #define MACB_MAN_CODE				2
> 
> +/* Capability mask bits */
> +#define MACB_CAPS_ISR_CLEAR_ON_WRITE		0x1
> +
> /* Bit manipulation macros */
> #define MACB_BIT(name)					\
> 	(1 << MACB_##name##_OFFSET)
> @@ -574,6 +577,8 @@ struct macb {
> 	unsigned int 		speed;
> 	unsigned int 		duplex;
> 
> +	u32			caps;
> +
> 	phy_interface_t		phy_interface;
> 
> 	/* AT91RM9200 transmit */
> -- 
> 1.8.0
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-13 16:05 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2013-05-14  0:58   ` Hein Tibosch
  2013-05-14  5:52     ` Jean-Christophe PLAGNIOL-VILLARD
  2013-05-14  9:16   ` Nicolas Ferre
  1 sibling, 1 reply; 22+ messages in thread
From: Hein Tibosch @ 2013-05-14  0:58 UTC (permalink / raw)
  To: Nicolas Ferre, Jean-Christophe PLAGNIOL-VILLARD
  Cc: michal.simek, Ludovic Desroches, s.trumtrar, linux-arm-kernel,
	linux-kernel, netdev

On 5/14/2013 12:05 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>
>> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
>> introduces clear-on-write on ISR register. This behavior is not always
>> implemented when using Cadence MACB/GEM and is breaking other platforms.
>> We are using a new Device Tree compatibility string and a capability
>> property to actually activate this clear-on-write behavior on ISR.
>>
>> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> can we detect it via the IP?

This was my first proposal, have it based on the value of MACB's
register 'MID' (offset 0x00fc, lower 16 bits).
On avr32 it reads: 0x0000010D, on Zynq it reports 0x00000119

So for the moment, CAPS_ISR_CLEAR_ON_WRITE could be set if the revision
equals to 0x00000119?

>> ---
>> Documentation/devicetree/bindings/net/macb.txt |  2 ++
>> drivers/net/ethernet/cadence/macb.c            | 15 +++++++++++++--
>> drivers/net/ethernet/cadence/macb.h            |  5 +++++
>> 3 files changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
>> index 44afa0e..13ec4f6 100644
>>
>> <snip>
>>
>> @@ -1474,6 +1476,10 @@ static const struct of_device_id macb_dt_ids[] = {
>> 	{ .compatible = "cdns,macb" },
>> 	{ .compatible = "cdns,pc302-gem" },
>> 	{ .compatible = "cdns,gem" },
>> +	{
>> +		.compatible = "cdns,zynq-7000-gem",
>> +		.data = (void *)MACB_CAPS_ISR_CLEAR_ON_WRITE,
>> +	},
>> 	{ /* sentinel */ }
>> };
>> MODULE_DEVICE_TABLE(of, macb_dt_ids);
>> @@ -1484,6 +1490,7 @@ static int __init macb_probe(struct platform_device *pdev)
>> 	struct macb_platform_data *pdata;
>> 	struct resource *regs;
>> 	struct net_device *dev;
>> +	const struct of_device_id *dev_id;
>> 	struct macb *bp;
>> 	struct phy_device *phydev;
>> 	u32 config;
>> @@ -1558,6 +1565,10 @@ static int __init macb_probe(struct platform_device *pdev)
>>
>> 	dev->base_addr = regs->start;
>>
>> +	dev_id = of_match_device(macb_dt_ids, &pdev->dev);
As avr32 doesn't yet define CONFIG_OF:
    macb.c:1601: error:  'macb_dt_ids' undeclared

Hein

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-14  0:58   ` Hein Tibosch
@ 2013-05-14  5:52     ` Jean-Christophe PLAGNIOL-VILLARD
  2013-05-14  7:18       ` Hein Tibosch
  2013-06-04  6:15       ` Michal Simek
  0 siblings, 2 replies; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-14  5:52 UTC (permalink / raw)
  To: Hein Tibosch
  Cc: netdev, Nicolas Ferre, michal.simek, linux-kernel,
	Ludovic Desroches, s.trumtrar, linux-arm-kernel

On 08:58 Tue 14 May     , Hein Tibosch wrote:
> On 5/14/2013 12:05 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
> >
> >> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
> >> introduces clear-on-write on ISR register. This behavior is not always
> >> implemented when using Cadence MACB/GEM and is breaking other platforms.
> >> We are using a new Device Tree compatibility string and a capability
> >> property to actually activate this clear-on-write behavior on ISR.
> >>
> >> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
> >> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> > can we detect it via the IP?
> 
> This was my first proposal, have it based on the value of MACB's
> register 'MID' (offset 0x00fc, lower 16 bits).
> On avr32 it reads: 0x0000010D, on Zynq it reports 0x00000119
> 
> So for the moment, CAPS_ISR_CLEAR_ON_WRITE could be set if the revision
> equals to 0x00000119?
so no it will not work

as the gem on sama5 is 00020119

so version 0x119 too

nico 
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

Best Regards,
J.
> 
> >> ---
> >> Documentation/devicetree/bindings/net/macb.txt |  2 ++
> >> drivers/net/ethernet/cadence/macb.c            | 15 +++++++++++++--
> >> drivers/net/ethernet/cadence/macb.h            |  5 +++++
> >> 3 files changed, 20 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
> >> index 44afa0e..13ec4f6 100644
> >>
> >> <snip>
> >>
> >> @@ -1474,6 +1476,10 @@ static const struct of_device_id macb_dt_ids[] = {
> >> 	{ .compatible = "cdns,macb" },
> >> 	{ .compatible = "cdns,pc302-gem" },
> >> 	{ .compatible = "cdns,gem" },
> >> +	{
> >> +		.compatible = "cdns,zynq-7000-gem",
> >> +		.data = (void *)MACB_CAPS_ISR_CLEAR_ON_WRITE,
> >> +	},
> >> 	{ /* sentinel */ }
> >> };
> >> MODULE_DEVICE_TABLE(of, macb_dt_ids);
> >> @@ -1484,6 +1490,7 @@ static int __init macb_probe(struct platform_device *pdev)
> >> 	struct macb_platform_data *pdata;
> >> 	struct resource *regs;
> >> 	struct net_device *dev;
> >> +	const struct of_device_id *dev_id;
> >> 	struct macb *bp;
> >> 	struct phy_device *phydev;
> >> 	u32 config;
> >> @@ -1558,6 +1565,10 @@ static int __init macb_probe(struct platform_device *pdev)
> >>
> >> 	dev->base_addr = regs->start;
> >>
> >> +	dev_id = of_match_device(macb_dt_ids, &pdev->dev);
> As avr32 doesn't yet define CONFIG_OF:
>     macb.c:1601: error:  'macb_dt_ids' undeclared
> 
> Hein

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-13 16:05 [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC Nicolas Ferre
  2013-05-13 16:05 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2013-05-14  7:01 ` Steffen Trumtrar
  2013-05-14 13:00 ` [PATCH v2] " Nicolas Ferre
  2 siblings, 0 replies; 22+ messages in thread
From: Steffen Trumtrar @ 2013-05-14  7:01 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Jean-Christophe PLAGNIOL-VILLARD, hein_tibosch, michal.simek,
	Ludovic Desroches, linux-arm-kernel, linux-kernel, netdev

Hi!

On Mon, May 13, 2013 at 06:05:05PM +0200, Nicolas Ferre wrote:
> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
> introduces clear-on-write on ISR register. This behavior is not always
> implemented when using Cadence MACB/GEM and is breaking other platforms.
> We are using a new Device Tree compatibility string and a capability
> property to actually activate this clear-on-write behavior on ISR.
> 
> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
>  Documentation/devicetree/bindings/net/macb.txt |  2 ++
>  drivers/net/ethernet/cadence/macb.c            | 15 +++++++++++++--
>  drivers/net/ethernet/cadence/macb.h            |  5 +++++
>  3 files changed, 20 insertions(+), 2 deletions(-)
> 

Looks okay to me:

Acked-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>

Thanks,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-14  5:52     ` Jean-Christophe PLAGNIOL-VILLARD
@ 2013-05-14  7:18       ` Hein Tibosch
  2013-05-14  7:22         ` Jean-Christophe PLAGNIOL-VILLARD
  2013-06-04  6:15       ` Michal Simek
  1 sibling, 1 reply; 22+ messages in thread
From: Hein Tibosch @ 2013-05-14  7:18 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD, Nicolas Ferre
  Cc: michal.simek, Ludovic Desroches, s.trumtrar, linux-arm-kernel,
	linux-kernel, netdev


On 5/14/2013 1:52 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 08:58 Tue 14 May     , Hein Tibosch wrote:
>> On 5/14/2013 12:05 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>>>
>>>> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
>>>> introduces clear-on-write on ISR register. This behavior is not always
>>>> implemented when using Cadence MACB/GEM and is breaking other platforms.
>>>> We are using a new Device Tree compatibility string and a capability
>>>> property to actually activate this clear-on-write behavior on ISR.
>>>>
>>>> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>> can we detect it via the IP?
>> This was my first proposal, have it based on the value of MACB's
>> register 'MID' (offset 0x00fc, lower 16 bits).
>> On avr32 it reads: 0x0000010D, on Zynq it reports 0x00000119
>>
>> So for the moment, CAPS_ISR_CLEAR_ON_WRITE could be set if the revision
>> equals to 0x00000119?
> so no it will not work
>
> as the gem on sama5 is 00020119
>
> so version 0x119 too
>
> nico

All right, that's a pity.

The only issue that remains then is the obligation to use CONFIG_OF,
or:

+#if defined(CONFIG_OF)
+	dev_id = of_match_device(macb_dt_ids, &pdev->dev);
+	if (dev_id)
+		bp->caps = (u32)dev_id->data;
+
+#endif

?

Hein

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-14  7:18       ` Hein Tibosch
@ 2013-05-14  7:22         ` Jean-Christophe PLAGNIOL-VILLARD
  2013-05-14  7:31           ` Hein Tibosch
  0 siblings, 1 reply; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-14  7:22 UTC (permalink / raw)
  To: Hein Tibosch
  Cc: Jean-Christophe PLAGNIOL-VILLARD, Nicolas Ferre, michal.simek,
	Ludovic Desroches, s.trumtrar, linux-arm-kernel, linux-kernel,
	netdev


On May 14, 2013, at 3:18 PM, Hein Tibosch <hein_tibosch@yahoo.es> wrote:

> 
> On 5/14/2013 1:52 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On 08:58 Tue 14 May     , Hein Tibosch wrote:
>>> On 5/14/2013 12:05 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>> On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>>>> 
>>>>> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
>>>>> introduces clear-on-write on ISR register. This behavior is not always
>>>>> implemented when using Cadence MACB/GEM and is breaking other platforms.
>>>>> We are using a new Device Tree compatibility string and a capability
>>>>> property to actually activate this clear-on-write behavior on ISR.
>>>>> 
>>>>> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
>>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>> can we detect it via the IP?
>>> This was my first proposal, have it based on the value of MACB's
>>> register 'MID' (offset 0x00fc, lower 16 bits).
>>> On avr32 it reads: 0x0000010D, on Zynq it reports 0x00000119
>>> 
>>> So for the moment, CAPS_ISR_CLEAR_ON_WRITE could be set if the revision
>>> equals to 0x00000119?
>> so no it will not work
>> 
>> as the gem on sama5 is 00020119
>> 
>> so version 0x119 too
>> 
>> nico
> 
> All right, that's a pity.
> 
> The only issue that remains then is the obligation to use CONFIG_OF,
> or:
> 
> +#if defined(CONFIG_OF)
> +	dev_id = of_match_device(macb_dt_ids, &pdev->dev);
> +	if (dev_id)
> +		bp->caps = (u32)dev_id->data;
> +
> +#endif
> 
> ?

no need as of_match_device is a inline of !OF

Best Regards,
J.
> 
> Hein
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-14  7:22         ` Jean-Christophe PLAGNIOL-VILLARD
@ 2013-05-14  7:31           ` Hein Tibosch
  2013-05-14  7:49             ` Michal Simek
  0 siblings, 1 reply; 22+ messages in thread
From: Hein Tibosch @ 2013-05-14  7:31 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD
  Cc: Nicolas Ferre, michal.simek, Ludovic Desroches, s.trumtrar,
	linux-arm-kernel, linux-kernel, netdev

On 5/14/2013 3:22 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On May 14, 2013, at 3:18 PM, Hein Tibosch <hein_tibosch@yahoo.es> wrote:
>
>> On 5/14/2013 1:52 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On 08:58 Tue 14 May     , Hein Tibosch wrote:
>>>> On 5/14/2013 12:05 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>> On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>>>>>
>>>>>> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
>>>>>> introduces clear-on-write on ISR register. This behavior is not always
>>>>>> implemented when using Cadence MACB/GEM and is breaking other platforms.
>>>>>> We are using a new Device Tree compatibility string and a capability
>>>>>> property to actually activate this clear-on-write behavior on ISR.
>>>>>>
>>>>>> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
>>>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>>> can we detect it via the IP?
>>>> This was my first proposal, have it based on the value of MACB's
>>>> register 'MID' (offset 0x00fc, lower 16 bits).
>>>> On avr32 it reads: 0x0000010D, on Zynq it reports 0x00000119
>>>>
>>>> So for the moment, CAPS_ISR_CLEAR_ON_WRITE could be set if the revision
>>>> equals to 0x00000119?
>>> so no it will not work
>>>
>>> as the gem on sama5 is 00020119
>>>
>>> so version 0x119 too
>>>
>>> nico
>> All right, that's a pity.
>>
>> The only issue that remains then is the obligation to use CONFIG_OF,
>> or:
>>
>> +#if defined(CONFIG_OF)
>> +	dev_id = of_match_device(macb_dt_ids, &pdev->dev);
>> +	if (dev_id)
>> +		bp->caps = (u32)dev_id->data;
>> +
>> +#endif
>>
>> ?
> no need as of_match_device is a inline of !OF
Sorry, here's the complete compiler error:
drivers/net/ethernet/cadence/macb.c: In function 'macb_probe':
drivers/net/ethernet/cadence/macb.c:1601: error: 'macb_dt_ids' undeclared (first use in this function)

Earlier, 'macb_dt_ids' is only defined when using OF

Best regards,
Hein

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-14  7:31           ` Hein Tibosch
@ 2013-05-14  7:49             ` Michal Simek
  2013-05-14  8:32               ` Hein Tibosch
  0 siblings, 1 reply; 22+ messages in thread
From: Michal Simek @ 2013-05-14  7:49 UTC (permalink / raw)
  To: Hein Tibosch
  Cc: Jean-Christophe PLAGNIOL-VILLARD, Nicolas Ferre, michal.simek,
	Ludovic Desroches, s.trumtrar, linux-arm-kernel, linux-kernel,
	netdev

[-- Attachment #1: Type: text/plain, Size: 2688 bytes --]

On 05/14/2013 09:31 AM, Hein Tibosch wrote:
> On 5/14/2013 3:22 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On May 14, 2013, at 3:18 PM, Hein Tibosch <hein_tibosch@yahoo.es> wrote:
>>
>>> On 5/14/2013 1:52 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>> On 08:58 Tue 14 May     , Hein Tibosch wrote:
>>>>> On 5/14/2013 12:05 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>>> On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>>>>>>
>>>>>>> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
>>>>>>> introduces clear-on-write on ISR register. This behavior is not always
>>>>>>> implemented when using Cadence MACB/GEM and is breaking other platforms.
>>>>>>> We are using a new Device Tree compatibility string and a capability
>>>>>>> property to actually activate this clear-on-write behavior on ISR.
>>>>>>>
>>>>>>> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
>>>>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>>>> can we detect it via the IP?
>>>>> This was my first proposal, have it based on the value of MACB's
>>>>> register 'MID' (offset 0x00fc, lower 16 bits).
>>>>> On avr32 it reads: 0x0000010D, on Zynq it reports 0x00000119
>>>>>
>>>>> So for the moment, CAPS_ISR_CLEAR_ON_WRITE could be set if the revision
>>>>> equals to 0x00000119?
>>>> so no it will not work
>>>>
>>>> as the gem on sama5 is 00020119
>>>>
>>>> so version 0x119 too
>>>>
>>>> nico
>>> All right, that's a pity.
>>>
>>> The only issue that remains then is the obligation to use CONFIG_OF,
>>> or:
>>>
>>> +#if defined(CONFIG_OF)
>>> +	dev_id = of_match_device(macb_dt_ids, &pdev->dev);
>>> +	if (dev_id)
>>> +		bp->caps = (u32)dev_id->data;
>>> +
>>> +#endif
>>>
>>> ?
>> no need as of_match_device is a inline of !OF
> Sorry, here's the complete compiler error:
> drivers/net/ethernet/cadence/macb.c: In function 'macb_probe':
> drivers/net/ethernet/cadence/macb.c:1601: error: 'macb_dt_ids' undeclared (first use in this function)
> 
> Earlier, 'macb_dt_ids' is only defined when using OF

The trick is in using of_match_ptr. It means remove that CONFIG_OF around macb_dt_ids too.

[linux-2.6.x]$ grep -rn "of_match_ptr" include/linux/
include/linux/of.h:314:#define of_match_ptr(_ptr)	(_ptr)
include/linux/of.h:508:#define of_match_ptr(_ptr)	NULL

Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-14  7:49             ` Michal Simek
@ 2013-05-14  8:32               ` Hein Tibosch
  0 siblings, 0 replies; 22+ messages in thread
From: Hein Tibosch @ 2013-05-14  8:32 UTC (permalink / raw)
  To: monstr, Nicolas Ferre
  Cc: Jean-Christophe PLAGNIOL-VILLARD, michal.simek,
	Ludovic Desroches, s.trumtrar, linux-arm-kernel, linux-kernel,
	netdev


On 5/14/2013 3:49 PM, Michal Simek wrote:
> On 05/14/2013 09:31 AM, Hein Tibosch wrote:
>> On 5/14/2013 3:22 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On May 14, 2013, at 3:18 PM, Hein Tibosch <hein_tibosch@yahoo.es> wrote:
>>>
>>>> On 5/14/2013 1:52 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>> On 08:58 Tue 14 May     , Hein Tibosch wrote:
>>>>>> On 5/14/2013 12:05 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>>>> On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>>>>>>>
>>>>>>>> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
>>>>>>>> introduces clear-on-write on ISR register. This behavior is not always
>>>>>>>> implemented when using Cadence MACB/GEM and is breaking other platforms.
>>>>>>>> We are using a new Device Tree compatibility string and a capability
>>>>>>>> property to actually activate this clear-on-write behavior on ISR.
>>>>>>>>
>>>>>>>> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
>>>>>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>>>>> can we detect it via the IP?
>>>>>> This was my first proposal, have it based on the value of MACB's
>>>>>> register 'MID' (offset 0x00fc, lower 16 bits).
>>>>>> On avr32 it reads: 0x0000010D, on Zynq it reports 0x00000119
>>>>>>
>>>>>> So for the moment, CAPS_ISR_CLEAR_ON_WRITE could be set if the revision
>>>>>> equals to 0x00000119?
>>>>> so no it will not work
>>>>>
>>>>> as the gem on sama5 is 00020119
>>>>>
>>>>> so version 0x119 too
>>>>>
>>>>> nico
>>>> All right, that's a pity.
>>>>
>>>> The only issue that remains then is the obligation to use CONFIG_OF,
>>>> or:
>>>>
>>>> +#if defined(CONFIG_OF)
>>>> +	dev_id = of_match_device(macb_dt_ids, &pdev->dev);
>>>> +	if (dev_id)
>>>> +		bp->caps = (u32)dev_id->data;
>>>> +
>>>> +#endif
>>>>
>>>> ?
>>> no need as of_match_device is a inline of !OF
>> Sorry, here's the complete compiler error:
>> drivers/net/ethernet/cadence/macb.c: In function 'macb_probe':
>> drivers/net/ethernet/cadence/macb.c:1601: error: 'macb_dt_ids' undeclared (first use in this function)
>>
>> Earlier, 'macb_dt_ids' is only defined when using OF
> The trick is in using of_match_ptr. It means remove that CONFIG_OF around macb_dt_ids too.
>
> [linux-2.6.x]$ grep -rn "of_match_ptr" include/linux/
> include/linux/of.h:314:#define of_match_ptr(_ptr)	(_ptr)
> include/linux/of.h:508:#define of_match_ptr(_ptr)	NULL
yes of course, clever.

I tested the patch with that change on my avr32 platform and like to add:

Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
Tested-by: Hein Tibosch <hein_tibosch@yahoo.es>

Thanks,
Hein

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-13 16:05 ` Jean-Christophe PLAGNIOL-VILLARD
  2013-05-14  0:58   ` Hein Tibosch
@ 2013-05-14  9:16   ` Nicolas Ferre
  2013-05-14 11:38     ` Michal Simek
  1 sibling, 1 reply; 22+ messages in thread
From: Nicolas Ferre @ 2013-05-14  9:16 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD, michal.simek, s.trumtrar
  Cc: hein_tibosch, Ludovic Desroches, linux-arm-kernel, linux-kernel, netdev

On 13/05/2013 18:05, Jean-Christophe PLAGNIOL-VILLARD :
>
> On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>
>> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
>> introduces clear-on-write on ISR register. This behavior is not always
>> implemented when using Cadence MACB/GEM and is breaking other platforms.
>> We are using a new Device Tree compatibility string and a capability
>> property to actually activate this clear-on-write behavior on ISR.
>>
>> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>
> can we detect it via the IP?

As said by Hein, we cannot use the IP revision number. *But* we may have 
the opportunity to read this integration configuration in the Design 
Configuration Register 1 (DCFG1 already used for determining data bus 
width).

So, Michal or Steffen, can you please tell me the value of:
-> bit 23 at register address 0x280: mine is "1" which should mean "IRQ 
read clear", yours should be "0".

Hein, in case of use of the MACB, we do not have this register included, 
so I will avoid to run the test when using MACB (we already have this 
information).

If it works, I plan to rewrite the patch but taking this information 
instead of the device tree compatibility string.

Best regards,

>> ---
>> Documentation/devicetree/bindings/net/macb.txt |  2 ++
>> drivers/net/ethernet/cadence/macb.c            | 15 +++++++++++++--
>> drivers/net/ethernet/cadence/macb.h            |  5 +++++
>> 3 files changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
>> index 44afa0e..13ec4f6 100644
>> --- a/Documentation/devicetree/bindings/net/macb.txt
>> +++ b/Documentation/devicetree/bindings/net/macb.txt
>> @@ -6,6 +6,8 @@ Required properties:
>>    Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb".
>>    Use "cnds,pc302-gem" for Picochip picoXcell pc302 and later devices based on
>>    the Cadence GEM, or the generic form: "cdns,gem".
>> +  Use "cdns,zynq-7000-gem" for devices based on Cadence GEM with alternative
>> +  options enabled (ISR clear on write).
>> - reg: Address and length of the register set for the device
>> - interrupts: Should contain macb interrupt
>> - phy-mode: String, operation mode of the PHY interface.
>> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
>> index 6be513d..628f2b0 100644
>> --- a/drivers/net/ethernet/cadence/macb.c
>> +++ b/drivers/net/ethernet/cadence/macb.c
>> @@ -485,7 +485,8 @@ static void macb_tx_interrupt(struct macb *bp)
>> 	status = macb_readl(bp, TSR);
>> 	macb_writel(bp, TSR, status);
>>
>> -	macb_writel(bp, ISR, MACB_BIT(TCOMP));
>> +	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
>> +		macb_writel(bp, ISR, MACB_BIT(TCOMP));
>>
>> 	netdev_vdbg(bp->dev, "macb_tx_interrupt status = 0x%03lx\n",
>> 		(unsigned long)status);
>> @@ -738,7 +739,8 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
>> 			 * now.
>> 			 */
>> 			macb_writel(bp, IDR, MACB_RX_INT_FLAGS);
>> -			macb_writel(bp, ISR, MACB_BIT(RCOMP));
>> +			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
>> +				macb_writel(bp, ISR, MACB_BIT(RCOMP));
>>
>> 			if (napi_schedule_prep(&bp->napi)) {
>> 				netdev_vdbg(bp->dev, "scheduling RX softirq\n");
>> @@ -1474,6 +1476,10 @@ static const struct of_device_id macb_dt_ids[] = {
>> 	{ .compatible = "cdns,macb" },
>> 	{ .compatible = "cdns,pc302-gem" },
>> 	{ .compatible = "cdns,gem" },
>> +	{
>> +		.compatible = "cdns,zynq-7000-gem",
>> +		.data = (void *)MACB_CAPS_ISR_CLEAR_ON_WRITE,
>> +	},
>> 	{ /* sentinel */ }
>> };
>> MODULE_DEVICE_TABLE(of, macb_dt_ids);
>> @@ -1484,6 +1490,7 @@ static int __init macb_probe(struct platform_device *pdev)
>> 	struct macb_platform_data *pdata;
>> 	struct resource *regs;
>> 	struct net_device *dev;
>> +	const struct of_device_id *dev_id;
>> 	struct macb *bp;
>> 	struct phy_device *phydev;
>> 	u32 config;
>> @@ -1558,6 +1565,10 @@ static int __init macb_probe(struct platform_device *pdev)
>>
>> 	dev->base_addr = regs->start;
>>
>> +	dev_id = of_match_device(macb_dt_ids, &pdev->dev);
>> +	if (dev_id)
>> +		bp->caps = (u32)dev_id->data;
>> +
>> 	/* Set MII management clock divider */
>> 	config = macb_mdc_clk_div(bp);
>> 	config |= macb_dbw(bp);
>> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
>> index 993d703..5d622fe 100644
>> --- a/drivers/net/ethernet/cadence/macb.h
>> +++ b/drivers/net/ethernet/cadence/macb.h
>> @@ -323,6 +323,9 @@
>> #define MACB_MAN_READ				2
>> #define MACB_MAN_CODE				2
>>
>> +/* Capability mask bits */
>> +#define MACB_CAPS_ISR_CLEAR_ON_WRITE		0x1
>> +
>> /* Bit manipulation macros */
>> #define MACB_BIT(name)					\
>> 	(1 << MACB_##name##_OFFSET)
>> @@ -574,6 +577,8 @@ struct macb {
>> 	unsigned int 		speed;
>> 	unsigned int 		duplex;
>>
>> +	u32			caps;
>> +
>> 	phy_interface_t		phy_interface;
>>
>> 	/* AT91RM9200 transmit */
>> --
>> 1.8.0
>>
>
>


-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-14  9:16   ` Nicolas Ferre
@ 2013-05-14 11:38     ` Michal Simek
  2013-05-14 12:30       ` Nicolas Ferre
  0 siblings, 1 reply; 22+ messages in thread
From: Michal Simek @ 2013-05-14 11:38 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Jean-Christophe PLAGNIOL-VILLARD, michal.simek, s.trumtrar,
	hein_tibosch, Ludovic Desroches, linux-arm-kernel, linux-kernel,
	netdev, Anirudha Sarangi

[-- Attachment #1: Type: text/plain, Size: 5306 bytes --]

On 05/14/2013 11:16 AM, Nicolas Ferre wrote:
> On 13/05/2013 18:05, Jean-Christophe PLAGNIOL-VILLARD :
>>
>> On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>>
>>> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
>>> introduces clear-on-write on ISR register. This behavior is not always
>>> implemented when using Cadence MACB/GEM and is breaking other platforms.
>>> We are using a new Device Tree compatibility string and a capability
>>> property to actually activate this clear-on-write behavior on ISR.
>>>
>>> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>
>> can we detect it via the IP?
> 
> As said by Hein, we cannot use the IP revision number. *But* we may have the opportunity to read this integration configuration in the Design Configuration Register 1 (DCFG1 already used for determining data bus width).
> 
> So, Michal or Steffen, can you please tell me the value of:
> -> bit 23 at register address 0x280: mine is "1" which should mean "IRQ read clear", yours should be "0".

here is the whole reg map for zynq.
Reg 0x280 is undocumented in our TRM.
Please decode it not sure if bit 0 is LSB or MSB.

U-Boot-PetaLinux> md e000b000
e000b000: 0000001c 000e0013 00000006 00000000    ................
e000b010: 00180704 00000021 3ffba2e4 3ffbd32c    ....!......?,..?
e000b020: 00000003 00000000 00000000 00000000    ................
e000b030: 07ffffff 63c66f08 00000000 0000ffff    .....o.c........
e000b040: 000003ff 000003ff 00000000 00000000    ................
e000b050: 00000000 00000000 00000000 00000000    ................
e000b060: 00000000 00000000 00000000 00000000    ................
e000b070: 00000000 00000000 00000000 00000000    ................
e000b080: 00000000 00000000 00350a00 00001c48    ..........5.H...
e000b090: 00000000 00000000 00000000 00000000    ................
e000b0a0: 00000000 00000000 00000000 00000000    ................
e000b0b0: 00000000 00000000 00000000 00000000    ................
e000b0c0: 00000000 00000000 00000000 00000000    ................
e000b0d0: 00000000 00000000 00000000 00000000    ................
e000b0e0: 00000000 00000000 00000000 00000000    ................
e000b0f0: 00000000 00000000 00000000 00020118    ................
U-Boot-PetaLinux>
e000b100: 00014796 00000000 0000051e 00000001    .G..............
e000b110: 00000000 00000000 0000051d 00000001    ................
e000b120: 00000000 00000000 00000000 00000000    ................
e000b130: 00000000 00000000 00000000 00000000    ................
e000b140: 00000000 00000000 00000000 00000000    ................
e000b150: 001e58b6 00000000 00000526 00000000    .X......&.......
e000b160: 00000004 00000000 00000004 00000001    ................
e000b170: 00000000 00000004 00000000 0000051d    ................
e000b180: 00000000 00000000 00000000 00000000    ................
e000b190: 00000000 00000000 00000000 00000000    ................
e000b1a0: 00000038 00000000 00000000 00000000    8...............
e000b1b0: 00000000 00000000 00000000 00000000    ................
e000b1c0: 00000000 00000000 00000000 00000000    ................
e000b1d0: 00000000 00000000 00000000 00000000    ................
e000b1e0: 00000000 00000000 00000000 00000000    ................
e000b1f0: 00000000 00000000 00000000 00000000    ................
U-Boot-PetaLinux>
e000b200: 00000000 00000000 00000000 00000000    ................
e000b210: 00000000 00000000 00000000 00000000    ................
e000b220: 00000000 00000000 00000000 00000000    ................
e000b230: 00000000 00000000 00000000 00000000    ................
e000b240: 00000000 00000000 00000000 00000000    ................
e000b250: 00000000 00000000 00000000 00000000    ................
e000b260: 00000000 00000000 00000000 00000000    ................
e000b270: 00000000 00000000 00000000 00000000    ................
e000b280: 02500111 2ab13fff 00000000 00000000    ..P..?.*........
e000b290: 002f2145 00000200 00000000 00000000    E!/.............
e000b2a0: 00000000 00000000 00000000 00000000    ................
e000b2b0: 00000000 00000000 00000000 00000000    ................
e000b2c0: 00000000 00000000 00000000 00000000    ................
e000b2d0: 00000000 00000000 00000000 00000000    ................
e000b2e0: 00000000 00000000 00000000 00000000    ................
e000b2f0: 00000000 00000000 00000000 00000000    ................
U-Boot-PetaLinux>



> 
> Hein, in case of use of the MACB, we do not have this register included, so I will avoid to run the test when using MACB (we already have this information).
> 
> If it works, I plan to rewrite the patch but taking this information instead of the device tree compatibility string.

yep. Will be good to detect it instead of new compatible string.

Also is there an option to remove "CONFIG_ARCH_AT91"?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-14 11:38     ` Michal Simek
@ 2013-05-14 12:30       ` Nicolas Ferre
  0 siblings, 0 replies; 22+ messages in thread
From: Nicolas Ferre @ 2013-05-14 12:30 UTC (permalink / raw)
  To: monstr
  Cc: Jean-Christophe PLAGNIOL-VILLARD, michal.simek, s.trumtrar,
	hein_tibosch, Ludovic Desroches, linux-arm-kernel, linux-kernel,
	netdev, Anirudha Sarangi

On 14/05/2013 13:38, Michal Simek :
> On 05/14/2013 11:16 AM, Nicolas Ferre wrote:
>> On 13/05/2013 18:05, Jean-Christophe PLAGNIOL-VILLARD :
>>>
>>> On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>>>
>>>> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
>>>> introduces clear-on-write on ISR register. This behavior is not always
>>>> implemented when using Cadence MACB/GEM and is breaking other platforms.
>>>> We are using a new Device Tree compatibility string and a capability
>>>> property to actually activate this clear-on-write behavior on ISR.
>>>>
>>>> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>
>>> can we detect it via the IP?
>>
>> As said by Hein, we cannot use the IP revision number. *But* we may have the opportunity to read this integration configuration in the Design Configuration Register 1 (DCFG1 already used for determining data bus width).
>>
>> So, Michal or Steffen, can you please tell me the value of:
>> -> bit 23 at register address 0x280: mine is "1" which should mean "IRQ read clear", yours should be "0".
>
> here is the whole reg map for zynq.
> Reg 0x280 is undocumented in our TRM.
> Please decode it not sure if bit 0 is LSB or MSB.

Bit 0 is LSB.

Value of DCFG1 is 0x02500111 so I read '0' for this value which should 
be good.

I write a new patch immediately.


> U-Boot-PetaLinux> md e000b000
> e000b000: 0000001c 000e0013 00000006 00000000    ................
> e000b010: 00180704 00000021 3ffba2e4 3ffbd32c    ....!......?,..?
> e000b020: 00000003 00000000 00000000 00000000    ................
> e000b030: 07ffffff 63c66f08 00000000 0000ffff    .....o.c........
> e000b040: 000003ff 000003ff 00000000 00000000    ................
> e000b050: 00000000 00000000 00000000 00000000    ................
> e000b060: 00000000 00000000 00000000 00000000    ................
> e000b070: 00000000 00000000 00000000 00000000    ................
> e000b080: 00000000 00000000 00350a00 00001c48    ..........5.H...
> e000b090: 00000000 00000000 00000000 00000000    ................
> e000b0a0: 00000000 00000000 00000000 00000000    ................
> e000b0b0: 00000000 00000000 00000000 00000000    ................
> e000b0c0: 00000000 00000000 00000000 00000000    ................
> e000b0d0: 00000000 00000000 00000000 00000000    ................
> e000b0e0: 00000000 00000000 00000000 00000000    ................
> e000b0f0: 00000000 00000000 00000000 00020118    ................
> U-Boot-PetaLinux>
> e000b100: 00014796 00000000 0000051e 00000001    .G..............
> e000b110: 00000000 00000000 0000051d 00000001    ................
> e000b120: 00000000 00000000 00000000 00000000    ................
> e000b130: 00000000 00000000 00000000 00000000    ................
> e000b140: 00000000 00000000 00000000 00000000    ................
> e000b150: 001e58b6 00000000 00000526 00000000    .X......&.......
> e000b160: 00000004 00000000 00000004 00000001    ................
> e000b170: 00000000 00000004 00000000 0000051d    ................
> e000b180: 00000000 00000000 00000000 00000000    ................
> e000b190: 00000000 00000000 00000000 00000000    ................
> e000b1a0: 00000038 00000000 00000000 00000000    8...............
> e000b1b0: 00000000 00000000 00000000 00000000    ................
> e000b1c0: 00000000 00000000 00000000 00000000    ................
> e000b1d0: 00000000 00000000 00000000 00000000    ................
> e000b1e0: 00000000 00000000 00000000 00000000    ................
> e000b1f0: 00000000 00000000 00000000 00000000    ................
> U-Boot-PetaLinux>
> e000b200: 00000000 00000000 00000000 00000000    ................
> e000b210: 00000000 00000000 00000000 00000000    ................
> e000b220: 00000000 00000000 00000000 00000000    ................
> e000b230: 00000000 00000000 00000000 00000000    ................
> e000b240: 00000000 00000000 00000000 00000000    ................
> e000b250: 00000000 00000000 00000000 00000000    ................
> e000b260: 00000000 00000000 00000000 00000000    ................
> e000b270: 00000000 00000000 00000000 00000000    ................
> e000b280: 02500111 2ab13fff 00000000 00000000    ..P..?.*........
> e000b290: 002f2145 00000200 00000000 00000000    E!/.............
> e000b2a0: 00000000 00000000 00000000 00000000    ................
> e000b2b0: 00000000 00000000 00000000 00000000    ................
> e000b2c0: 00000000 00000000 00000000 00000000    ................
> e000b2d0: 00000000 00000000 00000000 00000000    ................
> e000b2e0: 00000000 00000000 00000000 00000000    ................
> e000b2f0: 00000000 00000000 00000000 00000000    ................
> U-Boot-PetaLinux>
>
>
>
>>
>> Hein, in case of use of the MACB, we do not have this register included, so I will avoid to run the test when using MACB (we already have this information).
>>
>> If it works, I plan to rewrite the patch but taking this information instead of the device tree compatibility string.
>
> yep. Will be good to detect it instead of new compatible string.
>
> Also is there an option to remove "CONFIG_ARCH_AT91"?

Okay, I have a look at this as-well.

Best regards,
-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-13 16:05 [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC Nicolas Ferre
  2013-05-13 16:05 ` Jean-Christophe PLAGNIOL-VILLARD
  2013-05-14  7:01 ` Steffen Trumtrar
@ 2013-05-14 13:00 ` Nicolas Ferre
  2013-05-14 13:43   ` Hein Tibosch
  2013-05-14 16:24   ` Jean-Christophe PLAGNIOL-VILLARD
  2 siblings, 2 replies; 22+ messages in thread
From: Nicolas Ferre @ 2013-05-14 13:00 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD, hein_tibosch
  Cc: michal.simek, Ludovic Desroches, s.trumtrar, linux-arm-kernel,
	linux-kernel, netdev, Nicolas Ferre

Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
introduces clear-on-write on ISR register. This behavior is not always
implemented when using Cadence MACB/GEM and is breaking other platforms.
We are using the Design Configuration Register 1 information and a capability
property to actually activate this clear-on-write behavior on ISR.

Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
v2: - use DCFG1 bit 23 integration information instead of device tree
      compatibility string to retreive information about c-o-r vs. c-o-w ISR.
    - move configuration in macb_init_hw() function instead of probe() - at
      "open" time.

As I have changed the most of this patch, I will collect your Ack and feedback
just like a new patch. Thanks for your help...

Bye,

 drivers/net/ethernet/cadence/macb.c | 18 ++++++++++++++++--
 drivers/net/ethernet/cadence/macb.h |  7 +++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 6be513d..c89aa41 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -485,7 +485,8 @@ static void macb_tx_interrupt(struct macb *bp)
 	status = macb_readl(bp, TSR);
 	macb_writel(bp, TSR, status);
 
-	macb_writel(bp, ISR, MACB_BIT(TCOMP));
+	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+		macb_writel(bp, ISR, MACB_BIT(TCOMP));
 
 	netdev_vdbg(bp->dev, "macb_tx_interrupt status = 0x%03lx\n",
 		(unsigned long)status);
@@ -738,7 +739,8 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 			 * now.
 			 */
 			macb_writel(bp, IDR, MACB_RX_INT_FLAGS);
-			macb_writel(bp, ISR, MACB_BIT(RCOMP));
+			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+				macb_writel(bp, ISR, MACB_BIT(RCOMP));
 
 			if (napi_schedule_prep(&bp->napi)) {
 				netdev_vdbg(bp->dev, "scheduling RX softirq\n");
@@ -1062,6 +1064,17 @@ static void macb_configure_dma(struct macb *bp)
 	}
 }
 
+/*
+ * Configure peripheral capacities according to integration options used
+ */
+static void macb_configure_caps(struct macb *bp)
+{
+	if (macb_is_gem(bp)) {
+		if (GEM_BF(IRQCOR, gem_readl(bp, DCFG1)) == 0)
+			bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
+	}
+}
+
 static void macb_init_hw(struct macb *bp)
 {
 	u32 config;
@@ -1084,6 +1097,7 @@ static void macb_init_hw(struct macb *bp)
 	bp->duplex = DUPLEX_HALF;
 
 	macb_configure_dma(bp);
+	macb_configure_caps(bp);
 
 	/* Initialize TX and RX buffers */
 	macb_writel(bp, RBQP, bp->rx_ring_dma);
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 993d703..548c0ec 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -300,6 +300,8 @@
 #define MACB_REV_SIZE				16
 
 /* Bitfields in DCFG1. */
+#define GEM_IRQCOR_OFFSET			23
+#define GEM_IRQCOR_SIZE				1
 #define GEM_DBWDEF_OFFSET			25
 #define GEM_DBWDEF_SIZE				3
 
@@ -323,6 +325,9 @@
 #define MACB_MAN_READ				2
 #define MACB_MAN_CODE				2
 
+/* Capability mask bits */
+#define MACB_CAPS_ISR_CLEAR_ON_WRITE		0x1
+
 /* Bit manipulation macros */
 #define MACB_BIT(name)					\
 	(1 << MACB_##name##_OFFSET)
@@ -574,6 +579,8 @@ struct macb {
 	unsigned int 		speed;
 	unsigned int 		duplex;
 
+	u32			caps;
+
 	phy_interface_t		phy_interface;
 
 	/* AT91RM9200 transmit */
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-14 13:00 ` [PATCH v2] " Nicolas Ferre
@ 2013-05-14 13:43   ` Hein Tibosch
  2013-05-14 16:24   ` Jean-Christophe PLAGNIOL-VILLARD
  1 sibling, 0 replies; 22+ messages in thread
From: Hein Tibosch @ 2013-05-14 13:43 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Jean-Christophe PLAGNIOL-VILLARD, michal.simek,
	Ludovic Desroches, s.trumtrar, linux-arm-kernel, linux-kernel,
	netdev

Hi Nicolas,

On 5/14/2013 9:00 PM, Nicolas Ferre wrote:
> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
> introduces clear-on-write on ISR register. This behavior is not always
> implemented when using Cadence MACB/GEM and is breaking other platforms.
> We are using the Design Configuration Register 1 information and a capability
> property to actually activate this clear-on-write behavior on ISR.
>
> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> v2: - use DCFG1 bit 23 integration information instead of device tree
>       compatibility string to retreive information about c-o-r vs. c-o-w ISR.
>     - move configuration in macb_init_hw() function instead of probe() - at
>       "open" time.
>
> As I have changed the most of this patch, I will collect your Ack and feedback
> just like a new patch. Thanks for your help...
>
> Bye,
>
>  drivers/net/ethernet/cadence/macb.c | 18 ++++++++++++++++--
>  drivers/net/ethernet/cadence/macb.h |  7 +++++++
>  2 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 6be513d..c89aa41 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -485,7 +485,8 @@ static void macb_tx_interrupt(struct macb *bp)
>  	status = macb_readl(bp, TSR);
>  	macb_writel(bp, TSR, status);
>  
> -	macb_writel(bp, ISR, MACB_BIT(TCOMP));
> +	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +		macb_writel(bp, ISR, MACB_BIT(TCOMP));
>  
>  	netdev_vdbg(bp->dev, "macb_tx_interrupt status = 0x%03lx\n",
>  		(unsigned long)status);
> @@ -738,7 +739,8 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
>  			 * now.
>  			 */
>  			macb_writel(bp, IDR, MACB_RX_INT_FLAGS);
> -			macb_writel(bp, ISR, MACB_BIT(RCOMP));
> +			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +				macb_writel(bp, ISR, MACB_BIT(RCOMP));
>  
>  			if (napi_schedule_prep(&bp->napi)) {
>  				netdev_vdbg(bp->dev, "scheduling RX softirq\n");
> @@ -1062,6 +1064,17 @@ static void macb_configure_dma(struct macb *bp)
>  	}
>  }
>  
> +/*
> + * Configure peripheral capacities according to integration options used
> + */
> +static void macb_configure_caps(struct macb *bp)
> +{
> +	if (macb_is_gem(bp)) {
> +		if (GEM_BF(IRQCOR, gem_readl(bp, DCFG1)) == 0)
> +			bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
> +	}
> +}

As you'll know on avr32 (AP7), macb_is_gem() returns false and thus the cap
won't be set, which is correct.

> +
>  static void macb_init_hw(struct macb *bp)
>  {
>  	u32 config;
> @@ -1084,6 +1097,7 @@ static void macb_init_hw(struct macb *bp)
>  	bp->duplex = DUPLEX_HALF;
>  
>  	macb_configure_dma(bp);
> +	macb_configure_caps(bp);
>  
>  	/* Initialize TX and RX buffers */
>  	macb_writel(bp, RBQP, bp->rx_ring_dma);
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 993d703..548c0ec 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -300,6 +300,8 @@
>  #define MACB_REV_SIZE				16
>  
>  /* Bitfields in DCFG1. */
> +#define GEM_IRQCOR_OFFSET			23
> +#define GEM_IRQCOR_SIZE				1
>  #define GEM_DBWDEF_OFFSET			25
>  #define GEM_DBWDEF_SIZE				3
>  
> @@ -323,6 +325,9 @@
>  #define MACB_MAN_READ				2
>  #define MACB_MAN_CODE				2
>  
> +/* Capability mask bits */
> +#define MACB_CAPS_ISR_CLEAR_ON_WRITE		0x1
> +
>  /* Bit manipulation macros */
>  #define MACB_BIT(name)					\
>  	(1 << MACB_##name##_OFFSET)
> @@ -574,6 +579,8 @@ struct macb {
>  	unsigned int 		speed;
>  	unsigned int 		duplex;
>  
> +	u32			caps;
> +
>  	phy_interface_t		phy_interface;
>  
>  	/* AT91RM9200 transmit */
Just re-tested with the above patch and it works like before,
also iperf results are unaffected (57/100 Mbit).

Tested-by: Hein Tibosch <hein_tibosch@yahoo.es>

Thanks a lot,
Hein

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-14 13:00 ` [PATCH v2] " Nicolas Ferre
  2013-05-14 13:43   ` Hein Tibosch
@ 2013-05-14 16:24   ` Jean-Christophe PLAGNIOL-VILLARD
  2013-05-14 20:04     ` David Miller
  1 sibling, 1 reply; 22+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-14 16:24 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: hein_tibosch, michal.simek, Ludovic Desroches, s.trumtrar,
	linux-arm-kernel, linux-kernel, netdev

On 15:00 Tue 14 May     , Nicolas Ferre wrote:
> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
> introduces clear-on-write on ISR register. This behavior is not always
> implemented when using Cadence MACB/GEM and is breaking other platforms.
> We are using the Design Configuration Register 1 information and a capability
> property to actually activate this clear-on-write behavior on ISR.
> 
> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
> v2: - use DCFG1 bit 23 integration information instead of device tree
>       compatibility string to retreive information about c-o-r vs. c-o-w ISR.
>     - move configuration in macb_init_hw() function instead of probe() - at
>       "open" time.
> 
> As I have changed the most of this patch, I will collect your Ack and feedback
> just like a new patch. Thanks for your help...
> 
> Bye,
> 
>  drivers/net/ethernet/cadence/macb.c | 18 ++++++++++++++++--
>  drivers/net/ethernet/cadence/macb.h |  7 +++++++
>  2 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 6be513d..c89aa41 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -485,7 +485,8 @@ static void macb_tx_interrupt(struct macb *bp)
>  	status = macb_readl(bp, TSR);
>  	macb_writel(bp, TSR, status);
>  
> -	macb_writel(bp, ISR, MACB_BIT(TCOMP));
> +	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +		macb_writel(bp, ISR, MACB_BIT(TCOMP));
>  
>  	netdev_vdbg(bp->dev, "macb_tx_interrupt status = 0x%03lx\n",
>  		(unsigned long)status);
> @@ -738,7 +739,8 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
>  			 * now.
>  			 */
>  			macb_writel(bp, IDR, MACB_RX_INT_FLAGS);
> -			macb_writel(bp, ISR, MACB_BIT(RCOMP));
> +			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> +				macb_writel(bp, ISR, MACB_BIT(RCOMP));
>  
>  			if (napi_schedule_prep(&bp->napi)) {
>  				netdev_vdbg(bp->dev, "scheduling RX softirq\n");
> @@ -1062,6 +1064,17 @@ static void macb_configure_dma(struct macb *bp)
>  	}
>  }
>  
> +/*
> + * Configure peripheral capacities according to integration options used
> + */
> +static void macb_configure_caps(struct macb *bp)
> +{
> +	if (macb_is_gem(bp)) {
> +		if (GEM_BF(IRQCOR, gem_readl(bp, DCFG1)) == 0)
> +			bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
> +	}
> +}
> +
>  static void macb_init_hw(struct macb *bp)
>  {
>  	u32 config;
> @@ -1084,6 +1097,7 @@ static void macb_init_hw(struct macb *bp)
>  	bp->duplex = DUPLEX_HALF;
>  
>  	macb_configure_dma(bp);
> +	macb_configure_caps(bp);
>  
>  	/* Initialize TX and RX buffers */
>  	macb_writel(bp, RBQP, bp->rx_ring_dma);
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 993d703..548c0ec 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -300,6 +300,8 @@
>  #define MACB_REV_SIZE				16
>  
>  /* Bitfields in DCFG1. */
> +#define GEM_IRQCOR_OFFSET			23
> +#define GEM_IRQCOR_SIZE				1
>  #define GEM_DBWDEF_OFFSET			25
>  #define GEM_DBWDEF_SIZE				3
>  
> @@ -323,6 +325,9 @@
>  #define MACB_MAN_READ				2
>  #define MACB_MAN_CODE				2
>  
> +/* Capability mask bits */
> +#define MACB_CAPS_ISR_CLEAR_ON_WRITE		0x1
> +
>  /* Bit manipulation macros */
>  #define MACB_BIT(name)					\
>  	(1 << MACB_##name##_OFFSET)
> @@ -574,6 +579,8 @@ struct macb {
>  	unsigned int 		speed;
>  	unsigned int 		duplex;
>  
> +	u32			caps;
> +
>  	phy_interface_t		phy_interface;
>  
>  	/* AT91RM9200 transmit */
> -- 
> 1.8.0
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-14 16:24   ` Jean-Christophe PLAGNIOL-VILLARD
@ 2013-05-14 20:04     ` David Miller
  0 siblings, 0 replies; 22+ messages in thread
From: David Miller @ 2013-05-14 20:04 UTC (permalink / raw)
  To: plagnioj
  Cc: nicolas.ferre, hein_tibosch, michal.simek, ludovic.desroches,
	s.trumtrar, linux-arm-kernel, linux-kernel, netdev

From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Date: Tue, 14 May 2013 18:24:50 +0200

> On 15:00 Tue 14 May     , Nicolas Ferre wrote:
>> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
>> introduces clear-on-write on ISR register. This behavior is not always
>> implemented when using Cadence MACB/GEM and is breaking other platforms.
>> We are using the Design Configuration Register 1 information and a capability
>> property to actually activate this clear-on-write behavior on ISR.
>> 
>> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

Applied, thanks.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-05-14  5:52     ` Jean-Christophe PLAGNIOL-VILLARD
  2013-05-14  7:18       ` Hein Tibosch
@ 2013-06-04  6:15       ` Michal Simek
  2013-06-04  6:49         ` Steffen Trumtrar
  1 sibling, 1 reply; 22+ messages in thread
From: Michal Simek @ 2013-06-04  6:15 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD
  Cc: Hein Tibosch, netdev, Nicolas Ferre, michal.simek, linux-kernel,
	Ludovic Desroches, s.trumtrar, linux-arm-kernel,
	Sören Brinkmann

[-- Attachment #1: Type: text/plain, Size: 1754 bytes --]

On 05/14/2013 07:52 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 08:58 Tue 14 May     , Hein Tibosch wrote:
>> On 5/14/2013 12:05 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>>>
>>>> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
>>>> introduces clear-on-write on ISR register. This behavior is not always
>>>> implemented when using Cadence MACB/GEM and is breaking other platforms.
>>>> We are using a new Device Tree compatibility string and a capability
>>>> property to actually activate this clear-on-write behavior on ISR.
>>>>
>>>> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>> can we detect it via the IP?
>>
>> This was my first proposal, have it based on the value of MACB's
>> register 'MID' (offset 0x00fc, lower 16 bits).
>> On avr32 it reads: 0x0000010D, on Zynq it reports 0x00000119
>>
>> So for the moment, CAPS_ISR_CLEAR_ON_WRITE could be set if the revision
>> equals to 0x00000119?
> so no it will not work
> 
> as the gem on sama5 is 00020119
> 
> so version 0x119 too
> 
> nico 
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

Was this added to any queue or branch?
I would like to enable macb for zynq and not sure if "cdns,zynq-7000-gem"
compatible string goes to mainline.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-06-04  6:15       ` Michal Simek
@ 2013-06-04  6:49         ` Steffen Trumtrar
  2013-06-04  6:54           ` Michal Simek
  2013-06-04  7:51           ` Nicolas Ferre
  0 siblings, 2 replies; 22+ messages in thread
From: Steffen Trumtrar @ 2013-06-04  6:49 UTC (permalink / raw)
  To: Michal Simek
  Cc: Jean-Christophe PLAGNIOL-VILLARD, Hein Tibosch, netdev,
	Nicolas Ferre, michal.simek, linux-kernel, Ludovic Desroches,
	linux-arm-kernel, Sören Brinkmann

On Tue, Jun 04, 2013 at 08:15:45AM +0200, Michal Simek wrote:
> On 05/14/2013 07:52 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 08:58 Tue 14 May     , Hein Tibosch wrote:
> >> On 5/14/2013 12:05 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>> On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
> >>>
> >>>> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
> >>>> introduces clear-on-write on ISR register. This behavior is not always
> >>>> implemented when using Cadence MACB/GEM and is breaking other platforms.
> >>>> We are using a new Device Tree compatibility string and a capability
> >>>> property to actually activate this clear-on-write behavior on ISR.
> >>>>
> >>>> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
> >>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> >>> can we detect it via the IP?
> >>
> >> This was my first proposal, have it based on the value of MACB's
> >> register 'MID' (offset 0x00fc, lower 16 bits).
> >> On avr32 it reads: 0x0000010D, on Zynq it reports 0x00000119
> >>
> >> So for the moment, CAPS_ISR_CLEAR_ON_WRITE could be set if the revision
> >> equals to 0x00000119?
> > so no it will not work
> > 
> > as the gem on sama5 is 00020119
> > 
> > so version 0x119 too
> > 
> > nico 
> > Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
> Was this added to any queue or branch?
> I would like to enable macb for zynq and not sure if "cdns,zynq-7000-gem"
> compatible string goes to mainline.
> 

Hi!

This is already in next, but you can use the default compatible as the
DCR1 is used instead of DT binding.

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-06-04  6:49         ` Steffen Trumtrar
@ 2013-06-04  6:54           ` Michal Simek
  2013-06-04  7:51           ` Nicolas Ferre
  1 sibling, 0 replies; 22+ messages in thread
From: Michal Simek @ 2013-06-04  6:54 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: Jean-Christophe PLAGNIOL-VILLARD, Hein Tibosch, netdev,
	Nicolas Ferre, michal.simek, linux-kernel, Ludovic Desroches,
	linux-arm-kernel, Sören Brinkmann

[-- Attachment #1: Type: text/plain, Size: 2223 bytes --]

On 06/04/2013 08:49 AM, Steffen Trumtrar wrote:
> On Tue, Jun 04, 2013 at 08:15:45AM +0200, Michal Simek wrote:
>> On 05/14/2013 07:52 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On 08:58 Tue 14 May     , Hein Tibosch wrote:
>>>> On 5/14/2013 12:05 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>> On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>>>>>
>>>>>> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
>>>>>> introduces clear-on-write on ISR register. This behavior is not always
>>>>>> implemented when using Cadence MACB/GEM and is breaking other platforms.
>>>>>> We are using a new Device Tree compatibility string and a capability
>>>>>> property to actually activate this clear-on-write behavior on ISR.
>>>>>>
>>>>>> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
>>>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>>> can we detect it via the IP?
>>>>
>>>> This was my first proposal, have it based on the value of MACB's
>>>> register 'MID' (offset 0x00fc, lower 16 bits).
>>>> On avr32 it reads: 0x0000010D, on Zynq it reports 0x00000119
>>>>
>>>> So for the moment, CAPS_ISR_CLEAR_ON_WRITE could be set if the revision
>>>> equals to 0x00000119?
>>> so no it will not work
>>>
>>> as the gem on sama5 is 00020119
>>>
>>> so version 0x119 too
>>>
>>> nico 
>>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>
>> Was this added to any queue or branch?
>> I would like to enable macb for zynq and not sure if "cdns,zynq-7000-gem"
>> compatible string goes to mainline.
>>
> 
> Hi!
> 
> This is already in next, but you can use the default compatible as the
> DCR1 is used instead of DT binding.

Ah ok. It was added there without cdns,zynq-7000-gem compatible string.

BTW: I have asked Soren to update your patch on the top of his clock changes.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-06-04  6:49         ` Steffen Trumtrar
  2013-06-04  6:54           ` Michal Simek
@ 2013-06-04  7:51           ` Nicolas Ferre
  2013-06-04  7:57             ` Michal Simek
  1 sibling, 1 reply; 22+ messages in thread
From: Nicolas Ferre @ 2013-06-04  7:51 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: Michal Simek, Jean-Christophe PLAGNIOL-VILLARD, Hein Tibosch,
	netdev, michal.simek, linux-kernel, Ludovic Desroches,
	linux-arm-kernel, Sören Brinkmann

On 04/06/2013 08:49, Steffen Trumtrar :
> On Tue, Jun 04, 2013 at 08:15:45AM +0200, Michal Simek wrote:
>> On 05/14/2013 07:52 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On 08:58 Tue 14 May     , Hein Tibosch wrote:
>>>> On 5/14/2013 12:05 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>> On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>>>>>
>>>>>> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
>>>>>> introduces clear-on-write on ISR register. This behavior is not always
>>>>>> implemented when using Cadence MACB/GEM and is breaking other platforms.
>>>>>> We are using a new Device Tree compatibility string and a capability
>>>>>> property to actually activate this clear-on-write behavior on ISR.
>>>>>>
>>>>>> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
>>>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>>> can we detect it via the IP?
>>>>
>>>> This was my first proposal, have it based on the value of MACB's
>>>> register 'MID' (offset 0x00fc, lower 16 bits).
>>>> On avr32 it reads: 0x0000010D, on Zynq it reports 0x00000119
>>>>
>>>> So for the moment, CAPS_ISR_CLEAR_ON_WRITE could be set if the revision
>>>> equals to 0x00000119?
>>> so no it will not work
>>>
>>> as the gem on sama5 is 00020119
>>>
>>> so version 0x119 too
>>>
>>> nico
>>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>
>> Was this added to any queue or branch?
>> I would like to enable macb for zynq and not sure if "cdns,zynq-7000-gem"
>> compatible string goes to mainline.
>>
>
> Hi!
>
> This is already in next, but you can use the default compatible as the

Even more: already in Linus' tree!

> DCR1 is used instead of DT binding.

Absolutely.

Best regards,
-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC
  2013-06-04  7:51           ` Nicolas Ferre
@ 2013-06-04  7:57             ` Michal Simek
  0 siblings, 0 replies; 22+ messages in thread
From: Michal Simek @ 2013-06-04  7:57 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Steffen Trumtrar, Jean-Christophe PLAGNIOL-VILLARD, Hein Tibosch,
	netdev, michal.simek, linux-kernel, Ludovic Desroches,
	linux-arm-kernel, Sören Brinkmann

[-- Attachment #1: Type: text/plain, Size: 2253 bytes --]

On 06/04/2013 09:51 AM, Nicolas Ferre wrote:
> On 04/06/2013 08:49, Steffen Trumtrar :
>> On Tue, Jun 04, 2013 at 08:15:45AM +0200, Michal Simek wrote:
>>> On 05/14/2013 07:52 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>> On 08:58 Tue 14 May     , Hein Tibosch wrote:
>>>>> On 5/14/2013 12:05 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>>> On May 14, 2013, at 12:05 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>>>>>>
>>>>>>> Commit 749a2b6 (net/macb: clear tx/rx completion flags in ISR)
>>>>>>> introduces clear-on-write on ISR register. This behavior is not always
>>>>>>> implemented when using Cadence MACB/GEM and is breaking other platforms.
>>>>>>> We are using a new Device Tree compatibility string and a capability
>>>>>>> property to actually activate this clear-on-write behavior on ISR.
>>>>>>>
>>>>>>> Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
>>>>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>>>> can we detect it via the IP?
>>>>>
>>>>> This was my first proposal, have it based on the value of MACB's
>>>>> register 'MID' (offset 0x00fc, lower 16 bits).
>>>>> On avr32 it reads: 0x0000010D, on Zynq it reports 0x00000119
>>>>>
>>>>> So for the moment, CAPS_ISR_CLEAR_ON_WRITE could be set if the revision
>>>>> equals to 0x00000119?
>>>> so no it will not work
>>>>
>>>> as the gem on sama5 is 00020119
>>>>
>>>> so version 0x119 too
>>>>
>>>> nico
>>>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>>
>>> Was this added to any queue or branch?
>>> I would like to enable macb for zynq and not sure if "cdns,zynq-7000-gem"
>>> compatible string goes to mainline.
>>>
>>
>> Hi!
>>
>> This is already in next, but you can use the default compatible as the
> 
> Even more: already in Linus' tree!

Ah I see. v2 is in thunderbird in the same thread and I didn't check this
version for compatible string.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2013-06-04  7:57 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-13 16:05 [PATCH] net/macb: fix ISR clear-on-write behavior only for some SoC Nicolas Ferre
2013-05-13 16:05 ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-14  0:58   ` Hein Tibosch
2013-05-14  5:52     ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-14  7:18       ` Hein Tibosch
2013-05-14  7:22         ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-14  7:31           ` Hein Tibosch
2013-05-14  7:49             ` Michal Simek
2013-05-14  8:32               ` Hein Tibosch
2013-06-04  6:15       ` Michal Simek
2013-06-04  6:49         ` Steffen Trumtrar
2013-06-04  6:54           ` Michal Simek
2013-06-04  7:51           ` Nicolas Ferre
2013-06-04  7:57             ` Michal Simek
2013-05-14  9:16   ` Nicolas Ferre
2013-05-14 11:38     ` Michal Simek
2013-05-14 12:30       ` Nicolas Ferre
2013-05-14  7:01 ` Steffen Trumtrar
2013-05-14 13:00 ` [PATCH v2] " Nicolas Ferre
2013-05-14 13:43   ` Hein Tibosch
2013-05-14 16:24   ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-14 20:04     ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).