2024-04-03 12:52:40

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v2 0/8] net: dsa: microchip: ksz8: refactor FDB dump path

Refactor FDB dump code path for Microchip KSZ8xxx series. This series
mostly makes some cosmetic reworks and allows to forward errors detected
by the regmap.

Change logs are part of patch commit messages.

Oleksij Rempel (8):
net: dsa: microchip: Remove unused FDB timestamp support in
ksz8_r_dyn_mac_table()
net: dsa: microchip: Make ksz8_r_dyn_mac_table() static
net: dsa: microchip: ksz8: Refactor ksz8_fdb_dump()
net: dsa: microchip: ksz8: Refactor ksz8_r_dyn_mac_table() for
readability
net: dsa: microchip: ksz8: Unify variable naming in
ksz8_r_dyn_mac_table()
net: dsa: microchip: ksz8_r_dyn_mac_table(): ksz: do not return EAGAIN
on timeout
net: dsa: microchip: ksz8_r_dyn_mac_table(): return read/write error
if we got any
net: dsa: microchip: ksz8_r_dyn_mac_table(): use entries variable to
signal 0 entries

drivers/net/dsa/microchip/ksz8.h | 2 -
drivers/net/dsa/microchip/ksz8795.c | 135 ++++++++++++------------
drivers/net/dsa/microchip/ksz8795_reg.h | 1 +
3 files changed, 69 insertions(+), 69 deletions(-)

--
2.39.2



2024-04-03 12:54:25

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v2 4/8] net: dsa: microchip: ksz8: Refactor ksz8_r_dyn_mac_table() for readability

Move the code out of a long if statement scope in ksz8_r_dyn_mac_table()
to improve code readability.

Reviewed-by: Vladimir Oltean <[email protected]>
Acked-by: Arun Ramadoss <[email protected]>
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 60 +++++++++++++++--------------
1 file changed, 31 insertions(+), 29 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index c93eb351ab3d5..d258fb607b4af 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -416,7 +416,9 @@ static int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
const u32 *masks;
const u16 *regs;
u16 ctrl_addr;
+ u64 buf = 0;
u8 data;
+ int cnt;
int rc;

shifts = dev->info->shifts;
@@ -432,38 +434,38 @@ static int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
if (rc == -EAGAIN) {
if (addr == 0)
*entries = 0;
+ goto unlock_alu;
} else if (rc == -ENXIO) {
*entries = 0;
- /* At least one valid entry in the table. */
- } else {
- u64 buf = 0;
- int cnt;
-
- ksz_read64(dev, regs[REG_IND_DATA_HI], &buf);
- data_hi = (u32)(buf >> 32);
- data_lo = (u32)buf;
-
- /* Check out how many valid entry in the table. */
- cnt = data & masks[DYNAMIC_MAC_TABLE_ENTRIES_H];
- cnt <<= shifts[DYNAMIC_MAC_ENTRIES_H];
- cnt |= (data_hi & masks[DYNAMIC_MAC_TABLE_ENTRIES]) >>
- shifts[DYNAMIC_MAC_ENTRIES];
- *entries = cnt + 1;
-
- *fid = (data_hi & masks[DYNAMIC_MAC_TABLE_FID]) >>
- shifts[DYNAMIC_MAC_FID];
- *src_port = (data_hi & masks[DYNAMIC_MAC_TABLE_SRC_PORT]) >>
- shifts[DYNAMIC_MAC_SRC_PORT];
-
- mac_addr[5] = (u8)data_lo;
- mac_addr[4] = (u8)(data_lo >> 8);
- mac_addr[3] = (u8)(data_lo >> 16);
- mac_addr[2] = (u8)(data_lo >> 24);
-
- mac_addr[1] = (u8)data_hi;
- mac_addr[0] = (u8)(data_hi >> 8);
- rc = 0;
+ goto unlock_alu;
}
+
+ ksz_read64(dev, regs[REG_IND_DATA_HI], &buf);
+ data_hi = (u32)(buf >> 32);
+ data_lo = (u32)buf;
+
+ /* Check out how many valid entry in the table. */
+ cnt = data & masks[DYNAMIC_MAC_TABLE_ENTRIES_H];
+ cnt <<= shifts[DYNAMIC_MAC_ENTRIES_H];
+ cnt |= (data_hi & masks[DYNAMIC_MAC_TABLE_ENTRIES]) >>
+ shifts[DYNAMIC_MAC_ENTRIES];
+ *entries = cnt + 1;
+
+ *fid = (data_hi & masks[DYNAMIC_MAC_TABLE_FID]) >>
+ shifts[DYNAMIC_MAC_FID];
+ *src_port = (data_hi & masks[DYNAMIC_MAC_TABLE_SRC_PORT]) >>
+ shifts[DYNAMIC_MAC_SRC_PORT];
+
+ mac_addr[5] = (u8)data_lo;
+ mac_addr[4] = (u8)(data_lo >> 8);
+ mac_addr[3] = (u8)(data_lo >> 16);
+ mac_addr[2] = (u8)(data_lo >> 24);
+
+ mac_addr[1] = (u8)data_hi;
+ mac_addr[0] = (u8)(data_hi >> 8);
+ rc = 0;
+
+unlock_alu:
mutex_unlock(&dev->alu_mutex);

return rc;
--
2.39.2


2024-04-03 12:54:25

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v2 2/8] net: dsa: microchip: Make ksz8_r_dyn_mac_table() static

ksz8_r_dyn_mac_table() is not used outside the source file. Make it
static.

Reviewed-by: Vladimir Oltean <[email protected]>
Acked-by: Arun Ramadoss <[email protected]>
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8.h | 2 --
drivers/net/dsa/microchip/ksz8795.c | 4 ++--
2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8.h b/drivers/net/dsa/microchip/ksz8.h
index 5b38cbb7b058b..571c26ce71e47 100644
--- a/drivers/net/dsa/microchip/ksz8.h
+++ b/drivers/net/dsa/microchip/ksz8.h
@@ -19,8 +19,6 @@ void ksz8_flush_dyn_mac_table(struct ksz_device *dev, int port);
void ksz8_port_setup(struct ksz_device *dev, int port, bool cpu_port);
int ksz8_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val);
int ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val);
-int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
- u8 *fid, u8 *src_port, u16 *entries);
void ksz8_r_mib_cnt(struct ksz_device *dev, int port, u16 addr, u64 *cnt);
void ksz8_r_mib_pkt(struct ksz_device *dev, int port, u16 addr,
u64 *dropped, u64 *cnt);
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index f59a03e6981d2..e407111db6637 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -408,8 +408,8 @@ static int ksz8_valid_dyn_entry(struct ksz_device *dev, u8 *data)
return 0;
}

-int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
- u8 *fid, u8 *src_port, u16 *entries)
+static int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
+ u8 *fid, u8 *src_port, u16 *entries)
{
u32 data_hi, data_lo;
const u8 *shifts;
--
2.39.2


2024-04-03 13:01:24

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH net-next v2 2/8] net: dsa: microchip: Make ksz8_r_dyn_mac_table() static



On 4/3/2024 5:50 AM, Oleksij Rempel wrote:
> ksz8_r_dyn_mac_table() is not used outside the source file. Make it
> static.
>
> Reviewed-by: Vladimir Oltean <[email protected]>
> Acked-by: Arun Ramadoss <[email protected]>
> Signed-off-by: Oleksij Rempel <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2024-04-03 13:04:40

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH net-next v2 4/8] net: dsa: microchip: ksz8: Refactor ksz8_r_dyn_mac_table() for readability



On 4/3/2024 5:50 AM, Oleksij Rempel wrote:
> Move the code out of a long if statement scope in ksz8_r_dyn_mac_table()
> to improve code readability.
>
> Reviewed-by: Vladimir Oltean <[email protected]>
> Acked-by: Arun Ramadoss <[email protected]>
> Signed-off-by: Oleksij Rempel <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2024-04-03 13:04:51

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v2 6/8] net: dsa: microchip: ksz8_r_dyn_mac_table(): ksz: do not return EAGAIN on timeout

EAGAIN was not used by previous code and not used by current code. So,
remove it and use proper error value.

Reviewed-by: Vladimir Oltean <[email protected]>
Acked-by: Arun Ramadoss <[email protected]>
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 5765d23bc6edc..eeb32df0939d1 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -396,7 +396,7 @@ static int ksz8_valid_dyn_entry(struct ksz_device *dev, u8 *data)

/* Entry is not ready for accessing. */
if (*data & masks[DYNAMIC_MAC_TABLE_NOT_READY]) {
- return -EAGAIN;
+ return -ETIMEDOUT;
/* Entry is ready for accessing. */
} else {
ksz_read8(dev, regs[REG_IND_DATA_8], data);
@@ -431,15 +431,14 @@ static int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
ksz_write16(dev, regs[REG_IND_CTRL_0], ctrl_addr);

ret = ksz8_valid_dyn_entry(dev, &data);
- if (ret == -EAGAIN) {
- if (addr == 0)
- *entries = 0;
- goto unlock_alu;
- } else if (ret == -ENXIO) {
+ if (ret == -ENXIO) {
*entries = 0;
goto unlock_alu;
}

+ if (ret)
+ goto unlock_alu;
+
ksz_read64(dev, regs[REG_IND_DATA_HI], &buf);
data_hi = (u32)(buf >> 32);
data_lo = (u32)buf;
--
2.39.2


2024-04-03 13:06:35

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v2 8/8] net: dsa: microchip: ksz8_r_dyn_mac_table(): use entries variable to signal 0 entries

We already have a variable to provide number of entries. So use it,
instead of using error number.

Reviewed-by: Vladimir Oltean <[email protected]>
Acked-by: Arun Ramadoss <[email protected]>
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index de3d8357da855..ecef6f6f830b3 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -399,19 +399,11 @@ static int ksz8_valid_dyn_entry(struct ksz_device *dev, u8 *data)
} while ((*data & masks[DYNAMIC_MAC_TABLE_NOT_READY]) && timeout);

/* Entry is not ready for accessing. */
- if (*data & masks[DYNAMIC_MAC_TABLE_NOT_READY]) {
+ if (*data & masks[DYNAMIC_MAC_TABLE_NOT_READY])
return -ETIMEDOUT;
- /* Entry is ready for accessing. */
- } else {
- ret = ksz_read8(dev, regs[REG_IND_DATA_8], data);
- if (ret)
- return ret;

- /* There is no valid entry in the table. */
- if (*data & masks[DYNAMIC_MAC_TABLE_MAC_EMPTY])
- return -ENXIO;
- }
- return 0;
+ /* Entry is ready for accessing. */
+ return ksz_read8(dev, regs[REG_IND_DATA_8], data);
}

static int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
@@ -439,13 +431,13 @@ static int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
goto unlock_alu;

ret = ksz8_valid_dyn_entry(dev, &data);
- if (ret == -ENXIO) {
- *entries = 0;
+ if (ret)
goto unlock_alu;
- }

- if (ret)
+ if (data & masks[DYNAMIC_MAC_TABLE_MAC_EMPTY]) {
+ *entries = 0;
goto unlock_alu;
+ }

ret = ksz_read64(dev, regs[REG_IND_DATA_HI], &buf);
if (ret)
@@ -1210,8 +1202,6 @@ int ksz8_fdb_dump(struct ksz_device *dev, int port,
for (i = 0; i < KSZ8_DYN_MAC_ENTRIES; i++) {
ret = ksz8_r_dyn_mac_table(dev, i, mac, &fid, &src_port,
&entries);
- if (ret == -ENXIO)
- return 0;
if (ret)
return ret;

--
2.39.2


2024-04-05 02:40:52

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/8] net: dsa: microchip: ksz8: refactor FDB dump path

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <[email protected]>:

On Wed, 3 Apr 2024 14:50:31 +0200 you wrote:
> Refactor FDB dump code path for Microchip KSZ8xxx series. This series
> mostly makes some cosmetic reworks and allows to forward errors detected
> by the regmap.
>
> Change logs are part of patch commit messages.
>
> Oleksij Rempel (8):
> net: dsa: microchip: Remove unused FDB timestamp support in
> ksz8_r_dyn_mac_table()
> net: dsa: microchip: Make ksz8_r_dyn_mac_table() static
> net: dsa: microchip: ksz8: Refactor ksz8_fdb_dump()
> net: dsa: microchip: ksz8: Refactor ksz8_r_dyn_mac_table() for
> readability
> net: dsa: microchip: ksz8: Unify variable naming in
> ksz8_r_dyn_mac_table()
> net: dsa: microchip: ksz8_r_dyn_mac_table(): ksz: do not return EAGAIN
> on timeout
> net: dsa: microchip: ksz8_r_dyn_mac_table(): return read/write error
> if we got any
> net: dsa: microchip: ksz8_r_dyn_mac_table(): use entries variable to
> signal 0 entries
>
> [...]

Here is the summary with links:
- [net-next,v2,1/8] net: dsa: microchip: Remove unused FDB timestamp support in ksz8_r_dyn_mac_table()
https://git.kernel.org/netdev/net-next/c/a4a0306fbe1c
- [net-next,v2,2/8] net: dsa: microchip: Make ksz8_r_dyn_mac_table() static
https://git.kernel.org/netdev/net-next/c/860cf29eabf1
- [net-next,v2,3/8] net: dsa: microchip: ksz8: Refactor ksz8_fdb_dump()
https://git.kernel.org/netdev/net-next/c/0499bb09f412
- [net-next,v2,4/8] net: dsa: microchip: ksz8: Refactor ksz8_r_dyn_mac_table() for readability
https://git.kernel.org/netdev/net-next/c/5f1b7f838f5f
- [net-next,v2,5/8] net: dsa: microchip: ksz8: Unify variable naming in ksz8_r_dyn_mac_table()
https://git.kernel.org/netdev/net-next/c/08736e083e4b
- [net-next,v2,6/8] net: dsa: microchip: ksz8_r_dyn_mac_table(): ksz: do not return EAGAIN on timeout
https://git.kernel.org/netdev/net-next/c/081e84f267ad
- [net-next,v2,7/8] net: dsa: microchip: ksz8_r_dyn_mac_table(): return read/write error if we got any
https://git.kernel.org/netdev/net-next/c/7caed786b374
- [net-next,v2,8/8] net: dsa: microchip: ksz8_r_dyn_mac_table(): use entries variable to signal 0 entries
https://git.kernel.org/netdev/net-next/c/8d5758120dd2

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html