Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1278380 > unrolled thread

[PATCH v2 net 0/6] Marvell Armada XP/370/38X Neta fixes

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 net 0/6] Marvell Armada XP/370/38X Neta fixes Marcin Wojtas <mw@semihalf.com> - 2015-11-26 19:10 +0100
    [PATCH v2 net 6/6] net: mvneta: enable IP checksum with jumbo frames for Armada 38x on Port0 Marcin Wojtas <mw@semihalf.com> - 2015-11-26 19:10 +0100
      Re: [PATCH v2 net 6/6] net: mvneta: enable IP checksum with jumbo  frames for Armada 38x on Port0 Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2015-11-26 19:40 +0100
        Re: [PATCH v2 net 6/6] net: mvneta: enable IP checksum with jumbo  frames for Armada 38x on Port0 Marcin Wojtas <mw@semihalf.com> - 2015-11-26 19:50 +0100
    [PATCH v2 net 1/6] net: mvneta: add configuration for MBUS windows access protection Marcin Wojtas <mw@semihalf.com> - 2015-11-26 19:20 +0100
      Re: [PATCH v2 net 1/6] net: mvneta: add configuration for MBUS  windows access protection Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2015-11-26 19:30 +0100
        Re: [PATCH v2 net 1/6] net: mvneta: add configuration for MBUS  windows access protection Marcin Wojtas <mw@semihalf.com> - 2015-11-26 19:40 +0100
    [PATCH v2 net 2/6] net: mvneta: fix bit assignment in MVNETA_RXQ_CONFIG_REG Marcin Wojtas <mw@semihalf.com> - 2015-11-26 19:20 +0100
      Re: [PATCH v2 net 2/6] net: mvneta: fix bit assignment in  MVNETA_RXQ_CONFIG_REG Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2015-11-26 19:30 +0100

#1278380 — [PATCH v2 net 0/6] Marvell Armada XP/370/38X Neta fixes

Hi,

According to David's remark I split the original patchset into two. First
one, which I submit is a list of fixes, all stable-CC'ed.

On the occasion two new bugs were fixed - wrong define in one of registers
and, more important, an error path in mvneta_rx() after failing of skb
build.

I'm looking forward to the comments or remarks.

Best regards,
Marcin Wojtas

Changes from v1:
* update MBUS windows access protection register once, after whole loop
* add fixing value of MVNETA_RXQ_INTR_ENABLE_ALL_MASK
* add fixing error path for skb_build()
* add possibility of setting custom TX IP checksum limit in DT property

Marcin Wojtas (6):
  net: mvneta: add configuration for MBUS windows access protection
  net: mvneta: fix bit assignment in MVNETA_RXQ_CONFIG_REG
  net: mvneta: fix bit assignment for RX packet irq enable
  net: mvneta: fix error path for building skb
  net: mvneta: enable setting custom TX IP checksum limit
  net: mvneta: enable IP checksum with jumbo frames for Armada 38x on
    Port0

 .../bindings/net/marvell-armada-370-neta.txt       |  6 ++++
 arch/arm/boot/dts/armada-38x.dtsi                  |  1 +
 drivers/net/ethernet/marvell/mvneta.c              | 34 ++++++++++++++++++----
 3 files changed, 35 insertions(+), 6 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1278382 — [PATCH v2 net 6/6] net: mvneta: enable IP checksum with jumbo frames for Armada 38x on Port0

The Ethernet controller found in the Armada 38x SoC's family support
TCP/IP checksumming with frame sizes larger than 1600 bytes, however
only on port 0.

This commit enables it by setting 'tx-csum-limit' to 9800B in
'ethernet@70000' node.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Cc: <stable@vger.kernel.org> # v3.18+
---
 arch/arm/boot/dts/armada-38x.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index c6a0e9d..e8b7f67 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -498,6 +498,7 @@
 				reg = <0x70000 0x4000>;
 				interrupts-extended = <&mpic 8>;
 				clocks = <&gateclk 4>;
+				tx-csum-limit = <9800>;
 				status = "disabled";
 			};
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278394 — Re: [PATCH v2 net 6/6] net: mvneta: enable IP checksum with jumbo frames for Armada 38x on Port0

Marcin,

On Thu, 26 Nov 2015 19:08:13 +0100, Marcin Wojtas wrote:
> The Ethernet controller found in the Armada 38x SoC's family support
> TCP/IP checksumming with frame sizes larger than 1600 bytes, however
> only on port 0.
> 
> This commit enables it by setting 'tx-csum-limit' to 9800B in
> 'ethernet@70000' node.
> 
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> Cc: <stable@vger.kernel.org> # v3.18+

This and the previous patch is not a fix, and has no business going in
stable. It is enabling a better functionality, but it is clearly not a
fix.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278397 — Re: [PATCH v2 net 6/6] net: mvneta: enable IP checksum with jumbo frames for Armada 38x on Port0

Thomas,


2015-11-26 19:33 GMT+01:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Marcin,
>
> On Thu, 26 Nov 2015 19:08:13 +0100, Marcin Wojtas wrote:
>> The Ethernet controller found in the Armada 38x SoC's family support
>> TCP/IP checksumming with frame sizes larger than 1600 bytes, however
>> only on port 0.
>>
>> This commit enables it by setting 'tx-csum-limit' to 9800B in
>> 'ethernet@70000' node.
>>
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> Cc: <stable@vger.kernel.org> # v3.18+
>
> This and the previous patch is not a fix, and has no business going in
> stable. It is enabling a better functionality, but it is clearly not a
> fix.
>

Ok. I thought that patches enabling tx_csum_limit were adding a sort
of regression, but I can agree to your interpretation. In such case I
will leave stable notification only for patch #4.

Thanks,
Marcin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278383 — [PATCH v2 net 1/6] net: mvneta: add configuration for MBUS windows access protection

This commit adds missing configuration of MBUS windows access protection
in mvneta_conf_mbus_windows function - a dedicated variable for that
purpose remained there unused since v3.8 initial mvneta support. Because
of that the register contents were inherited from the bootloader.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Cc: <stable@vger.kernel.org> # v3.8+
Fixes: c5aff18204da ("net: mvneta: driver for Marvell Armada 370/XP network
unit")
---
 drivers/net/ethernet/marvell/mvneta.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index e84c7f2..c4327b9 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -62,6 +62,7 @@
 #define MVNETA_WIN_SIZE(w)                      (0x2204 + ((w) << 3))
 #define MVNETA_WIN_REMAP(w)                     (0x2280 + ((w) << 2))
 #define MVNETA_BASE_ADDR_ENABLE                 0x2290
+#define MVNETA_ACCESS_PROTECT_ENABLE		0x2294
 #define MVNETA_PORT_CONFIG                      0x2400
 #define      MVNETA_UNI_PROMISC_MODE            BIT(0)
 #define      MVNETA_DEF_RXQ(q)                  ((q) << 1)
@@ -3188,9 +3189,11 @@ static void mvneta_conf_mbus_windows(struct mvneta_port *pp,
 
 		win_enable &= ~(1 << i);
 		win_protect |= 3 << (2 * i);
+
 	}
 
 	mvreg_write(pp, MVNETA_BASE_ADDR_ENABLE, win_enable);
+	mvreg_write(pp, MVNETA_ACCESS_PROTECT_ENABLE, win_protect);
 }
 
 /* Power up the port */
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278391 — Re: [PATCH v2 net 1/6] net: mvneta: add configuration for MBUS windows access protection

Marcin,

On Thu, 26 Nov 2015 19:08:08 +0100, Marcin Wojtas wrote:
> This commit adds missing configuration of MBUS windows access protection
> in mvneta_conf_mbus_windows function - a dedicated variable for that
> purpose remained there unused since v3.8 initial mvneta support. Because
> of that the register contents were inherited from the bootloader.
> 
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Cc: <stable@vger.kernel.org> # v3.8+
> Fixes: c5aff18204da ("net: mvneta: driver for Marvell Armada 370/XP network
> unit")

Is this actually fixing a user visible problem? If not, then I am not
sure it qualifies for stable.

> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index e84c7f2..c4327b9 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -62,6 +62,7 @@
>  #define MVNETA_WIN_SIZE(w)                      (0x2204 + ((w) << 3))
>  #define MVNETA_WIN_REMAP(w)                     (0x2280 + ((w) << 2))
>  #define MVNETA_BASE_ADDR_ENABLE                 0x2290
> +#define MVNETA_ACCESS_PROTECT_ENABLE		0x2294

Spaces are used for indentation in the definitions of all other
constants in this driver, so I think you should stick with this
convention, at least to remain consistent.

>  #define MVNETA_PORT_CONFIG                      0x2400
>  #define      MVNETA_UNI_PROMISC_MODE            BIT(0)
>  #define      MVNETA_DEF_RXQ(q)                  ((q) << 1)
> @@ -3188,9 +3189,11 @@ static void mvneta_conf_mbus_windows(struct mvneta_port *pp,
>  
>  		win_enable &= ~(1 << i);
>  		win_protect |= 3 << (2 * i);
> +

Unneeded change.

>  	}
>  
>  	mvreg_write(pp, MVNETA_BASE_ADDR_ENABLE, win_enable);
> +	mvreg_write(pp, MVNETA_ACCESS_PROTECT_ENABLE, win_protect);
>  }

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278396 — Re: [PATCH v2 net 1/6] net: mvneta: add configuration for MBUS windows access protection

Hi Thomas,

2015-11-26 19:25 GMT+01:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Marcin,
>
> On Thu, 26 Nov 2015 19:08:08 +0100, Marcin Wojtas wrote:
>> This commit adds missing configuration of MBUS windows access protection
>> in mvneta_conf_mbus_windows function - a dedicated variable for that
>> purpose remained there unused since v3.8 initial mvneta support. Because
>> of that the register contents were inherited from the bootloader.
>>
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Cc: <stable@vger.kernel.org> # v3.8+
>> Fixes: c5aff18204da ("net: mvneta: driver for Marvell Armada 370/XP network
>> unit")
>
> Is this actually fixing a user visible problem? If not, then I am not
> sure it qualifies for stable.

Indeed, it's not fixing a visible thing, unless you resume from
suspend to ram. However, the original driver declares and updates
'win_protect ' variable, which have remained unused for past 3 years:)
If you think it's not worth cc'ing stable despite that, I will remove
the notification.

>
>> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
>> index e84c7f2..c4327b9 100644
>> --- a/drivers/net/ethernet/marvell/mvneta.c
>> +++ b/drivers/net/ethernet/marvell/mvneta.c
>> @@ -62,6 +62,7 @@
>>  #define MVNETA_WIN_SIZE(w)                      (0x2204 + ((w) << 3))
>>  #define MVNETA_WIN_REMAP(w)                     (0x2280 + ((w) << 2))
>>  #define MVNETA_BASE_ADDR_ENABLE                 0x2290
>> +#define MVNETA_ACCESS_PROTECT_ENABLE         0x2294
>
> Spaces are used for indentation in the definitions of all other
> constants in this driver, so I think you should stick with this
> convention, at least to remain consistent.
>

Ok.

>>  #define MVNETA_PORT_CONFIG                      0x2400
>>  #define      MVNETA_UNI_PROMISC_MODE            BIT(0)
>>  #define      MVNETA_DEF_RXQ(q)                  ((q) << 1)
>> @@ -3188,9 +3189,11 @@ static void mvneta_conf_mbus_windows(struct mvneta_port *pp,
>>
>>               win_enable &= ~(1 << i);
>>               win_protect |= 3 << (2 * i);
>> +
>
> Unneeded change.
>

Indeed, my mistake.

Thanks,
Marcin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278389 — [PATCH v2 net 2/6] net: mvneta: fix bit assignment in MVNETA_RXQ_CONFIG_REG

MVNETA_RXQ_HW_BUF_ALLOC bit which controls enabling hardware buffer
allocation was mistakenly set as BIT(1). This commit fixes the assignment.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Cc: <stable@vger.kernel.org> # v3.8+
Fixes: c5aff18204da ("net: mvneta: driver for Marvell Armada 370/XP network
unit")
---
 drivers/net/ethernet/marvell/mvneta.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index c4327b9..d78edbb 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -36,7 +36,7 @@
 
 /* Registers */
 #define MVNETA_RXQ_CONFIG_REG(q)                (0x1400 + ((q) << 2))
-#define      MVNETA_RXQ_HW_BUF_ALLOC            BIT(1)
+#define      MVNETA_RXQ_HW_BUF_ALLOC            BIT(0)
 #define      MVNETA_RXQ_PKT_OFFSET_ALL_MASK     (0xf    << 8)
 #define      MVNETA_RXQ_PKT_OFFSET_MASK(offs)   ((offs) << 8)
 #define MVNETA_RXQ_THRESHOLD_REG(q)             (0x14c0 + ((q) << 2))
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278393 — Re: [PATCH v2 net 2/6] net: mvneta: fix bit assignment in MVNETA_RXQ_CONFIG_REG

Dear Marcin Wojtas,

On Thu, 26 Nov 2015 19:08:09 +0100, Marcin Wojtas wrote:
> MVNETA_RXQ_HW_BUF_ALLOC bit which controls enabling hardware buffer
> allocation was mistakenly set as BIT(1). This commit fixes the assignment.
> 
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Cc: <stable@vger.kernel.org> # v3.8+
> Fixes: c5aff18204da ("net: mvneta: driver for Marvell Armada 370/XP network
> unit")
> ---
>  drivers/net/ethernet/marvell/mvneta.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

What is the user-visible impact of this bug? Indeed, the code is wrong,
but things were working fine until now.

So I am not questioning that this patch should be merged, but only
questioning whether the stable tag is appropriate.

Same questions for your patch 3/6.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web