[PATCH 0/4] Add support for the ethernet switch on the EXPRESSObin

[PATCH 0/4] Add support for the ethernet switch on the EXPRESSObin

[Romain Perier]

This set of patches adds support for the Marvell ethernet switch 88E6341.
It also add the devicetree definition of thid switch to the DT board.

Romain Perier (4):
  net: dsa: mv88e6xxx: Allow mv88e6xxx_smi_init() to be used at address
    0x1
  net: dsa: mv88e6xxx: Don't forbid MDIO I/Os for PHY addr >=
    num_of_ports
  net: dsa: mv88e6xxx: Add support for ethernet switch 88E6341/88E6141
  arm64: dts: marvell: Add ethernet switch definition for the
    EXPRESSObin

 .../boot/dts/marvell/armada-3720-espressobin.dts   | 67 ++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/chip.c                   | 24 ++++----
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h              |  4 +-
 3 files changed, 84 insertions(+), 11 deletions(-)

-- 
2.9.3

[PATCH 1/4] net: dsa: mv88e6xxx: Allow mv88e6xxx_smi_init() to be used at address 0x1

[Romain Perier]

Currently, the function mv88e6xxx_smi_init() returns -EINVAL if the bit
zero of sw_addr is 0x1. However, on some platforms, ethernet switches
are configured in Multi chip addressing mode and available at MDIO
address 0x1.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index f7222dc..b5f0e1e 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4240,10 +4240,6 @@ static void mv88e6xxx_phy_destroy(struct mv88e6xxx_chip *chip)
 static int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip,
 			      struct mii_bus *bus, int sw_addr)
 {
-	/* ADDR[0] pin is unavailable externally and considered zero */
-	if (sw_addr & 0x1)
-		return -EINVAL;
-
 	if (sw_addr == 0)
 		chip->smi_ops = &mv88e6xxx_smi_single_chip_ops;
 	else if (mv88e6xxx_has(chip, MV88E6XXX_FLAGS_MULTI_CHIP))
-- 
2.9.3

Re: [PATCH 1/4] net: dsa: mv88e6xxx: Allow mv88e6xxx_smi_init() to be used at address 0x1

[Andrew Lunn]

On Mon, Dec 19, 2016 at 03:16:06PM +0100, Romain Perier wrote:
> Currently, the function mv88e6xxx_smi_init() returns -EINVAL if the bit
> zero of sw_addr is 0x1. However, on some platforms, ethernet switches
> are configured in Multi chip addressing mode and available at MDIO
> address 0x1.

Hi Romain

What branch is this against? net-next?

Please see:

https://www.spinics.net/lists/netdev/msg409156.html

It would be nicer to use Volodymyr, since it has been reviewed. 

Volodymyr, what happened to your version 2? Did David accept it?

	   Andrew

Re: [PATCH 1/4] net: dsa: mv88e6xxx: Allow mv88e6xxx_smi_init() to be used at address 0x1

[Romain Perier]

Hi Andrew,

Le 19/12/2016 à 15:38, Andrew Lunn a écrit :
> On Mon, Dec 19, 2016 at 03:16:06PM +0100, Romain Perier wrote:
>> Currently, the function mv88e6xxx_smi_init() returns -EINVAL if the bit
>> zero of sw_addr is 0x1. However, on some platforms, ethernet switches
>> are configured in Multi chip addressing mode and available at MDIO
>> address 0x1.
>
> Hi Romain
>
> What branch is this against? net-next?

Until last friday it was based onto next-20161216, I rebased onto the 
torvalds's tree this morning (so ~4.10-pre-rc1).

>
> Please see:
>
> https://www.spinics.net/lists/netdev/msg409156.html

Oh, it's already fixed, good. I did not see this patch :)

>
> It would be nicer to use Volodymyr, since it has been reviewed.


As the fix is already there, I will use it, sure.

Thanks,
Romain

Re: [PATCH 1/4] net: dsa: mv88e6xxx: Allow mv88e6xxx_smi_init() to be used at address 0x1

[Andrew Lunn]

On Mon, Dec 19, 2016 at 03:56:34PM +0100, Romain Perier wrote:
> Hi Andrew,
> 
> Le 19/12/2016 à 15:38, Andrew Lunn a écrit :
> >On Mon, Dec 19, 2016 at 03:16:06PM +0100, Romain Perier wrote:
> >>Currently, the function mv88e6xxx_smi_init() returns -EINVAL if the bit
> >>zero of sw_addr is 0x1. However, on some platforms, ethernet switches
> >>are configured in Multi chip addressing mode and available at MDIO
> >>address 0x1.
> >
> >Hi Romain
> >
> >What branch is this against? net-next?
> 
> Until last friday it was based onto next-20161216, I rebased onto
> the torvalds's tree this morning (so ~4.10-pre-rc1).

This patchset is 80% networking. So please see:

Documentation/networking/netdev-FAQ.txt

> Oh, it's already fixed, good. I did not see this patch :)
> 
> >
> >It would be nicer to use Volodymyr, since it has been reviewed.
> 
> 
> As the fix is already there, I will use it, sure.

Im not sure what happened to it. It might of fallen through the
cracks. Lets see what Volodymyr says. It might need resubmitting once
netdev reopens.

       Andrew

Re: [PATCH 1/4] net: dsa: mv88e6xxx: Allow mv88e6xxx_smi_init() to be used at address 0x1

[Andrew Lunn]

On Mon, Dec 19, 2016 at 04:04:32PM +0100, Volodymyr Bendiuga wrote:
> Hi Andrew,
> 
> No, it did not get accepted. Or at least I did not see
> David accepting it. Let me know if I should resubmit it.

Hi Volodymyr

Please do resend it. Probably netdev will reopen sometime after the
25th.

Don't forget to include the reviewed-by i gave.

Thanks

	Andrew

[PATCH 2/4] net: dsa: mv88e6xxx: Don't forbid MDIO I/Os for PHY addr >= num_of_ports

[Romain Perier]

Some Marvell ethernet switches have internal ethernet transceivers with
hardcoded phy addresses. These addresses can be grearer than the number
of ports or its value might be different than the associated port number.
This is for example the case for MV88E6341 that has 6 ports and internal
Port 1 to Port4 PHYs mapped at SMI addresses from 0x11 to 0x14.

This commits fixes the issue by removing the condition in MDIO callbacks.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b5f0e1e..76d944e 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2881,9 +2881,6 @@ static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
 	u16 val;
 	int err;
 
-	if (phy >= mv88e6xxx_num_ports(chip))
-		return 0xffff;
-
 	mutex_lock(&chip->reg_lock);
 	err = mv88e6xxx_phy_read(chip, phy, reg, &val);
 	mutex_unlock(&chip->reg_lock);
@@ -2896,9 +2893,6 @@ static int mv88e6xxx_mdio_write(struct mii_bus *bus, int phy, int reg, u16 val)
 	struct mv88e6xxx_chip *chip = bus->priv;
 	int err;
 
-	if (phy >= mv88e6xxx_num_ports(chip))
-		return 0xffff;
-
 	mutex_lock(&chip->reg_lock);
 	err = mv88e6xxx_phy_write(chip, phy, reg, val);
 	mutex_unlock(&chip->reg_lock);
-- 
2.9.3

Re: [PATCH 2/4] net: dsa: mv88e6xxx: Don't forbid MDIO I/Os for PHY addr >= num_of_ports

[Andrew Lunn]

On Mon, Dec 19, 2016 at 03:16:07PM +0100, Romain Perier wrote:
> Some Marvell ethernet switches have internal ethernet transceivers with
> hardcoded phy addresses. These addresses can be grearer than the number
> of ports or its value might be different than the associated port number.
> This is for example the case for MV88E6341 that has 6 ports and internal
> Port 1 to Port4 PHYs mapped at SMI addresses from 0x11 to 0x14.
> 
> This commits fixes the issue by removing the condition in MDIO callbacks.
> 
> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>

So it is not quite compatible with the 6352. Thanks Marvell :-(

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH 3/4] net: dsa: mv88e6xxx: Add support for ethernet switch 88E6341/88E6141

[Romain Perier]

The Marvell 88E6341 device is single-chip, 6-port ethernet switch with
four integrated 10/100/1000Mbps ethernet transceivers and one high speed
SerDes interfaces. It is compatible with switches of family 88E6352.

This commit adds basic support for this switch by describing its
capabilities to the driver.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 14 ++++++++++++++
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |  4 +++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 76d944e..72ba24b 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4086,6 +4086,20 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.ops = &mv88e6321_ops,
 	},
 
+	[MV88E6341] = {
+		.prod_num = PORT_SWITCH_ID_PROD_NUM_6341,
+		.family = MV88E6XXX_FAMILY_6352,
+		.name = "Marvell 88E6341",
+		.num_databases = 4096,
+		.num_ports = 6,
+		.port_base_addr = 0x10,
+		.global1_addr = 0x1b,
+		.age_time_coeff = 15000,
+		.tag_protocol = DSA_TAG_PROTO_EDSA,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
+		.ops = &mv88e6352_ops,
+	},
+
 	[MV88E6350] = {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6350,
 		.family = MV88E6XXX_FAMILY_6351,
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index af54bae..176d6c4 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -86,6 +86,7 @@
 #define PORT_SWITCH_ID_PROD_NUM_6097	0x099
 #define PORT_SWITCH_ID_PROD_NUM_6131	0x106
 #define PORT_SWITCH_ID_PROD_NUM_6320	0x115
+#define PORT_SWITCH_ID_PROD_NUM_6341	0x340
 #define PORT_SWITCH_ID_PROD_NUM_6123	0x121
 #define PORT_SWITCH_ID_PROD_NUM_6161	0x161
 #define PORT_SWITCH_ID_PROD_NUM_6165	0x165
@@ -432,6 +433,7 @@ enum mv88e6xxx_model {
 	MV88E6290,
 	MV88E6320,
 	MV88E6321,
+	MV88E6341,
 	MV88E6350,
 	MV88E6351,
 	MV88E6352,
@@ -448,7 +450,7 @@ enum mv88e6xxx_family {
 	MV88E6XXX_FAMILY_6185,	/* 6108 6121 6122 6131 6152 6155 6182 6185 */
 	MV88E6XXX_FAMILY_6320,	/* 6320 6321 */
 	MV88E6XXX_FAMILY_6351,	/* 6171 6175 6350 6351 */
-	MV88E6XXX_FAMILY_6352,	/* 6172 6176 6240 6352 */
+	MV88E6XXX_FAMILY_6352,	/* 6172 6176 6240 6341 6352 */
 	MV88E6XXX_FAMILY_6390,  /* 6190 6190X 6191 6290 6390 6390X */
 };
 
-- 
2.9.3

Re: [PATCH 3/4] net: dsa: mv88e6xxx: Add support for ethernet switch 88E6341/88E6141

[Andrew Lunn]

Hi Romain

> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 76d944e..72ba24b 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -4086,6 +4086,20 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.ops = &mv88e6321_ops,
>  	},
>  
> +	[MV88E6341] = {
> +		.prod_num = PORT_SWITCH_ID_PROD_NUM_6341,
> +		.family = MV88E6XXX_FAMILY_6352,
> +		.name = "Marvell 88E6341",
> +		.num_databases = 4096,
> +		.num_ports = 6,
> +		.port_base_addr = 0x10,
> +		.global1_addr = 0x1b,
> +		.age_time_coeff = 15000,
> +		.tag_protocol = DSA_TAG_PROTO_EDSA,
> +		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
> +		.ops = &mv88e6352_ops,

Even if it i 100% compatible with the 6532, you should still add an
ops structure for it. All chips have their own, even when the are
exactly the same as other in the family.

> --- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
> +++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
> @@ -86,6 +86,7 @@
>  #define PORT_SWITCH_ID_PROD_NUM_6097	0x099
>  #define PORT_SWITCH_ID_PROD_NUM_6131	0x106
>  #define PORT_SWITCH_ID_PROD_NUM_6320	0x115
> +#define PORT_SWITCH_ID_PROD_NUM_6341	0x340
>  #define PORT_SWITCH_ID_PROD_NUM_6123	0x121
>  #define PORT_SWITCH_ID_PROD_NUM_6161	0x161

Ah, err..

These should be in numerical order of the macro.
PORT_SWITCH_ID_PROD_NUM_6320 is however in the wrong place.  Please
put the 6341 after the 6321.

Thanks
	Andrew

Re: [PATCH 3/4] net: dsa: mv88e6xxx: Add support for ethernet switch 88E6341/88E6141

[Romain Perier]

Hi Andrew,

Le 19/12/2016 à 15:52, Andrew Lunn a écrit :
> Hi Romain
>
>> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
>> index 76d944e..72ba24b 100644
>> --- a/drivers/net/dsa/mv88e6xxx/chip.c
>> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
>> @@ -4086,6 +4086,20 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>>  		.ops = &mv88e6321_ops,
>>  	},
>>
>> +	[MV88E6341] = {
>> +		.prod_num = PORT_SWITCH_ID_PROD_NUM_6341,
>> +		.family = MV88E6XXX_FAMILY_6352,
>> +		.name = "Marvell 88E6341",
>> +		.num_databases = 4096,
>> +		.num_ports = 6,
>> +		.port_base_addr = 0x10,
>> +		.global1_addr = 0x1b,
>> +		.age_time_coeff = 15000,
>> +		.tag_protocol = DSA_TAG_PROTO_EDSA,
>> +		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
>> +		.ops = &mv88e6352_ops,
>
> Even if it i 100% compatible with the 6532, you should still add an
> ops structure for it. All chips have their own, even when the are
> exactly the same as other in the family.

Ack, I will fix this.

>
>> --- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
>> +++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
>> @@ -86,6 +86,7 @@
>>  #define PORT_SWITCH_ID_PROD_NUM_6097	0x099
>>  #define PORT_SWITCH_ID_PROD_NUM_6131	0x106
>>  #define PORT_SWITCH_ID_PROD_NUM_6320	0x115
>> +#define PORT_SWITCH_ID_PROD_NUM_6341	0x340
>>  #define PORT_SWITCH_ID_PROD_NUM_6123	0x121
>>  #define PORT_SWITCH_ID_PROD_NUM_6161	0x161
>
> Ah, err..
>
> These should be in numerical order of the macro.
> PORT_SWITCH_ID_PROD_NUM_6320 is however in the wrong place.  Please
> put the 6341 after the 6321.

good catch, ok.

Thanks,
Romain

[PATCH 4/4] arm64: dts: marvell: Add ethernet switch definition for the EXPRESSObin

[Romain Perier]

This defines and enables the Marvell ethernet switch MVE886341 on the
Marvell EXPRESSObin board.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 .../boot/dts/marvell/armada-3720-espressobin.dts   | 67 ++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
index 83178d9..8ba6c0b 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
@@ -80,3 +80,70 @@
 &usb3 {
 	status = "okay";
 };
+
+&mdio {
+	switch0: switch0@0 {
+		compatible = "marvell,mv88e6085";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <1>;
+
+		dsa,member = <0 0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+				label = "cpu";
+				ethernet = <&eth0>;
+			};
+
+			port@1 {
+				reg = <1>;
+				label = "wan";
+				phy-handle = <&switch0phy0>;
+			};
+
+			port@2 {
+				reg = <2>;
+				label = "lan0";
+				phy-handle = <&switch0phy1>;
+			};
+
+			port@3 {
+				reg = <3>;
+				label = "lan1";
+				phy-handle = <&switch0phy2>;
+			};
+
+		};
+
+		mdio {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <1>;
+
+			switch0phy0: switch0phy0@0 {
+				reg = <0x11>;
+			};
+			switch0phy1: switch0phy1@1 {
+				reg = <0x12>;
+			};
+			switch0phy2: switch0phy2@2 {
+				reg = <0x13>;
+			};
+		};
+	};
+};
+
+&eth0 {
+	phy-mode = "rgmii-id";
+	status = "okay";
+
+	fixed-link {
+		speed = <1000>;
+		full-duplex;
+	};
+};
-- 
2.9.3

Re: [PATCH 4/4] arm64: dts: marvell: Add ethernet switch definition for the EXPRESSObin

[Andrew Lunn]

> +		mdio {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <1>;
> +
> +			switch0phy0: switch0phy0@0 {
> +				reg = <0x11>;

Since the reg is 0x11, this should be called switch0phy0@11.  Please
follow the same scheme for the other phys.

       Andrew

Re: [PATCH 4/4] arm64: dts: marvell: Add ethernet switch definition for the EXPRESSObin

[Thomas Petazzoni]

Hello,

On Mon, 19 Dec 2016 15:16:09 +0100, Romain Perier wrote:
> This defines and enables the Marvell ethernet switch MVE886341 on the
> Marvell EXPRESSObin board.
> 
> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>

I didn't want to make this (silly) comment but since you got another
comment that will let you send a new iteration, here is my silly
comment: the name of the board is EspressoBin, not ExpressoBin.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 4/4] arm64: dts: marvell: Add ethernet switch definition for the EXPRESSObin

[Romain Perier]

Hi,

Le 19/12/2016 à 15:58, Thomas Petazzoni a écrit :
> Hello,
>
> On Mon, 19 Dec 2016 15:16:09 +0100, Romain Perier wrote:
>> This defines and enables the Marvell ethernet switch MVE886341 on the
>> Marvell EXPRESSObin board.
>>
>> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
>
> I didn't want to make this (silly) comment but since you got another
> comment that will let you send a new iteration, here is my silly
> comment: the name of the board is EspressoBin, not ExpressoBin.
>
> Thanks!
>
> Thomas
>

Ack for the two feedback (I don't know why I have replaced "es" by "ex" 
:/, anyway...)

Thanks,
Romain