[PATCH 01/13] net: mvneta: add configuration for MBUS windows access protection

[PATCH 01/13] net: mvneta: add configuration for MBUS windows access protection

[Marcin Wojtas]

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>
Cc: <stable@vger.kernel.org> # v3.8+
---
 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..0f30aaa 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,6 +3189,8 @@ static void mvneta_conf_mbus_windows(struct mvneta_port *pp,
 
 		win_enable &= ~(1 << i);
 		win_protect |= 3 << (2 * i);
+
+		mvreg_write(pp, MVNETA_ACCESS_PROTECT_ENABLE, win_protect);
 	}
 
 	mvreg_write(pp, MVNETA_BASE_ADDR_ENABLE, win_enable);
-- 
1.8.3.1

Re: [PATCH 01/13] net: mvneta: add configuration for MBUS windows access protection

[Gregory CLEMENT]

Hi Marcin,
 
 On dim., nov. 22 2015, Marcin Wojtas <mw@semihalf.com> 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.

It looks OK for me and at least after applying the driver continues
working :)

I guess you find it when you tested suspend to ram.

Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

Thanks,

Gregory


>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> Cc: <stable@vger.kernel.org> # v3.8+
> ---
>  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..0f30aaa 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,6 +3189,8 @@ static void mvneta_conf_mbus_windows(struct mvneta_port *pp,
>  
>  		win_enable &= ~(1 << i);
>  		win_protect |= 3 << (2 * i);
> +
> +		mvreg_write(pp, MVNETA_ACCESS_PROTECT_ENABLE, win_protect);
>  	}
>  
>  	mvreg_write(pp, MVNETA_BASE_ADDR_ENABLE, win_enable);
> -- 
> 1.8.3.1
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 02/13] net: mvneta: enable IP checksum with jumbo frames for Armada 38x on Port0

[Marcin Wojtas]

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 this feature by using 'marvell,armada-xp-neta' 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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index c6a0e9d..b7868b2 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -494,7 +494,7 @@
 			};
 
 			eth0: ethernet@70000 {
-				compatible = "marvell,armada-370-neta";
+				compatible = "marvell,armada-xp-neta";
 				reg = <0x70000 0x4000>;
 				interrupts-extended = <&mpic 8>;
 				clocks = <&gateclk 4>;
-- 
1.8.3.1

Re: [PATCH 02/13] net: mvneta: enable IP checksum with jumbo frames for Armada 38x on Port0

[Arnd Bergmann]

On Sunday 22 November 2015 08:53:48 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 this feature by using 'marvell,armada-xp-neta' 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 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
> index c6a0e9d..b7868b2 100644
> --- a/arch/arm/boot/dts/armada-38x.dtsi
> +++ b/arch/arm/boot/dts/armada-38x.dtsi
> @@ -494,7 +494,7 @@
>                         };
>  
>                         eth0: ethernet@70000 {
> -                               compatible = "marvell,armada-370-neta";
> +                               compatible = "marvell,armada-xp-neta";
>                                 reg = <0x70000 0x4000>;
>                                 interrupts-extended = <&mpic 8>;
>                                 clocks = <&gateclk 4>;
> 

As it's clear that they are not 100% backwards compatible, please
add a SoC specific compatible string here as well, like

	compatible = "marvell,armada-380-neta", "marvell,armada-xp-neta";

Maybe also leave the 370 string in place.

	Arnd

Re: [PATCH 02/13] net: mvneta: enable IP checksum with jumbo frames for Armada 38x on Port0

[Marcin Wojtas]

Arnd,

2015-11-22 21:00 GMT+01:00 Arnd Bergmann <arnd@arndb.de>:
> On Sunday 22 November 2015 08:53:48 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 this feature by using 'marvell,armada-xp-neta' 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 | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
>> index c6a0e9d..b7868b2 100644
>> --- a/arch/arm/boot/dts/armada-38x.dtsi
>> +++ b/arch/arm/boot/dts/armada-38x.dtsi
>> @@ -494,7 +494,7 @@
>>                         };
>>
>>                         eth0: ethernet@70000 {
>> -                               compatible = "marvell,armada-370-neta";
>> +                               compatible = "marvell,armada-xp-neta";
>>                                 reg = <0x70000 0x4000>;
>>                                 interrupts-extended = <&mpic 8>;
>>                                 clocks = <&gateclk 4>;
>>
>
> As it's clear that they are not 100% backwards compatible, please
> add a SoC specific compatible string here as well, like
>
>         compatible = "marvell,armada-380-neta", "marvell,armada-xp-neta";
>

Wouldn't be one sufficient ("marvell,armada-380-neta")?

> Maybe also leave the 370 string in place.
>

Now 370 string disables ip checksum for jumbo frames, so I don't think
it's appropriate to keep it for port 0.

Best regards,
Marcin

Re: [PATCH 02/13] net: mvneta: enable IP checksum with jumbo frames for Armada 38x on Port0

[Arnd Bergmann]

On Sunday 22 November 2015 22:04:38 Marcin Wojtas wrote:
> 2015-11-22 21:00 GMT+01:00 Arnd Bergmann <arnd@arndb.de>:
> > On Sunday 22 November 2015 08:53:48 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 this feature by using 'marvell,armada-xp-neta' 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 | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
> >> index c6a0e9d..b7868b2 100644
> >> --- a/arch/arm/boot/dts/armada-38x.dtsi
> >> +++ b/arch/arm/boot/dts/armada-38x.dtsi
> >> @@ -494,7 +494,7 @@
> >>                         };
> >>
> >>                         eth0: ethernet@70000 {
> >> -                               compatible = "marvell,armada-370-neta";
> >> +                               compatible = "marvell,armada-xp-neta";
> >>                                 reg = <0x70000 0x4000>;
> >>                                 interrupts-extended = <&mpic 8>;
> >>                                 clocks = <&gateclk 4>;
> >>
> >
> > As it's clear that they are not 100% backwards compatible, please
> > add a SoC specific compatible string here as well, like
> >
> >         compatible = "marvell,armada-380-neta", "marvell,armada-xp-neta";
> >
> 
> Wouldn't be one sufficient ("marvell,armada-380-neta")?

If they are basically compatible, you want to the original one in,
to make sure it keeps running on operating systems that only know
about the older string.

> > Maybe also leave the 370 string in place.
> >
> 
> Now 370 string disables ip checksum for jumbo frames, so I don't think
> it's appropriate to keep it for port 0.

Ok, I see. We should probably have done it the other way round and
kept the default as checksum-disabled and only override it when
the newer compatible string is also present. Basically the device
*is* compatible to an Armada 370, it just has additional features that
work correctly.

If the feature set depends on the port number, we should think about
the way it gets handled again, as this is probably better not described
as something that depends (just) on the SoC, but on the way it gets
integrated. Maybe we can introduce an additional property for the
checksums on jumbo frames and use that if present but fall back to
identifying by compatible string otherwise.

	Arnd

Re: [PATCH 02/13] net: mvneta: enable IP checksum with jumbo frames for Armada 38x on Port0

[Marcin Wojtas]

Arnd,

>
> If the feature set depends on the port number, we should think about
> the way it gets handled again, as this is probably better not described
> as something that depends (just) on the SoC, but on the way it gets
> integrated. Maybe we can introduce an additional property for the
> checksums on jumbo frames and use that if present but fall back to
> identifying by compatible string otherwise.
>

I think adding a property, taking also compatible strings will be the
best solution.

Best regards,
Marcin

[PATCH 03/13] net: mvneta: fix bit assignment in MVNETA_RXQ_CONFIG_REG

[Marcin Wojtas]

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>
Cc: <stable@vger.kernel.org> # v3.8+
---
 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 0f30aaa..d12b8c6 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

Re: [PATCH 03/13] net: mvneta: fix bit assignment in MVNETA_RXQ_CONFIG_REG

[Gregory CLEMENT]

Hi Marcin,
 
 On dim., nov. 22 2015, Marcin Wojtas <mw@semihalf.com> wrote:

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

I confirm it from the datasheet I got:

Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

Thanks,

Gregory


>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> Cc: <stable@vger.kernel.org> # v3.8+
> ---
>  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 0f30aaa..d12b8c6 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
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com