[PATCH net 0/2] Add a property to disable MACB USRIO register

[PATCH net 0/2] Add a property to disable MACB USRIO register

[Neil Armstrong]

This patchet introduces an optional DT property to disable usage of the
USRIO register on platform not implementing it thus avaiding some external
impresise aborts of ARM based platforms.

Neil Armstrong (2):
  net: cadence: macb: Disable USRIO register on some platforms
  bindings: net: macb: add no-usrio optional property

 Documentation/devicetree/bindings/net/macb.txt |  3 +++
 drivers/net/ethernet/cadence/macb.c            | 29 +++++++++++++++-----------
 2 files changed, 20 insertions(+), 12 deletions(-)

-- 
1.9.1

[PATCH net 1/2] net: cadence: macb: Disable USRIO register on some platforms

[Neil Armstrong]

On some platforms, the macb integration does not use the USRIO
register to configure the (R)MII port and clocks.
When the register is not implemented and the MACB error signal
is connected to the bus error, reading or writing to the USRIO
register can trigger some Imprecise External Aborts on ARM platforms.
---
 drivers/net/ethernet/cadence/macb.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 169059c..3897620 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2122,7 +2122,9 @@ static void macb_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 	regs_buff[10] = macb_tx_dma(&bp->queues[0], tail);
 	regs_buff[11] = macb_tx_dma(&bp->queues[0], head);
 
-	regs_buff[12] = macb_or_gem_readl(bp, USRIO);
+	if (!of_property_read_bool(bp->pdev->dev.of_node, "no-usrio")) {
+		regs_buff[12] = macb_or_gem_readl(bp, USRIO);
+	}
 	if (macb_is_gem(bp)) {
 		regs_buff[13] = gem_readl(bp, DMACFG);
 	}
@@ -2401,19 +2403,22 @@ static int macb_init(struct platform_device *pdev)
 		dev->hw_features &= ~NETIF_F_SG;
 	dev->features = dev->hw_features;
 
-	val = 0;
-	if (bp->phy_interface == PHY_INTERFACE_MODE_RGMII)
-		val = GEM_BIT(RGMII);
-	else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII &&
-		 (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
-		val = MACB_BIT(RMII);
-	else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
-		val = MACB_BIT(MII);
+	/* Some platform do not implement the USRIO register */
+	if (!of_property_read_bool(pdev->dev.of_node, "no-usrio")) {
+		val = 0;
+		if (bp->phy_interface == PHY_INTERFACE_MODE_RGMII)
+			val = GEM_BIT(RGMII);
+		else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII &&
+			 (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
+			val = MACB_BIT(RMII);
+		else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
+			val = MACB_BIT(MII);
 
-	if (bp->caps & MACB_CAPS_USRIO_HAS_CLKEN)
-		val |= MACB_BIT(CLKEN);
+		if (bp->caps & MACB_CAPS_USRIO_HAS_CLKEN)
+			val |= MACB_BIT(CLKEN);
 
-	macb_or_gem_writel(bp, USRIO, val);
+		macb_or_gem_writel(bp, USRIO, val);
+	}
 
 	/* Set MII management clock divider */
 	val = macb_mdc_clk_div(bp);
-- 
1.9.1

[PATCH net 2/2] bindings: net: macb: add no-usrio optional property

[Neil Armstrong]

Add the no-usrio optional property to disable usage of the USRIO
register on platforms not implementing it.
---
 Documentation/devicetree/bindings/net/macb.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index b5d7976..a15ee54 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -19,6 +19,9 @@ Required properties:
 	Optional elements: 'tx_clk'
 - clocks: Phandles to input clocks.
 
+Optional properties:
+- no-usrio: Indicates the platform integration does not implement the USRIO register.
+
 Examples:
 
 	macb0: ethernet@fffc4000 {
-- 
1.9.1

Re: [PATCH net 1/2] net: cadence: macb: Disable USRIO register on some platforms

[David Miller]

From: Neil Armstrong <narmstrong@baylibre.com>
Date: Mon,  7 Dec 2015 11:58:33 +0100

> -	regs_buff[12] = macb_or_gem_readl(bp, USRIO);
> +	if (!of_property_read_bool(bp->pdev->dev.of_node, "no-usrio")) {
> +		regs_buff[12] = macb_or_gem_readl(bp, USRIO);
> +	}

Single statement basic blocks shall not be enclosed in curly braces.

Re: [PATCH net 1/2] net: cadence: macb: Disable USRIO register on some platforms

[Josh Cartwright]

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

On Mon, Dec 07, 2015 at 11:58:33AM +0100, Neil Armstrong wrote:
> On some platforms, the macb integration does not use the USRIO
> register to configure the (R)MII port and clocks.
> When the register is not implemented and the MACB error signal
> is connected to the bus error, reading or writing to the USRIO
> register can trigger some Imprecise External Aborts on ARM platforms.
> ---

Does this make sense to even be a separate bool device tree property?

This sort of configuration is typically done by:
   1. Creating a new 'caps' bit; relevant codepaths check that bit
   2. Creating a new "compatible" string for your platform's macb
      instance
   3. Creating a new 'struct macb_config' instance for your platform,
      setting any relevant caps bits when it is selected.

  Josh

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH net 1/2] net: cadence: macb: Disable USRIO register on some platforms

[Neil Armstrong]

Hi Josh,

2015-12-07 20:32 GMT+01:00 Josh Cartwright <joshc@ni.com>:
> On Mon, Dec 07, 2015 at 11:58:33AM +0100, Neil Armstrong wrote:
>> On some platforms, the macb integration does not use the USRIO
>> register to configure the (R)MII port and clocks.
>> When the register is not implemented and the MACB error signal
>> is connected to the bus error, reading or writing to the USRIO
>> register can trigger some Imprecise External Aborts on ARM platforms.
>> ---
>
> Does this make sense to even be a separate bool device tree property?
>
> This sort of configuration is typically done by:
>    1. Creating a new 'caps' bit; relevant codepaths check that bit
>    2. Creating a new "compatible" string for your platform's macb
>       instance
>    3. Creating a new 'struct macb_config' instance for your platform,
>       setting any relevant caps bits when it is selected.
>
>   Josh

I see the point, but according to the User Guide :
>User I/O Register
> The MACB design provides up to 16 inputs and 16 outputs,
> for which the state of the I/O may
> be read or set under the control of the processor interface.
> If the user I/O is disabled as a configuration option, this address space is defined
> as reserved, and hence will be a read-only register of value 0x0.

On the design I worked on, the macb_user_* signals were commented,
thus disabling this register.

The implementation is not mandatory, and the "generic" macb compatible
"cdns,macb" should disable
usage of USRIO register by default and be only used for platform
specific macb instances...

Is it OK if I add a new 'caps' bit and use it for the "generic" macb instance ?

For the device tree property, it should be safe to have the generic
instances of macb and gem to
rely on these properties instead of hardcoded instances.
(it's the biggest aim of device tree, no ? no more hardcoded 'caps' bit ?)
The "no-usrio" and other should eventually map 'caps' bits along the
generic instances.

Neil

Re: [PATCH net 1/2] net: cadence: macb: Disable USRIO register on some platforms

[Nicolas Ferre]

Le 08/12/2015 10:21, Neil Armstrong a écrit :
> Hi Josh,
> 
> 2015-12-07 20:32 GMT+01:00 Josh Cartwright <joshc@ni.com>:
>> On Mon, Dec 07, 2015 at 11:58:33AM +0100, Neil Armstrong wrote:
>>> On some platforms, the macb integration does not use the USRIO
>>> register to configure the (R)MII port and clocks.
>>> When the register is not implemented and the MACB error signal
>>> is connected to the bus error, reading or writing to the USRIO
>>> register can trigger some Imprecise External Aborts on ARM platforms.
>>> ---
>>
>> Does this make sense to even be a separate bool device tree property?
>>
>> This sort of configuration is typically done by:
>>    1. Creating a new 'caps' bit; relevant codepaths check that bit
>>    2. Creating a new "compatible" string for your platform's macb
>>       instance
>>    3. Creating a new 'struct macb_config' instance for your platform,
>>       setting any relevant caps bits when it is selected.
>>
>>   Josh
> 
> I see the point, but according to the User Guide :
>> User I/O Register
>> The MACB design provides up to 16 inputs and 16 outputs,
>> for which the state of the I/O may
>> be read or set under the control of the processor interface.
>> If the user I/O is disabled as a configuration option, this address space is defined
>> as reserved, and hence will be a read-only register of value 0x0.
> 
> On the design I worked on, the macb_user_* signals were commented,
> thus disabling this register.
> 
> The implementation is not mandatory, and the "generic" macb compatible
> "cdns,macb" should disable
> usage of USRIO register by default and be only used for platform
> specific macb instances...
> 
> Is it OK if I add a new 'caps' bit and use it for the "generic" macb instance ?

I would say no as some platform may already use this compatibility
string. So If you need a different capability set, please create a new
compatible string and use this one.

> For the device tree property, it should be safe to have the generic
> instances of macb and gem to
> rely on these properties instead of hardcoded instances.
> (it's the biggest aim of device tree, no ? no more hardcoded 'caps' bit ?)
> The "no-usrio" and other should eventually map 'caps' bits along the
> generic instances.

It has been decided a log time ago to use these capabilities and to have
mixed approach to the actual configuration of the IP:
- from the compatibility string
- from the configuration registers.

I may be sometimes challenging to figure out where the final property
comes from but it has proven to be pretty well adapted to any kind of
situation.

So let's continue with this and not insert additional properties to this
binding.

Best regards,
-- 
Nicolas Ferre

[PATCH v2 net-next 0/3] Add new capability and parse from DT

[Neil Armstrong]

The first patch introduces a new capability bit to disable usage of the
USRIO register on platform not implementing it thus avoiding some external
imprecise aborts on ARM based platforms.
The two last patchs implements a DT fallback to get the software defined 
capabilities from DT in the case of the generic compatible form is used.
These properties will permit support of future implementations of macb/gem
without the need for adding new static structures in the code.

v1: http://lkml.kernel.org/r/1449485914-12883-1-git-send-email-narmstrong@baylibre.com
v2: switch to CAPS_* bit and add all software CAPS_* as DT properties

Neil Armstrong (3):
  net: ethernet: cadence-macb: Add disabled usrio caps
  net: ethernet: cadence-macb: Add fallback to read DT provided caps
  bindings: ethernet: macb: Add optional caps properties

 Documentation/devicetree/bindings/net/macb.txt | 10 ++++
 drivers/net/ethernet/cadence/macb.c            | 72 +++++++++++++++++++++-----
 drivers/net/ethernet/cadence/macb.h            |  1 +
 3 files changed, 71 insertions(+), 12 deletions(-)

-- 
1.9.1

[PATCH v2 net-next 1/3] net: ethernet: cadence-macb: Add disabled usrio caps

[Neil Armstrong]

On some platforms, the macb integration does not use the USRIO
register to configure the (R)MII port and clocks.
When the register is not implemented and the MACB error signal
is connected to the bus error, reading or writing to the USRIO
register can trigger some Imprecise External Aborts on ARM platforms.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/net/ethernet/cadence/macb.c | 27 +++++++++++++++------------
 drivers/net/ethernet/cadence/macb.h |  1 +
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 169059c..9325140 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2122,7 +2122,8 @@ static void macb_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 	regs_buff[10] = macb_tx_dma(&bp->queues[0], tail);
 	regs_buff[11] = macb_tx_dma(&bp->queues[0], head);
 
-	regs_buff[12] = macb_or_gem_readl(bp, USRIO);
+	if (!(bp->caps & MACB_CAPS_USRIO_DISABLED))
+		regs_buff[12] = macb_or_gem_readl(bp, USRIO);
 	if (macb_is_gem(bp)) {
 		regs_buff[13] = gem_readl(bp, DMACFG);
 	}
@@ -2401,19 +2402,21 @@ static int macb_init(struct platform_device *pdev)
 		dev->hw_features &= ~NETIF_F_SG;
 	dev->features = dev->hw_features;
 
-	val = 0;
-	if (bp->phy_interface == PHY_INTERFACE_MODE_RGMII)
-		val = GEM_BIT(RGMII);
-	else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII &&
-		 (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
-		val = MACB_BIT(RMII);
-	else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
-		val = MACB_BIT(MII);
+	if (!(bp->caps & MACB_CAPS_USRIO_DISABLED)) {
+		val = 0;
+		if (bp->phy_interface == PHY_INTERFACE_MODE_RGMII)
+			val = GEM_BIT(RGMII);
+		else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII &&
+			 (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
+			val = MACB_BIT(RMII);
+		else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
+			val = MACB_BIT(MII);
 
-	if (bp->caps & MACB_CAPS_USRIO_HAS_CLKEN)
-		val |= MACB_BIT(CLKEN);
+		if (bp->caps & MACB_CAPS_USRIO_HAS_CLKEN)
+			val |= MACB_BIT(CLKEN);
 
-	macb_or_gem_writel(bp, USRIO, val);
+		macb_or_gem_writel(bp, USRIO, val);
+	}
 
 	/* Set MII management clock divider */
 	val = macb_mdc_clk_div(bp);
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index d83b0db..65ea049 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -400,6 +400,7 @@
 #define MACB_CAPS_USRIO_HAS_CLKEN		0x00000002
 #define MACB_CAPS_USRIO_DEFAULT_IS_MII		0x00000004
 #define MACB_CAPS_NO_GIGABIT_HALF		0x00000008
+#define MACB_CAPS_USRIO_DISABLED		0x00000010
 #define MACB_CAPS_FIFO_MODE			0x10000000
 #define MACB_CAPS_GIGABIT_MODE_AVAILABLE	0x20000000
 #define MACB_CAPS_SG_DISABLED			0x40000000
-- 
1.9.1

[PATCH v2 net-next 2/3] net: ethernet: cadence-macb: Add fallback to read DT provided caps

[Neil Armstrong]

Add 1:1 mapping of software defines caps parsing from DT in case the
generic macb compatible form is used.
These properties will provide support for futures implementations
only defined from DT without need to update the driver code to support
new variants.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/net/ethernet/cadence/macb.c | 45 +++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 9325140..28a9a8b 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2795,6 +2795,48 @@ static const struct macb_config zynq_config = {
 	.init = macb_init,
 };
 
+static const struct macb_config *macb_parse_dt_caps(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct macb_config *macb_config;
+	u32 val;
+
+	macb_config = devm_kzalloc(dev, sizeof(*macb_config), GFP_KERNEL);
+	if (!macb_config)
+		return NULL;
+
+	if (of_property_read_bool(np, "cdns,usrio-has-clken"))
+		macb_config->caps |= MACB_CAPS_USRIO_HAS_CLKEN;
+
+	if (of_property_read_bool(np, "cdns,usrio-default-mii"))
+		macb_config->caps |= MACB_CAPS_USRIO_DEFAULT_IS_MII;
+
+	if (of_property_read_bool(np, "cdns,no-gigabit-half"))
+		macb_config->caps |= MACB_CAPS_NO_GIGABIT_HALF;
+
+	if (of_property_read_bool(np, "cdns,usrio-disabled"))
+		macb_config->caps |= MACB_CAPS_USRIO_DISABLED;
+
+	if (of_property_read_bool(np, "cdns,gem-sg-disabled"))
+		macb_config->caps |= MACB_CAPS_SG_DISABLED;
+
+	if (of_property_read_bool(np, "cdns,gem-has-gigabit"))
+		macb_config->caps |= MACB_CAPS_GIGABIT_MODE_AVAILABLE;
+
+	if (of_property_read_bool(np, "cdns,usrio-disabled"))
+		macb_config->caps |= MACB_CAPS_USRIO_DISABLED;
+
+	if (!of_property_read_u32(np, "cdns,dma-burst-length", &val))
+		macb_config->dma_burst_length = val;
+
+	if (!of_property_read_u32(np, "cdns,jumbo-max-length", &val)) {
+		macb_config->jumbo_max_len = val;
+		macb_config->caps |= MACB_CAPS_JUMBO;
+	}
+
+	return macb_config;
+}
+
 static const struct of_device_id macb_dt_ids[] = {
 	{ .compatible = "cdns,at32ap7000-macb" },
 	{ .compatible = "cdns,at91sam9260-macb", .data = &at91sam9260_config },
@@ -2847,6 +2889,9 @@ static int macb_probe(struct platform_device *pdev)
 			clk_init = macb_config->clk_init;
 			init = macb_config->init;
 		}
+
+		if (!macb_config)
+			macb_config = macb_parse_dt_caps(&pdev->dev);
 	}
 
 	err = clk_init(pdev, &pclk, &hclk, &tx_clk);
-- 
1.9.1

[PATCH v2 net-next 3/3] bindings: ethernet: macb: Add optional caps properties

[Neil Armstrong]

Add generic caps properties to the binding in order to support
future macb/gem implementations with the generic macb compatible form.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 Documentation/devicetree/bindings/net/macb.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index b5d7976..891c72e 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -19,6 +19,16 @@ Required properties:
 	Optional elements: 'tx_clk'
 - clocks: Phandles to input clocks.
 
+Optional properties:
+- cdns,usrio-has-clken: Boolean; The USRIO register has an clock-enable bit.
+- cdns,usrio-default-mii: Boolean; The USRIO register defaults to MII mode.
+- cdns,usrio-disabled: Boolean; The USRIO register is not implemented.
+- cdns,no-gigabit-half: Boolean; The MAC does not support 1000/Half link mode.
+- cdns,gem-sg-disabled: Boolean; The MAC does not support Scatter-Gather mode.
+- cdns,gem-has-gigabit: Boolean; The MAC supports Gigabit Ethernet.
+- cdns,dma-burst-length: Should countain the DMA burst length.
+- cdns,jumbo-max-length: Should countain the maximum Jumbo frame length.
+
 Examples:
 
 	macb0: ethernet@fffc4000 {
-- 
1.9.1

Re: [PATCH v2 net-next 2/3] net: ethernet: cadence-macb: Add fallback to read DT provided caps

[Arnd Bergmann]

On Tuesday 08 December 2015 14:52:05 Neil Armstrong wrote:
> Add 1:1 mapping of software defines caps parsing from DT in case the
> generic macb compatible form is used.
> These properties will provide support for futures implementations
> only defined from DT without need to update the driver code to support
> new variants.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> 

Translating the Linux implementation specific configuration into
DT properties directly is usually not the best way.

Could we instead have a lookup table by compatible string to set the
flags? It seems that there are lots of different flags but only a
couple of different users of this IP block. Also, the fact that
you are now adding yet another quirk tells me that the set you
define today is unlikely to cover all the future requirements.

	Arnd

Re: [PATCH net 2/2] bindings: net: macb: add no-usrio optional property

[Rob Herring]

On Mon, Dec 07, 2015 at 11:58:34AM +0100, Neil Armstrong wrote:
> Add the no-usrio optional property to disable usage of the USRIO
> register on platforms not implementing it.
> ---
>  Documentation/devicetree/bindings/net/macb.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
> index b5d7976..a15ee54 100644
> --- a/Documentation/devicetree/bindings/net/macb.txt
> +++ b/Documentation/devicetree/bindings/net/macb.txt
> @@ -19,6 +19,9 @@ Required properties:
>  	Optional elements: 'tx_clk'
>  - clocks: Phandles to input clocks.
>  
> +Optional properties:
> +- no-usrio: Indicates the platform integration does not implement the USRIO register.

Add a vendor prefix please.

Rob

Re: [PATCH v2 net-next 2/3] net: ethernet: cadence-macb: Add fallback to read DT provided caps

[Nicolas Ferre]

Le 08/12/2015 16:00, Arnd Bergmann a écrit :
> On Tuesday 08 December 2015 14:52:05 Neil Armstrong wrote:
>> Add 1:1 mapping of software defines caps parsing from DT in case the
>> generic macb compatible form is used.
>> These properties will provide support for futures implementations
>> only defined from DT without need to update the driver code to support
>> new variants.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>
> 
> Translating the Linux implementation specific configuration into
> DT properties directly is usually not the best way.
> 
> Could we instead have a lookup table by compatible string to set the
> flags? It seems that there are lots of different flags but only a
> couple of different users of this IP block. Also, the fact that
> you are now adding yet another quirk tells me that the set you
> define today is unlikely to cover all the future requirements.

This is basically what I told Neil in my previous email.

I understand you point Neil, but I don't find it makes sense and Arnd
described it better that I did.
So please find a proper compatibility string and simply use it. What
about:
"cdns,the_name_of_the_product_that_first_implemented_this_no_usrio_special_case-gem"?

Bye,
-- 
Nicolas Ferre

Re: [PATCH v2 net-next 3/3] bindings: ethernet: macb: Add optional caps properties

[Rob Herring]

On Tue, Dec 08, 2015 at 02:52:06PM +0100, Neil Armstrong wrote:
> Add generic caps properties to the binding in order to support
> future macb/gem implementations with the generic macb compatible form.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

Acked-by: Rob Herring <robh@kernel.org>

> ---
>  Documentation/devicetree/bindings/net/macb.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
> index b5d7976..891c72e 100644
> --- a/Documentation/devicetree/bindings/net/macb.txt
> +++ b/Documentation/devicetree/bindings/net/macb.txt
> @@ -19,6 +19,16 @@ Required properties:
>  	Optional elements: 'tx_clk'
>  - clocks: Phandles to input clocks.
>  
> +Optional properties:
> +- cdns,usrio-has-clken: Boolean; The USRIO register has an clock-enable bit.
> +- cdns,usrio-default-mii: Boolean; The USRIO register defaults to MII mode.
> +- cdns,usrio-disabled: Boolean; The USRIO register is not implemented.
> +- cdns,no-gigabit-half: Boolean; The MAC does not support 1000/Half link mode.
> +- cdns,gem-sg-disabled: Boolean; The MAC does not support Scatter-Gather mode.
> +- cdns,gem-has-gigabit: Boolean; The MAC supports Gigabit Ethernet.
> +- cdns,dma-burst-length: Should countain the DMA burst length.
> +- cdns,jumbo-max-length: Should countain the maximum Jumbo frame length.
> +
>  Examples:
>  
>  	macb0: ethernet@fffc4000 {
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 net-next] net: ethernet: cadence-macb: Add disabled usrio caps

[Neil Armstrong]

On some platforms, the macb integration does not use the USRIO
register to configure the (R)MII port and clocks.
When the register is not implemented and the MACB error signal
is connected to the bus error, reading or writing to the USRIO
register can trigger some Imprecise External Aborts on ARM platforms.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/net/ethernet/cadence/macb.c | 27 +++++++++++++++------------
 drivers/net/ethernet/cadence/macb.h |  1 +
 2 files changed, 16 insertions(+), 12 deletions(-)

Nicolas,
I post only the first patch of the previous set posted here :
http://lkml.kernel.org/r/1449582726-6148-1-git-send-email-narmstrong@baylibre.com
to hopefully make it into the 4.5 merge time,
I'll post the vendor prefix once this patch will hit mainline.

Regards,
Neil

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 8b45bc9..fa53bc3 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2124,7 +2124,8 @@ static void macb_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 	regs_buff[10] = macb_tx_dma(&bp->queues[0], tail);
 	regs_buff[11] = macb_tx_dma(&bp->queues[0], head);
 
-	regs_buff[12] = macb_or_gem_readl(bp, USRIO);
+	if (!(bp->caps & MACB_CAPS_USRIO_DISABLED))
+		regs_buff[12] = macb_or_gem_readl(bp, USRIO);
 	if (macb_is_gem(bp)) {
 		regs_buff[13] = gem_readl(bp, DMACFG);
 	}
@@ -2403,19 +2404,21 @@ static int macb_init(struct platform_device *pdev)
 		dev->hw_features &= ~NETIF_F_SG;
 	dev->features = dev->hw_features;
 
-	val = 0;
-	if (bp->phy_interface == PHY_INTERFACE_MODE_RGMII)
-		val = GEM_BIT(RGMII);
-	else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII &&
-		 (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
-		val = MACB_BIT(RMII);
-	else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
-		val = MACB_BIT(MII);
+	if (!(bp->caps & MACB_CAPS_USRIO_DISABLED)) {
+		val = 0;
+		if (bp->phy_interface == PHY_INTERFACE_MODE_RGMII)
+			val = GEM_BIT(RGMII);
+		else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII &&
+			 (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
+			val = MACB_BIT(RMII);
+		else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
+			val = MACB_BIT(MII);
 
-	if (bp->caps & MACB_CAPS_USRIO_HAS_CLKEN)
-		val |= MACB_BIT(CLKEN);
+		if (bp->caps & MACB_CAPS_USRIO_HAS_CLKEN)
+			val |= MACB_BIT(CLKEN);
 
-	macb_or_gem_writel(bp, USRIO, val);
+		macb_or_gem_writel(bp, USRIO, val);
+	}
 
 	/* Set MII management clock divider */
 	val = macb_mdc_clk_div(bp);
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 5c03e81..0d4ecfc 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -400,6 +400,7 @@
 #define MACB_CAPS_USRIO_HAS_CLKEN		0x00000002
 #define MACB_CAPS_USRIO_DEFAULT_IS_MII		0x00000004
 #define MACB_CAPS_NO_GIGABIT_HALF		0x00000008
+#define MACB_CAPS_USRIO_DISABLED		0x00000010
 #define MACB_CAPS_FIFO_MODE			0x10000000
 #define MACB_CAPS_GIGABIT_MODE_AVAILABLE	0x20000000
 #define MACB_CAPS_SG_DISABLED			0x40000000
-- 
1.9.1

Re: [PATCH v3 net-next] net: ethernet: cadence-macb: Add disabled usrio caps

[Nicolas Ferre]

Le 04/01/2016 10:01, Neil Armstrong a écrit :
> On some platforms, the macb integration does not use the USRIO
> register to configure the (R)MII port and clocks.
> When the register is not implemented and the MACB error signal
> is connected to the bus error, reading or writing to the USRIO
> register can trigger some Imprecise External Aborts on ARM platforms.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/net/ethernet/cadence/macb.c | 27 +++++++++++++++------------
>  drivers/net/ethernet/cadence/macb.h |  1 +
>  2 files changed, 16 insertions(+), 12 deletions(-)
> 
> Nicolas,
> I post only the first patch of the previous set posted here :
> http://lkml.kernel.org/r/1449582726-6148-1-git-send-email-narmstrong@baylibre.com
> to hopefully make it into the 4.5 merge time,
> I'll post the vendor prefix once this patch will hit mainline.

Okay, but I don't see how you will activate this capability (or I lost
track of it). So, before I can accept one solution, can you please
repost the whole solution as a v4.

Thanks. Bye,


> 
> Regards,
> Neil
> 
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 8b45bc9..fa53bc3 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -2124,7 +2124,8 @@ static void macb_get_regs(struct net_device *dev, struct ethtool_regs *regs,
>  	regs_buff[10] = macb_tx_dma(&bp->queues[0], tail);
>  	regs_buff[11] = macb_tx_dma(&bp->queues[0], head);
>  
> -	regs_buff[12] = macb_or_gem_readl(bp, USRIO);
> +	if (!(bp->caps & MACB_CAPS_USRIO_DISABLED))
> +		regs_buff[12] = macb_or_gem_readl(bp, USRIO);
>  	if (macb_is_gem(bp)) {
>  		regs_buff[13] = gem_readl(bp, DMACFG);
>  	}
> @@ -2403,19 +2404,21 @@ static int macb_init(struct platform_device *pdev)
>  		dev->hw_features &= ~NETIF_F_SG;
>  	dev->features = dev->hw_features;
>  
> -	val = 0;
> -	if (bp->phy_interface == PHY_INTERFACE_MODE_RGMII)
> -		val = GEM_BIT(RGMII);
> -	else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII &&
> -		 (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
> -		val = MACB_BIT(RMII);
> -	else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
> -		val = MACB_BIT(MII);
> +	if (!(bp->caps & MACB_CAPS_USRIO_DISABLED)) {
> +		val = 0;
> +		if (bp->phy_interface == PHY_INTERFACE_MODE_RGMII)
> +			val = GEM_BIT(RGMII);
> +		else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII &&
> +			 (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
> +			val = MACB_BIT(RMII);
> +		else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
> +			val = MACB_BIT(MII);
>  
> -	if (bp->caps & MACB_CAPS_USRIO_HAS_CLKEN)
> -		val |= MACB_BIT(CLKEN);
> +		if (bp->caps & MACB_CAPS_USRIO_HAS_CLKEN)
> +			val |= MACB_BIT(CLKEN);
>  
> -	macb_or_gem_writel(bp, USRIO, val);
> +		macb_or_gem_writel(bp, USRIO, val);
> +	}
>  
>  	/* Set MII management clock divider */
>  	val = macb_mdc_clk_div(bp);
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 5c03e81..0d4ecfc 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -400,6 +400,7 @@
>  #define MACB_CAPS_USRIO_HAS_CLKEN		0x00000002
>  #define MACB_CAPS_USRIO_DEFAULT_IS_MII		0x00000004
>  #define MACB_CAPS_NO_GIGABIT_HALF		0x00000008
> +#define MACB_CAPS_USRIO_DISABLED		0x00000010
>  #define MACB_CAPS_FIFO_MODE			0x10000000
>  #define MACB_CAPS_GIGABIT_MODE_AVAILABLE	0x20000000
>  #define MACB_CAPS_SG_DISABLED			0x40000000
> 


-- 
Nicolas Ferre

[PATCH v4 net-next 0/3] Add new capability and macb DT variant

[Neil Armstrong]

The first patch introduces a new capability bit to disable usage of the
USRIO register on platform not implementing it thus avoiding some external
imprecise aborts on ARM based platforms.
The two last patchs adds a new macb variant compatible name using the
capability, the NPx name is temporary and must be fixed when the first patch
hits mainline.

Only the first patch should be merged right now until the compatible name
is fixed.

v1: http://lkml.kernel.org/r/1449485914-12883-1-git-send-email-narmstrong@baylibre.com
v2: http://lkml.kernel.org/r/1449582726-6148-1-git-send-email-narmstrong@baylibre.com
v3: http://lkml.kernel.org/r/1451898103-21868-1-git-send-email-narmstrong@baylibre.com
v4: as nicolas suggested, use a new macb config and a new product/vendor prefix

Neil Armstrong (3):
  net: ethernet: cadence-macb: Add disabled usrio caps
  net: macb: Add NPx macb config using USRIO_DISABLED cap
  dt-bindings: net: macb: Add NPx macb variant

 Documentation/devicetree/bindings/net/macb.txt |  1 +
 drivers/net/ethernet/cadence/macb.c            | 33 ++++++++++++++++----------
 drivers/net/ethernet/cadence/macb.h            |  1 +
 3 files changed, 23 insertions(+), 12 deletions(-)

-- 
1.9.1

[PATCH v4 net-next 1/3] net: ethernet: cadence-macb: Add disabled usrio caps

[Neil Armstrong]

On some platforms, the macb integration does not use the USRIO
register to configure the (R)MII port and clocks.
When the register is not implemented and the MACB error signal
is connected to the bus error, reading or writing to the USRIO
register can trigger some Imprecise External Aborts on ARM platforms.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/net/ethernet/cadence/macb.c | 27 +++++++++++++++------------
 drivers/net/ethernet/cadence/macb.h |  1 +
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 8b45bc9..fa53bc3 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2124,7 +2124,8 @@ static void macb_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 	regs_buff[10] = macb_tx_dma(&bp->queues[0], tail);
 	regs_buff[11] = macb_tx_dma(&bp->queues[0], head);
 
-	regs_buff[12] = macb_or_gem_readl(bp, USRIO);
+	if (!(bp->caps & MACB_CAPS_USRIO_DISABLED))
+		regs_buff[12] = macb_or_gem_readl(bp, USRIO);
 	if (macb_is_gem(bp)) {
 		regs_buff[13] = gem_readl(bp, DMACFG);
 	}
@@ -2403,19 +2404,21 @@ static int macb_init(struct platform_device *pdev)
 		dev->hw_features &= ~NETIF_F_SG;
 	dev->features = dev->hw_features;
 
-	val = 0;
-	if (bp->phy_interface == PHY_INTERFACE_MODE_RGMII)
-		val = GEM_BIT(RGMII);
-	else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII &&
-		 (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
-		val = MACB_BIT(RMII);
-	else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
-		val = MACB_BIT(MII);
+	if (!(bp->caps & MACB_CAPS_USRIO_DISABLED)) {
+		val = 0;
+		if (bp->phy_interface == PHY_INTERFACE_MODE_RGMII)
+			val = GEM_BIT(RGMII);
+		else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII &&
+			 (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
+			val = MACB_BIT(RMII);
+		else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
+			val = MACB_BIT(MII);
 
-	if (bp->caps & MACB_CAPS_USRIO_HAS_CLKEN)
-		val |= MACB_BIT(CLKEN);
+		if (bp->caps & MACB_CAPS_USRIO_HAS_CLKEN)
+			val |= MACB_BIT(CLKEN);
 
-	macb_or_gem_writel(bp, USRIO, val);
+		macb_or_gem_writel(bp, USRIO, val);
+	}
 
 	/* Set MII management clock divider */
 	val = macb_mdc_clk_div(bp);
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 5c03e81..0d4ecfc 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -400,6 +400,7 @@
 #define MACB_CAPS_USRIO_HAS_CLKEN		0x00000002
 #define MACB_CAPS_USRIO_DEFAULT_IS_MII		0x00000004
 #define MACB_CAPS_NO_GIGABIT_HALF		0x00000008
+#define MACB_CAPS_USRIO_DISABLED		0x00000010
 #define MACB_CAPS_FIFO_MODE			0x10000000
 #define MACB_CAPS_GIGABIT_MODE_AVAILABLE	0x20000000
 #define MACB_CAPS_SG_DISABLED			0x40000000
-- 
1.9.1

[PATCH v4 net-next 2/3] net: macb: Add NPx macb config using USRIO_DISABLED cap

[Neil Armstrong]

Declare a new SoC variant for NPx SoCs having USRIO_DISABLED as
capability bit.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/net/ethernet/cadence/macb.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index fa53bc3..a9e27a7 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2781,6 +2781,11 @@ static const struct macb_config emac_config = {
 	.init = at91ether_init,
 };
 
+static const struct macb_config npx_config = {
+	.caps = MACB_CAPS_USRIO_DISABLED,
+	.clk_init = macb_clk_init,
+	.init = macb_init,
+};
 
 static const struct macb_config zynqmp_config = {
 	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO,
@@ -2801,6 +2806,7 @@ static const struct of_device_id macb_dt_ids[] = {
 	{ .compatible = "cdns,at32ap7000-macb" },
 	{ .compatible = "cdns,at91sam9260-macb", .data = &at91sam9260_config },
 	{ .compatible = "cdns,macb" },
+	{ .compatible = "cdns,npx-macb", .data = &npx_config },
 	{ .compatible = "cdns,pc302-gem", .data = &pc302gem_config },
 	{ .compatible = "cdns,gem", .data = &pc302gem_config },
 	{ .compatible = "atmel,sama5d2-gem", .data = &sama5d2_config },
-- 
1.9.1

[PATCH v4 net-next 3/3] dt-bindings: net: macb: Add NPx macb variant

[Neil Armstrong]

Add NPx macb variant for NPx SoCs.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 Documentation/devicetree/bindings/net/macb.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 38c8e84..638cdde 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -4,6 +4,7 @@ Required properties:
 - compatible: Should be "cdns,[<chip>-]{macb|gem}"
   Use "cdns,at91sam9260-macb" for Atmel at91sam9 SoCs or the 10/100Mbit IP
   available on sama5d3 SoCs.
+  Use "cdns,npx-macb" for NPX SoCs devices.
   Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb".
   Use "cdns,pc302-gem" for Picochip picoXcell pc302 and later devices based on
   the Cadence GEM, or the generic form: "cdns,gem".
-- 
1.9.1

Re: [PATCH v4 net-next 2/3] net: macb: Add NPx macb config using USRIO_DISABLED cap

[Nicolas Ferre]

Le 04/01/2016 10:42, Neil Armstrong a écrit :
> Declare a new SoC variant for NPx SoCs having USRIO_DISABLED as
> capability bit.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/net/ethernet/cadence/macb.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index fa53bc3..a9e27a7 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -2781,6 +2781,11 @@ static const struct macb_config emac_config = {
>  	.init = at91ether_init,
>  };
>  
> +static const struct macb_config npx_config = {
> +	.caps = MACB_CAPS_USRIO_DISABLED,
> +	.clk_init = macb_clk_init,
> +	.init = macb_init,
> +};
>  
>  static const struct macb_config zynqmp_config = {
>  	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO,
> @@ -2801,6 +2806,7 @@ static const struct of_device_id macb_dt_ids[] = {
>  	{ .compatible = "cdns,at32ap7000-macb" },
>  	{ .compatible = "cdns,at91sam9260-macb", .data = &at91sam9260_config },
>  	{ .compatible = "cdns,macb" },
> +	{ .compatible = "cdns,npx-macb", .data = &npx_config },

I can accept that, but I think that you'd better make your device tree
compatibility string *not* generic. Name it by the first NPx SoC or
perfectly compatible SoC family that has this configuration and you'll
be able to make the NP(x+1) compatible with it.

It has proven to be much more future proof and even if in the early days
of DT on ARM we accepted some binding with generic strings like this one
below, It has proven to be a mistake.

>  	{ .compatible = "cdns,gem", .data = &pc302gem_config },
>  	{ .compatible = "atmel,sama5d2-gem", .data = &sama5d2_config },
> 


-- 
Nicolas Ferre

Re: [PATCH v4 net-next 2/3] net: macb: Add NPx macb config using USRIO_DISABLED cap

[Neil Armstrong]

On 01/04/2016 11:38 AM, Nicolas Ferre wrote:
> Le 04/01/2016 10:42, Neil Armstrong a écrit :
>>  static const struct macb_config zynqmp_config = {
>>  	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO,
>> @@ -2801,6 +2806,7 @@ static const struct of_device_id macb_dt_ids[] = {
>>  	{ .compatible = "cdns,at32ap7000-macb" },
>>  	{ .compatible = "cdns,at91sam9260-macb", .data = &at91sam9260_config },
>>  	{ .compatible = "cdns,macb" },
>> +	{ .compatible = "cdns,npx-macb", .data = &npx_config },
> 
> I can accept that, but I think that you'd better make your device tree
> compatibility string *not* generic. Name it by the first NPx SoC or
> perfectly compatible SoC family that has this configuration and you'll
> be able to make the NP(x+1) compatible with it.
Well, the first Soc having this configuration is Np4, would cdns,np4-macb be ok ?
> 
> It has proven to be much more future proof and even if in the early days
> of DT on ARM we accepted some binding with generic strings like this one
> below, It has proven to be a mistake.
> 
>>  	{ .compatible = "cdns,gem", .data = &pc302gem_config },
>>  	{ .compatible = "atmel,sama5d2-gem", .data = &sama5d2_config },
>>
> 
> 

Neil

Re: [PATCH v4 net-next 2/3] net: macb: Add NPx macb config using USRIO_DISABLED cap

[Nicolas Ferre]

Le 05/01/2016 13:20, Neil Armstrong a écrit :
> On 01/04/2016 11:38 AM, Nicolas Ferre wrote:
>> Le 04/01/2016 10:42, Neil Armstrong a écrit :
>>>  static const struct macb_config zynqmp_config = {
>>>  	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO,
>>> @@ -2801,6 +2806,7 @@ static const struct of_device_id macb_dt_ids[] = {
>>>  	{ .compatible = "cdns,at32ap7000-macb" },
>>>  	{ .compatible = "cdns,at91sam9260-macb", .data = &at91sam9260_config },
>>>  	{ .compatible = "cdns,macb" },
>>> +	{ .compatible = "cdns,npx-macb", .data = &npx_config },
>>
>> I can accept that, but I think that you'd better make your device tree
>> compatibility string *not* generic. Name it by the first NPx SoC or
>> perfectly compatible SoC family that has this configuration and you'll
>> be able to make the NP(x+1) compatible with it.
> Well, the first Soc having this configuration is Np4, would cdns,np4-macb be ok ?

Yes, absolutely.

Thanks

>> It has proven to be much more future proof and even if in the early days
>> of DT on ARM we accepted some binding with generic strings like this one
>> below, It has proven to be a mistake.
>>
>>>  	{ .compatible = "cdns,gem", .data = &pc302gem_config },
>>>  	{ .compatible = "atmel,sama5d2-gem", .data = &sama5d2_config },
>>>
>>
>>
> 
> Neil
> 


-- 
Nicolas Ferre

[PATCH v5 net-next 0/3] Add new capability and macb DT variant

[Neil Armstrong]

The first patch introduces a new capability bit to disable usage of the
USRIO register on platform not implementing it thus avoiding some external
imprecise aborts on ARM based platforms.
The two last patchs adds a new macb variant compatible name using the
capability, the NP4 SoC uses this particular hardware configuration.

v1: http://lkml.kernel.org/r/1449485914-12883-1-git-send-email-narmstrong@baylibre.com
v2: http://lkml.kernel.org/r/1449582726-6148-1-git-send-email-narmstrong@baylibre.com
v3: http://lkml.kernel.org/r/1451898103-21868-1-git-send-email-narmstrong@baylibre.com
v4: http://lkml.kernel.org/r/1451900573-22657-1-git-send-email-narmstrong@baylibre.com
v5: switch SoC name to non-generic NP4 name

Neil Armstrong (3):
  net: ethernet: cadence-macb: Add disabled usrio caps
  net: macb: Add NP4 macb config using USRIO_DISABLED
  dt-bindings: net: macb: Add NP4 macb variant

 Documentation/devicetree/bindings/net/macb.txt |  1 +
 drivers/net/ethernet/cadence/macb.c            | 33 ++++++++++++++++----------
 drivers/net/ethernet/cadence/macb.h            |  1 +
 3 files changed, 23 insertions(+), 12 deletions(-)

-- 
1.9.1

[PATCH v5 net-next 1/3] net: ethernet: cadence-macb: Add disabled usrio caps

[Neil Armstrong]

On some platforms, the macb integration does not use the USRIO
register to configure the (R)MII port and clocks.
When the register is not implemented and the MACB error signal
is connected to the bus error, reading or writing to the USRIO
register can trigger some Imprecise External Aborts on ARM platforms.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/net/ethernet/cadence/macb.c | 27 +++++++++++++++------------
 drivers/net/ethernet/cadence/macb.h |  1 +
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 8b45bc9..fa53bc3 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2124,7 +2124,8 @@ static void macb_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 	regs_buff[10] = macb_tx_dma(&bp->queues[0], tail);
 	regs_buff[11] = macb_tx_dma(&bp->queues[0], head);
 
-	regs_buff[12] = macb_or_gem_readl(bp, USRIO);
+	if (!(bp->caps & MACB_CAPS_USRIO_DISABLED))
+		regs_buff[12] = macb_or_gem_readl(bp, USRIO);
 	if (macb_is_gem(bp)) {
 		regs_buff[13] = gem_readl(bp, DMACFG);
 	}
@@ -2403,19 +2404,21 @@ static int macb_init(struct platform_device *pdev)
 		dev->hw_features &= ~NETIF_F_SG;
 	dev->features = dev->hw_features;
 
-	val = 0;
-	if (bp->phy_interface == PHY_INTERFACE_MODE_RGMII)
-		val = GEM_BIT(RGMII);
-	else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII &&
-		 (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
-		val = MACB_BIT(RMII);
-	else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
-		val = MACB_BIT(MII);
+	if (!(bp->caps & MACB_CAPS_USRIO_DISABLED)) {
+		val = 0;
+		if (bp->phy_interface == PHY_INTERFACE_MODE_RGMII)
+			val = GEM_BIT(RGMII);
+		else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII &&
+			 (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
+			val = MACB_BIT(RMII);
+		else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
+			val = MACB_BIT(MII);
 
-	if (bp->caps & MACB_CAPS_USRIO_HAS_CLKEN)
-		val |= MACB_BIT(CLKEN);
+		if (bp->caps & MACB_CAPS_USRIO_HAS_CLKEN)
+			val |= MACB_BIT(CLKEN);
 
-	macb_or_gem_writel(bp, USRIO, val);
+		macb_or_gem_writel(bp, USRIO, val);
+	}
 
 	/* Set MII management clock divider */
 	val = macb_mdc_clk_div(bp);
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 5c03e81..0d4ecfc 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -400,6 +400,7 @@
 #define MACB_CAPS_USRIO_HAS_CLKEN		0x00000002
 #define MACB_CAPS_USRIO_DEFAULT_IS_MII		0x00000004
 #define MACB_CAPS_NO_GIGABIT_HALF		0x00000008
+#define MACB_CAPS_USRIO_DISABLED		0x00000010
 #define MACB_CAPS_FIFO_MODE			0x10000000
 #define MACB_CAPS_GIGABIT_MODE_AVAILABLE	0x20000000
 #define MACB_CAPS_SG_DISABLED			0x40000000
-- 
1.9.1

[PATCH v5 net-next 2/3] net: macb: Add NP4 macb config using USRIO_DISABLED

[Neil Armstrong]

Declare a new NP4 SoC variant having USRIO_DISABLED as capability bit.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/net/ethernet/cadence/macb.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index fa53bc3..d12ee07 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2781,6 +2781,11 @@ static const struct macb_config emac_config = {
 	.init = at91ether_init,
 };
 
+static const struct macb_config np4_config = {
+	.caps = MACB_CAPS_USRIO_DISABLED,
+	.clk_init = macb_clk_init,
+	.init = macb_init,
+};
 
 static const struct macb_config zynqmp_config = {
 	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO,
@@ -2801,6 +2806,7 @@ static const struct of_device_id macb_dt_ids[] = {
 	{ .compatible = "cdns,at32ap7000-macb" },
 	{ .compatible = "cdns,at91sam9260-macb", .data = &at91sam9260_config },
 	{ .compatible = "cdns,macb" },
+	{ .compatible = "cdns,np4-macb", .data = &np4_config },
 	{ .compatible = "cdns,pc302-gem", .data = &pc302gem_config },
 	{ .compatible = "cdns,gem", .data = &pc302gem_config },
 	{ .compatible = "atmel,sama5d2-gem", .data = &sama5d2_config },
-- 
1.9.1

[PATCH v5 net-next 3/3] dt-bindings: net: macb: Add NP4 macb variant

[Neil Armstrong]

Add NP4 macb SoC variant.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 Documentation/devicetree/bindings/net/macb.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 38c8e84..5c397ca 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -4,6 +4,7 @@ Required properties:
 - compatible: Should be "cdns,[<chip>-]{macb|gem}"
   Use "cdns,at91sam9260-macb" for Atmel at91sam9 SoCs or the 10/100Mbit IP
   available on sama5d3 SoCs.
+  Use "cdns,np4-macb" for NP4 SoC devices.
   Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb".
   Use "cdns,pc302-gem" for Picochip picoXcell pc302 and later devices based on
   the Cadence GEM, or the generic form: "cdns,gem".
-- 
1.9.1

Re: [PATCH v5 net-next 1/3] net: ethernet: cadence-macb: Add disabled usrio caps

[Nicolas Ferre]

Le 05/01/2016 14:39, Neil Armstrong a écrit :
> On some platforms, the macb integration does not use the USRIO
> register to configure the (R)MII port and clocks.
> When the register is not implemented and the MACB error signal
> is connected to the bus error, reading or writing to the USRIO
> register can trigger some Imprecise External Aborts on ARM platforms.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Thanks!

> ---
>  drivers/net/ethernet/cadence/macb.c | 27 +++++++++++++++------------
>  drivers/net/ethernet/cadence/macb.h |  1 +
>  2 files changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 8b45bc9..fa53bc3 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -2124,7 +2124,8 @@ static void macb_get_regs(struct net_device *dev, struct ethtool_regs *regs,
>  	regs_buff[10] = macb_tx_dma(&bp->queues[0], tail);
>  	regs_buff[11] = macb_tx_dma(&bp->queues[0], head);
>  
> -	regs_buff[12] = macb_or_gem_readl(bp, USRIO);
> +	if (!(bp->caps & MACB_CAPS_USRIO_DISABLED))
> +		regs_buff[12] = macb_or_gem_readl(bp, USRIO);
>  	if (macb_is_gem(bp)) {
>  		regs_buff[13] = gem_readl(bp, DMACFG);
>  	}
> @@ -2403,19 +2404,21 @@ static int macb_init(struct platform_device *pdev)
>  		dev->hw_features &= ~NETIF_F_SG;
>  	dev->features = dev->hw_features;
>  
> -	val = 0;
> -	if (bp->phy_interface == PHY_INTERFACE_MODE_RGMII)
> -		val = GEM_BIT(RGMII);
> -	else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII &&
> -		 (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
> -		val = MACB_BIT(RMII);
> -	else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
> -		val = MACB_BIT(MII);
> +	if (!(bp->caps & MACB_CAPS_USRIO_DISABLED)) {
> +		val = 0;
> +		if (bp->phy_interface == PHY_INTERFACE_MODE_RGMII)
> +			val = GEM_BIT(RGMII);
> +		else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII &&
> +			 (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
> +			val = MACB_BIT(RMII);
> +		else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII))
> +			val = MACB_BIT(MII);
>  
> -	if (bp->caps & MACB_CAPS_USRIO_HAS_CLKEN)
> -		val |= MACB_BIT(CLKEN);
> +		if (bp->caps & MACB_CAPS_USRIO_HAS_CLKEN)
> +			val |= MACB_BIT(CLKEN);
>  
> -	macb_or_gem_writel(bp, USRIO, val);
> +		macb_or_gem_writel(bp, USRIO, val);
> +	}
>  
>  	/* Set MII management clock divider */
>  	val = macb_mdc_clk_div(bp);
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 5c03e81..0d4ecfc 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -400,6 +400,7 @@
>  #define MACB_CAPS_USRIO_HAS_CLKEN		0x00000002
>  #define MACB_CAPS_USRIO_DEFAULT_IS_MII		0x00000004
>  #define MACB_CAPS_NO_GIGABIT_HALF		0x00000008
> +#define MACB_CAPS_USRIO_DISABLED		0x00000010
>  #define MACB_CAPS_FIFO_MODE			0x10000000
>  #define MACB_CAPS_GIGABIT_MODE_AVAILABLE	0x20000000
>  #define MACB_CAPS_SG_DISABLED			0x40000000
> 


-- 
Nicolas Ferre

Re: [PATCH v5 net-next 2/3] net: macb: Add NP4 macb config using USRIO_DISABLED

[Nicolas Ferre]

Le 05/01/2016 14:39, Neil Armstrong a écrit :
> Declare a new NP4 SoC variant having USRIO_DISABLED as capability bit.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>


> ---
>  drivers/net/ethernet/cadence/macb.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index fa53bc3..d12ee07 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -2781,6 +2781,11 @@ static const struct macb_config emac_config = {
>  	.init = at91ether_init,
>  };
>  
> +static const struct macb_config np4_config = {
> +	.caps = MACB_CAPS_USRIO_DISABLED,
> +	.clk_init = macb_clk_init,
> +	.init = macb_init,
> +};
>  
>  static const struct macb_config zynqmp_config = {
>  	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO,
> @@ -2801,6 +2806,7 @@ static const struct of_device_id macb_dt_ids[] = {
>  	{ .compatible = "cdns,at32ap7000-macb" },
>  	{ .compatible = "cdns,at91sam9260-macb", .data = &at91sam9260_config },
>  	{ .compatible = "cdns,macb" },
> +	{ .compatible = "cdns,np4-macb", .data = &np4_config },
>  	{ .compatible = "cdns,pc302-gem", .data = &pc302gem_config },
>  	{ .compatible = "cdns,gem", .data = &pc302gem_config },
>  	{ .compatible = "atmel,sama5d2-gem", .data = &sama5d2_config },
> 


-- 
Nicolas Ferre

Re: [PATCH v5 net-next 3/3] dt-bindings: net: macb: Add NP4 macb variant

[Nicolas Ferre]

Le 05/01/2016 14:39, Neil Armstrong a écrit :
> Add NP4 macb SoC variant.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Neil, thanks for your understanding and reactivity concerning this patch
series.

Bye,

> ---
>  Documentation/devicetree/bindings/net/macb.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
> index 38c8e84..5c397ca 100644
> --- a/Documentation/devicetree/bindings/net/macb.txt
> +++ b/Documentation/devicetree/bindings/net/macb.txt
> @@ -4,6 +4,7 @@ Required properties:
>  - compatible: Should be "cdns,[<chip>-]{macb|gem}"
>    Use "cdns,at91sam9260-macb" for Atmel at91sam9 SoCs or the 10/100Mbit IP
>    available on sama5d3 SoCs.
> +  Use "cdns,np4-macb" for NP4 SoC devices.
>    Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb".
>    Use "cdns,pc302-gem" for Picochip picoXcell pc302 and later devices based on
>    the Cadence GEM, or the generic form: "cdns,gem".
> 


-- 
Nicolas Ferre

Re: [PATCH v5 net-next 3/3] dt-bindings: net: macb: Add NP4 macb variant

[Rob Herring]

On Tue, Jan 05, 2016 at 02:39:18PM +0100, Neil Armstrong wrote:
> Add NP4 macb SoC variant.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  Documentation/devicetree/bindings/net/macb.txt | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 net-next 0/3] Add new capability and macb DT variant

[David Miller]

From: Neil Armstrong <narmstrong@baylibre.com>
Date: Tue,  5 Jan 2016 14:39:15 +0100

> The first patch introduces a new capability bit to disable usage of the
> USRIO register on platform not implementing it thus avoiding some external
> imprecise aborts on ARM based platforms.
> The two last patchs adds a new macb variant compatible name using the
> capability, the NP4 SoC uses this particular hardware configuration.
> 
> v1: http://lkml.kernel.org/r/1449485914-12883-1-git-send-email-narmstrong@baylibre.com
> v2: http://lkml.kernel.org/r/1449582726-6148-1-git-send-email-narmstrong@baylibre.com
> v3: http://lkml.kernel.org/r/1451898103-21868-1-git-send-email-narmstrong@baylibre.com
> v4: http://lkml.kernel.org/r/1451900573-22657-1-git-send-email-narmstrong@baylibre.com
> v5: switch SoC name to non-generic NP4 name

Series applied to net-next, thanks.