[PATCH 00/11] net: dsa: microchip: make ksz8795 driver more dynamic

[PATCH 00/11] net: dsa: microchip: make ksz8795 driver more dynamic

[Michael Grzeschik]

This series changes the ksz8795 driver to use more dynamic variables
instead of static defines. The purpose is to prepare the driver to be
used with other microchip switches with a similar layout.

Michael Grzeschik (11):
  net: dsa: microchip: ksz8795: remove unused last_port variable
  net: dsa: microchip: ksz8795: remove superfluous port_cnt assignment
  net: dsa: microchip: ksz8795: move variable assignments from detect to init
  net: dsa: microchip: ksz8795: use reg_mib_cnt where possible
  net: dsa: microchip: ksz8795: use mib_cnt where possible
  net: dsa: microchip: ksz8795: use phy_port_cnt where possible
  net: dsa: microchip: remove superfluous num_ports asignment
  net: dsa: microchip: ksz8795: align port_cnt usage with other microchip drivers
  net: dsa: microchip: remove usage of mib_port_count
  net: dsa: microchip: ksz8795: dynamic allocate memory for flush_dyn_mac_table
  net: dsa: microchip: ksz8795: use num_vlans where possible

 drivers/net/dsa/microchip/ksz8795.c     | 74 ++++++++++++-------------
 drivers/net/dsa/microchip/ksz8795_reg.h | 10 ----
 drivers/net/dsa/microchip/ksz9477.c     | 14 ++---
 drivers/net/dsa/microchip/ksz_common.c  |  4 +-
 drivers/net/dsa/microchip/ksz_common.h  |  2 -
 5 files changed, 43 insertions(+), 61 deletions(-)

-- 
2.29.2

[PATCH 01/11] net: dsa: microchip: ksz8795: remove unused last_port variable

[Michael Grzeschik]

The variable last_port is not used anywhere, this patch removes it.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz8795.c    | 1 -
 drivers/net/dsa/microchip/ksz_common.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 1e101ab56cea113..8a3d2e607283581 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -1163,7 +1163,6 @@ static int ksz8795_switch_detect(struct ksz_device *dev)
 			id2 = 0x65;
 	} else if (id2 == CHIP_ID_94) {
 		dev->port_cnt--;
-		dev->last_port = dev->port_cnt;
 		id2 = 0x94;
 	}
 	id16 &= ~0xff;
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index cf866e48ff664b6..629700a63702435 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -72,7 +72,6 @@ struct ksz_device {
 	int reg_mib_cnt;
 	int mib_cnt;
 	int mib_port_cnt;
-	int last_port;			/* ports after that not used */
 	phy_interface_t compat_interface;
 	u32 regs_size;
 	bool phy_errata_9477;
-- 
2.29.2

Re: [PATCH 01/11] net: dsa: microchip: ksz8795: remove unused last_port variable

[Andrew Lunn]

On Wed, Nov 18, 2020 at 11:03:47PM +0100, Michael Grzeschik wrote:
> The variable last_port is not used anywhere, this patch removes it.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

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

    Andrew

Re: [PATCH 01/11] net: dsa: microchip: ksz8795: remove unused last_port variable

[Florian Fainelli]

On 11/18/2020 2:03 PM, Michael Grzeschik wrote:
> The variable last_port is not used anywhere, this patch removes it.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH 02/11] net: dsa: microchip: ksz8795: remove superfluous port_cnt assignment

[Michael Grzeschik]

The port_cnt assignment will be done again in the init function.
This patch removes the previous assignment in the detect function.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz8795.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 8a3d2e607283581..853a0805e08f239 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -1152,7 +1152,6 @@ static int ksz8795_switch_detect(struct ksz_device *dev)
 
 	dev->mib_port_cnt = TOTAL_PORT_NUM;
 	dev->phy_port_cnt = SWITCH_PORT_NUM;
-	dev->port_cnt = SWITCH_PORT_NUM;
 
 	if (id2 == CHIP_ID_95) {
 		u8 val;
@@ -1162,7 +1161,6 @@ static int ksz8795_switch_detect(struct ksz_device *dev)
 		if (val & PORT_FIBER_MODE)
 			id2 = 0x65;
 	} else if (id2 == CHIP_ID_94) {
-		dev->port_cnt--;
 		id2 = 0x94;
 	}
 	id16 &= ~0xff;
-- 
2.29.2

Re: [PATCH 02/11] net: dsa: microchip: ksz8795: remove superfluous port_cnt assignment

[Andrew Lunn]

On Wed, Nov 18, 2020 at 11:03:48PM +0100, Michael Grzeschik wrote:
> The port_cnt assignment will be done again in the init function.
> This patch removes the previous assignment in the detect function.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

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

    Andrew

Re: [PATCH 02/11] net: dsa: microchip: ksz8795: remove superfluous port_cnt assignment

[Florian Fainelli]

On 11/18/2020 2:03 PM, Michael Grzeschik wrote:
> The port_cnt assignment will be done again in the init function.
> This patch removes the previous assignment in the detect function.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH 03/11] net: dsa: microchip: ksz8795: move variable assignments from detect to init

[Michael Grzeschik]

This patch moves all variable assignments to the init function. It
leaves the detect function for its single purpose to detect which chip
version is found.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz8795.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 853a0805e08f239..1164c745ce940d0 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -1150,9 +1150,6 @@ static int ksz8795_switch_detect(struct ksz_device *dev)
 	    (id2 != CHIP_ID_94 && id2 != CHIP_ID_95))
 		return -ENODEV;
 
-	dev->mib_port_cnt = TOTAL_PORT_NUM;
-	dev->phy_port_cnt = SWITCH_PORT_NUM;
-
 	if (id2 == CHIP_ID_95) {
 		u8 val;
 
@@ -1167,9 +1164,6 @@ static int ksz8795_switch_detect(struct ksz_device *dev)
 	id16 |= id2;
 	dev->chip_id = id16;
 
-	dev->cpu_port = dev->mib_port_cnt - 1;
-	dev->host_mask = BIT(dev->cpu_port);
-
 	return 0;
 }
 
@@ -1244,6 +1238,12 @@ static int ksz8795_switch_init(struct ksz_device *dev)
 	dev->reg_mib_cnt = SWITCH_COUNTER_NUM;
 	dev->mib_cnt = TOTAL_SWITCH_COUNTER_NUM;
 
+	dev->mib_port_cnt = TOTAL_PORT_NUM;
+	dev->phy_port_cnt = SWITCH_PORT_NUM;
+
+	dev->cpu_port = dev->mib_port_cnt - 1;
+	dev->host_mask = BIT(dev->cpu_port);
+
 	i = dev->mib_port_cnt;
 	dev->ports = devm_kzalloc(dev->dev, sizeof(struct ksz_port) * i,
 				  GFP_KERNEL);
-- 
2.29.2

Re: [PATCH 03/11] net: dsa: microchip: ksz8795: move variable assignments from detect to init

[Andrew Lunn]

On Wed, Nov 18, 2020 at 11:03:49PM +0100, Michael Grzeschik wrote:
> This patch moves all variable assignments to the init function. It
> leaves the detect function for its single purpose to detect which chip
> version is found.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

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

    Andrew

Re: [PATCH 03/11] net: dsa: microchip: ksz8795: move variable assignments from detect to init

[Florian Fainelli]

On 11/18/2020 2:03 PM, Michael Grzeschik wrote:
> This patch moves all variable assignments to the init function. It
> leaves the detect function for its single purpose to detect which chip
> version is found.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH 04/11] net: dsa: microchip: ksz8795: use reg_mib_cnt where possible

[Michael Grzeschik]

The extra define SWITCH_COUNTER_NUM is a copy of the KSZ8795_COUNTER_NUM
define. This patch initializes reg_mib_cnt with KSZ8795_COUNTER_NUM,
makes use of reg_mib_cnt everywhere instead and removes the extra
define.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz8795.c     | 6 +++---
 drivers/net/dsa/microchip/ksz8795_reg.h | 1 -
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 1164c745ce940d0..04a571bde7e6a4f 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -125,7 +125,7 @@ static void ksz8795_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
 	u8 check;
 	int loop;
 
-	ctrl_addr = addr + SWITCH_COUNTER_NUM * port;
+	ctrl_addr = addr + dev->reg_mib_cnt * port;
 	ctrl_addr |= IND_ACC_TABLE(TABLE_MIB | TABLE_READ);
 
 	mutex_lock(&dev->alu_mutex);
@@ -156,7 +156,7 @@ static void ksz8795_r_mib_pkt(struct ksz_device *dev, int port, u16 addr,
 	u8 check;
 	int loop;
 
-	addr -= SWITCH_COUNTER_NUM;
+	addr -= dev->reg_mib_cnt;
 	ctrl_addr = (KS_MIB_TOTAL_RX_1 - KS_MIB_TOTAL_RX_0) * port;
 	ctrl_addr += addr + KS_MIB_TOTAL_RX_0;
 	ctrl_addr |= IND_ACC_TABLE(TABLE_MIB | TABLE_READ);
@@ -1235,7 +1235,7 @@ static int ksz8795_switch_init(struct ksz_device *dev)
 	dev->port_mask = BIT(dev->port_cnt) - 1;
 	dev->port_mask |= dev->host_mask;
 
-	dev->reg_mib_cnt = SWITCH_COUNTER_NUM;
+	dev->reg_mib_cnt = KSZ8795_COUNTER_NUM;
 	dev->mib_cnt = TOTAL_SWITCH_COUNTER_NUM;
 
 	dev->mib_port_cnt = TOTAL_PORT_NUM;
diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h b/drivers/net/dsa/microchip/ksz8795_reg.h
index 3a50462df8fa9d2..25840719b936650 100644
--- a/drivers/net/dsa/microchip/ksz8795_reg.h
+++ b/drivers/net/dsa/microchip/ksz8795_reg.h
@@ -854,7 +854,6 @@
 #define KSZ8795_COUNTER_NUM		0x20
 #define TOTAL_KSZ8795_COUNTER_NUM	(KSZ8795_COUNTER_NUM + 4)
 
-#define SWITCH_COUNTER_NUM		KSZ8795_COUNTER_NUM
 #define TOTAL_SWITCH_COUNTER_NUM	TOTAL_KSZ8795_COUNTER_NUM
 
 /* Common names used by other drivers */
-- 
2.29.2

Re: [PATCH 04/11] net: dsa: microchip: ksz8795: use reg_mib_cnt where possible

[Andrew Lunn]

On Wed, Nov 18, 2020 at 11:03:50PM +0100, Michael Grzeschik wrote:
> The extra define SWITCH_COUNTER_NUM is a copy of the KSZ8795_COUNTER_NUM
> define. This patch initializes reg_mib_cnt with KSZ8795_COUNTER_NUM,
> makes use of reg_mib_cnt everywhere instead and removes the extra
> define.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

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

    Andrew

Re: [PATCH 04/11] net: dsa: microchip: ksz8795: use reg_mib_cnt where possible

[Florian Fainelli]

On 11/18/2020 2:03 PM, Michael Grzeschik wrote:
> The extra define SWITCH_COUNTER_NUM is a copy of the KSZ8795_COUNTER_NUM
> define. This patch initializes reg_mib_cnt with KSZ8795_COUNTER_NUM,
> makes use of reg_mib_cnt everywhere instead and removes the extra
> define.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH 05/11] net: dsa: microchip: ksz8795: use mib_cnt where possible

[Michael Grzeschik]

The variable mib_cnt is assigned with TOTAL_SWITCH_COUNTER_NUM. This
value can also be derived from the array size of mib_names. This patch
uses this calculated value instead, removes the extra define and uses
mib_cnt everywhere possible instead of the static define
TOTAL_SWITCH_COUNTER_NUM.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz8795.c     | 8 ++++----
 drivers/net/dsa/microchip/ksz8795_reg.h | 3 ---
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 04a571bde7e6a4f..6ddba2de2d3026e 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -23,7 +23,7 @@
 
 static const struct {
 	char string[ETH_GSTRING_LEN];
-} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
+} mib_names[] = {
 	{ "rx_hi" },
 	{ "rx_undersize" },
 	{ "rx_fragments" },
@@ -656,7 +656,7 @@ static void ksz8795_get_strings(struct dsa_switch *ds, int port,
 {
 	int i;
 
-	for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
+	for (i = 0; i < dev->mib_cnt; i++) {
 		memcpy(buf + i * ETH_GSTRING_LEN, mib_names[i].string,
 		       ETH_GSTRING_LEN);
 	}
@@ -1236,7 +1236,7 @@ static int ksz8795_switch_init(struct ksz_device *dev)
 	dev->port_mask |= dev->host_mask;
 
 	dev->reg_mib_cnt = KSZ8795_COUNTER_NUM;
-	dev->mib_cnt = TOTAL_SWITCH_COUNTER_NUM;
+	dev->mib_cnt = ARRAY_SIZE(mib_names);
 
 	dev->mib_port_cnt = TOTAL_PORT_NUM;
 	dev->phy_port_cnt = SWITCH_PORT_NUM;
@@ -1254,7 +1254,7 @@ static int ksz8795_switch_init(struct ksz_device *dev)
 		dev->ports[i].mib.counters =
 			devm_kzalloc(dev->dev,
 				     sizeof(u64) *
-				     (TOTAL_SWITCH_COUNTER_NUM + 1),
+				     (dev->mib_cnt + 1),
 				     GFP_KERNEL);
 		if (!dev->ports[i].mib.counters)
 			return -ENOMEM;
diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h b/drivers/net/dsa/microchip/ksz8795_reg.h
index 25840719b936650..c131224850135bd 100644
--- a/drivers/net/dsa/microchip/ksz8795_reg.h
+++ b/drivers/net/dsa/microchip/ksz8795_reg.h
@@ -852,9 +852,6 @@
 #define SWITCH_PORT_NUM			(TOTAL_PORT_NUM - 1)
 
 #define KSZ8795_COUNTER_NUM		0x20
-#define TOTAL_KSZ8795_COUNTER_NUM	(KSZ8795_COUNTER_NUM + 4)
-
-#define TOTAL_SWITCH_COUNTER_NUM	TOTAL_KSZ8795_COUNTER_NUM
 
 /* Common names used by other drivers */
 
-- 
2.29.2

Re: [PATCH 05/11] net: dsa: microchip: ksz8795: use mib_cnt where possible

[Andrew Lunn]

On Wed, Nov 18, 2020 at 11:03:51PM +0100, Michael Grzeschik wrote:
> The variable mib_cnt is assigned with TOTAL_SWITCH_COUNTER_NUM. This
> value can also be derived from the array size of mib_names. This patch
> uses this calculated value instead, removes the extra define and uses
> mib_cnt everywhere possible instead of the static define
> TOTAL_SWITCH_COUNTER_NUM.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/net/dsa/microchip/ksz8795.c     | 8 ++++----
>  drivers/net/dsa/microchip/ksz8795_reg.h | 3 ---
>  2 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> index 04a571bde7e6a4f..6ddba2de2d3026e 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -23,7 +23,7 @@
>  
>  static const struct {
>  	char string[ETH_GSTRING_LEN];
> -} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
> +} mib_names[] = {
>  	{ "rx_hi" },
>  	{ "rx_undersize" },
>  	{ "rx_fragments" },
> @@ -656,7 +656,7 @@ static void ksz8795_get_strings(struct dsa_switch *ds, int port,
>  {
>  	int i;
>  
> -	for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
> +	for (i = 0; i < dev->mib_cnt; i++) {
>  		memcpy(buf + i * ETH_GSTRING_LEN, mib_names[i].string,
>  		       ETH_GSTRING_LEN);
>  	}
> @@ -1236,7 +1236,7 @@ static int ksz8795_switch_init(struct ksz_device *dev)
>  	dev->port_mask |= dev->host_mask;
>  
>  	dev->reg_mib_cnt = KSZ8795_COUNTER_NUM;
> -	dev->mib_cnt = TOTAL_SWITCH_COUNTER_NUM;
> +	dev->mib_cnt = ARRAY_SIZE(mib_names);

Hi Michael

I think it would be better to just use ARRAY_SIZE(mib_names)
everywhere. It is one less hoop to jump through when looking for array
overruns, etc.

	Andrew

Re: [PATCH 05/11] net: dsa: microchip: ksz8795: use mib_cnt where possible

[Michael Grzeschik]

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

Hi Andrew!

On Thu, Nov 19, 2020 at 01:20:47AM +0100, Andrew Lunn wrote:
>On Wed, Nov 18, 2020 at 11:03:51PM +0100, Michael Grzeschik wrote:
>> The variable mib_cnt is assigned with TOTAL_SWITCH_COUNTER_NUM. This
>> value can also be derived from the array size of mib_names. This patch
>> uses this calculated value instead, removes the extra define and uses
>> mib_cnt everywhere possible instead of the static define
>> TOTAL_SWITCH_COUNTER_NUM.
>>
>> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>> ---
>>  drivers/net/dsa/microchip/ksz8795.c     | 8 ++++----
>>  drivers/net/dsa/microchip/ksz8795_reg.h | 3 ---
>>  2 files changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
>> index 04a571bde7e6a4f..6ddba2de2d3026e 100644
>> --- a/drivers/net/dsa/microchip/ksz8795.c
>> +++ b/drivers/net/dsa/microchip/ksz8795.c
>> @@ -23,7 +23,7 @@
>>
>>  static const struct {
>>  	char string[ETH_GSTRING_LEN];
>> -} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
>> +} mib_names[] = {
>>  	{ "rx_hi" },
>>  	{ "rx_undersize" },
>>  	{ "rx_fragments" },
>> @@ -656,7 +656,7 @@ static void ksz8795_get_strings(struct dsa_switch *ds, int port,
>>  {
>>  	int i;
>>
>> -	for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
>> +	for (i = 0; i < dev->mib_cnt; i++) {
>>  		memcpy(buf + i * ETH_GSTRING_LEN, mib_names[i].string,
>>  		       ETH_GSTRING_LEN);
>>  	}
>> @@ -1236,7 +1236,7 @@ static int ksz8795_switch_init(struct ksz_device *dev)
>>  	dev->port_mask |= dev->host_mask;
>>
>>  	dev->reg_mib_cnt = KSZ8795_COUNTER_NUM;
>> -	dev->mib_cnt = TOTAL_SWITCH_COUNTER_NUM;
>> +	dev->mib_cnt = ARRAY_SIZE(mib_names);
>
>Hi Michael
>
>I think it would be better to just use ARRAY_SIZE(mib_names)
>everywhere. It is one less hoop to jump through when looking for array
>overruns, etc.

I would better stay with the variable, because of two reasons. First it
is also used in ksz_common.c and ksz_9477.c. Also in my next patches
I will introduce another mib_names array. We will have ksz8863_mib_names
and ksz8795_mib_names. In the init function then the mib_cnt will be set
regarding to the chip that is found.

Do you agree that the extra variable makes the code more readable in
that case?

Regards,
Michael

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

Re: [PATCH 05/11] net: dsa: microchip: ksz8795: use mib_cnt where possible

[Florian Fainelli]

On 11/18/2020 2:03 PM, Michael Grzeschik wrote:
> The variable mib_cnt is assigned with TOTAL_SWITCH_COUNTER_NUM. This
> value can also be derived from the array size of mib_names. This patch
> uses this calculated value instead, removes the extra define and uses
> mib_cnt everywhere possible instead of the static define
> TOTAL_SWITCH_COUNTER_NUM.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH 05/11] net: dsa: microchip: ksz8795: use mib_cnt where possible

[kernel test robot]

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

Hi Michael,

I love your patch! Yet something to improve:

[auto build test ERROR on net/master]
[also build test ERROR on net-next/master ipvs/master linus/master v5.10-rc4 next-20201118]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Michael-Grzeschik/net-dsa-microchip-make-ksz8795-driver-more-dynamic/20201119-060705
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git a3dcb3e7e70c72a68a79b30fc3a3adad5612731c
config: h8300-randconfig-r031-20201119 (attached as .config)
compiler: h8300-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/9debc1a2179402cc4391e253c57fafa0ef357e05
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Michael-Grzeschik/net-dsa-microchip-make-ksz8795-driver-more-dynamic/20201119-060705
        git checkout 9debc1a2179402cc4391e253c57fafa0ef357e05
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=h8300 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/net/dsa/microchip/ksz8795.c: In function 'ksz8795_get_strings':
>> drivers/net/dsa/microchip/ksz8795.c:659:18: error: 'dev' undeclared (first use in this function); did you mean 'cdev'?
     659 |  for (i = 0; i < dev->mib_cnt; i++) {
         |                  ^~~
         |                  cdev
   drivers/net/dsa/microchip/ksz8795.c:659:18: note: each undeclared identifier is reported only once for each function it appears in

vim +659 drivers/net/dsa/microchip/ksz8795.c

   653	
   654	static void ksz8795_get_strings(struct dsa_switch *ds, int port,
   655					u32 stringset, uint8_t *buf)
   656	{
   657		int i;
   658	
 > 659		for (i = 0; i < dev->mib_cnt; i++) {
   660			memcpy(buf + i * ETH_GSTRING_LEN, mib_names[i].string,
   661			       ETH_GSTRING_LEN);
   662		}
   663	}
   664	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28874 bytes --]

[PATCH 06/11] net: dsa: microchip: ksz8795: use phy_port_cnt where possible

[Michael Grzeschik]

Since the driver can be used on more switches it needs
to use flexible port count where ever possible.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v1: - based on "[PATCH v4 04/11] net: dsa: microchip: ksz8795: use port_cnt where possible"
    - lore: https://lore.kernel.org/netdev/20200803054442.20089-5-m.grzeschik@pengutronix.de/
---
 drivers/net/dsa/microchip/ksz8795.c     | 20 ++++++++++----------
 drivers/net/dsa/microchip/ksz8795_reg.h |  3 ---
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 6ddba2de2d3026e..7114902495a0ebb 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -418,8 +418,8 @@ static void ksz8795_r_vlan_entries(struct ksz_device *dev, u16 addr)
 	int i;
 
 	ksz8795_r_table(dev, TABLE_VLAN, addr, &data);
-	addr *= 4;
-	for (i = 0; i < 4; i++) {
+	addr *= dev->phy_port_cnt;
+	for (i = 0; i < dev->phy_port_cnt; i++) {
 		dev->vlan_cache[addr + i].table[0] = (u16)data;
 		data >>= VLAN_TABLE_S;
 	}
@@ -433,7 +433,7 @@ static void ksz8795_r_vlan_table(struct ksz_device *dev, u16 vid, u16 *vlan)
 	u64 buf;
 
 	data = (u16 *)&buf;
-	addr = vid / 4;
+	addr = vid / dev->phy_port_cnt;
 	index = vid & 3;
 	ksz8795_r_table(dev, TABLE_VLAN, addr, &buf);
 	*vlan = data[index];
@@ -447,7 +447,7 @@ static void ksz8795_w_vlan_table(struct ksz_device *dev, u16 vid, u16 vlan)
 	u64 buf;
 
 	data = (u16 *)&buf;
-	addr = vid / 4;
+	addr = vid / dev->phy_port_cnt;
 	index = vid & 3;
 	ksz8795_r_table(dev, TABLE_VLAN, addr, &buf);
 	data[index] = vlan;
@@ -691,12 +691,12 @@ static void ksz8795_port_stp_state_set(struct dsa_switch *ds, int port,
 	switch (state) {
 	case BR_STATE_DISABLED:
 		data |= PORT_LEARN_DISABLE;
-		if (port < SWITCH_PORT_NUM)
+		if (port < dev->phy_port_cnt)
 			member = 0;
 		break;
 	case BR_STATE_LISTENING:
 		data |= (PORT_RX_ENABLE | PORT_LEARN_DISABLE);
-		if (port < SWITCH_PORT_NUM &&
+		if (port < dev->phy_port_cnt &&
 		    p->stp_state == BR_STATE_DISABLED)
 			member = dev->host_mask | p->vid_member;
 		break;
@@ -720,7 +720,7 @@ static void ksz8795_port_stp_state_set(struct dsa_switch *ds, int port,
 		break;
 	case BR_STATE_BLOCKING:
 		data |= PORT_LEARN_DISABLE;
-		if (port < SWITCH_PORT_NUM &&
+		if (port < dev->phy_port_cnt &&
 		    p->stp_state == BR_STATE_DISABLED)
 			member = dev->host_mask | p->vid_member;
 		break;
@@ -1005,7 +1005,7 @@ static void ksz8795_config_cpu_port(struct dsa_switch *ds)
 	ksz8795_port_setup(dev, dev->cpu_port, true);
 	dev->member = dev->host_mask;
 
-	for (i = 0; i < SWITCH_PORT_NUM; i++) {
+	for (i = 0; i < dev->phy_port_cnt; i++) {
 		p = &dev->ports[i];
 
 		/* Initialize to non-zero so that ksz_cfg_port_member() will
@@ -1016,7 +1016,7 @@ static void ksz8795_config_cpu_port(struct dsa_switch *ds)
 		ksz8795_port_stp_state_set(ds, i, BR_STATE_DISABLED);
 
 		/* Last port may be disabled. */
-		if (i == dev->port_cnt)
+		if (i == dev->phy_port_cnt)
 			break;
 		p->on = 1;
 		p->phy = 1;
@@ -1239,7 +1239,7 @@ static int ksz8795_switch_init(struct ksz_device *dev)
 	dev->mib_cnt = ARRAY_SIZE(mib_names);
 
 	dev->mib_port_cnt = TOTAL_PORT_NUM;
-	dev->phy_port_cnt = SWITCH_PORT_NUM;
+	dev->phy_port_cnt = dev->port_cnt;
 
 	dev->cpu_port = dev->mib_port_cnt - 1;
 	dev->host_mask = BIT(dev->cpu_port);
diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h b/drivers/net/dsa/microchip/ksz8795_reg.h
index c131224850135bd..6377165a236fdf3 100644
--- a/drivers/net/dsa/microchip/ksz8795_reg.h
+++ b/drivers/net/dsa/microchip/ksz8795_reg.h
@@ -848,9 +848,6 @@
 
 #define TOTAL_PORT_NUM			5
 
-/* Host port can only be last of them. */
-#define SWITCH_PORT_NUM			(TOTAL_PORT_NUM - 1)
-
 #define KSZ8795_COUNTER_NUM		0x20
 
 /* Common names used by other drivers */
-- 
2.29.2

Re: [PATCH 06/11] net: dsa: microchip: ksz8795: use phy_port_cnt where possible

[Andrew Lunn]

>  	case BR_STATE_DISABLED:
>  		data |= PORT_LEARN_DISABLE;
> -		if (port < SWITCH_PORT_NUM)
> +		if (port < dev->phy_port_cnt)
>  			member = 0;
>  		break;

So this, unlike all the other patches so far, is not obviously
correct. What exactly does phy_port_cnt mean? Can there be ports
without PHYs? What if the PHYs are external? You still need to be able
to change the STP state of such ports.

Andrew

Re: [PATCH 06/11] net: dsa: microchip: ksz8795: use phy_port_cnt where possible

[Michael Grzeschik]

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

On Thu, Nov 19, 2020 at 01:29:15AM +0100, Andrew Lunn wrote:
>>  	case BR_STATE_DISABLED:
>>  		data |= PORT_LEARN_DISABLE;
>> -		if (port < SWITCH_PORT_NUM)
>> +		if (port < dev->phy_port_cnt)
>>  			member = 0;
>>  		break;
>
>So this, unlike all the other patches so far, is not obviously
>correct. What exactly does phy_port_cnt mean? Can there be ports
>without PHYs? What if the PHYs are external? You still need to be able
>to change the STP state of such ports.

The variable phy_port_cnt is referring to all external physical
available ports, that are not connected internally to the cpu.

That means that the previous code path was already broken, when stp
handling should include the cpu_port.

Michael

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

Re: [PATCH 06/11] net: dsa: microchip: ksz8795: use phy_port_cnt where possible

[Andrew Lunn]

On Thu, Nov 19, 2020 at 09:40:05AM +0100, Michael Grzeschik wrote:
> On Thu, Nov 19, 2020 at 01:29:15AM +0100, Andrew Lunn wrote:
> > >  	case BR_STATE_DISABLED:
> > >  		data |= PORT_LEARN_DISABLE;
> > > -		if (port < SWITCH_PORT_NUM)
> > > +		if (port < dev->phy_port_cnt)
> > >  			member = 0;
> > >  		break;
> > 
> > So this, unlike all the other patches so far, is not obviously
> > correct. What exactly does phy_port_cnt mean? Can there be ports
> > without PHYs? What if the PHYs are external? You still need to be able
> > to change the STP state of such ports.
> 
> The variable phy_port_cnt is referring to all external physical
> available ports, that are not connected internally to the cpu.
> 
> That means that the previous code path was already broken, when stp
> handling should include the cpu_port.

So using DSA names, it is the number of user ports. And the assumption
is, the last port is the CPU port.

Please add to the commit message that this patch fixes the code as
well. That sort of comment helps the reviewer understand why it is not
just an obvious mechanical change.

	Andrew

Re: [PATCH 06/11] net: dsa: microchip: ksz8795: use phy_port_cnt where possible

[Florian Fainelli]

On 11/18/2020 2:03 PM, Michael Grzeschik wrote:
> Since the driver can be used on more switches it needs
> to use flexible port count where ever possible.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH 07/11] net: dsa: microchip: remove superfluous num_ports asignment

[Michael Grzeschik]

The variable num_ports is already assigned in the init function.
This patch removes the extra assignment of the variable.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz8795.c | 2 --
 drivers/net/dsa/microchip/ksz9477.c | 2 --
 2 files changed, 4 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 7114902495a0ebb..17dc720df2340b0 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -992,8 +992,6 @@ static void ksz8795_config_cpu_port(struct dsa_switch *ds)
 	u8 remote;
 	int i;
 
-	ds->num_ports = dev->port_cnt + 1;
-
 	/* Switch marks the maximum frame with extra byte as oversize. */
 	ksz_cfg(dev, REG_SW_CTRL_2, SW_LEGAL_PACKET_DISABLE, true);
 	ksz_cfg(dev, S_TAIL_TAG_CTRL, SW_TAIL_TAG_ENABLE, true);
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index abfd3802bb51706..2119965da10ae1e 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1267,8 +1267,6 @@ static void ksz9477_config_cpu_port(struct dsa_switch *ds)
 	struct ksz_port *p;
 	int i;
 
-	ds->num_ports = dev->port_cnt;
-
 	for (i = 0; i < dev->port_cnt; i++) {
 		if (dsa_is_cpu_port(ds, i) && (dev->cpu_ports & (1 << i))) {
 			phy_interface_t interface;
-- 
2.29.2

Re: [PATCH 07/11] net: dsa: microchip: remove superfluous num_ports asignment

[Andrew Lunn]

On Wed, Nov 18, 2020 at 11:03:53PM +0100, Michael Grzeschik wrote:
> The variable num_ports is already assigned in the init function.
> This patch removes the extra assignment of the variable.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/net/dsa/microchip/ksz8795.c | 2 --
>  drivers/net/dsa/microchip/ksz9477.c | 2 --
>  2 files changed, 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> index 7114902495a0ebb..17dc720df2340b0 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -992,8 +992,6 @@ static void ksz8795_config_cpu_port(struct dsa_switch *ds)
>  	u8 remote;
>  	int i;
>  
> -	ds->num_ports = dev->port_cnt + 1;
> -
>  	/* Switch marks the maximum frame with extra byte as oversize. */
>  	ksz_cfg(dev, REG_SW_CTRL_2, SW_LEGAL_PACKET_DISABLE, true);
>  	ksz_cfg(dev, S_TAIL_TAG_CTRL, SW_TAIL_TAG_ENABLE, true);
> diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
> index abfd3802bb51706..2119965da10ae1e 100644
> --- a/drivers/net/dsa/microchip/ksz9477.c
> +++ b/drivers/net/dsa/microchip/ksz9477.c
> @@ -1267,8 +1267,6 @@ static void ksz9477_config_cpu_port(struct dsa_switch *ds)
>  	struct ksz_port *p;
>  	int i;
>  
> -	ds->num_ports = dev->port_cnt;

Please could you give a clue in the commit message that the init
function handles the difference between these two, the + 1 in ksz8795.

	 Andrew

Re: [PATCH 07/11] net: dsa: microchip: remove superfluous num_ports asignment

[Florian Fainelli]

On 11/18/2020 2:03 PM, Michael Grzeschik wrote:
> The variable num_ports is already assigned in the init function.
> This patch removes the extra assignment of the variable.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH 08/11] net: dsa: microchip: ksz8795: align port_cnt usage with other microchip drivers

[Michael Grzeschik]

The ksz8795 driver is using port_cnt differently to the other microchip
DSA drivers. It sets it to the external physical port count, than the
whole port count (including the cpu port). This patch is aligning the
variables purpose with the other microchip drivers.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz8795.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 17dc720df2340b0..10c9b301833dd59 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -1183,7 +1183,7 @@ static const struct ksz_chip_data ksz8795_switch_chips[] = {
 		.num_alus = 0,
 		.num_statics = 8,
 		.cpu_ports = 0x10,	/* can be configured as cpu port */
-		.port_cnt = 4,		/* total physical port count */
+		.port_cnt = 5,
 	},
 	{
 		.chip_id = 0x8794,
@@ -1192,7 +1192,7 @@ static const struct ksz_chip_data ksz8795_switch_chips[] = {
 		.num_alus = 0,
 		.num_statics = 8,
 		.cpu_ports = 0x10,	/* can be configured as cpu port */
-		.port_cnt = 3,		/* total physical port count */
+		.port_cnt = 4,
 	},
 	{
 		.chip_id = 0x8765,
@@ -1201,7 +1201,7 @@ static const struct ksz_chip_data ksz8795_switch_chips[] = {
 		.num_alus = 0,
 		.num_statics = 8,
 		.cpu_ports = 0x10,	/* can be configured as cpu port */
-		.port_cnt = 4,		/* total physical port count */
+		.port_cnt = 5,
 	},
 };
 
@@ -1237,7 +1237,7 @@ static int ksz8795_switch_init(struct ksz_device *dev)
 	dev->mib_cnt = ARRAY_SIZE(mib_names);
 
 	dev->mib_port_cnt = TOTAL_PORT_NUM;
-	dev->phy_port_cnt = dev->port_cnt;
+	dev->phy_port_cnt = dev->port_cnt - 1;
 
 	dev->cpu_port = dev->mib_port_cnt - 1;
 	dev->host_mask = BIT(dev->cpu_port);
@@ -1259,7 +1259,7 @@ static int ksz8795_switch_init(struct ksz_device *dev)
 	}
 
 	/* set the real number of ports */
-	dev->ds->num_ports = dev->port_cnt + 1;
+	dev->ds->num_ports = dev->port_cnt;
 
 	return 0;
 }
-- 
2.29.2

Re: [PATCH 08/11] net: dsa: microchip: ksz8795: align port_cnt usage with other microchip drivers

[Andrew Lunn]

On Wed, Nov 18, 2020 at 11:03:54PM +0100, Michael Grzeschik wrote:
> The ksz8795 driver is using port_cnt differently to the other microchip
> DSA drivers. It sets it to the external physical port count, than the
> whole port count (including the cpu port). This patch is aligning the
> variables purpose with the other microchip drivers.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/net/dsa/microchip/ksz8795.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> index 17dc720df2340b0..10c9b301833dd59 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -1183,7 +1183,7 @@ static const struct ksz_chip_data ksz8795_switch_chips[] = {
>  		.num_alus = 0,
>  		.num_statics = 8,
>  		.cpu_ports = 0x10,	/* can be configured as cpu port */
> -		.port_cnt = 4,		/* total physical port count */
> +		.port_cnt = 5,

Rather than remove the comment, please could you update the
comment. port_cnt is too generic to know its exact meaning without a
helpful comment. And this might be why this driver is different...

	Andrew

RE: [PATCH 08/11] net: dsa: microchip: ksz8795: align port_cnt usage with other microchip drivers

[Tristram.Ha]

> On Wed, Nov 18, 2020 at 11:03:54PM +0100, Michael Grzeschik wrote:
> > The ksz8795 driver is using port_cnt differently to the other microchip
> > DSA drivers. It sets it to the external physical port count, than the
> > whole port count (including the cpu port). This patch is aligning the
> > variables purpose with the other microchip drivers.
> >
> > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> > ---
> >  drivers/net/dsa/microchip/ksz8795.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/dsa/microchip/ksz8795.c
> b/drivers/net/dsa/microchip/ksz8795.c
> > index 17dc720df2340b0..10c9b301833dd59 100644
> > --- a/drivers/net/dsa/microchip/ksz8795.c
> > +++ b/drivers/net/dsa/microchip/ksz8795.c
> > @@ -1183,7 +1183,7 @@ static const struct ksz_chip_data
> ksz8795_switch_chips[] = {
> >               .num_alus = 0,
> >               .num_statics = 8,
> >               .cpu_ports = 0x10,      /* can be configured as cpu port */
> > -             .port_cnt = 4,          /* total physical port count */
> > +             .port_cnt = 5,
> 
> Rather than remove the comment, please could you update the
> comment. port_cnt is too generic to know its exact meaning without a
> helpful comment. And this might be why this driver is different...

At one time there are 3 distinctions of the ports used in the drivers for KSZ switches.
Physical ports require valid link to operate.  They are the usual ports users interact with.
The total port count is usually physical port count + 1.  The last port is the host port.
They all have the usual port controls like receive, transmit, QoS, and other functions.
That last port may not have MIB counters.  That is why another variable is used to
manage handling of MIB counters in a loop.

The KSZ9477/KSZ9897 family of switches is a new design where any port can be a host port.
It also has extra RGMII/SGMII port that makes the term "physical port" ambiguous.

KSZ8795 has 5 ports.  The last is always the host port.
KSZ8794 has 3 physical ports, but the last port is still 5.  Port 4 is disabled.

There is another KSZ8895 switch which also has 5 ports.  It has a variation KSZ8864 which
disables the first port.

Now the DSA layer treats each port individually and there is less use of a loop of ports
Inside the switch driver it is good to consolidate those port variables.

Re: [PATCH 08/11] net: dsa: microchip: ksz8795: align port_cnt usage with other microchip drivers

[Florian Fainelli]

On 11/18/2020 2:03 PM, Michael Grzeschik wrote:
> The ksz8795 driver is using port_cnt differently to the other microchip
> DSA drivers. It sets it to the external physical port count, than the
> whole port count (including the cpu port). This patch is aligning the
> variables purpose with the other microchip drivers.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

With Andrew's comment addressed on clarifying the intent of port_cnt:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH 09/11] net: dsa: microchip: remove usage of mib_port_count

[Michael Grzeschik]

The variable mib_port_cnt has the same meaning as port_cnt.
This driver removes the extra variable and is using port_cnt
everywhere instead.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz8795.c    | 11 +++++------
 drivers/net/dsa/microchip/ksz9477.c    | 12 +++++-------
 drivers/net/dsa/microchip/ksz_common.c |  4 ++--
 drivers/net/dsa/microchip/ksz_common.h |  1 -
 4 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 10c9b301833dd59..9ea5ec61513023f 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -760,7 +760,7 @@ static void ksz8795_flush_dyn_mac_table(struct ksz_device *dev, int port)
 	} else {
 		/* Flush all ports. */
 		first = 0;
-		cnt = dev->mib_port_cnt;
+		cnt = dev->port_cnt;
 	}
 	for (index = first; index < cnt; index++) {
 		p = &dev->ports[index];
@@ -1236,18 +1236,17 @@ static int ksz8795_switch_init(struct ksz_device *dev)
 	dev->reg_mib_cnt = KSZ8795_COUNTER_NUM;
 	dev->mib_cnt = ARRAY_SIZE(mib_names);
 
-	dev->mib_port_cnt = TOTAL_PORT_NUM;
 	dev->phy_port_cnt = dev->port_cnt - 1;
 
-	dev->cpu_port = dev->mib_port_cnt - 1;
+	dev->cpu_port = dev->port_cnt - 1;
 	dev->host_mask = BIT(dev->cpu_port);
 
-	i = dev->mib_port_cnt;
-	dev->ports = devm_kzalloc(dev->dev, sizeof(struct ksz_port) * i,
+	dev->ports = devm_kzalloc(dev->dev,
+				  dev->port_cnt * sizeof(struct ksz_port),
 				  GFP_KERNEL);
 	if (!dev->ports)
 		return -ENOMEM;
-	for (i = 0; i < dev->mib_port_cnt; i++) {
+	for (i = 0; i < dev->port_cnt; i++) {
 		mutex_init(&dev->ports[i].mib.cnt_mutex);
 		dev->ports[i].mib.counters =
 			devm_kzalloc(dev->dev,
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 2119965da10ae1e..42e647b67abd500 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -478,7 +478,7 @@ static void ksz9477_flush_dyn_mac_table(struct ksz_device *dev, int port)
 			   SW_FLUSH_OPTION_M << SW_FLUSH_OPTION_S,
 			   SW_FLUSH_OPTION_DYN_MAC << SW_FLUSH_OPTION_S);
 
-	if (port < dev->mib_port_cnt) {
+	if (port < dev->port_cnt) {
 		/* flush individual port */
 		ksz_pread8(dev, port, P_STP_CTRL, &data);
 		if (!(data & PORT_LEARN_DISABLE))
@@ -1317,7 +1317,7 @@ static void ksz9477_config_cpu_port(struct dsa_switch *ds)
 
 	dev->member = dev->host_mask;
 
-	for (i = 0; i < dev->mib_port_cnt; i++) {
+	for (i = 0; i < dev->port_cnt; i++) {
 		if (i == dev->cpu_port)
 			continue;
 		p = &dev->ports[i];
@@ -1444,7 +1444,6 @@ static int ksz9477_switch_detect(struct ksz_device *dev)
 		return ret;
 
 	/* Number of ports can be reduced depending on chip. */
-	dev->mib_port_cnt = TOTAL_PORT_NUM;
 	dev->phy_port_cnt = 5;
 
 	/* Default capability is gigabit capable. */
@@ -1461,7 +1460,6 @@ static int ksz9477_switch_detect(struct ksz_device *dev)
 		/* Chip does not support gigabit. */
 		if (data8 & SW_QW_ABLE)
 			dev->features &= ~GBIT_SUPPORT;
-		dev->mib_port_cnt = 3;
 		dev->phy_port_cnt = 2;
 	} else {
 		dev_info(dev->dev, "Found KSZ9477 or compatible\n");
@@ -1564,12 +1562,12 @@ static int ksz9477_switch_init(struct ksz_device *dev)
 	dev->reg_mib_cnt = SWITCH_COUNTER_NUM;
 	dev->mib_cnt = TOTAL_SWITCH_COUNTER_NUM;
 
-	i = dev->mib_port_cnt;
-	dev->ports = devm_kzalloc(dev->dev, sizeof(struct ksz_port) * i,
+	dev->ports = devm_kzalloc(dev->dev,
+				  dev->port_cnt * sizeof(struct ksz_port),
 				  GFP_KERNEL);
 	if (!dev->ports)
 		return -ENOMEM;
-	for (i = 0; i < dev->mib_port_cnt; i++) {
+	for (i = 0; i < dev->port_cnt; i++) {
 		mutex_init(&dev->ports[i].mib.cnt_mutex);
 		dev->ports[i].mib.counters =
 			devm_kzalloc(dev->dev,
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 0ef854911f215fa..566edfe1323ee69 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -72,7 +72,7 @@ static void ksz_mib_read_work(struct work_struct *work)
 	struct ksz_port *p;
 	int i;
 
-	for (i = 0; i < dev->mib_port_cnt; i++) {
+	for (i = 0; i < dev->port_cnt; i++) {
 		if (dsa_is_unused_port(dev->ds, i))
 			continue;
 
@@ -103,7 +103,7 @@ void ksz_init_mib_timer(struct ksz_device *dev)
 
 	INIT_DELAYED_WORK(&dev->mib_read, ksz_mib_read_work);
 
-	for (i = 0; i < dev->mib_port_cnt; i++)
+	for (i = 0; i < dev->port_cnt; i++)
 		dev->dev_ops->port_init_cnt(dev, i);
 }
 EXPORT_SYMBOL_GPL(ksz_init_mib_timer);
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 629700a63702435..720f22275c84448 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -71,7 +71,6 @@ struct ksz_device {
 	int port_cnt;
 	int reg_mib_cnt;
 	int mib_cnt;
-	int mib_port_cnt;
 	phy_interface_t compat_interface;
 	u32 regs_size;
 	bool phy_errata_9477;
-- 
2.29.2

Re: [PATCH 09/11] net: dsa: microchip: remove usage of mib_port_count

[Andrew Lunn]

On Wed, Nov 18, 2020 at 11:03:55PM +0100, Michael Grzeschik wrote:
> The variable mib_port_cnt has the same meaning as port_cnt.
> This driver removes the extra variable and is using port_cnt
> everywhere instead.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

Nice to see another poorly defined variable disappear.

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

    Andrew

Re: [PATCH 09/11] net: dsa: microchip: remove usage of mib_port_count

[Florian Fainelli]

On 11/18/2020 2:03 PM, Michael Grzeschik wrote:
> The variable mib_port_cnt has the same meaning as port_cnt.
> This driver removes the extra variable and is using port_cnt
> everywhere instead.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH 10/11] net: dsa: microchip: ksz8795: dynamic allocate memory for flush_dyn_mac_table

[Michael Grzeschik]

To get the driver working with other chips using different port counts
the dyn_mac_table should be flushed depending on the amount of physical
ports.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v1: - based on "[PATCH v4 05/11] net: dsa: microchip: ksz8795: dynamica allocate memory for flush_dyn_mac_table"
    - lore: https://lore.kernel.org/netdev/20200803054442.20089-6-m.grzeschik@pengutronix.de/
---
 drivers/net/dsa/microchip/ksz8795.c     | 8 ++++++--
 drivers/net/dsa/microchip/ksz8795_reg.h | 2 --
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 9ea5ec61513023f..418f71e5b90761c 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -750,11 +750,14 @@ static void ksz8795_port_stp_state_set(struct dsa_switch *ds, int port,
 
 static void ksz8795_flush_dyn_mac_table(struct ksz_device *dev, int port)
 {
-	u8 learn[TOTAL_PORT_NUM];
 	int first, index, cnt;
 	struct ksz_port *p;
+	u8 *learn = kzalloc(dev->port_cnt, GFP_KERNEL);
 
-	if ((uint)port < TOTAL_PORT_NUM) {
+	if (!learn)
+		return;
+
+	if ((uint)port < dev->port_cnt) {
 		first = port;
 		cnt = port + 1;
 	} else {
@@ -779,6 +782,7 @@ static void ksz8795_flush_dyn_mac_table(struct ksz_device *dev, int port)
 		if (!(learn[index] & PORT_LEARN_DISABLE))
 			ksz_pwrite8(dev, index, P_STP_CTRL, learn[index]);
 	}
+	kfree(learn);
 }
 
 static int ksz8795_port_vlan_filtering(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h b/drivers/net/dsa/microchip/ksz8795_reg.h
index 6377165a236fdf3..681d19ab27b89da 100644
--- a/drivers/net/dsa/microchip/ksz8795_reg.h
+++ b/drivers/net/dsa/microchip/ksz8795_reg.h
@@ -846,8 +846,6 @@
 
 #define KS_PRIO_IN_REG			4
 
-#define TOTAL_PORT_NUM			5
-
 #define KSZ8795_COUNTER_NUM		0x20
 
 /* Common names used by other drivers */
-- 
2.29.2

Re: [PATCH 10/11] net: dsa: microchip: ksz8795: dynamic allocate memory for flush_dyn_mac_table

[Andrew Lunn]

On Wed, Nov 18, 2020 at 11:03:56PM +0100, Michael Grzeschik wrote:
> To get the driver working with other chips using different port counts
> the dyn_mac_table should be flushed depending on the amount of physical
> ports.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> 
> ---
> v1: - based on "[PATCH v4 05/11] net: dsa: microchip: ksz8795: dynamica allocate memory for flush_dyn_mac_table"
>     - lore: https://lore.kernel.org/netdev/20200803054442.20089-6-m.grzeschik@pengutronix.de/
> ---
>  drivers/net/dsa/microchip/ksz8795.c     | 8 ++++++--
>  drivers/net/dsa/microchip/ksz8795_reg.h | 2 --
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> index 9ea5ec61513023f..418f71e5b90761c 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -750,11 +750,14 @@ static void ksz8795_port_stp_state_set(struct dsa_switch *ds, int port,
>  
>  static void ksz8795_flush_dyn_mac_table(struct ksz_device *dev, int port)
>  {
> -	u8 learn[TOTAL_PORT_NUM];
>  	int first, index, cnt;
>  	struct ksz_port *p;
> +	u8 *learn = kzalloc(dev->port_cnt, GFP_KERNEL);

Using DSA_MAX_PORTS makes things simpler.

      Andrew

Re: [PATCH 10/11] net: dsa: microchip: ksz8795: dynamic allocate memory for flush_dyn_mac_table

[Florian Fainelli]

On 11/18/2020 2:03 PM, Michael Grzeschik wrote:
> To get the driver working with other chips using different port counts
> the dyn_mac_table should be flushed depending on the amount of physical
> ports.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH 11/11] net: dsa: microchip: ksz8795: use num_vlans where possible

[Michael Grzeschik]

The value of the define VLAN_TABLE_ENTRIES can be derived from
num_vlans. This patch is using the variable num_vlans instead and
removes the extra define.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz8795.c     | 2 +-
 drivers/net/dsa/microchip/ksz8795_reg.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 418f71e5b90761c..ca44959b49126e3 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -1087,7 +1087,7 @@ static int ksz8795_setup(struct dsa_switch *ds)
 			   (BROADCAST_STORM_VALUE *
 			   BROADCAST_STORM_PROT_RATE) / 100);
 
-	for (i = 0; i < VLAN_TABLE_ENTRIES; i++)
+	for (i = 0; i < (dev->num_vlans / 4); i++)
 		ksz8795_r_vlan_entries(dev, i);
 
 	/* Setup STP address for STP operation. */
diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h b/drivers/net/dsa/microchip/ksz8795_reg.h
index 681d19ab27b89da..40372047d40d828 100644
--- a/drivers/net/dsa/microchip/ksz8795_reg.h
+++ b/drivers/net/dsa/microchip/ksz8795_reg.h
@@ -989,7 +989,6 @@
 #define TAIL_TAG_OVERRIDE		BIT(6)
 #define TAIL_TAG_LOOKUP			BIT(7)
 
-#define VLAN_TABLE_ENTRIES		(4096 / 4)
 #define FID_ENTRIES			128
 
 #endif
-- 
2.29.2

Re: [PATCH 11/11] net: dsa: microchip: ksz8795: use num_vlans where possible

[Andrew Lunn]

On Wed, Nov 18, 2020 at 11:03:57PM +0100, Michael Grzeschik wrote:
> The value of the define VLAN_TABLE_ENTRIES can be derived from
> num_vlans. This patch is using the variable num_vlans instead and
> removes the extra define.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

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

    Andrew

Re: [PATCH 11/11] net: dsa: microchip: ksz8795: use num_vlans where possible

[Florian Fainelli]

On 11/18/2020 2:03 PM, Michael Grzeschik wrote:
> The value of the define VLAN_TABLE_ENTRIES can be derived from
> num_vlans. This patch is using the variable num_vlans instead and
> removes the extra define.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian