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
The only place where this variable would be set to false is the
ksz8_config_cpu_port() function. But it is done in a bogus way:
for (i = 0; i < dev->phy_port_cnt; i++) {
if (i == dev->phy_port_cnt) <--- will be never executed.
break;
p->on = 1;
So, we never have a situation where p->on = 0. In this case, we can just
remove it.
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 20 +-------------------
drivers/net/dsa/microchip/ksz_common.h | 1 -
2 files changed, 1 insertion(+), 20 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index ea08bdea193e..618366fadfb5 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -952,7 +952,6 @@ void ksz8_flush_dyn_mac_table(struct ksz_device *dev, int port)
{
u8 learn[DSA_MAX_PORTS];
int first, index, cnt;
- struct ksz_port *p;
const u16 *regs;
regs = dev->info->regs;
@@ -966,9 +965,6 @@ void ksz8_flush_dyn_mac_table(struct ksz_device *dev, int port)
cnt = dev->info->port_cnt;
}
for (index = first; index < cnt; index++) {
- p = &dev->ports[index];
- if (!p->on)
- continue;
ksz_pread8(dev, index, regs[P_STP_CTRL], &learn[index]);
if (!(learn[index] & PORT_LEARN_DISABLE))
ksz_pwrite8(dev, index, regs[P_STP_CTRL],
@@ -976,9 +972,6 @@ void ksz8_flush_dyn_mac_table(struct ksz_device *dev, int port)
}
ksz_cfg(dev, S_FLUSH_TABLE_CTRL, SW_FLUSH_DYN_MAC_TABLE, true);
for (index = first; index < cnt; index++) {
- p = &dev->ports[index];
- if (!p->on)
- continue;
if (!(learn[index] & PORT_LEARN_DISABLE))
ksz_pwrite8(dev, index, regs[P_STP_CTRL], learn[index]);
}
@@ -1368,25 +1361,14 @@ void ksz8_config_cpu_port(struct dsa_switch *ds)
ksz_cfg(dev, regs[S_TAIL_TAG_CTRL], masks[SW_TAIL_TAG_ENABLE], true);
- p = &dev->ports[dev->cpu_port];
- p->on = 1;
-
ksz8_port_setup(dev, dev->cpu_port, true);
for (i = 0; i < dev->phy_port_cnt; i++) {
- p = &dev->ports[i];
-
ksz_port_stp_state_set(ds, i, BR_STATE_DISABLED);
-
- /* Last port may be disabled. */
- if (i == dev->phy_port_cnt)
- break;
- p->on = 1;
}
for (i = 0; i < dev->phy_port_cnt; i++) {
p = &dev->ports[i];
- if (!p->on)
- continue;
+
if (!ksz_is_ksz88x3(dev)) {
ksz_pread8(dev, i, regs[P_REMOTE_STATUS], &remote);
if (remote & KSZ8_PORT_FIBER_MODE)
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 055d61ff3fb8..504ad07842a0 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -87,7 +87,6 @@ struct ksz_port {
int stp_state;
struct phy_device phydev;
- u32 on:1; /* port is not disabled by hardware */
u32 fiber:1; /* port is fiber */
u32 force:1;
u32 read:1; /* read MIB counters in background */
--
2.30.2
EAGAIN was not used by previous code and not used by current code. So,
remove it and use proper error value.
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 1677f61b138a..1b067e0cacd5 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -382,7 +382,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);
@@ -417,15 +417,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.30.2
Current ksz8_fdb_dump() is able to extract max 249 entries on
the ksz8863/ksz8873 series of switches. This happened due to wrong
bit mask and offset calculation.
With this patch, we will be able to extract all possible 1024 entries.
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 423f944cc34c..43d94131417b 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -398,10 +398,10 @@ static const u32 ksz8863_masks[] = {
[STATIC_MAC_TABLE_FID] = GENMASK(29, 26),
[STATIC_MAC_TABLE_OVERRIDE] = BIT(20),
[STATIC_MAC_TABLE_FWD_PORTS] = GENMASK(18, 16),
- [DYNAMIC_MAC_TABLE_ENTRIES_H] = GENMASK(5, 0),
+ [DYNAMIC_MAC_TABLE_ENTRIES_H] = GENMASK(1, 0),
[DYNAMIC_MAC_TABLE_MAC_EMPTY] = BIT(7),
[DYNAMIC_MAC_TABLE_NOT_READY] = BIT(7),
- [DYNAMIC_MAC_TABLE_ENTRIES] = GENMASK(31, 28),
+ [DYNAMIC_MAC_TABLE_ENTRIES] = GENMASK(31, 24),
[DYNAMIC_MAC_TABLE_FID] = GENMASK(19, 16),
[DYNAMIC_MAC_TABLE_SRC_PORT] = GENMASK(21, 20),
[DYNAMIC_MAC_TABLE_TIMESTAMP] = GENMASK(23, 22),
@@ -411,7 +411,7 @@ static u8 ksz8863_shifts[] = {
[VLAN_TABLE_MEMBERSHIP_S] = 16,
[STATIC_MAC_FWD_PORTS] = 16,
[STATIC_MAC_FID] = 22,
- [DYNAMIC_MAC_ENTRIES_H] = 3,
+ [DYNAMIC_MAC_ENTRIES_H] = 8,
[DYNAMIC_MAC_ENTRIES] = 24,
[DYNAMIC_MAC_FID] = 16,
[DYNAMIC_MAC_TIMESTAMP] = 24,
--
2.30.2
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
With current code loopback is not working and selftest will always fail.
Fix register and bit offsets to make loopback on KSZ88x3 switches.
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 24 ++++++++++++++++++------
drivers/net/dsa/microchip/ksz8795_reg.h | 1 +
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 208cf4dde397..a6d5de41a754 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -625,8 +625,13 @@ int ksz8_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
if (ret)
return ret;
- if (restart & PORT_PHY_LOOPBACK)
- data |= BMCR_LOOPBACK;
+ if (ksz_is_ksz88x3(dev)) {
+ if (restart & KSZ8873_PORT_PHY_LOOPBACK)
+ data |= BMCR_LOOPBACK;
+ } else {
+ if (restart & PORT_PHY_LOOPBACK)
+ data |= BMCR_LOOPBACK;
+ }
if (ctrl & PORT_FORCE_100_MBIT)
data |= BMCR_SPEED100;
if (ksz_is_ksz88x3(dev)) {
@@ -849,10 +854,17 @@ int ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
data |= PORT_FORCE_MDIX;
else
data &= ~PORT_FORCE_MDIX;
- if (val & BMCR_LOOPBACK)
- data |= PORT_PHY_LOOPBACK;
- else
- data &= ~PORT_PHY_LOOPBACK;
+ if (ksz_is_ksz88x3(dev)) {
+ if (val & BMCR_LOOPBACK)
+ data |= KSZ8873_PORT_PHY_LOOPBACK;
+ else
+ data &= ~KSZ8873_PORT_PHY_LOOPBACK;
+ } else {
+ if (val & BMCR_LOOPBACK)
+ data |= PORT_PHY_LOOPBACK;
+ else
+ data &= ~PORT_PHY_LOOPBACK;
+ }
if (data != restart) {
ret = ksz_pwrite8(dev, p, regs[P_NEG_RESTART_CTRL],
diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h b/drivers/net/dsa/microchip/ksz8795_reg.h
index 0bdceb534192..08204da7d621 100644
--- a/drivers/net/dsa/microchip/ksz8795_reg.h
+++ b/drivers/net/dsa/microchip/ksz8795_reg.h
@@ -262,6 +262,7 @@
#define PORT_AUTO_MDIX_DISABLE BIT(2)
#define PORT_FORCE_MDIX BIT(1)
#define PORT_MAC_LOOPBACK BIT(0)
+#define KSZ8873_PORT_PHY_LOOPBACK BIT(0)
#define REG_PORT_1_STATUS_2 0x1E
#define REG_PORT_2_STATUS_2 0x2E
--
2.30.2
Read/write operations may fail. So, make use of return values.
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 33 ++++++++++++++++-------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 9c1450782314..ea08bdea193e 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -358,19 +358,26 @@ static int ksz8_r_table(struct ksz_device *dev, int table, u16 addr, u64 *data)
return ret;
}
-static void ksz8_w_table(struct ksz_device *dev, int table, u16 addr, u64 data)
+static int ksz8_w_table(struct ksz_device *dev, int table, u16 addr, u64 data)
{
const u16 *regs;
u16 ctrl_addr;
+ int ret;
regs = dev->info->regs;
ctrl_addr = IND_ACC_TABLE(table) | addr;
mutex_lock(&dev->alu_mutex);
- ksz_write64(dev, regs[REG_IND_DATA_HI], data);
- ksz_write16(dev, regs[REG_IND_CTRL_0], ctrl_addr);
+ ret = ksz_write64(dev, regs[REG_IND_DATA_HI], data);
+ if (ret)
+ goto unlock_alu;
+
+ ret = ksz_write16(dev, regs[REG_IND_CTRL_0], ctrl_addr);
+unlock_alu:
mutex_unlock(&dev->alu_mutex);
+
+ return ret;
}
static int ksz8_valid_dyn_entry(struct ksz_device *dev, u8 *data)
@@ -509,8 +516,8 @@ static int ksz8_r_sta_mac_table(struct ksz_device *dev, u16 addr,
return 0;
}
-static void ksz8_w_sta_mac_table(struct ksz_device *dev, u16 addr,
- struct alu_struct *alu)
+static int ksz8_w_sta_mac_table(struct ksz_device *dev, u16 addr,
+ struct alu_struct *alu)
{
u32 data_hi, data_lo;
const u8 *shifts;
@@ -538,7 +545,8 @@ static void ksz8_w_sta_mac_table(struct ksz_device *dev, u16 addr,
data_hi &= ~masks[STATIC_MAC_TABLE_OVERRIDE];
data = (u64)data_hi << 32 | data_lo;
- ksz8_w_table(dev, TABLE_STATIC_MAC, addr, data);
+
+ return ksz8_w_table(dev, TABLE_STATIC_MAC, addr, data);
}
static void ksz8_from_vlan(struct ksz_device *dev, u32 vlan, u8 *fid,
@@ -1065,9 +1073,8 @@ static int ksz8_add_sta_mac(struct ksz_device *dev, int port,
/* Need a way to map VID to FID. */
alu.fid = vid;
}
- ksz8_w_sta_mac_table(dev, index, &alu);
- return 0;
+ return ksz8_w_sta_mac_table(dev, index, &alu);
}
static int ksz8_del_sta_mac(struct ksz_device *dev, int port,
@@ -1091,16 +1098,14 @@ static int ksz8_del_sta_mac(struct ksz_device *dev, int port,
/* no available entry */
if (index == dev->info->num_statics)
- goto exit;
+ return 0;
/* clear port */
alu.port_forward &= ~BIT(port);
if (!alu.port_forward)
alu.is_static = false;
- ksz8_w_sta_mac_table(dev, index, &alu);
-exit:
- return 0;
+ return ksz8_w_sta_mac_table(dev, index, &alu);
}
int ksz8_mdb_add(struct ksz_device *dev, int port,
@@ -1424,9 +1429,7 @@ int ksz8_enable_stp_addr(struct ksz_device *dev)
alu.is_override = true;
alu.port_forward = dev->info->cpu_ports;
- ksz8_w_sta_mac_table(dev, 0, &alu);
-
- return 0;
+ return ksz8_w_sta_mac_table(dev, 0, &alu);
}
int ksz8_setup(struct dsa_switch *ds)
--
2.30.2
We already have a variable to provide number of entries. So use it,
instead of using error number.
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 3348f5c25942..31c77e086a9d 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -385,19 +385,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,
@@ -425,13 +417,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)
@@ -997,8 +989,6 @@ int ksz8_fdb_dump(struct ksz_device *dev, int port,
u8 src_port;
ret = ksz8_r_dyn_mac_table(dev, i, mac, &src_port, &entries);
- if (ret == -ENXIO)
- return 0;
if (ret)
return ret;
--
2.30.2
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
Read/write operations may fail. So, make use of return values.
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index b7487be91f67..1de33ceb50de 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -336,19 +336,26 @@ void ksz8_port_init_cnt(struct ksz_device *dev, int port)
}
}
-static void ksz8_r_table(struct ksz_device *dev, int table, u16 addr, u64 *data)
+static int ksz8_r_table(struct ksz_device *dev, int table, u16 addr, u64 *data)
{
const u16 *regs;
u16 ctrl_addr;
+ int ret;
regs = dev->info->regs;
ctrl_addr = IND_ACC_TABLE(table | TABLE_READ) | addr;
mutex_lock(&dev->alu_mutex);
- ksz_write16(dev, regs[REG_IND_CTRL_0], ctrl_addr);
- ksz_read64(dev, regs[REG_IND_DATA_HI], data);
+ ret = ksz_write16(dev, regs[REG_IND_CTRL_0], ctrl_addr);
+ if (ret)
+ goto unlock_alu;
+
+ ret = ksz_read64(dev, regs[REG_IND_DATA_HI], data);
+unlock_alu:
mutex_unlock(&dev->alu_mutex);
+
+ return ret;
}
static void ksz8_w_table(struct ksz_device *dev, int table, u16 addr, u64 data)
@@ -463,11 +470,15 @@ static int ksz8_r_sta_mac_table(struct ksz_device *dev, u16 addr,
const u8 *shifts;
const u32 *masks;
u64 data;
+ int ret;
shifts = dev->info->shifts;
masks = dev->info->masks;
- ksz8_r_table(dev, TABLE_STATIC_MAC, addr, &data);
+ ret = ksz8_r_table(dev, TABLE_STATIC_MAC, addr, &data);
+ if (ret)
+ return ret;
+
data_hi = data >> 32;
data_lo = (u32)data;
--
2.30.2
This is a preparation for the next patch, to make use of read/write errors.
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 94 +++++++++++++++++------------
1 file changed, 54 insertions(+), 40 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 1c08103c9f50..b7487be91f67 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -457,7 +457,7 @@ static int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
}
static int ksz8_r_sta_mac_table(struct ksz_device *dev, u16 addr,
- struct alu_struct *alu)
+ struct alu_struct *alu, bool *valid)
{
u32 data_hi, data_lo;
const u8 *shifts;
@@ -470,28 +470,32 @@ static int ksz8_r_sta_mac_table(struct ksz_device *dev, u16 addr,
ksz8_r_table(dev, TABLE_STATIC_MAC, addr, &data);
data_hi = data >> 32;
data_lo = (u32)data;
- if (data_hi & (masks[STATIC_MAC_TABLE_VALID] |
- masks[STATIC_MAC_TABLE_OVERRIDE])) {
- alu->mac[5] = (u8)data_lo;
- alu->mac[4] = (u8)(data_lo >> 8);
- alu->mac[3] = (u8)(data_lo >> 16);
- alu->mac[2] = (u8)(data_lo >> 24);
- alu->mac[1] = (u8)data_hi;
- alu->mac[0] = (u8)(data_hi >> 8);
- alu->port_forward =
- (data_hi & masks[STATIC_MAC_TABLE_FWD_PORTS]) >>
- shifts[STATIC_MAC_FWD_PORTS];
- alu->is_override =
- (data_hi & masks[STATIC_MAC_TABLE_OVERRIDE]) ? 1 : 0;
- data_hi >>= 1;
- alu->is_static = true;
- alu->is_use_fid =
- (data_hi & masks[STATIC_MAC_TABLE_USE_FID]) ? 1 : 0;
- alu->fid = (data_hi & masks[STATIC_MAC_TABLE_FID]) >>
- shifts[STATIC_MAC_FID];
+
+ if (!(data_hi & (masks[STATIC_MAC_TABLE_VALID] |
+ masks[STATIC_MAC_TABLE_OVERRIDE]))) {
+ *valid = false;
return 0;
}
- return -ENXIO;
+
+ alu->mac[5] = (u8)data_lo;
+ alu->mac[4] = (u8)(data_lo >> 8);
+ alu->mac[3] = (u8)(data_lo >> 16);
+ alu->mac[2] = (u8)(data_lo >> 24);
+ alu->mac[1] = (u8)data_hi;
+ alu->mac[0] = (u8)(data_hi >> 8);
+ alu->port_forward =
+ (data_hi & masks[STATIC_MAC_TABLE_FWD_PORTS]) >>
+ shifts[STATIC_MAC_FWD_PORTS];
+ alu->is_override = (data_hi & masks[STATIC_MAC_TABLE_OVERRIDE]) ? 1 : 0;
+ data_hi >>= 1;
+ alu->is_static = true;
+ alu->is_use_fid = (data_hi & masks[STATIC_MAC_TABLE_USE_FID]) ? 1 : 0;
+ alu->fid = (data_hi & masks[STATIC_MAC_TABLE_FID]) >>
+ shifts[STATIC_MAC_FID];
+
+ *valid = true;
+
+ return 0;
}
void ksz8_w_sta_mac_table(struct ksz_device *dev, u16 addr,
@@ -969,12 +973,13 @@ int ksz8_fdb_dump(struct ksz_device *dev, int port,
for (i = 0; i < dev->info->num_statics; i++) {
struct alu_struct alu;
+ bool valid;
- ret = ksz8_r_sta_mac_table(dev, i, &alu);
- if (ret == -ENXIO)
- continue;
+ ret = ksz8_r_sta_mac_table(dev, i, &alu, &valid);
if (ret)
return ret;
+ if (!valid)
+ continue;
if (!(alu.port_forward & BIT(port)))
continue;
@@ -1010,20 +1015,25 @@ static int ksz8_add_sta_mac(struct ksz_device *dev, int port,
const unsigned char *addr, u16 vid)
{
struct alu_struct alu;
- int index;
+ int index, ret;
int empty = 0;
alu.port_forward = 0;
for (index = 0; index < dev->info->num_statics; index++) {
- if (!ksz8_r_sta_mac_table(dev, index, &alu)) {
- /* Found one already in static MAC table. */
- if (!memcmp(alu.mac, addr, ETH_ALEN) &&
- alu.fid == vid)
- break;
- /* Remember the first empty entry. */
- } else if (!empty) {
- empty = index + 1;
+ bool valid;
+
+ ret = ksz8_r_sta_mac_table(dev, index, &alu, &valid);
+ if (ret)
+ return ret;
+ if (!valid) {
+ /* Remember the first empty entry. */
+ if (!empty)
+ empty = index + 1;
+ continue;
}
+
+ if (!memcmp(alu.mac, addr, ETH_ALEN) && alu.fid == vid)
+ break;
}
/* no available entry */
@@ -1053,15 +1063,19 @@ static int ksz8_del_sta_mac(struct ksz_device *dev, int port,
const unsigned char *addr, u16 vid)
{
struct alu_struct alu;
- int index;
+ int index, ret;
for (index = 0; index < dev->info->num_statics; index++) {
- if (!ksz8_r_sta_mac_table(dev, index, &alu)) {
- /* Found one already in static MAC table. */
- if (!memcmp(alu.mac, addr, ETH_ALEN) &&
- alu.fid == vid)
- break;
- }
+ bool valid;
+
+ ret = ksz8_r_sta_mac_table(dev, index, &alu, &valid);
+ if (ret)
+ return ret;
+ if (!valid)
+ continue;
+
+ if (!memcmp(alu.mac, addr, ETH_ALEN) && alu.fid == vid)
+ break;
}
/* no available entry */
--
2.30.2
After fixing different bugs we can refactor this function:
- be paranoid - read only max possibly amount of entries supported by
the HW.
- pass error values returned by regmap
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 41 ++++++++++++++-----------
drivers/net/dsa/microchip/ksz8795_reg.h | 1 +
2 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 736cf4e54333..308b46bb2ce5 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -949,26 +949,31 @@ void ksz8_flush_dyn_mac_table(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 ret = 0;
- u16 i = 0;
- u16 entries = 0;
- u8 src_port;
- u8 mac[ETH_ALEN];
+ u16 i, entries = 0;
+ int ret;
- do {
- ret = ksz8_r_dyn_mac_table(dev, i, mac, &src_port,
- &entries);
- if (!ret && port == src_port) {
- ret = cb(mac, 0, false, data);
- if (ret)
- break;
- }
- i++;
- } while (i < entries);
- if (i >= entries)
- ret = 0;
+ for (i = 0; i < KSZ8_DYN_MAC_ENTRIES; i++) {
+ u8 mac[ETH_ALEN];
+ u8 src_port;
- return ret;
+ ret = ksz8_r_dyn_mac_table(dev, i, mac, &src_port, &entries);
+ if (ret == -ENXIO)
+ return 0;
+ if (ret)
+ return ret;
+
+ if (i >= entries)
+ return 0;
+
+ if (port != src_port)
+ continue;
+
+ ret = cb(mac, 0, false, data);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
}
int ksz8_mdb_add(struct ksz_device *dev, int port,
diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h b/drivers/net/dsa/microchip/ksz8795_reg.h
index 7a57c6088f80..0bdceb534192 100644
--- a/drivers/net/dsa/microchip/ksz8795_reg.h
+++ b/drivers/net/dsa/microchip/ksz8795_reg.h
@@ -811,5 +811,6 @@
#define TAIL_TAG_LOOKUP BIT(7)
#define FID_ENTRIES 128
+#define KSZ8_DYN_MAC_ENTRIES 1024
#endif
--
2.30.2
Other parts of this file use ret. So, unify it.
Signed-off-by: Oleksij Rempel <[email protected]>
---
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 2674fd553851..1677f61b138a 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -405,7 +405,7 @@ static int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
u64 buf = 0;
u8 data;
int cnt;
- int rc;
+ int ret;
shifts = dev->info->shifts;
masks = dev->info->masks;
@@ -416,12 +416,12 @@ static int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
mutex_lock(&dev->alu_mutex);
ksz_write16(dev, regs[REG_IND_CTRL_0], ctrl_addr);
- rc = ksz8_valid_dyn_entry(dev, &data);
- if (rc == -EAGAIN) {
+ ret = ksz8_valid_dyn_entry(dev, &data);
+ if (ret == -EAGAIN) {
if (addr == 0)
*entries = 0;
goto unlock_alu;
- } else if (rc == -ENXIO) {
+ } else if (ret == -ENXIO) {
*entries = 0;
goto unlock_alu;
}
@@ -447,12 +447,12 @@ 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);
- rc = 0;
+ ret = 0;
unlock_alu:
mutex_unlock(&dev->alu_mutex);
- return rc;
+ return ret;
}
int ksz8_r_sta_mac_table(struct ksz_device *dev, u16 addr,
--
2.30.2
On Mon, Nov 28, 2022 at 12:59:32PM +0100, Oleksij Rempel wrote:
> 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):
The netdev FAQ says:
* don’t post large series (> 15 patches), break them up
This seems like something which could easily be broken up.
Andrew
On Mon, 28 Nov 2022 14:51:47 +0100 Andrew Lunn wrote:
> * don’t post large series (> 15 patches), break them up
>
> This seems like something which could easily be broken up.
And name the target tree in the subject tag. And make sure it applies
cleanly to that tree (last patch is to blame AFAICT). And don't repost
within 24 hours.
On 11/28/2022 3:59 AM, Oleksij Rempel wrote:
> 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
>
My understanding is that we typically limit series to 15 patches. Do you
have some justification for why this goes over 15 and can't reasonably
be split into two series?
At a glance it seems like a bunch of smaller cleanups.
On Mon, 2022-11-28 at 12:59 +0100, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> This is a preparation for the next patch, to make use of read/write
> errors.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---
> drivers/net/dsa/microchip/ksz8795.c | 94 +++++++++++++++++--------
> ----
> 1 file changed, 54 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/net/dsa/microchip/ksz8795.c
> b/drivers/net/dsa/microchip/ksz8795.c
> index 1c08103c9f50..b7487be91f67 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -457,7 +457,7 @@ static int ksz8_r_dyn_mac_table(struct ksz_device
> *dev, u16 addr, u8 *mac_addr,
> }
>
> static int ksz8_r_sta_mac_table(struct ksz_device *dev, u16 addr,
> - struct alu_struct *alu)
> + struct alu_struct *alu, bool *valid)
> {
> u32 data_hi, data_lo;
> const u8 *shifts;
> @@ -470,28 +470,32 @@ static int ksz8_r_sta_mac_table(struct
> ksz_device *dev, u16 addr,
> ksz8_r_table(dev, TABLE_STATIC_MAC, addr, &data);
> data_hi = data >> 32;
> data_lo = (u32)data;
> - if (data_hi & (masks[STATIC_MAC_TABLE_VALID] |
> - masks[STATIC_MAC_TABLE_OVERRIDE])) {
> - alu->mac[5] = (u8)data_lo;
> - alu->mac[4] = (u8)(data_lo >> 8);
> - alu->mac[3] = (u8)(data_lo >> 16);
> - alu->mac[2] = (u8)(data_lo >> 24);
> - alu->mac[1] = (u8)data_hi;
> - alu->mac[0] = (u8)(data_hi >> 8);
> - alu->port_forward =
> - (data_hi & masks[STATIC_MAC_TABLE_FWD_PORTS])
> >>
> - shifts[STATIC_MAC_FWD_PORTS];
> - alu->is_override =
> - (data_hi & masks[STATIC_MAC_TABLE_OVERRIDE])
> ? 1 : 0;
> - data_hi >>= 1;
> - alu->is_static = true;
> - alu->is_use_fid =
> - (data_hi & masks[STATIC_MAC_TABLE_USE_FID]) ?
> 1 : 0;
> - alu->fid = (data_hi & masks[STATIC_MAC_TABLE_FID]) >>
> - shifts[STATIC_MAC_FID];
> +
> + if (!(data_hi & (masks[STATIC_MAC_TABLE_VALID] |
> + masks[STATIC_MAC_TABLE_OVERRIDE]))) {
> + *valid = false;
> return 0;
> }
> - return -ENXIO;
> +
> + alu->mac[5] = (u8)data_lo;
> + alu->mac[4] = (u8)(data_lo >> 8);
> + alu->mac[3] = (u8)(data_lo >> 16);
> + alu->mac[2] = (u8)(data_lo >> 24);
> + alu->mac[1] = (u8)data_hi;
> + alu->mac[0] = (u8)(data_hi >> 8);
u64_to_ether_addr macro can be used to store address into array.
> + alu->port_forward =
> + (data_hi & masks[STATIC_MAC_TABLE_FWD_PORTS]) >>
> + shifts[STATIC_MAC_FWD_PORTS];
> + alu->is_override = (data_hi &
> masks[STATIC_MAC_TABLE_OVERRIDE]) ? 1 : 0;
> + data_hi >>= 1;
> + alu->is_static = true;
> + alu->is_use_fid = (data_hi & masks[STATIC_MAC_TABLE_USE_FID])
> ? 1 : 0;
> + alu->fid = (data_hi & masks[STATIC_MAC_TABLE_FID]) >>
> + shifts[STATIC_MAC_FID];
Instead of masks and shifts, you consider using
FIELD_GET(STATIC_MAC_TABLE_FID, data_hi);
> +
> + *valid = true;
> +
> + return 0;
> }
>
> void ksz8_w_sta_mac_table(struct ksz_device *dev, u16 addr,
> @@ -969,12 +973,13 @@ int ksz8_fdb_dump(struct ksz_device *dev, int
> port,
>
> for (i = 0; i < dev->info->num_statics; i++) {
> struct alu_struct alu;
> + bool valid;
>
> - ret = ksz8_r_sta_mac_table(dev, i, &alu);
> - if (ret == -ENXIO)
> - continue;
> + ret = ksz8_r_sta_mac_table(dev, i, &alu, &valid);
> if (ret)
> return ret;
> + if (!valid)
> + continue;
>
> if (!(alu.port_forward & BIT(port)))
> continue;
> @@ -1010,20 +1015,25 @@ static int ksz8_add_sta_mac(struct ksz_device
> *dev, int port,
> const unsigned char *addr, u16 vid)
> {
> struct alu_struct alu;
> - int index;
> + int index, ret;
> int empty = 0;
>
> alu.port_forward = 0;
> for (index = 0; index < dev->info->num_statics; index++) {
> - if (!ksz8_r_sta_mac_table(dev, index, &alu)) {
> - /* Found one already in static MAC table. */
> - if (!memcmp(alu.mac, addr, ETH_ALEN) &&
> - alu.fid == vid)
> - break;
> - /* Remember the first empty entry. */
> - } else if (!empty) {
> - empty = index + 1;
> + bool valid;
> +
> + ret = ksz8_r_sta_mac_table(dev, index, &alu, &valid);
> + if (ret)
> + return ret;
> + if (!valid) {
> + /* Remember the first empty entry. */
> + if (!empty)
> + empty = index + 1;
> + continue;
> }
> +
> + if (!memcmp(alu.mac, addr, ETH_ALEN) && alu.fid ==
> vid)
> + break;
> }
>
> /* no available entry */
> @@ -1053,15 +1063,19 @@ static int ksz8_del_sta_mac(struct ksz_device
> *dev, int port,
> const unsigned char *addr, u16 vid)
> {
> struct alu_struct alu;
> - int index;
> + int index, ret;
Variable declaration in separate line.
>
> for (index = 0; index < dev->info->num_statics; index++) {
> - if (!ksz8_r_sta_mac_table(dev, index, &alu)) {
> - /* Found one already in static MAC table. */
> - if (!memcmp(alu.mac, addr, ETH_ALEN) &&
> - alu.fid == vid)
> - break;
> - }
> + bool valid;
> +
> + ret = ksz8_r_sta_mac_table(dev, index, &alu, &valid);
> + if (ret)
> + return ret;
> + if (!valid)
> + continue;
> +
> + if (!memcmp(alu.mac, addr, ETH_ALEN) && alu.fid ==
> vid)
> + break;
> }
>
> /* no available entry */
> --
> 2.30.2
>
On Mon, Nov 28, 2022 at 03:09:19PM -0800, Jacob Keller wrote:
>
>
> On 11/28/2022 3:59 AM, Oleksij Rempel wrote:
> > 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
> >
>
>
> My understanding is that we typically limit series to 15 patches. Do you
> have some justification for why this goes over 15 and can't reasonably be
> split into two series?
>
> At a glance it seems like a bunch of smaller cleanups.
The previous patch set got request to do more clean ups:
https://lore.kernel.org/all/[email protected]/
I need to show, there are already more patches in the queue.
Regards,
Oleksij
--
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 |
On Tue, Nov 29, 2022 at 05:25:55AM +0000, [email protected] wrote:
> On Mon, 2022-11-28 at 12:59 +0100, Oleksij Rempel wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> >
> > This is a preparation for the next patch, to make use of read/write
> > errors.
> >
> > Signed-off-by: Oleksij Rempel <[email protected]>
> > ---
> > drivers/net/dsa/microchip/ksz8795.c | 94 +++++++++++++++++--------
> > ----
> > 1 file changed, 54 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/net/dsa/microchip/ksz8795.c
> > b/drivers/net/dsa/microchip/ksz8795.c
> > index 1c08103c9f50..b7487be91f67 100644
> > --- a/drivers/net/dsa/microchip/ksz8795.c
> > +++ b/drivers/net/dsa/microchip/ksz8795.c
> > @@ -457,7 +457,7 @@ static int ksz8_r_dyn_mac_table(struct ksz_device
> > *dev, u16 addr, u8 *mac_addr,
> > }
> >
> > static int ksz8_r_sta_mac_table(struct ksz_device *dev, u16 addr,
> > - struct alu_struct *alu)
> > + struct alu_struct *alu, bool *valid)
> > {
> > u32 data_hi, data_lo;
> > const u8 *shifts;
> > @@ -470,28 +470,32 @@ static int ksz8_r_sta_mac_table(struct
> > ksz_device *dev, u16 addr,
> > ksz8_r_table(dev, TABLE_STATIC_MAC, addr, &data);
> > data_hi = data >> 32;
> > data_lo = (u32)data;
> > - if (data_hi & (masks[STATIC_MAC_TABLE_VALID] |
> > - masks[STATIC_MAC_TABLE_OVERRIDE])) {
> > - alu->mac[5] = (u8)data_lo;
> > - alu->mac[4] = (u8)(data_lo >> 8);
> > - alu->mac[3] = (u8)(data_lo >> 16);
> > - alu->mac[2] = (u8)(data_lo >> 24);
> > - alu->mac[1] = (u8)data_hi;
> > - alu->mac[0] = (u8)(data_hi >> 8);
> > - alu->port_forward =
> > - (data_hi & masks[STATIC_MAC_TABLE_FWD_PORTS])
> > >>
> > - shifts[STATIC_MAC_FWD_PORTS];
> > - alu->is_override =
> > - (data_hi & masks[STATIC_MAC_TABLE_OVERRIDE])
> > ? 1 : 0;
> > - data_hi >>= 1;
> > - alu->is_static = true;
> > - alu->is_use_fid =
> > - (data_hi & masks[STATIC_MAC_TABLE_USE_FID]) ?
> > 1 : 0;
> > - alu->fid = (data_hi & masks[STATIC_MAC_TABLE_FID]) >>
> > - shifts[STATIC_MAC_FID];
> > +
> > + if (!(data_hi & (masks[STATIC_MAC_TABLE_VALID] |
> > + masks[STATIC_MAC_TABLE_OVERRIDE]))) {
> > + *valid = false;
> > return 0;
> > }
> > - return -ENXIO;
> > +
> > + alu->mac[5] = (u8)data_lo;
> > + alu->mac[4] = (u8)(data_lo >> 8);
> > + alu->mac[3] = (u8)(data_lo >> 16);
> > + alu->mac[2] = (u8)(data_lo >> 24);
> > + alu->mac[1] = (u8)data_hi;
> > + alu->mac[0] = (u8)(data_hi >> 8);
>
> u64_to_ether_addr macro can be used to store address into array.
This should not be done in this patch.
> > + alu->port_forward =
> > + (data_hi & masks[STATIC_MAC_TABLE_FWD_PORTS]) >>
> > + shifts[STATIC_MAC_FWD_PORTS];
>
>
>
> > + alu->is_override = (data_hi &
> > masks[STATIC_MAC_TABLE_OVERRIDE]) ? 1 : 0;
> > + data_hi >>= 1;
> > + alu->is_static = true;
> > + alu->is_use_fid = (data_hi & masks[STATIC_MAC_TABLE_USE_FID])
> > ? 1 : 0;
> > + alu->fid = (data_hi & masks[STATIC_MAC_TABLE_FID]) >>
> > + shifts[STATIC_MAC_FID];
>
> Instead of masks and shifts, you consider using
> FIELD_GET(STATIC_MAC_TABLE_FID, data_hi);
I would love to do this, but in this case, complete driver should be
splittet in to KSZ88X3 and KSZ879X portions. FIELD_GET can't work with
dynamic masks and shifts.
I would say, it is not for this patch set.
> > +
> > + *valid = true;
> > +
> > + return 0;
> > }
> >
> > void ksz8_w_sta_mac_table(struct ksz_device *dev, u16 addr,
> > @@ -969,12 +973,13 @@ int ksz8_fdb_dump(struct ksz_device *dev, int
> > port,
> >
> > for (i = 0; i < dev->info->num_statics; i++) {
> > struct alu_struct alu;
> > + bool valid;
> >
> > - ret = ksz8_r_sta_mac_table(dev, i, &alu);
> > - if (ret == -ENXIO)
> > - continue;
> > + ret = ksz8_r_sta_mac_table(dev, i, &alu, &valid);
> > if (ret)
> > return ret;
> > + if (!valid)
> > + continue;
> >
> > if (!(alu.port_forward & BIT(port)))
> > continue;
> > @@ -1010,20 +1015,25 @@ static int ksz8_add_sta_mac(struct ksz_device
> > *dev, int port,
> > const unsigned char *addr, u16 vid)
> > {
> > struct alu_struct alu;
> > - int index;
> > + int index, ret;
> > int empty = 0;
> >
> > alu.port_forward = 0;
> > for (index = 0; index < dev->info->num_statics; index++) {
> > - if (!ksz8_r_sta_mac_table(dev, index, &alu)) {
> > - /* Found one already in static MAC table. */
> > - if (!memcmp(alu.mac, addr, ETH_ALEN) &&
> > - alu.fid == vid)
> > - break;
> > - /* Remember the first empty entry. */
> > - } else if (!empty) {
> > - empty = index + 1;
> > + bool valid;
> > +
> > + ret = ksz8_r_sta_mac_table(dev, index, &alu, &valid);
> > + if (ret)
> > + return ret;
> > + if (!valid) {
> > + /* Remember the first empty entry. */
> > + if (!empty)
> > + empty = index + 1;
> > + continue;
> > }
> > +
> > + if (!memcmp(alu.mac, addr, ETH_ALEN) && alu.fid ==
> > vid)
> > + break;
> > }
> >
> > /* no available entry */
> > @@ -1053,15 +1063,19 @@ static int ksz8_del_sta_mac(struct ksz_device
> > *dev, int port,
> > const unsigned char *addr, u16 vid)
> > {
> > struct alu_struct alu;
> > - int index;
> > + int index, ret;
>
> Variable declaration in separate line.
Is it a new coding style requirement? I can't find it here:
https://www.kernel.org/doc/html/v6.0/process/coding-style.html
> >
> > for (index = 0; index < dev->info->num_statics; index++) {
> > - if (!ksz8_r_sta_mac_table(dev, index, &alu)) {
> > - /* Found one already in static MAC table. */
> > - if (!memcmp(alu.mac, addr, ETH_ALEN) &&
> > - alu.fid == vid)
> > - break;
> > - }
> > + bool valid;
> > +
> > + ret = ksz8_r_sta_mac_table(dev, index, &alu, &valid);
> > + if (ret)
> > + return ret;
> > + if (!valid)
> > + continue;
> > +
> > + if (!memcmp(alu.mac, addr, ETH_ALEN) && alu.fid ==
> > vid)
> > + break;
> > }
> >
> > /* no available entry */
> > --
> > 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 |
Hi Oleksij,
On Mon, 2022-11-28 at 12:59 +0100, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> With current code loopback is not working and selftest will always
> fail.
> Fix register and bit offsets to make loopback on KSZ88x3 switches.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---
> drivers/net/dsa/microchip/ksz8795.c | 24 ++++++++++++++++++-----
> -
> drivers/net/dsa/microchip/ksz8795_reg.h | 1 +
> 2 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/dsa/microchip/ksz8795.c
> b/drivers/net/dsa/microchip/ksz8795.c
> index 208cf4dde397..a6d5de41a754 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -625,8 +625,13 @@ int ksz8_r_phy(struct ksz_device *dev, u16 phy,
> u16 reg, u16 *val)
> if (ret)
> return ret;
>
> - if (restart & PORT_PHY_LOOPBACK)
> - data |= BMCR_LOOPBACK;
> + if (ksz_is_ksz88x3(dev)) {
> + if (restart & KSZ8873_PORT_PHY_LOOPBACK)
> + data |= BMCR_LOOPBACK;
> + } else {
> + if (restart & PORT_PHY_LOOPBACK)
> + data |= BMCR_LOOPBACK;
> + }
Can you consider using ksz8795_masks[] and ksz8863_masks[] to check the
loopback. Like if (restart & mask[PHY_LOOPBAK)) to avoid two checks.
> if (ctrl & PORT_FORCE_100_MBIT)
> data |= BMCR_SPEED100;
> if (ksz_is_ksz88x3(dev)) {
> diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h
> b/drivers/net/dsa/microchip/ksz8795_reg.h
> index 0bdceb534192..08204da7d621 100644
> --- a/drivers/net/dsa/microchip/ksz8795_reg.h
> +++ b/drivers/net/dsa/microchip/ksz8795_reg.h
> @@ -262,6 +262,7 @@
> #define PORT_AUTO_MDIX_DISABLE BIT(2)
> #define PORT_FORCE_MDIX BIT(1)
> #define PORT_MAC_LOOPBACK BIT(0)
> +#define KSZ8873_PORT_PHY_LOOPBACK BIT(0)
>
> #define REG_PORT_1_STATUS_2 0x1E
> #define REG_PORT_2_STATUS_2 0x2E
> --
> 2.30.2
>
Hi Oleksij,
On Mon, 2022-11-28 at 12:59 +0100, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> After fixing different bugs we can refactor this function:
> - be paranoid - read only max possibly amount of entries supported by
> the HW.
> - pass error values returned by regmap
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---
> drivers/net/dsa/microchip/ksz8795.c | 41 ++++++++++++++---------
> --
> drivers/net/dsa/microchip/ksz8795_reg.h | 1 +
> 2 files changed, 24 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/dsa/microchip/ksz8795.c
> b/drivers/net/dsa/microchip/ksz8795.c
> index 736cf4e54333..308b46bb2ce5 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -949,26 +949,31 @@ void ksz8_flush_dyn_mac_table(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 ret = 0;
> - u16 i = 0;
> - u16 entries = 0;
> - u8 src_port;
> - u8 mac[ETH_ALEN];
> + u16 i, entries = 0;
> + int ret;
>
> - do {
> - ret = ksz8_r_dyn_mac_table(dev, i, mac, &src_port,
> - &entries);
> - if (!ret && port == src_port) {
> - ret = cb(mac, 0, false, data);
> - if (ret)
> - break;
> - }
> - i++;
> - } while (i < entries);
> - if (i >= entries)
> - ret = 0;
> + for (i = 0; i < KSZ8_DYN_MAC_ENTRIES; i++) {
> + u8 mac[ETH_ALEN];
> + u8 src_port;
Any specific reason for declaring variable within for loop instead of
outside.
>
> - return ret;
> + ret = ksz8_r_dyn_mac_table(dev, i, mac, &src_port,
> &entries);
> + if (ret == -ENXIO)
> + return 0;
> + if (ret)
> + return ret;
> +
> + if (i >= entries)
> + return 0;
> +
> + if (port != src_port)
> + continue;
> +
> + ret = cb(mac, 0, false, data);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> }
>
> int ksz8_mdb_add(struct ksz_device *dev, int port,
> diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h
> b/drivers/net/dsa/microchip/ksz8795_reg.h
> index 7a57c6088f80..0bdceb534192 100644
> --- a/drivers/net/dsa/microchip/ksz8795_reg.h
> +++ b/drivers/net/dsa/microchip/ksz8795_reg.h
> @@ -811,5 +811,6 @@
> #define TAIL_TAG_LOOKUP BIT(7)
>
> #define FID_ENTRIES 128
> +#define KSZ8_DYN_MAC_ENTRIES 1024
>
> #endif
> --
> 2.30.2
>
On Tue, Nov 29, 2022 at 08:29:47AM +0000, [email protected] wrote:
> Hi Oleksij,
>
> On Mon, 2022-11-28 at 12:59 +0100, Oleksij Rempel wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> >
> > After fixing different bugs we can refactor this function:
> > - be paranoid - read only max possibly amount of entries supported by
> > the HW.
> > - pass error values returned by regmap
> >
> > Signed-off-by: Oleksij Rempel <[email protected]>
> > ---
> > drivers/net/dsa/microchip/ksz8795.c | 41 ++++++++++++++---------
> > --
> > drivers/net/dsa/microchip/ksz8795_reg.h | 1 +
> > 2 files changed, 24 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/net/dsa/microchip/ksz8795.c
> > b/drivers/net/dsa/microchip/ksz8795.c
> > index 736cf4e54333..308b46bb2ce5 100644
> > --- a/drivers/net/dsa/microchip/ksz8795.c
> > +++ b/drivers/net/dsa/microchip/ksz8795.c
> > @@ -949,26 +949,31 @@ void ksz8_flush_dyn_mac_table(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 ret = 0;
> > - u16 i = 0;
> > - u16 entries = 0;
> > - u8 src_port;
> > - u8 mac[ETH_ALEN];
> > + u16 i, entries = 0;
> > + int ret;
> >
> > - do {
> > - ret = ksz8_r_dyn_mac_table(dev, i, mac, &src_port,
> > - &entries);
> > - if (!ret && port == src_port) {
> > - ret = cb(mac, 0, false, data);
> > - if (ret)
> > - break;
> > - }
> > - i++;
> > - } while (i < entries);
> > - if (i >= entries)
> > - ret = 0;
> > + for (i = 0; i < KSZ8_DYN_MAC_ENTRIES; i++) {
> > + u8 mac[ETH_ALEN];
> > + u8 src_port;
>
> Any specific reason for declaring variable within for loop instead of
> outside.
No. It is personal preference to declare variables within the scope where
variable is used.
--
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 |
On Tue, Nov 29, 2022 at 06:35:39AM +0100, Oleksij Rempel wrote:
> On Mon, Nov 28, 2022 at 03:09:19PM -0800, Jacob Keller wrote:
> >
> >
> > On 11/28/2022 3:59 AM, Oleksij Rempel wrote:
> > > 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
> > >
> >
> >
> > My understanding is that we typically limit series to 15 patches. Do you
> > have some justification for why this goes over 15 and can't reasonably be
> > split into two series?
> >
> > At a glance it seems like a bunch of smaller cleanups.
>
> The previous patch set got request to do more clean ups:
> https://lore.kernel.org/all/[email protected]/
>
> I need to show, there are already more patches in the queue.
There is some psychology involved here. I see 26 patches and decide i
need to allocate 30 minutes to this sometime, and put the review off
until later, without even looking at them. If i get 5 patches, i
probably just do it, knowing i will be finished pretty quickly. My
guess is, 5 patches a day for 5 days will be merged faster than 26
patches in one go.
Andrew
On Tue, Nov 29, 2022 at 03:15:03PM +0100, Andrew Lunn wrote:
> On Tue, Nov 29, 2022 at 06:35:39AM +0100, Oleksij Rempel wrote:
> > On Mon, Nov 28, 2022 at 03:09:19PM -0800, Jacob Keller wrote:
> > >
> > > My understanding is that we typically limit series to 15 patches. Do you
> > > have some justification for why this goes over 15 and can't reasonably be
> > > split into two series?
> > >
> > > At a glance it seems like a bunch of smaller cleanups.
> >
> > The previous patch set got request to do more clean ups:
> > https://lore.kernel.org/all/[email protected]/
> >
> > I need to show, there are already more patches in the queue.
>
> There is some psychology involved here. I see 26 patches and decide i
> need to allocate 30 minutes to this sometime, and put the review off
> until later, without even looking at them. If i get 5 patches, i
> probably just do it, knowing i will be finished pretty quickly. My
> guess is, 5 patches a day for 5 days will be merged faster than 26
> patches in one go.
Good point. Thx!
I'll split this patch set.
Regards,
Oleksij
--
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 |