2022-11-28 12:23:38

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH v1 00/26] net: dsa: microchip: stats64, fdb, error

This patch series is a result of maintaining work on ksz8 part of
microchip driver. It includes stats64 and fdb support. Error handling.
Loopback fix and so on...

Oleksij Rempel (26):
net: dsa: microchip: add stats64 support for ksz8 series of switches
net: dsa: microchip: ksz8: ksz8_fdb_dump: fix port validation and VID
information
net: dsa: microchip: ksz8: ksz8_fdb_dump: fix not complete fdb
extraction
net: dsa: microchip: ksz8: ksz8_fdb_dump: fix time stamp extraction
net: dsa: microchip: ksz8: ksz8_fdb_dump: do not extract ghost entry
from empty table
net: dsa: microchip: ksz8863_smi: fix bulk access
net: dsa: microchip: ksz8_r_dyn_mac_table(): remove timestamp support
net: dsa: microchip: make ksz8_r_dyn_mac_table() static
net: dsa: microchip: ksz8_r_dyn_mac_table(): remove fid support
net: dsa: microchip: ksz8: refactor ksz8_fdb_dump()
net: dsa: microchip: ksz8: ksz8_fdb_dump: dump static MAC table
net: dsa: microchip: ksz8: move static mac table operations to a
separate functions
net: dsa: microchip: ksz8: add fdb_add/del support
net: dsa: microchip: KSZ88x3 fix loopback support
net: dsa: microchip: ksz8_r_dyn_mac_table(): move main part of the
code out of if statement
net: dsa: microchip: ksz8_r_dyn_mac_table(): use ret instead of rc
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
net: dsa: microchip: make ksz8_r_sta_mac_table() static
net: dsa: microchip: ksz8_r_sta_mac_table(): do not use error code for
empty entries
net: dsa: microchip: ksz8_r_sta_mac_table(): make use of error values
provided by read/write functions
net: dsa: microchip: make ksz8_w_sta_mac_table() static
net: dsa: microchip: ksz8_w_sta_mac_table(): make use of error values
provided by read/write functions
net: dsa: microchip: remove ksz_port:on variable
net: dsa: microchip: ksz8: do not force flow control by default

drivers/net/dsa/microchip/ksz8.h | 14 +-
drivers/net/dsa/microchip/ksz8795.c | 440 +++++++++++++++---------
drivers/net/dsa/microchip/ksz8795_reg.h | 2 +
drivers/net/dsa/microchip/ksz8863_smi.c | 10 +-
drivers/net/dsa/microchip/ksz_common.c | 100 +++++-
drivers/net/dsa/microchip/ksz_common.h | 2 +-
6 files changed, 377 insertions(+), 191 deletions(-)

--
2.30.2


2022-11-28 12:25:10

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH v1 15/26] net: dsa: microchip: ksz8_r_dyn_mac_table(): move main part of the code out of if statement

Start reworking error handling of this function

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 56 +++++++++++++++--------------
1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index a6d5de41a754..2674fd553851 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -402,7 +402,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;
@@ -418,36 +420,36 @@ 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;
-
- *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;
+
+ *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.30.2

2022-11-28 12:25:25

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH v1 18/26] net: dsa: microchip: ksz8_r_dyn_mac_table(): return read/write error if we got any

The read/write path may fail. So, return error if we got it.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 1b067e0cacd5..3348f5c25942 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -371,12 +371,16 @@ static int ksz8_valid_dyn_entry(struct ksz_device *dev, u8 *data)
int timeout = 100;
const u32 *masks;
const u16 *regs;
+ int ret;

masks = dev->info->masks;
regs = dev->info->regs;

do {
- ksz_read8(dev, regs[REG_IND_DATA_CHECK], data);
+ ret = ksz_read8(dev, regs[REG_IND_DATA_CHECK], data);
+ if (ret)
+ return ret;
+
timeout--;
} while ((*data & masks[DYNAMIC_MAC_TABLE_NOT_READY]) && timeout);

@@ -385,7 +389,9 @@ static int ksz8_valid_dyn_entry(struct ksz_device *dev, u8 *data)
return -ETIMEDOUT;
/* Entry is ready for accessing. */
} else {
- ksz_read8(dev, regs[REG_IND_DATA_8], data);
+ 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])
@@ -414,7 +420,9 @@ static int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
ctrl_addr = IND_ACC_TABLE(TABLE_DYNAMIC_MAC | TABLE_READ) | addr;

mutex_lock(&dev->alu_mutex);
- ksz_write16(dev, regs[REG_IND_CTRL_0], ctrl_addr);
+ ret = ksz_write16(dev, regs[REG_IND_CTRL_0], ctrl_addr);
+ if (ret)
+ goto unlock_alu;

ret = ksz8_valid_dyn_entry(dev, &data);
if (ret == -ENXIO) {
@@ -425,7 +433,10 @@ static int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
if (ret)
goto unlock_alu;

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

@@ -446,7 +457,6 @@ static int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,

mac_addr[1] = (u8)data_hi;
mac_addr[0] = (u8)(data_hi >> 8);
- ret = 0;

unlock_alu:
mutex_unlock(&dev->alu_mutex);
--
2.30.2

2022-11-28 12:41:06

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH v1 00/26] net: dsa: microchip: stats64, fdb, error

On Mon, Nov 28, 2022 at 01:00:08PM +0100, Oleksij Rempel wrote:

Sorry, I double send this series by accident. Please ignore the second
one.

> This patch series is a result of maintaining work on ksz8 part of
> microchip driver. It includes stats64 and fdb support. Error handling.
> Loopback fix and so on...
>
> Oleksij Rempel (26):
> net: dsa: microchip: add stats64 support for ksz8 series of switches
> net: dsa: microchip: ksz8: ksz8_fdb_dump: fix port validation and VID
> information
> net: dsa: microchip: ksz8: ksz8_fdb_dump: fix not complete fdb
> extraction
> net: dsa: microchip: ksz8: ksz8_fdb_dump: fix time stamp extraction
> net: dsa: microchip: ksz8: ksz8_fdb_dump: do not extract ghost entry
> from empty table
> net: dsa: microchip: ksz8863_smi: fix bulk access
> net: dsa: microchip: ksz8_r_dyn_mac_table(): remove timestamp support
> net: dsa: microchip: make ksz8_r_dyn_mac_table() static
> net: dsa: microchip: ksz8_r_dyn_mac_table(): remove fid support
> net: dsa: microchip: ksz8: refactor ksz8_fdb_dump()
> net: dsa: microchip: ksz8: ksz8_fdb_dump: dump static MAC table
> net: dsa: microchip: ksz8: move static mac table operations to a
> separate functions
> net: dsa: microchip: ksz8: add fdb_add/del support
> net: dsa: microchip: KSZ88x3 fix loopback support
> net: dsa: microchip: ksz8_r_dyn_mac_table(): move main part of the
> code out of if statement
> net: dsa: microchip: ksz8_r_dyn_mac_table(): use ret instead of rc
> 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
> net: dsa: microchip: make ksz8_r_sta_mac_table() static
> net: dsa: microchip: ksz8_r_sta_mac_table(): do not use error code for
> empty entries
> net: dsa: microchip: ksz8_r_sta_mac_table(): make use of error values
> provided by read/write functions
> net: dsa: microchip: make ksz8_w_sta_mac_table() static
> net: dsa: microchip: ksz8_w_sta_mac_table(): make use of error values
> provided by read/write functions
> net: dsa: microchip: remove ksz_port:on variable
> net: dsa: microchip: ksz8: do not force flow control by default
>
> drivers/net/dsa/microchip/ksz8.h | 14 +-
> drivers/net/dsa/microchip/ksz8795.c | 440 +++++++++++++++---------
> drivers/net/dsa/microchip/ksz8795_reg.h | 2 +
> drivers/net/dsa/microchip/ksz8863_smi.c | 10 +-
> drivers/net/dsa/microchip/ksz_common.c | 100 +++++-
> drivers/net/dsa/microchip/ksz_common.h | 2 +-
> 6 files changed, 377 insertions(+), 191 deletions(-)
>
> --
> 2.30.2
>
>

--
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 |

2022-11-28 12:44:13

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH v1 20/26] net: dsa: microchip: make ksz8_r_sta_mac_table() static

It is used only in ksz8795.c, no need to export it.

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 a28fa7cd4d98..ed72ec626593 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_sta_mac_table(struct ksz_device *dev, u16 addr,
- struct alu_struct *alu);
void ksz8_w_sta_mac_table(struct ksz_device *dev, u16 addr,
struct alu_struct *alu);
void ksz8_r_mib_cnt(struct ksz_device *dev, int port, u16 addr, u64 *cnt);
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 31c77e086a9d..1c08103c9f50 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -456,8 +456,8 @@ static int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
return ret;
}

-int ksz8_r_sta_mac_table(struct ksz_device *dev, u16 addr,
- struct alu_struct *alu)
+static int ksz8_r_sta_mac_table(struct ksz_device *dev, u16 addr,
+ struct alu_struct *alu)
{
u32 data_hi, data_lo;
const u8 *shifts;
--
2.30.2

2022-11-28 12:46:54

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH v1 13/26] net: dsa: microchip: ksz8: add fdb_add/del support

Provide access to the static MAC table over fdb_add/del callbacks.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8.h | 4 ++++
drivers/net/dsa/microchip/ksz8795.c | 12 ++++++++++++
drivers/net/dsa/microchip/ksz_common.c | 2 ++
3 files changed, 18 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz8.h b/drivers/net/dsa/microchip/ksz8.h
index 96d453af17c5..a28fa7cd4d98 100644
--- a/drivers/net/dsa/microchip/ksz8.h
+++ b/drivers/net/dsa/microchip/ksz8.h
@@ -30,6 +30,10 @@ void ksz8_freeze_mib(struct ksz_device *dev, int port, bool freeze);
void ksz8_port_init_cnt(struct ksz_device *dev, int port);
int ksz8_fdb_dump(struct ksz_device *dev, int port,
dsa_fdb_dump_cb_t *cb, void *data);
+int ksz8_fdb_add(struct ksz_device *dev, int port, const unsigned char *addr,
+ u16 vid, struct dsa_db db);
+int ksz8_fdb_del(struct ksz_device *dev, int port, const unsigned char *addr,
+ u16 vid, struct dsa_db db);
int ksz8_mdb_add(struct ksz_device *dev, int port,
const struct switchdev_obj_port_mdb *mdb, struct dsa_db db);
int ksz8_mdb_del(struct ksz_device *dev, int port,
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index d16dc8e5ed18..208cf4dde397 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -1077,6 +1077,18 @@ int ksz8_mdb_del(struct ksz_device *dev, int port,
return ksz8_del_sta_mac(dev, port, mdb->addr, mdb->vid);
}

+int ksz8_fdb_add(struct ksz_device *dev, int port, const unsigned char *addr,
+ u16 vid, struct dsa_db db)
+{
+ return ksz8_add_sta_mac(dev, port, addr, vid);
+}
+
+int ksz8_fdb_del(struct ksz_device *dev, int port, const unsigned char *addr,
+ u16 vid, struct dsa_db db)
+{
+ return ksz8_del_sta_mac(dev, port, addr, vid);
+}
+
int ksz8_port_vlan_filtering(struct ksz_device *dev, int port, bool flag,
struct netlink_ext_ack *extack)
{
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index afb846c18b57..171cb0063fbf 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -197,6 +197,8 @@ static const struct ksz_dev_ops ksz8_dev_ops = {
.freeze_mib = ksz8_freeze_mib,
.port_init_cnt = ksz8_port_init_cnt,
.fdb_dump = ksz8_fdb_dump,
+ .fdb_add = ksz8_fdb_add,
+ .fdb_del = ksz8_fdb_del,
.mdb_add = ksz8_mdb_add,
.mdb_del = ksz8_mdb_del,
.vlan_filtering = ksz8_port_vlan_filtering,
--
2.30.2

2022-11-28 12:47:33

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH v1 06/26] net: dsa: microchip: ksz8863_smi: fix bulk access

Current regmap bulk access is broken, resulting to wrong reads/writes
if ksz_read64/ksz_write64 functions are used.
Mostly this issue was visible by using ksz8_fdb_dump(), which returned
corrupt MAC address.

The reason is that regmap was configured to have max_raw_read/write,
even if ksz8863_mdio_read/write functions are able to handle unlimited
read/write accesses. On ksz_read64 function we are using multiple 32bit
accesses by incrementing each access by 1 instead of 4. Resulting buffer
had 01234567.12345678 instead of 01234567.89abcdef.

We have multiple ways to fix it:
- enable 4 byte alignment for 32bit accesses. Since the HW do not have
this requirement. It will break driver.
- disable max_raw_* limit.

This patch is removing max_raw_* limit for regmap accesses in ksz8863_smi.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8863_smi.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8863_smi.c b/drivers/net/dsa/microchip/ksz8863_smi.c
index 2f4623f3bd85..2516c9db7fec 100644
--- a/drivers/net/dsa/microchip/ksz8863_smi.c
+++ b/drivers/net/dsa/microchip/ksz8863_smi.c
@@ -70,6 +70,7 @@ static int ksz8863_mdio_write(void *ctx, const void *data, size_t count)
tmp, val[i]);
if (ret < 0)
goto out;
+
}

out:
@@ -82,22 +83,16 @@ static const struct regmap_bus regmap_smi[] = {
{
.read = ksz8863_mdio_read,
.write = ksz8863_mdio_write,
- .max_raw_read = 1,
- .max_raw_write = 1,
},
{
.read = ksz8863_mdio_read,
.write = ksz8863_mdio_write,
.val_format_endian_default = REGMAP_ENDIAN_BIG,
- .max_raw_read = 2,
- .max_raw_write = 2,
},
{
.read = ksz8863_mdio_read,
.write = ksz8863_mdio_write,
.val_format_endian_default = REGMAP_ENDIAN_BIG,
- .max_raw_read = 4,
- .max_raw_write = 4,
}
};

@@ -108,7 +103,6 @@ static const struct regmap_config ksz8863_regmap_config[] = {
.pad_bits = 24,
.val_bits = 8,
.cache_type = REGCACHE_NONE,
- .use_single_read = 1,
.lock = ksz_regmap_lock,
.unlock = ksz_regmap_unlock,
},
@@ -118,7 +112,6 @@ static const struct regmap_config ksz8863_regmap_config[] = {
.pad_bits = 24,
.val_bits = 16,
.cache_type = REGCACHE_NONE,
- .use_single_read = 1,
.lock = ksz_regmap_lock,
.unlock = ksz_regmap_unlock,
},
@@ -128,7 +121,6 @@ static const struct regmap_config ksz8863_regmap_config[] = {
.pad_bits = 24,
.val_bits = 32,
.cache_type = REGCACHE_NONE,
- .use_single_read = 1,
.lock = ksz_regmap_lock,
.unlock = ksz_regmap_unlock,
}
--
2.30.2