[RFC PATCH 0/5] net: dsa: POC support for mv88e6250

[RFC PATCH 0/5] net: dsa: POC support for mv88e6250

[Rasmus Villemoes]

This is some mostly-working code adding support for the mv88e6250
chip. I'm not quite done checking whether all the methods in the _ops
structure are appropriate, but basic switchdev functionality seems to
work.

Rasmus Villemoes (5):
  net: dsa: mv88e6xxx: introduce support for two chips using direct smi
    addressing
  net: dsa: mv88e6xxx: rename smi read/write functions
  net: dsa: prepare mv88e6xxx_g1_atu_op() for the mv88e6250
  net: dsa: implement vtu_getnext and vtu_loadpurge for mv88e6250
  net: dsa: add support for mv88e6250

 drivers/net/dsa/mv88e6xxx/chip.c        | 125 +++++++++++++++++++-----
 drivers/net/dsa/mv88e6xxx/chip.h        |   7 ++
 drivers/net/dsa/mv88e6xxx/global1.c     |  20 ++++
 drivers/net/dsa/mv88e6xxx/global1.h     |   5 +
 drivers/net/dsa/mv88e6xxx/global1_atu.c |   5 +-
 drivers/net/dsa/mv88e6xxx/global1_vtu.c |  58 +++++++++++
 drivers/net/dsa/mv88e6xxx/port.h        |   1 +
 7 files changed, 196 insertions(+), 25 deletions(-)

-- 
2.20.1

[RFC PATCH 1/5] net: dsa: mv88e6xxx: introduce support for two chips using direct smi addressing

[Rasmus Villemoes]

The 88e6250 (as well as 6220, 6071, 6070, 6020) do not support
multi-chip (indirect) addressing. However, one can still have two of
them on the same mdio bus, since the device only uses 16 of the 32
possible addresses, either addresses 0x00-0x0F or 0x10-0x1F depending
on the ADDR4 pin at reset [since ADDR4 is internally pulled high, the
latter is the default].

In order to prepare for supporting the 88e6250 and friends, introduce
mv88e6xxx_info::dual_chip to allow having a non-zero sw_addr while
still using direct addressing.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 10 +++++++---
 drivers/net/dsa/mv88e6xxx/chip.h |  5 +++++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index c078c791f481..f66daa77774b 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -62,6 +62,10 @@ static void assert_reg_lock(struct mv88e6xxx_chip *chip)
  * When ADDR is non-zero, the chip uses Multi-chip Addressing Mode, allowing
  * multiple devices to share the SMI interface. In this mode it responds to only
  * 2 registers, used to indirectly access the internal SMI devices.
+ *
+ * Some chips use a different scheme: Only the ADDR4 pin is used for
+ * configuration, and the device responds to 16 of the 32 SMI
+ * addresses, allowing two to coexist on the same SMI interface.
  */
 
 static int mv88e6xxx_smi_read(struct mv88e6xxx_chip *chip,
@@ -87,7 +91,7 @@ static int mv88e6xxx_smi_single_chip_read(struct mv88e6xxx_chip *chip,
 {
 	int ret;
 
-	ret = mdiobus_read_nested(chip->bus, addr, reg);
+	ret = mdiobus_read_nested(chip->bus, addr + chip->sw_addr, reg);
 	if (ret < 0)
 		return ret;
 
@@ -101,7 +105,7 @@ static int mv88e6xxx_smi_single_chip_write(struct mv88e6xxx_chip *chip,
 {
 	int ret;
 
-	ret = mdiobus_write_nested(chip->bus, addr, reg, val);
+	ret = mdiobus_write_nested(chip->bus, addr + chip->sw_addr, reg, val);
 	if (ret < 0)
 		return ret;
 
@@ -4548,7 +4552,7 @@ static struct mv88e6xxx_chip *mv88e6xxx_alloc_chip(struct device *dev)
 static int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip,
 			      struct mii_bus *bus, int sw_addr)
 {
-	if (sw_addr == 0)
+	if (sw_addr == 0 || chip->info->dual_chip)
 		chip->smi_ops = &mv88e6xxx_smi_single_chip_ops;
 	else if (chip->info->multi_chip)
 		chip->smi_ops = &mv88e6xxx_smi_multi_chip_ops;
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 546651d8c3e1..5759fffbd39c 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -123,6 +123,11 @@ struct mv88e6xxx_info {
 	 * when it is non-zero, and use indirect access to internal registers.
 	 */
 	bool multi_chip;
+	/* Dual-chip Addressing Mode
+	 * Some chips respond to only half of the 32 SMI addresses,
+	 * allowing two to coexist on the same SMI interface.
+	 */
+	bool dual_chip;
 	enum dsa_tag_protocol tag_protocol;
 
 	/* Mask for FromPort and ToPort value of PortVec used in ATU Move
-- 
2.20.1

Re: [RFC PATCH 1/5] net: dsa: mv88e6xxx: introduce support for two chips using direct smi addressing

[Andrew Lunn]

On Wed, May 01, 2019 at 07:32:10PM +0000, Rasmus Villemoes wrote:
> The 88e6250 (as well as 6220, 6071, 6070, 6020) do not support
> multi-chip (indirect) addressing. However, one can still have two of
> them on the same mdio bus, since the device only uses 16 of the 32
> possible addresses, either addresses 0x00-0x0F or 0x10-0x1F depending
> on the ADDR4 pin at reset [since ADDR4 is internally pulled high, the
> latter is the default].
> 
> In order to prepare for supporting the 88e6250 and friends, introduce
> mv88e6xxx_info::dual_chip to allow having a non-zero sw_addr while
> still using direct addressing.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 10 +++++++---
>  drivers/net/dsa/mv88e6xxx/chip.h |  5 +++++
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index c078c791f481..f66daa77774b 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -62,6 +62,10 @@ static void assert_reg_lock(struct mv88e6xxx_chip *chip)
>   * When ADDR is non-zero, the chip uses Multi-chip Addressing Mode, allowing
>   * multiple devices to share the SMI interface. In this mode it responds to only
>   * 2 registers, used to indirectly access the internal SMI devices.
> + *
> + * Some chips use a different scheme: Only the ADDR4 pin is used for
> + * configuration, and the device responds to 16 of the 32 SMI
> + * addresses, allowing two to coexist on the same SMI interface.
>   */
>  
>  static int mv88e6xxx_smi_read(struct mv88e6xxx_chip *chip,
> @@ -87,7 +91,7 @@ static int mv88e6xxx_smi_single_chip_read(struct mv88e6xxx_chip *chip,
>  {
>  	int ret;
>  
> -	ret = mdiobus_read_nested(chip->bus, addr, reg);
> +	ret = mdiobus_read_nested(chip->bus, addr + chip->sw_addr, reg);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -101,7 +105,7 @@ static int mv88e6xxx_smi_single_chip_write(struct mv88e6xxx_chip *chip,
>  {
>  	int ret;
>  
> -	ret = mdiobus_write_nested(chip->bus, addr, reg, val);
> +	ret = mdiobus_write_nested(chip->bus, addr + chip->sw_addr, reg, val);
>  	if (ret < 0)
>  		return ret;

Hi Rasmus

This works, but i think i prefer adding mv88e6xxx_smi_dual_chip_write,
mv88e6xxx_smi_dual_chip_read, and create a
mv88e6xxx_smi_single_chip_ops.

>  
> @@ -4548,7 +4552,7 @@ static struct mv88e6xxx_chip *mv88e6xxx_alloc_chip(struct device *dev)
>  static int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip,
>  			      struct mii_bus *bus, int sw_addr)
>  {
> -	if (sw_addr == 0)
> +	if (sw_addr == 0 || chip->info->dual_chip)
>  		chip->smi_ops = &mv88e6xxx_smi_single_chip_ops;
>  	else if (chip->info->multi_chip)
>  		chip->smi_ops = &mv88e6xxx_smi_multi_chip_ops;

And then select the dual chip ops here. That seems be to more in
keeping with the current code.

Thanks
	Andrew

Re: [RFC PATCH 1/5] net: dsa: mv88e6xxx: introduce support for two chips using direct smi addressing

[Rasmus Villemoes]

On 01/05/2019 22.19, Andrew Lunn wrote:
> On Wed, May 01, 2019 at 07:32:10PM +0000, Rasmus Villemoes wrote:
>> The 88e6250 (as well as 6220, 6071, 6070, 6020) do not support
>> multi-chip (indirect) addressing. However, one can still have two of
>> them on the same mdio bus, since the device only uses 16 of the 32
>> possible addresses, either addresses 0x00-0x0F or 0x10-0x1F depending
>> on the ADDR4 pin at reset [since ADDR4 is internally pulled high, the
>> latter is the default].
>>
>> In order to prepare for supporting the 88e6250 and friends, introduce
>> mv88e6xxx_info::dual_chip to allow having a non-zero sw_addr while
>> still using direct addressing.
>>
>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>> ---
>>  drivers/net/dsa/mv88e6xxx/chip.c | 10 +++++++---
>>  drivers/net/dsa/mv88e6xxx/chip.h |  5 +++++
>>  2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
>> index c078c791f481..f66daa77774b 100644
>> --- a/drivers/net/dsa/mv88e6xxx/chip.c
>> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
>> @@ -62,6 +62,10 @@ static void assert_reg_lock(struct mv88e6xxx_chip *chip)
>>   * When ADDR is non-zero, the chip uses Multi-chip Addressing Mode, allowing
>>   * multiple devices to share the SMI interface. In this mode it responds to only
>>   * 2 registers, used to indirectly access the internal SMI devices.
>> + *
>> + * Some chips use a different scheme: Only the ADDR4 pin is used for
>> + * configuration, and the device responds to 16 of the 32 SMI
>> + * addresses, allowing two to coexist on the same SMI interface.
>>   */
>>  
>>  static int mv88e6xxx_smi_read(struct mv88e6xxx_chip *chip,
>> @@ -87,7 +91,7 @@ static int mv88e6xxx_smi_single_chip_read(struct mv88e6xxx_chip *chip,
>>  {
>>  	int ret;
>>  
>> -	ret = mdiobus_read_nested(chip->bus, addr, reg);
>> +	ret = mdiobus_read_nested(chip->bus, addr + chip->sw_addr, reg);
>>  	if (ret < 0)
>>  		return ret;
>>  
>> @@ -101,7 +105,7 @@ static int mv88e6xxx_smi_single_chip_write(struct mv88e6xxx_chip *chip,
>>  {
>>  	int ret;
>>  
>> -	ret = mdiobus_write_nested(chip->bus, addr, reg, val);
>> +	ret = mdiobus_write_nested(chip->bus, addr + chip->sw_addr, reg, val);
>>  	if (ret < 0)
>>  		return ret;
> 
> Hi Rasmus
> 
> This works, but i think i prefer adding mv88e6xxx_smi_dual_chip_write,
> mv88e6xxx_smi_dual_chip_read, and create a
> mv88e6xxx_smi_single_chip_ops.

Hi Andrew

Now that Vivien's "net: dsa: mv88e6xxx: refine SMI support" is in
master, do you still prefer introducing a third bus_ops structure
(mv88e6xxx_smi_dual_direct_ops ?), or would the approach of adding
chip->sw_addr in the smi_direct_{read/write} functions be ok (which
would then require changing the indirect callers to pass 0 instead of
chip->swaddr).

Thanks,
Rasmus

Re: [RFC PATCH 1/5] net: dsa: mv88e6xxx: introduce support for two chips using direct smi addressing

[Andrew Lunn]

> > Hi Rasmus
> > 
> > This works, but i think i prefer adding mv88e6xxx_smi_dual_chip_write,
> > mv88e6xxx_smi_dual_chip_read, and create a
> > mv88e6xxx_smi_single_chip_ops.
> 
> Hi Andrew
> 
> Now that Vivien's "net: dsa: mv88e6xxx: refine SMI support" is in
> master, do you still prefer introducing a third bus_ops structure
> (mv88e6xxx_smi_dual_direct_ops ?), or would the approach of adding
> chip->sw_addr in the smi_direct_{read/write} functions be ok (which
> would then require changing the indirect callers to pass 0 instead of
> chip->swaddr).

Hi Rasmus

I would still prefer a new bus_ops.

Thanks
	Andrew

Re: [RFC PATCH 1/5] net: dsa: mv88e6xxx: introduce support for two chips using direct smi addressing

[Vivien Didelot]

Hi Rasmus,

On Wed, 8 May 2019 13:47:15 +0200, Andrew Lunn <andrew@lunn.ch> wrote:
> > > 
> > > This works, but i think i prefer adding mv88e6xxx_smi_dual_chip_write,
> > > mv88e6xxx_smi_dual_chip_read, and create a
> > > mv88e6xxx_smi_single_chip_ops.
> > 
> > Now that Vivien's "net: dsa: mv88e6xxx: refine SMI support" is in
> > master, do you still prefer introducing a third bus_ops structure
> > (mv88e6xxx_smi_dual_direct_ops ?), or would the approach of adding
> > chip->sw_addr in the smi_direct_{read/write} functions be ok (which
> > would then require changing the indirect callers to pass 0 instead of
> > chip->swaddr).
> 
> I would still prefer a new bus_ops.

Even though those are direct read and write operations, having 3
mv88e6xxx_bus_ops structures will make it clear that there are 3 ways
for accessible the internal switch registers through SMI, depending
on the Marvell chip model.  So I would prefer a third ops as well.

Thanks,
Vivien

[RFC PATCH 2/5] net: dsa: mv88e6xxx: rename smi read/write functions

[Rasmus Villemoes]

With the previous patch adding support for two chips using direct SMI
addressing, the smi_single_chip_{read,write} functions are slightly
misnamed. Changing to smi_dual_chip_{read,write} would not be accurate
either.

Change the names to reflect how the access to the SMI registers is
done (direct/indirect) rather than the number of chips that can be
connected to the same SMI master. No functional change.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 42 ++++++++++++++++----------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index f66daa77774b..d8d8230a6bf5 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -86,8 +86,8 @@ static int mv88e6xxx_smi_write(struct mv88e6xxx_chip *chip,
 	return chip->smi_ops->write(chip, addr, reg, val);
 }
 
-static int mv88e6xxx_smi_single_chip_read(struct mv88e6xxx_chip *chip,
-					  int addr, int reg, u16 *val)
+static int mv88e6xxx_smi_direct_read(struct mv88e6xxx_chip *chip,
+				     int addr, int reg, u16 *val)
 {
 	int ret;
 
@@ -100,8 +100,8 @@ static int mv88e6xxx_smi_single_chip_read(struct mv88e6xxx_chip *chip,
 	return 0;
 }
 
-static int mv88e6xxx_smi_single_chip_write(struct mv88e6xxx_chip *chip,
-					   int addr, int reg, u16 val)
+static int mv88e6xxx_smi_direct_write(struct mv88e6xxx_chip *chip,
+				      int addr, int reg, u16 val)
 {
 	int ret;
 
@@ -112,12 +112,12 @@ static int mv88e6xxx_smi_single_chip_write(struct mv88e6xxx_chip *chip,
 	return 0;
 }
 
-static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_single_chip_ops = {
-	.read = mv88e6xxx_smi_single_chip_read,
-	.write = mv88e6xxx_smi_single_chip_write,
+static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_direct_ops = {
+	.read = mv88e6xxx_smi_direct_read,
+	.write = mv88e6xxx_smi_direct_write,
 };
 
-static int mv88e6xxx_smi_multi_chip_wait(struct mv88e6xxx_chip *chip)
+static int mv88e6xxx_smi_indirect_wait(struct mv88e6xxx_chip *chip)
 {
 	int ret;
 	int i;
@@ -134,13 +134,13 @@ static int mv88e6xxx_smi_multi_chip_wait(struct mv88e6xxx_chip *chip)
 	return -ETIMEDOUT;
 }
 
-static int mv88e6xxx_smi_multi_chip_read(struct mv88e6xxx_chip *chip,
-					 int addr, int reg, u16 *val)
+static int mv88e6xxx_smi_indirect_read(struct mv88e6xxx_chip *chip,
+				       int addr, int reg, u16 *val)
 {
 	int ret;
 
 	/* Wait for the bus to become free. */
-	ret = mv88e6xxx_smi_multi_chip_wait(chip);
+	ret = mv88e6xxx_smi_indirect_wait(chip);
 	if (ret < 0)
 		return ret;
 
@@ -151,7 +151,7 @@ static int mv88e6xxx_smi_multi_chip_read(struct mv88e6xxx_chip *chip,
 		return ret;
 
 	/* Wait for the read command to complete. */
-	ret = mv88e6xxx_smi_multi_chip_wait(chip);
+	ret = mv88e6xxx_smi_indirect_wait(chip);
 	if (ret < 0)
 		return ret;
 
@@ -165,13 +165,13 @@ static int mv88e6xxx_smi_multi_chip_read(struct mv88e6xxx_chip *chip,
 	return 0;
 }
 
-static int mv88e6xxx_smi_multi_chip_write(struct mv88e6xxx_chip *chip,
-					  int addr, int reg, u16 val)
+static int mv88e6xxx_smi_indirect_write(struct mv88e6xxx_chip *chip,
+					int addr, int reg, u16 val)
 {
 	int ret;
 
 	/* Wait for the bus to become free. */
-	ret = mv88e6xxx_smi_multi_chip_wait(chip);
+	ret = mv88e6xxx_smi_indirect_wait(chip);
 	if (ret < 0)
 		return ret;
 
@@ -187,16 +187,16 @@ static int mv88e6xxx_smi_multi_chip_write(struct mv88e6xxx_chip *chip,
 		return ret;
 
 	/* Wait for the write command to complete. */
-	ret = mv88e6xxx_smi_multi_chip_wait(chip);
+	ret = mv88e6xxx_smi_indirect_wait(chip);
 	if (ret < 0)
 		return ret;
 
 	return 0;
 }
 
-static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_multi_chip_ops = {
-	.read = mv88e6xxx_smi_multi_chip_read,
-	.write = mv88e6xxx_smi_multi_chip_write,
+static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_indirect_ops = {
+	.read = mv88e6xxx_smi_indirect_read,
+	.write = mv88e6xxx_smi_indirect_write,
 };
 
 int mv88e6xxx_read(struct mv88e6xxx_chip *chip, int addr, int reg, u16 *val)
@@ -4553,9 +4553,9 @@ static int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip,
 			      struct mii_bus *bus, int sw_addr)
 {
 	if (sw_addr == 0 || chip->info->dual_chip)
-		chip->smi_ops = &mv88e6xxx_smi_single_chip_ops;
+		chip->smi_ops = &mv88e6xxx_smi_direct_ops;
 	else if (chip->info->multi_chip)
-		chip->smi_ops = &mv88e6xxx_smi_multi_chip_ops;
+		chip->smi_ops = &mv88e6xxx_smi_indirect_ops;
 	else
 		return -EINVAL;
 
-- 
2.20.1

Re: [RFC PATCH 2/5] net: dsa: mv88e6xxx: rename smi read/write functions

[Vivien Didelot]

Hi Rasmus,

On Wed, 1 May 2019 19:32:11 +0000, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:

> -static int mv88e6xxx_smi_single_chip_read(struct mv88e6xxx_chip *chip,
> -					  int addr, int reg, u16 *val)
> +static int mv88e6xxx_smi_direct_read(struct mv88e6xxx_chip *chip,
> +				     int addr, int reg, u16 *val)

I have a preparatory patch which does almost exactly that. I'm sending it
to simplify this patchset.

Also please use my Gmail address as described by get_maintainer.pl please.

Thank you,
Vivien

Re: [RFC PATCH 2/5] net: dsa: mv88e6xxx: rename smi read/write functions

[Rasmus Villemoes]

On 03/05/2019 23.57, Vivien Didelot wrote:
> Hi Rasmus,
> 
> On Wed, 1 May 2019 19:32:11 +0000, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:
> 
>> -static int mv88e6xxx_smi_single_chip_read(struct mv88e6xxx_chip *chip,
>> -					  int addr, int reg, u16 *val)
>> +static int mv88e6xxx_smi_direct_read(struct mv88e6xxx_chip *chip,
>> +				     int addr, int reg, u16 *val)
> 
> I have a preparatory patch which does almost exactly that. I'm sending it
> to simplify this patchset.

OK, I'll hold off sending a v2 until I see how 1/5 and 2/5 are obsoleted
by your patch(es).

> Also please use my Gmail address as described by get_maintainer.pl please.

Sorry, I must have been running 'git send-email' (which invokes
get_maintainer via a --to-cmd script) from the 4.19 tree I'm currently
targeting. Will try to remember.

Thanks,
Rasmus

Re: [RFC PATCH 2/5] net: dsa: mv88e6xxx: rename smi read/write functions

[Vivien Didelot]

Hi Rasmus,

On Mon, 6 May 2019 05:57:11 +0000, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:

> > I have a preparatory patch which does almost exactly that. I'm sending it
> > to simplify this patchset.
> 
> OK, I'll hold off sending a v2 until I see how 1/5 and 2/5 are obsoleted
> by your patch(es).

You may rebase your patches now and add your new implementation of
register access through SMI in the smi.c file if that is necessary.


Thanks,

	Vivien

[RFC PATCH 3/5] net: dsa: prepare mv88e6xxx_g1_atu_op() for the mv88e6250

[Rasmus Villemoes]

All the currently supported chips have .num_databases either 256 or
4096, so this patch does not change behaviour for any of those. The
mv88e6250, however, has .num_databases == 64, and it does not put the
upper two bits in ATU control 13:12, but rather in ATU Operation
9:8. So change the logic to prepare for supporting mv88e6250.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/net/dsa/mv88e6xxx/global1_atu.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
index ea243840ee0f..1ae680bc0eff 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
@@ -94,7 +94,7 @@ static int mv88e6xxx_g1_atu_op(struct mv88e6xxx_chip *chip, u16 fid, u16 op)
 		if (err)
 			return err;
 	} else {
-		if (mv88e6xxx_num_databases(chip) > 16) {
+		if (mv88e6xxx_num_databases(chip) > 64) {
 			/* ATU DBNum[7:4] are located in ATU Control 15:12 */
 			err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_CTL,
 						&val);
@@ -106,6 +106,9 @@ static int mv88e6xxx_g1_atu_op(struct mv88e6xxx_chip *chip, u16 fid, u16 op)
 						 val);
 			if (err)
 				return err;
+		} else if (mv88e6xxx_num_databases(chip) > 16) {
+			/* ATU DBNum[5:4] are located in ATU Operation 9:8 */
+			op |= (fid & 0x30) << 4;
 		}
 
 		/* ATU DBNum[3:0] are located in ATU Operation 3:0 */
-- 
2.20.1

Re: [RFC PATCH 3/5] net: dsa: prepare mv88e6xxx_g1_atu_op() for the mv88e6250

[Andrew Lunn]

On Wed, May 01, 2019 at 07:32:12PM +0000, Rasmus Villemoes wrote:
> All the currently supported chips have .num_databases either 256 or
> 4096, so this patch does not change behaviour for any of those. The
> mv88e6250, however, has .num_databases == 64, and it does not put the
> upper two bits in ATU control 13:12, but rather in ATU Operation
> 9:8. So change the logic to prepare for supporting mv88e6250.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

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

    Andrew

[RFC PATCH 4/5] net: dsa: implement vtu_getnext and vtu_loadpurge for mv88e6250

[Rasmus Villemoes]

These are almost identical to the 6185 variants, but have fewer bits
for the FID.

Bit 10 of the VTU_OP register (offset 0x05) is the VidPolicy bit,
which one should probably preserve in mv88e6xxx_g1_vtu_op(), instead
of always writing a 0. However, on the 6352 family, that bit is
located at bit 12 in the VTU FID register (offset 0x02), and is always
unconditionally cleared by the mv88e6xxx_g1_vtu_fid_write()
function.

Since nothing in the existing driver seems to know or care about that
bit, it seems reasonable to not add the boilerplate to preserve it for
the 6250 (which would require adding a chip-specific vtu_op function,
or adding chip-quirks to the existing one).

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/net/dsa/mv88e6xxx/global1.h     |  4 ++
 drivers/net/dsa/mv88e6xxx/global1_vtu.c | 58 +++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/global1.h b/drivers/net/dsa/mv88e6xxx/global1.h
index bef01331266f..b205b0bba158 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.h
+++ b/drivers/net/dsa/mv88e6xxx/global1.h
@@ -305,6 +305,10 @@ int mv88e6185_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 			     struct mv88e6xxx_vtu_entry *entry);
 int mv88e6185_g1_vtu_loadpurge(struct mv88e6xxx_chip *chip,
 			       struct mv88e6xxx_vtu_entry *entry);
+int mv88e6250_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
+			     struct mv88e6xxx_vtu_entry *entry);
+int mv88e6250_g1_vtu_loadpurge(struct mv88e6xxx_chip *chip,
+			       struct mv88e6xxx_vtu_entry *entry);
 int mv88e6352_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 			     struct mv88e6xxx_vtu_entry *entry);
 int mv88e6352_g1_vtu_loadpurge(struct mv88e6xxx_chip *chip,
diff --git a/drivers/net/dsa/mv88e6xxx/global1_vtu.c b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
index 058326924f3e..a8ef268c32cb 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_vtu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
@@ -307,6 +307,35 @@ static int mv88e6xxx_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 	return mv88e6xxx_g1_vtu_vid_read(chip, entry);
 }
 
+int mv88e6250_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
+			     struct mv88e6xxx_vtu_entry *entry)
+{
+	u16 val;
+	int err;
+
+	err = mv88e6xxx_g1_vtu_getnext(chip, entry);
+	if (err)
+		return err;
+
+	if (entry->valid) {
+		err = mv88e6185_g1_vtu_data_read(chip, entry);
+		if (err)
+			return err;
+
+		/* VTU DBNum[3:0] are located in VTU Operation 3:0
+		 * VTU DBNum[5:4] are located in VTU Operation 9:8
+		 */
+		err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_VTU_OP, &val);
+		if (err)
+			return err;
+
+		entry->fid = val & 0x000f;
+		entry->fid |= (val & 0x0300) >> 4;
+	}
+
+	return 0;
+}
+
 int mv88e6185_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 			     struct mv88e6xxx_vtu_entry *entry)
 {
@@ -396,6 +425,35 @@ int mv88e6390_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 	return 0;
 }
 
+int mv88e6250_g1_vtu_loadpurge(struct mv88e6xxx_chip *chip,
+			       struct mv88e6xxx_vtu_entry *entry)
+{
+	u16 op = MV88E6XXX_G1_VTU_OP_VTU_LOAD_PURGE;
+	int err;
+
+	err = mv88e6xxx_g1_vtu_op_wait(chip);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_g1_vtu_vid_write(chip, entry);
+	if (err)
+		return err;
+
+	if (entry->valid) {
+		err = mv88e6185_g1_vtu_data_write(chip, entry);
+		if (err)
+			return err;
+
+		/* VTU DBNum[3:0] are located in VTU Operation 3:0
+		 * VTU DBNum[5:4] are located in VTU Operation 9:8
+		 */
+		op |= entry->fid & 0x000f;
+		op |= (entry->fid & 0x0030) << 8;
+	}
+
+	return mv88e6xxx_g1_vtu_op(chip, op);
+}
+
 int mv88e6185_g1_vtu_loadpurge(struct mv88e6xxx_chip *chip,
 			       struct mv88e6xxx_vtu_entry *entry)
 {
-- 
2.20.1

Re: [RFC PATCH 4/5] net: dsa: implement vtu_getnext and vtu_loadpurge for mv88e6250

[Andrew Lunn]

On Wed, May 01, 2019 at 07:32:13PM +0000, Rasmus Villemoes wrote:
> These are almost identical to the 6185 variants, but have fewer bits
> for the FID.
> 
> Bit 10 of the VTU_OP register (offset 0x05) is the VidPolicy bit,
> which one should probably preserve in mv88e6xxx_g1_vtu_op(), instead
> of always writing a 0. However, on the 6352 family, that bit is
> located at bit 12 in the VTU FID register (offset 0x02), and is always
> unconditionally cleared by the mv88e6xxx_g1_vtu_fid_write()
> function.
> 
> Since nothing in the existing driver seems to know or care about that
> bit, it seems reasonable to not add the boilerplate to preserve it for
> the 6250 (which would require adding a chip-specific vtu_op function,
> or adding chip-quirks to the existing one).
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

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

    Andrew

[RFC PATCH 5/5] net: dsa: add support for mv88e6250

[Rasmus Villemoes]

This is a very rough attempt at adding support for the Marvell
88E6250. The _info and _ops structures are based on those for 6240 (as
I have data sheets for both the 6240 and 6250), fixing the things that
I have determined to be different for the two chips - but some things
are almost certain to still be wrong.

The chip uses the new dual_chip option, and since its port registers
start at SMI address 0x08 or 0x18 (i.e., always 0x08 + sw_addr), we
need to introduce it as a new family in order for the
auto-identification in mv88e6xxx_detect() to work.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/net/dsa/mv88e6xxx/chip.c    | 73 +++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/chip.h    |  2 +
 drivers/net/dsa/mv88e6xxx/global1.c | 20 ++++++++
 drivers/net/dsa/mv88e6xxx/global1.h |  1 +
 drivers/net/dsa/mv88e6xxx/port.h    |  1 +
 5 files changed, 97 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d8d8230a6bf5..11b9d5df2e71 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3502,6 +3502,51 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.phylink_validate = mv88e6352_phylink_validate,
 };
 
+static const struct mv88e6xxx_ops mv88e6250_ops = {
+	/* MV88E6XXX_FAMILY_6250 */
+	.ieee_pri_map = mv88e6085_g1_ieee_pri_map,
+	.ip_pri_map = mv88e6085_g1_ip_pri_map,
+	.irl_init_all = mv88e6352_g2_irl_init_all,
+	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
+	.set_eeprom = mv88e6xxx_g2_set_eeprom16,
+	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
+	.phy_read = mv88e6xxx_g2_smi_phy_read,
+	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.port_set_link = mv88e6xxx_port_set_link,
+	.port_set_duplex = mv88e6xxx_port_set_duplex,
+	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
+	.port_set_speed = mv88e6352_port_set_speed,
+	.port_tag_remap = mv88e6095_port_tag_remap,
+	.port_set_frame_mode = mv88e6351_port_set_frame_mode,
+	.port_set_egress_floods = mv88e6352_port_set_egress_floods,
+	.port_set_ether_type = mv88e6351_port_set_ether_type,
+	.port_set_jumbo_size = mv88e6165_port_set_jumbo_size,
+	.port_egress_rate_limiting = mv88e6097_port_egress_rate_limiting,
+	.port_pause_limit = mv88e6097_port_pause_limit,
+	.port_disable_learn_limit = mv88e6xxx_port_disable_learn_limit,
+	.port_disable_pri_override = mv88e6xxx_port_disable_pri_override,
+	.port_link_state = mv88e6352_port_link_state,
+	.port_get_cmode = mv88e6352_port_get_cmode,
+	.stats_snapshot = mv88e6320_g1_stats_snapshot,
+	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
+	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
+	.stats_get_strings = mv88e6095_stats_get_strings,
+	.stats_get_stats = mv88e6095_stats_get_stats,
+	.set_cpu_port = mv88e6095_g1_set_cpu_port,
+	.set_egress_port = mv88e6095_g1_set_egress_port,
+	.watchdog_ops = &mv88e6097_watchdog_ops,
+	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
+	.pot_clear = mv88e6xxx_g2_pot_clear,
+	.reset = mv88e6250_g1_reset,
+	.rmu_disable = mv88e6352_g1_rmu_disable,
+	.vtu_getnext = mv88e6250_g1_vtu_getnext,
+	.vtu_loadpurge = mv88e6250_g1_vtu_loadpurge,
+	.gpio_ops = &mv88e6352_gpio_ops,
+	.avb_ops = &mv88e6352_avb_ops,
+	.ptp_ops = &mv88e6352_ptp_ops,
+	.phylink_validate = mv88e6352_phylink_validate,
+};
+
 static const struct mv88e6xxx_ops mv88e6290_ops = {
 	/* MV88E6XXX_FAMILY_6390 */
 	.setup_errata = mv88e6390_setup_errata,
@@ -4279,6 +4324,30 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.ops = &mv88e6240_ops,
 	},
 
+	[MV88E6250] = {
+		.prod_num = MV88E6XXX_PORT_SWITCH_ID_PROD_6250,
+		.family = MV88E6XXX_FAMILY_6250,
+		.name = "Marvell 88E6250",
+		.num_databases = 64,
+		.num_ports = 7,
+		.num_internal_phys = 5,
+		.num_gpio = 8,
+		.max_vid = 4095,
+		.port_base_addr = 0x08,
+		.phy_base_addr = 0x00,
+		.global1_addr = 0x0f,
+		.global2_addr = 0x07,
+		.age_time_coeff = 15000,
+		.g1_irqs = 9,
+		.g2_irqs = 10,
+		.atu_move_port_mask = 0xf,
+		.pvt = false,
+		.dual_chip = true,
+		.tag_protocol = DSA_TAG_PROTO_DSA,
+		.ptp_support = true,
+		.ops = &mv88e6250_ops,
+	},
+
 	[MV88E6290] = {
 		.prod_num = MV88E6XXX_PORT_SWITCH_ID_PROD_6290,
 		.family = MV88E6XXX_FAMILY_6390,
@@ -4933,6 +5002,10 @@ static const struct of_device_id mv88e6xxx_of_match[] = {
 		.compatible = "marvell,mv88e6190",
 		.data = &mv88e6xxx_table[MV88E6190],
 	},
+	{
+		.compatible = "marvell,mv88e6250",
+		.data = &mv88e6xxx_table[MV88E6250],
+	},
 	{ /* sentinel */ },
 };
 
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 5759fffbd39c..8da64c40aef6 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -73,6 +73,7 @@ enum mv88e6xxx_model {
 	MV88E6190X,
 	MV88E6191,
 	MV88E6240,
+	MV88E6250,
 	MV88E6290,
 	MV88E6320,
 	MV88E6321,
@@ -91,6 +92,7 @@ enum mv88e6xxx_family {
 	MV88E6XXX_FAMILY_6097,	/* 6046 6085 6096 6097 */
 	MV88E6XXX_FAMILY_6165,	/* 6123 6161 6165 */
 	MV88E6XXX_FAMILY_6185,	/* 6108 6121 6122 6131 6152 6155 6182 6185 */
+	MV88E6XXX_FAMILY_6250,	/* 6250 */
 	MV88E6XXX_FAMILY_6320,	/* 6320 6321 */
 	MV88E6XXX_FAMILY_6341,	/* 6141 6341 */
 	MV88E6XXX_FAMILY_6351,	/* 6171 6175 6350 6351 */
diff --git a/drivers/net/dsa/mv88e6xxx/global1.c b/drivers/net/dsa/mv88e6xxx/global1.c
index 38e399e0f30e..9f6817c61a4f 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.c
+++ b/drivers/net/dsa/mv88e6xxx/global1.c
@@ -205,6 +205,26 @@ int mv88e6352_g1_reset(struct mv88e6xxx_chip *chip)
 	return mv88e6352_g1_wait_ppu_polling(chip);
 }
 
+/* copy of mv88e6352_g1_reset, without tailcall of mv88e6352_g1_wait_ppu_polling. */
+int mv88e6250_g1_reset(struct mv88e6xxx_chip *chip)
+{
+	u16 val;
+	int err;
+
+	/* Set the SWReset bit 15 */
+	err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_CTL1, &val);
+	if (err)
+		return err;
+
+	val |= MV88E6XXX_G1_CTL1_SW_RESET;
+
+	err = mv88e6xxx_g1_write(chip, MV88E6XXX_G1_CTL1, val);
+	if (err)
+		return err;
+
+	return mv88e6xxx_g1_wait_init_ready(chip);
+}
+
 int mv88e6185_g1_ppu_enable(struct mv88e6xxx_chip *chip)
 {
 	u16 val;
diff --git a/drivers/net/dsa/mv88e6xxx/global1.h b/drivers/net/dsa/mv88e6xxx/global1.h
index b205b0bba158..e1b7a6b68365 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.h
+++ b/drivers/net/dsa/mv88e6xxx/global1.h
@@ -259,6 +259,7 @@ int mv88e6xxx_g1_set_switch_mac(struct mv88e6xxx_chip *chip, u8 *addr);
 
 int mv88e6185_g1_reset(struct mv88e6xxx_chip *chip);
 int mv88e6352_g1_reset(struct mv88e6xxx_chip *chip);
+int mv88e6250_g1_reset(struct mv88e6xxx_chip *chip);
 
 int mv88e6185_g1_ppu_enable(struct mv88e6xxx_chip *chip);
 int mv88e6185_g1_ppu_disable(struct mv88e6xxx_chip *chip);
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
index 95b59f5eb393..f1d2b3b88cf4 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -113,6 +113,7 @@
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6191	0x1910
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6185	0x1a70
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6240	0x2400
+#define MV88E6XXX_PORT_SWITCH_ID_PROD_6250	0x2500
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6290	0x2900
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6321	0x3100
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6141	0x3400
-- 
2.20.1

Re: [RFC PATCH 5/5] net: dsa: add support for mv88e6250

[Andrew Lunn]

> +++ b/drivers/net/dsa/mv88e6xxx/global1.c
> @@ -205,6 +205,26 @@ int mv88e6352_g1_reset(struct mv88e6xxx_chip *chip)
>  	return mv88e6352_g1_wait_ppu_polling(chip);
>  }
>  
> +/* copy of mv88e6352_g1_reset, without tailcall of mv88e6352_g1_wait_ppu_polling. */

Nit:

No need to have this comment. 

Otherwise 

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

    Andrew

[PATCH v2 0/5] net: dsa: support for mv88e6250

[Rasmus Villemoes]

This is some mostly-working code adding support for the mv88e6250
chip. I'm not quite done checking whether all the methods in the _ops
structure are appropriate, but basic switchdev functionality seems to
work.

v2:
- rebase on top of net-next/master
- add reviewed-by to two patches unchanged from v1 (2,3)
- add separate watchdog_ops

Rasmus Villemoes (5):
  net: dsa: mv88e6xxx: introduce support for two chips using direct smi
    addressing
  net: dsa: prepare mv88e6xxx_g1_atu_op() for the mv88e6250
  net: dsa: implement vtu_getnext and vtu_loadpurge for mv88e6250
  net: dsa: mv88e6xxx: implement watchdog_ops for mv88e6250
  net: dsa: add support for mv88e6250

 drivers/net/dsa/mv88e6xxx/chip.c        | 73 +++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/chip.h        |  8 +++
 drivers/net/dsa/mv88e6xxx/global1.c     | 19 +++++++
 drivers/net/dsa/mv88e6xxx/global1.h     |  5 ++
 drivers/net/dsa/mv88e6xxx/global1_atu.c |  5 +-
 drivers/net/dsa/mv88e6xxx/global1_vtu.c | 58 ++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/global2.c     | 26 +++++++++
 drivers/net/dsa/mv88e6xxx/global2.h     | 14 +++++
 drivers/net/dsa/mv88e6xxx/port.h        |  1 +
 drivers/net/dsa/mv88e6xxx/smi.c         | 25 ++++++++-
 10 files changed, 232 insertions(+), 2 deletions(-)

-- 
2.20.1

[PATCH v2 1/5] net: dsa: mv88e6xxx: introduce support for two chips using direct smi addressing

[Rasmus Villemoes]

The 88e6250 (as well as 6220, 6071, 6070, 6020) do not support
multi-chip (indirect) addressing. However, one can still have two of
them on the same mdio bus, since the device only uses 16 of the 32
possible addresses, either addresses 0x00-0x0F or 0x10-0x1F depending
on the ADDR4 pin at reset [since ADDR4 is internally pulled high, the
latter is the default].

In order to prepare for supporting the 88e6250 and friends, introduce
mv88e6xxx_info::dual_chip to allow having a non-zero sw_addr while
still using direct addressing.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/net/dsa/mv88e6xxx/chip.h |  6 ++++++
 drivers/net/dsa/mv88e6xxx/smi.c  | 25 ++++++++++++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index faa3fa889f19..74777c3bc313 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -112,6 +112,12 @@ struct mv88e6xxx_info {
 	 * when it is non-zero, and use indirect access to internal registers.
 	 */
 	bool multi_chip;
+	/* Dual-chip Addressing Mode
+	 * Some chips respond to only half of the 32 SMI addresses,
+	 * allowing two to coexist on the same SMI interface.
+	 */
+	bool dual_chip;
+
 	enum dsa_tag_protocol tag_protocol;
 
 	/* Mask for FromPort and ToPort value of PortVec used in ATU Move
diff --git a/drivers/net/dsa/mv88e6xxx/smi.c b/drivers/net/dsa/mv88e6xxx/smi.c
index 96f7d2685bdc..1151b5b493ea 100644
--- a/drivers/net/dsa/mv88e6xxx/smi.c
+++ b/drivers/net/dsa/mv88e6xxx/smi.c
@@ -24,6 +24,10 @@
  * When ADDR is non-zero, the chip uses Multi-chip Addressing Mode, allowing
  * multiple devices to share the SMI interface. In this mode it responds to only
  * 2 registers, used to indirectly access the internal SMI devices.
+ *
+ * Some chips use a different scheme: Only the ADDR4 pin is used for
+ * configuration, and the device responds to 16 of the 32 SMI
+ * addresses, allowing two to coexist on the same SMI interface.
  */
 
 static int mv88e6xxx_smi_direct_read(struct mv88e6xxx_chip *chip,
@@ -76,6 +80,23 @@ static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_direct_ops = {
 	.write = mv88e6xxx_smi_direct_write,
 };
 
+static int mv88e6xxx_smi_dual_direct_read(struct mv88e6xxx_chip *chip,
+					  int dev, int reg, u16 *data)
+{
+	return mv88e6xxx_smi_direct_read(chip, dev + chip->sw_addr, reg, data);
+}
+
+static int mv88e6xxx_smi_dual_direct_write(struct mv88e6xxx_chip *chip,
+					   int dev, int reg, u16 data)
+{
+	return mv88e6xxx_smi_direct_write(chip, dev + chip->sw_addr, reg, data);
+}
+
+static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_dual_direct_ops = {
+	.read = mv88e6xxx_smi_dual_direct_read,
+	.write = mv88e6xxx_smi_dual_direct_write,
+};
+
 /* Offset 0x00: SMI Command Register
  * Offset 0x01: SMI Data Register
  */
@@ -144,7 +165,9 @@ static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_indirect_ops = {
 int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip,
 		       struct mii_bus *bus, int sw_addr)
 {
-	if (sw_addr == 0)
+	if (chip->info->dual_chip)
+		chip->smi_ops = &mv88e6xxx_smi_dual_direct_ops;
+	else if (sw_addr == 0)
 		chip->smi_ops = &mv88e6xxx_smi_direct_ops;
 	else if (chip->info->multi_chip)
 		chip->smi_ops = &mv88e6xxx_smi_indirect_ops;
-- 
2.20.1

Re: [PATCH v2 1/5] net: dsa: mv88e6xxx: introduce support for two chips using direct smi addressing

[Andrew Lunn]

On Fri, May 24, 2019 at 09:00:24AM +0000, Rasmus Villemoes wrote:
> The 88e6250 (as well as 6220, 6071, 6070, 6020) do not support
> multi-chip (indirect) addressing. However, one can still have two of
> them on the same mdio bus, since the device only uses 16 of the 32
> possible addresses, either addresses 0x00-0x0F or 0x10-0x1F depending
> on the ADDR4 pin at reset [since ADDR4 is internally pulled high, the
> latter is the default].
> 
> In order to prepare for supporting the 88e6250 and friends, introduce
> mv88e6xxx_info::dual_chip to allow having a non-zero sw_addr while
> still using direct addressing.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

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

    Andrew

Re: [PATCH v2 1/5] net: dsa: mv88e6xxx: introduce support for two chips using direct smi addressing

[Vivien Didelot]

Hi Rasmus,

On Fri, 24 May 2019 09:00:24 +0000, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:
> The 88e6250 (as well as 6220, 6071, 6070, 6020) do not support
> multi-chip (indirect) addressing. However, one can still have two of
> them on the same mdio bus, since the device only uses 16 of the 32
> possible addresses, either addresses 0x00-0x0F or 0x10-0x1F depending
> on the ADDR4 pin at reset [since ADDR4 is internally pulled high, the
> latter is the default].
> 
> In order to prepare for supporting the 88e6250 and friends, introduce
> mv88e6xxx_info::dual_chip to allow having a non-zero sw_addr while
> still using direct addressing.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.h |  6 ++++++
>  drivers/net/dsa/mv88e6xxx/smi.c  | 25 ++++++++++++++++++++++++-
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
> index faa3fa889f19..74777c3bc313 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.h
> +++ b/drivers/net/dsa/mv88e6xxx/chip.h
> @@ -112,6 +112,12 @@ struct mv88e6xxx_info {
>  	 * when it is non-zero, and use indirect access to internal registers.
>  	 */
>  	bool multi_chip;
> +	/* Dual-chip Addressing Mode
> +	 * Some chips respond to only half of the 32 SMI addresses,
> +	 * allowing two to coexist on the same SMI interface.
> +	 */
> +	bool dual_chip;
> +
>  	enum dsa_tag_protocol tag_protocol;
>  
>  	/* Mask for FromPort and ToPort value of PortVec used in ATU Move
> diff --git a/drivers/net/dsa/mv88e6xxx/smi.c b/drivers/net/dsa/mv88e6xxx/smi.c
> index 96f7d2685bdc..1151b5b493ea 100644
> --- a/drivers/net/dsa/mv88e6xxx/smi.c
> +++ b/drivers/net/dsa/mv88e6xxx/smi.c
> @@ -24,6 +24,10 @@
>   * When ADDR is non-zero, the chip uses Multi-chip Addressing Mode, allowing
>   * multiple devices to share the SMI interface. In this mode it responds to only
>   * 2 registers, used to indirectly access the internal SMI devices.
> + *
> + * Some chips use a different scheme: Only the ADDR4 pin is used for
> + * configuration, and the device responds to 16 of the 32 SMI
> + * addresses, allowing two to coexist on the same SMI interface.
>   */
>  
>  static int mv88e6xxx_smi_direct_read(struct mv88e6xxx_chip *chip,
> @@ -76,6 +80,23 @@ static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_direct_ops = {
>  	.write = mv88e6xxx_smi_direct_write,
>  };
>  
> +static int mv88e6xxx_smi_dual_direct_read(struct mv88e6xxx_chip *chip,
> +					  int dev, int reg, u16 *data)
> +{
> +	return mv88e6xxx_smi_direct_read(chip, dev + chip->sw_addr, reg, data);

Using chip->sw_addr + dev seems more idiomatic to me than dev + chip->sw_addr.

> +}
> +
> +static int mv88e6xxx_smi_dual_direct_write(struct mv88e6xxx_chip *chip,
> +					   int dev, int reg, u16 data)
> +{
> +	return mv88e6xxx_smi_direct_write(chip, dev + chip->sw_addr, reg, data);
> +}
> +
> +static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_dual_direct_ops = {
> +	.read = mv88e6xxx_smi_dual_direct_read,
> +	.write = mv88e6xxx_smi_dual_direct_write,
> +};
> +
>  /* Offset 0x00: SMI Command Register
>   * Offset 0x01: SMI Data Register
>   */
> @@ -144,7 +165,9 @@ static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_indirect_ops = {
>  int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip,
>  		       struct mii_bus *bus, int sw_addr)
>  {
> -	if (sw_addr == 0)
> +	if (chip->info->dual_chip)
> +		chip->smi_ops = &mv88e6xxx_smi_dual_direct_ops;
> +	else if (sw_addr == 0)
>  		chip->smi_ops = &mv88e6xxx_smi_direct_ops;
>  	else if (chip->info->multi_chip)
>  		chip->smi_ops = &mv88e6xxx_smi_indirect_ops;

Please submit respins (v2, v3, and so on) as independent threads,
not as a reply to the previous version.

Otherwise this looks good to me:

Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>

Thanks,
Vivien

[PATCH v2 2/5] net: dsa: prepare mv88e6xxx_g1_atu_op() for the mv88e6250

[Rasmus Villemoes]

All the currently supported chips have .num_databases either 256 or
4096, so this patch does not change behaviour for any of those. The
mv88e6250, however, has .num_databases == 64, and it does not put the
upper two bits in ATU control 13:12, but rather in ATU Operation
9:8. So change the logic to prepare for supporting mv88e6250.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/net/dsa/mv88e6xxx/global1_atu.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
index ea243840ee0f..1ae680bc0eff 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
@@ -94,7 +94,7 @@ static int mv88e6xxx_g1_atu_op(struct mv88e6xxx_chip *chip, u16 fid, u16 op)
 		if (err)
 			return err;
 	} else {
-		if (mv88e6xxx_num_databases(chip) > 16) {
+		if (mv88e6xxx_num_databases(chip) > 64) {
 			/* ATU DBNum[7:4] are located in ATU Control 15:12 */
 			err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_CTL,
 						&val);
@@ -106,6 +106,9 @@ static int mv88e6xxx_g1_atu_op(struct mv88e6xxx_chip *chip, u16 fid, u16 op)
 						 val);
 			if (err)
 				return err;
+		} else if (mv88e6xxx_num_databases(chip) > 16) {
+			/* ATU DBNum[5:4] are located in ATU Operation 9:8 */
+			op |= (fid & 0x30) << 4;
 		}
 
 		/* ATU DBNum[3:0] are located in ATU Operation 3:0 */
-- 
2.20.1

Re: [PATCH v2 2/5] net: dsa: prepare mv88e6xxx_g1_atu_op() for the mv88e6250

[Vivien Didelot]

On Fri, 24 May 2019 09:00:26 +0000, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:
> All the currently supported chips have .num_databases either 256 or
> 4096, so this patch does not change behaviour for any of those. The
> mv88e6250, however, has .num_databases == 64, and it does not put the
> upper two bits in ATU control 13:12, but rather in ATU Operation
> 9:8. So change the logic to prepare for supporting mv88e6250.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>

[PATCH v2 3/5] net: dsa: implement vtu_getnext and vtu_loadpurge for mv88e6250

[Rasmus Villemoes]

These are almost identical to the 6185 variants, but have fewer bits
for the FID.

Bit 10 of the VTU_OP register (offset 0x05) is the VidPolicy bit,
which one should probably preserve in mv88e6xxx_g1_vtu_op(), instead
of always writing a 0. However, on the 6352 family, that bit is
located at bit 12 in the VTU FID register (offset 0x02), and is always
unconditionally cleared by the mv88e6xxx_g1_vtu_fid_write()
function.

Since nothing in the existing driver seems to know or care about that
bit, it seems reasonable to not add the boilerplate to preserve it for
the 6250 (which would require adding a chip-specific vtu_op function,
or adding chip-quirks to the existing one).

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/net/dsa/mv88e6xxx/global1.h     |  4 ++
 drivers/net/dsa/mv88e6xxx/global1_vtu.c | 58 +++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/global1.h b/drivers/net/dsa/mv88e6xxx/global1.h
index bef01331266f..b205b0bba158 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.h
+++ b/drivers/net/dsa/mv88e6xxx/global1.h
@@ -305,6 +305,10 @@ int mv88e6185_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 			     struct mv88e6xxx_vtu_entry *entry);
 int mv88e6185_g1_vtu_loadpurge(struct mv88e6xxx_chip *chip,
 			       struct mv88e6xxx_vtu_entry *entry);
+int mv88e6250_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
+			     struct mv88e6xxx_vtu_entry *entry);
+int mv88e6250_g1_vtu_loadpurge(struct mv88e6xxx_chip *chip,
+			       struct mv88e6xxx_vtu_entry *entry);
 int mv88e6352_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 			     struct mv88e6xxx_vtu_entry *entry);
 int mv88e6352_g1_vtu_loadpurge(struct mv88e6xxx_chip *chip,
diff --git a/drivers/net/dsa/mv88e6xxx/global1_vtu.c b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
index 058326924f3e..a8ef268c32cb 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_vtu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
@@ -307,6 +307,35 @@ static int mv88e6xxx_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 	return mv88e6xxx_g1_vtu_vid_read(chip, entry);
 }
 
+int mv88e6250_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
+			     struct mv88e6xxx_vtu_entry *entry)
+{
+	u16 val;
+	int err;
+
+	err = mv88e6xxx_g1_vtu_getnext(chip, entry);
+	if (err)
+		return err;
+
+	if (entry->valid) {
+		err = mv88e6185_g1_vtu_data_read(chip, entry);
+		if (err)
+			return err;
+
+		/* VTU DBNum[3:0] are located in VTU Operation 3:0
+		 * VTU DBNum[5:4] are located in VTU Operation 9:8
+		 */
+		err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_VTU_OP, &val);
+		if (err)
+			return err;
+
+		entry->fid = val & 0x000f;
+		entry->fid |= (val & 0x0300) >> 4;
+	}
+
+	return 0;
+}
+
 int mv88e6185_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 			     struct mv88e6xxx_vtu_entry *entry)
 {
@@ -396,6 +425,35 @@ int mv88e6390_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 	return 0;
 }
 
+int mv88e6250_g1_vtu_loadpurge(struct mv88e6xxx_chip *chip,
+			       struct mv88e6xxx_vtu_entry *entry)
+{
+	u16 op = MV88E6XXX_G1_VTU_OP_VTU_LOAD_PURGE;
+	int err;
+
+	err = mv88e6xxx_g1_vtu_op_wait(chip);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_g1_vtu_vid_write(chip, entry);
+	if (err)
+		return err;
+
+	if (entry->valid) {
+		err = mv88e6185_g1_vtu_data_write(chip, entry);
+		if (err)
+			return err;
+
+		/* VTU DBNum[3:0] are located in VTU Operation 3:0
+		 * VTU DBNum[5:4] are located in VTU Operation 9:8
+		 */
+		op |= entry->fid & 0x000f;
+		op |= (entry->fid & 0x0030) << 8;
+	}
+
+	return mv88e6xxx_g1_vtu_op(chip, op);
+}
+
 int mv88e6185_g1_vtu_loadpurge(struct mv88e6xxx_chip *chip,
 			       struct mv88e6xxx_vtu_entry *entry)
 {
-- 
2.20.1

Re: [PATCH v2 3/5] net: dsa: implement vtu_getnext and vtu_loadpurge for mv88e6250

[Vivien Didelot]

On Fri, 24 May 2019 09:00:27 +0000, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:
> These are almost identical to the 6185 variants, but have fewer bits
> for the FID.
> 
> Bit 10 of the VTU_OP register (offset 0x05) is the VidPolicy bit,
> which one should probably preserve in mv88e6xxx_g1_vtu_op(), instead
> of always writing a 0. However, on the 6352 family, that bit is
> located at bit 12 in the VTU FID register (offset 0x02), and is always
> unconditionally cleared by the mv88e6xxx_g1_vtu_fid_write()
> function.
> 
> Since nothing in the existing driver seems to know or care about that
> bit, it seems reasonable to not add the boilerplate to preserve it for
> the 6250 (which would require adding a chip-specific vtu_op function,
> or adding chip-quirks to the existing one).
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>

[PATCH v2 4/5] net: dsa: mv88e6xxx: implement watchdog_ops for mv88e6250

[Rasmus Villemoes]

The MV88E6352_G2_WDOG_CTL_* bits almost, but not quite, describe the
watchdog control register on the mv88e6250. Among those actually
referenced in the code, only QC_ENABLE differs (bit 6 rather than bit
5).

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/net/dsa/mv88e6xxx/global2.c | 26 ++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/global2.h | 14 ++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c
index 91a3cb2452ac..85984eb69ffd 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.c
+++ b/drivers/net/dsa/mv88e6xxx/global2.c
@@ -816,6 +816,32 @@ const struct mv88e6xxx_irq_ops mv88e6097_watchdog_ops = {
 	.irq_free = mv88e6097_watchdog_free,
 };
 
+static void mv88e6250_watchdog_free(struct mv88e6xxx_chip *chip)
+{
+	u16 reg;
+
+	mv88e6xxx_g2_read(chip, MV88E6250_G2_WDOG_CTL, &reg);
+
+	reg &= ~(MV88E6250_G2_WDOG_CTL_EGRESS_ENABLE |
+		 MV88E6250_G2_WDOG_CTL_QC_ENABLE);
+
+	mv88e6xxx_g2_write(chip, MV88E6250_G2_WDOG_CTL, reg);
+}
+
+static int mv88e6250_watchdog_setup(struct mv88e6xxx_chip *chip)
+{
+	return mv88e6xxx_g2_write(chip, MV88E6250_G2_WDOG_CTL,
+				  MV88E6250_G2_WDOG_CTL_EGRESS_ENABLE |
+				  MV88E6250_G2_WDOG_CTL_QC_ENABLE |
+				  MV88E6250_G2_WDOG_CTL_SWRESET);
+}
+
+const struct mv88e6xxx_irq_ops mv88e6250_watchdog_ops = {
+	.irq_action = mv88e6097_watchdog_action,
+	.irq_setup = mv88e6250_watchdog_setup,
+	.irq_free = mv88e6250_watchdog_free,
+};
+
 static int mv88e6390_watchdog_setup(struct mv88e6xxx_chip *chip)
 {
 	return mv88e6xxx_g2_update(chip, MV88E6390_G2_WDOG_CTL,
diff --git a/drivers/net/dsa/mv88e6xxx/global2.h b/drivers/net/dsa/mv88e6xxx/global2.h
index 194660d8c783..6205c6b75bc7 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.h
+++ b/drivers/net/dsa/mv88e6xxx/global2.h
@@ -205,6 +205,18 @@
 #define MV88E6XXX_G2_SCRATCH_MISC_PTR_MASK	0x7f00
 #define MV88E6XXX_G2_SCRATCH_MISC_DATA_MASK	0x00ff
 
+/* Offset 0x1B: Watch Dog Control Register */
+#define MV88E6250_G2_WDOG_CTL			0x1b
+#define MV88E6250_G2_WDOG_CTL_QC_HISTORY	0x0100
+#define MV88E6250_G2_WDOG_CTL_QC_EVENT		0x0080
+#define MV88E6250_G2_WDOG_CTL_QC_ENABLE		0x0040
+#define MV88E6250_G2_WDOG_CTL_EGRESS_HISTORY	0x0020
+#define MV88E6250_G2_WDOG_CTL_EGRESS_EVENT	0x0010
+#define MV88E6250_G2_WDOG_CTL_EGRESS_ENABLE	0x0008
+#define MV88E6250_G2_WDOG_CTL_FORCE_IRQ		0x0004
+#define MV88E6250_G2_WDOG_CTL_HISTORY		0x0002
+#define MV88E6250_G2_WDOG_CTL_SWRESET		0x0001
+
 /* Offset 0x1B: Watch Dog Control Register */
 #define MV88E6352_G2_WDOG_CTL			0x1b
 #define MV88E6352_G2_WDOG_CTL_EGRESS_EVENT	0x0080
@@ -334,6 +346,7 @@ int mv88e6xxx_g2_device_mapping_write(struct mv88e6xxx_chip *chip, int target,
 				      int port);
 
 extern const struct mv88e6xxx_irq_ops mv88e6097_watchdog_ops;
+extern const struct mv88e6xxx_irq_ops mv88e6250_watchdog_ops;
 extern const struct mv88e6xxx_irq_ops mv88e6390_watchdog_ops;
 
 extern const struct mv88e6xxx_avb_ops mv88e6165_avb_ops;
@@ -484,6 +497,7 @@ static inline int mv88e6xxx_g2_pot_clear(struct mv88e6xxx_chip *chip)
 }
 
 static const struct mv88e6xxx_irq_ops mv88e6097_watchdog_ops = {};
+static const struct mv88e6xxx_irq_ops mv88e6250_watchdog_ops = {};
 static const struct mv88e6xxx_irq_ops mv88e6390_watchdog_ops = {};
 
 static const struct mv88e6xxx_avb_ops mv88e6165_avb_ops = {};
-- 
2.20.1

Re: [PATCH v2 4/5] net: dsa: mv88e6xxx: implement watchdog_ops for mv88e6250

[Andrew Lunn]

On Fri, May 24, 2019 at 09:00:29AM +0000, Rasmus Villemoes wrote:
> The MV88E6352_G2_WDOG_CTL_* bits almost, but not quite, describe the
> watchdog control register on the mv88e6250. Among those actually
> referenced in the code, only QC_ENABLE differs (bit 6 rather than bit
> 5).

Marvell hardware engineers do like to keep software engineers busy :-(

> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

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

    Andrew

Re: [PATCH v2 4/5] net: dsa: mv88e6xxx: implement watchdog_ops for mv88e6250

[Vivien Didelot]

On Fri, 24 May 2019 09:00:29 +0000, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:
> The MV88E6352_G2_WDOG_CTL_* bits almost, but not quite, describe the
> watchdog control register on the mv88e6250. Among those actually
> referenced in the code, only QC_ENABLE differs (bit 6 rather than bit
> 5).
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

Clean patch,

Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>

Thanks,
Vivien

[PATCH v2 5/5] net: dsa: add support for mv88e6250

[Rasmus Villemoes]

This is a very rough attempt at adding support for the Marvell
88E6250. The _info and _ops structures are based on those for 6240 (as
I have data sheets for both the 6240 and 6250), fixing the things that
I have determined to be different for the two chips - but some things
are almost certain to still be wrong.

The chip uses the new dual_chip option, and since its port registers
start at SMI address 0x08 or 0x18 (i.e., always 0x08 + sw_addr), we
need to introduce it as a new family in order for the
auto-identification in mv88e6xxx_detect() to work.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/net/dsa/mv88e6xxx/chip.c    | 73 +++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/chip.h    |  2 +
 drivers/net/dsa/mv88e6xxx/global1.c | 19 ++++++++
 drivers/net/dsa/mv88e6xxx/global1.h |  1 +
 drivers/net/dsa/mv88e6xxx/port.h    |  1 +
 5 files changed, 96 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 28414db979b0..8cc9a76559ab 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3448,6 +3448,51 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.phylink_validate = mv88e6352_phylink_validate,
 };
 
+static const struct mv88e6xxx_ops mv88e6250_ops = {
+	/* MV88E6XXX_FAMILY_6250 */
+	.ieee_pri_map = mv88e6085_g1_ieee_pri_map,
+	.ip_pri_map = mv88e6085_g1_ip_pri_map,
+	.irl_init_all = mv88e6352_g2_irl_init_all,
+	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
+	.set_eeprom = mv88e6xxx_g2_set_eeprom16,
+	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
+	.phy_read = mv88e6xxx_g2_smi_phy_read,
+	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.port_set_link = mv88e6xxx_port_set_link,
+	.port_set_duplex = mv88e6xxx_port_set_duplex,
+	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
+	.port_set_speed = mv88e6352_port_set_speed,
+	.port_tag_remap = mv88e6095_port_tag_remap,
+	.port_set_frame_mode = mv88e6351_port_set_frame_mode,
+	.port_set_egress_floods = mv88e6352_port_set_egress_floods,
+	.port_set_ether_type = mv88e6351_port_set_ether_type,
+	.port_set_jumbo_size = mv88e6165_port_set_jumbo_size,
+	.port_egress_rate_limiting = mv88e6097_port_egress_rate_limiting,
+	.port_pause_limit = mv88e6097_port_pause_limit,
+	.port_disable_learn_limit = mv88e6xxx_port_disable_learn_limit,
+	.port_disable_pri_override = mv88e6xxx_port_disable_pri_override,
+	.port_link_state = mv88e6352_port_link_state,
+	.port_get_cmode = mv88e6352_port_get_cmode,
+	.stats_snapshot = mv88e6320_g1_stats_snapshot,
+	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
+	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
+	.stats_get_strings = mv88e6095_stats_get_strings,
+	.stats_get_stats = mv88e6095_stats_get_stats,
+	.set_cpu_port = mv88e6095_g1_set_cpu_port,
+	.set_egress_port = mv88e6095_g1_set_egress_port,
+	.watchdog_ops = &mv88e6250_watchdog_ops,
+	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
+	.pot_clear = mv88e6xxx_g2_pot_clear,
+	.reset = mv88e6250_g1_reset,
+	.rmu_disable = mv88e6352_g1_rmu_disable,
+	.vtu_getnext = mv88e6250_g1_vtu_getnext,
+	.vtu_loadpurge = mv88e6250_g1_vtu_loadpurge,
+	.gpio_ops = &mv88e6352_gpio_ops,
+	.avb_ops = &mv88e6352_avb_ops,
+	.ptp_ops = &mv88e6352_ptp_ops,
+	.phylink_validate = mv88e6352_phylink_validate,
+};
+
 static const struct mv88e6xxx_ops mv88e6290_ops = {
 	/* MV88E6XXX_FAMILY_6390 */
 	.setup_errata = mv88e6390_setup_errata,
@@ -4233,6 +4278,30 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.ops = &mv88e6240_ops,
 	},
 
+	[MV88E6250] = {
+		.prod_num = MV88E6XXX_PORT_SWITCH_ID_PROD_6250,
+		.family = MV88E6XXX_FAMILY_6250,
+		.name = "Marvell 88E6250",
+		.num_databases = 64,
+		.num_ports = 7,
+		.num_internal_phys = 5,
+		.num_gpio = 8,
+		.max_vid = 4095,
+		.port_base_addr = 0x08,
+		.phy_base_addr = 0x00,
+		.global1_addr = 0x0f,
+		.global2_addr = 0x07,
+		.age_time_coeff = 15000,
+		.g1_irqs = 9,
+		.g2_irqs = 10,
+		.atu_move_port_mask = 0xf,
+		.pvt = false,
+		.dual_chip = true,
+		.tag_protocol = DSA_TAG_PROTO_DSA,
+		.ptp_support = true,
+		.ops = &mv88e6250_ops,
+	},
+
 	[MV88E6290] = {
 		.prod_num = MV88E6XXX_PORT_SWITCH_ID_PROD_6290,
 		.family = MV88E6XXX_FAMILY_6390,
@@ -4841,6 +4910,10 @@ static const struct of_device_id mv88e6xxx_of_match[] = {
 		.compatible = "marvell,mv88e6190",
 		.data = &mv88e6xxx_table[MV88E6190],
 	},
+	{
+		.compatible = "marvell,mv88e6250",
+		.data = &mv88e6xxx_table[MV88E6250],
+	},
 	{ /* sentinel */ },
 };
 
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 74777c3bc313..2fbe72b7587b 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -62,6 +62,7 @@ enum mv88e6xxx_model {
 	MV88E6190X,
 	MV88E6191,
 	MV88E6240,
+	MV88E6250,
 	MV88E6290,
 	MV88E6320,
 	MV88E6321,
@@ -80,6 +81,7 @@ enum mv88e6xxx_family {
 	MV88E6XXX_FAMILY_6097,	/* 6046 6085 6096 6097 */
 	MV88E6XXX_FAMILY_6165,	/* 6123 6161 6165 */
 	MV88E6XXX_FAMILY_6185,	/* 6108 6121 6122 6131 6152 6155 6182 6185 */
+	MV88E6XXX_FAMILY_6250,	/* 6250 */
 	MV88E6XXX_FAMILY_6320,	/* 6320 6321 */
 	MV88E6XXX_FAMILY_6341,	/* 6141 6341 */
 	MV88E6XXX_FAMILY_6351,	/* 6171 6175 6350 6351 */
diff --git a/drivers/net/dsa/mv88e6xxx/global1.c b/drivers/net/dsa/mv88e6xxx/global1.c
index 38e399e0f30e..b373feeed06c 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.c
+++ b/drivers/net/dsa/mv88e6xxx/global1.c
@@ -182,6 +182,25 @@ int mv88e6185_g1_reset(struct mv88e6xxx_chip *chip)
 	return mv88e6185_g1_wait_ppu_polling(chip);
 }
 
+int mv88e6250_g1_reset(struct mv88e6xxx_chip *chip)
+{
+	u16 val;
+	int err;
+
+	/* Set the SWReset bit 15 */
+	err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_CTL1, &val);
+	if (err)
+		return err;
+
+	val |= MV88E6XXX_G1_CTL1_SW_RESET;
+
+	err = mv88e6xxx_g1_write(chip, MV88E6XXX_G1_CTL1, val);
+	if (err)
+		return err;
+
+	return mv88e6xxx_g1_wait_init_ready(chip);
+}
+
 int mv88e6352_g1_reset(struct mv88e6xxx_chip *chip)
 {
 	u16 val;
diff --git a/drivers/net/dsa/mv88e6xxx/global1.h b/drivers/net/dsa/mv88e6xxx/global1.h
index b205b0bba158..e1b7a6b68365 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.h
+++ b/drivers/net/dsa/mv88e6xxx/global1.h
@@ -259,6 +259,7 @@ int mv88e6xxx_g1_set_switch_mac(struct mv88e6xxx_chip *chip, u8 *addr);
 
 int mv88e6185_g1_reset(struct mv88e6xxx_chip *chip);
 int mv88e6352_g1_reset(struct mv88e6xxx_chip *chip);
+int mv88e6250_g1_reset(struct mv88e6xxx_chip *chip);
 
 int mv88e6185_g1_ppu_enable(struct mv88e6xxx_chip *chip);
 int mv88e6185_g1_ppu_disable(struct mv88e6xxx_chip *chip);
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
index 39c85e98fb92..541ef5c3f1d1 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -112,6 +112,7 @@
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6191	0x1910
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6185	0x1a70
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6240	0x2400
+#define MV88E6XXX_PORT_SWITCH_ID_PROD_6250	0x2500
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6290	0x2900
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6321	0x3100
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6141	0x3400
-- 
2.20.1

Re: [PATCH v2 5/5] net: dsa: add support for mv88e6250

[Andrew Lunn]

> @@ -4841,6 +4910,10 @@ static const struct of_device_id mv88e6xxx_of_match[] = {
>  		.compatible = "marvell,mv88e6190",
>  		.data = &mv88e6xxx_table[MV88E6190],
>  	},
> +	{
> +		.compatible = "marvell,mv88e6250",
> +		.data = &mv88e6xxx_table[MV88E6250],
> +	},
>  	{ /* sentinel */ },
>  };

Ah, yes. I had not thought about that. A device at address 0 would be
found, but a device at address 16 would be missed.

Please add this compatible string to Documentation/devicetree/bindings/net/dsa/marvell.txt 

> +++ b/drivers/net/dsa/mv88e6xxx/global1.c
> @@ -182,6 +182,25 @@ int mv88e6185_g1_reset(struct mv88e6xxx_chip *chip)
>  	return mv88e6185_g1_wait_ppu_polling(chip);
>  }
>  
> +int mv88e6250_g1_reset(struct mv88e6xxx_chip *chip)
> +{
> +	u16 val;
> +	int err;
> +
> +	/* Set the SWReset bit 15 */
> +	err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_CTL1, &val);
> +	if (err)
> +		return err;
> +
> +	val |= MV88E6XXX_G1_CTL1_SW_RESET;
> +
> +	err = mv88e6xxx_g1_write(chip, MV88E6XXX_G1_CTL1, val);
> +	if (err)
> +		return err;
> +
> +	return mv88e6xxx_g1_wait_init_ready(chip);
> +}

It looks like you could refactor mv88e6352_g1_reset() to call
this function, and then mv88e6352_g1_wait_ppu_polling(chip);

Otherwise, this looks good.

   Andrew

Re: [PATCH v2 5/5] net: dsa: add support for mv88e6250

[Rasmus Villemoes]

On 24/05/2019 16.27, Andrew Lunn wrote:
>> @@ -4841,6 +4910,10 @@ static const struct of_device_id mv88e6xxx_of_match[] = {
>>  		.compatible = "marvell,mv88e6190",
>>  		.data = &mv88e6xxx_table[MV88E6190],
>>  	},
>> +	{
>> +		.compatible = "marvell,mv88e6250",
>> +		.data = &mv88e6xxx_table[MV88E6250],
>> +	},
>>  	{ /* sentinel */ },
>>  };
> 
> Ah, yes. I had not thought about that. A device at address 0 would be
> found, but a device at address 16 would be missed.

Eh, no? The port registers are at offset 0x8, i.e. at either SMI address
8 or 24, so I don't think a 6250 at address 0 could be detected using
either of the existing families?

> Please add this compatible string to Documentation/devicetree/bindings/net/dsa/marvell.txt 

Will do.

>> +++ b/drivers/net/dsa/mv88e6xxx/global1.c
>> @@ -182,6 +182,25 @@ int mv88e6185_g1_reset(struct mv88e6xxx_chip *chip)
>>  	return mv88e6185_g1_wait_ppu_polling(chip);
>>  }
>>  
>> +int mv88e6250_g1_reset(struct mv88e6xxx_chip *chip)
>> +{
>> +	u16 val;
>> +	int err;
>> +
>> +	/* Set the SWReset bit 15 */
>> +	err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_CTL1, &val);
>> +	if (err)
>> +		return err;
>> +
>> +	val |= MV88E6XXX_G1_CTL1_SW_RESET;
>> +
>> +	err = mv88e6xxx_g1_write(chip, MV88E6XXX_G1_CTL1, val);
>> +	if (err)
>> +		return err;
>> +
>> +	return mv88e6xxx_g1_wait_init_ready(chip);
>> +}
> 
> It looks like you could refactor mv88e6352_g1_reset() to call
> this function, and then mv88e6352_g1_wait_ppu_polling(chip);

Yes, I actually deliberately moved the 6250 reset function further up in
v2 to allow that. I'll add that refactoring as a separate patch.

Thanks,
Rasmus

Re: [PATCH v2 5/5] net: dsa: add support for mv88e6250

[Andrew Lunn]

On Mon, Jun 03, 2019 at 08:52:38AM +0000, Rasmus Villemoes wrote:
> On 24/05/2019 16.27, Andrew Lunn wrote:
> >> @@ -4841,6 +4910,10 @@ static const struct of_device_id mv88e6xxx_of_match[] = {
> >>  		.compatible = "marvell,mv88e6190",
> >>  		.data = &mv88e6xxx_table[MV88E6190],
> >>  	},
> >> +	{
> >> +		.compatible = "marvell,mv88e6250",
> >> +		.data = &mv88e6xxx_table[MV88E6250],
> >> +	},
> >>  	{ /* sentinel */ },
> >>  };
> > 
> > Ah, yes. I had not thought about that. A device at address 0 would be
> > found, but a device at address 16 would be missed.
> 
> Eh, no? The port registers are at offset 0x8, i.e. at either SMI address
> 8 or 24, so I don't think a 6250 at address 0 could be detected using
> either of the existing families?

Even better.

The real problem is, people keep trying to add new compatible strings
here when they should not. The compatible string is about being able
to read the ID registers, not to list every single switch chip family.

This is one case where it really is needed, and i had not thought
about that.

      Andrew

Re: [PATCH v2 5/5] net: dsa: add support for mv88e6250

[Vivien Didelot]

Hi Rasmus,

On Fri, 24 May 2019 09:00:31 +0000, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:
> This is a very rough attempt at adding support for the Marvell
> 88E6250. The _info and _ops structures are based on those for 6240 (as
> I have data sheets for both the 6240 and 6250), fixing the things that
> I have determined to be different for the two chips - but some things
> are almost certain to still be wrong.

The idea is that for things that you're not certain about, simply
don't add the corresponding ops. The driver would simply return
-EOPNOTSUPP for the related features (if it doesn't behave like this,
we must fix this.)


Thanks,
Vivien