diff mbox series

[net-next,v4,5/5] net: dsa: microchip: apply KSZ87xx family fixes wrt datasheet

Message ID 20240812084945.578993-6-vtpieter@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: dsa: microchip: ksz8795: add Wake on LAN support | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 29 this patch: 29
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 29 this patch: 29
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 29 this patch: 29
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 107 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Pieter Aug. 12, 2024, 8:49 a.m. UTC 
From: Pieter Van Trappen <pieter.van.trappen@cern.ch>

The KSZ87xx switches have 32 static MAC address table entries and not
8. This fixes -ENOSPC non-critical errors from ksz8_add_sta_mac when
configured as a bridge.

Add a new ksz87xx_dev_ops structure to be able to use the
ksz_r_mib_stat64 pointer for this family; this corrects a wrong
mib->counters cast to ksz88xx_stats_raw. This fixes iproute2
statistics. Rename ksz8_dev_ops structure to ksz88x3_dev_ops, in line
with ksz_is_* naming conventions from ksz_common.h.

Fix the tag_ksz egress mask, the port is encoded in the two and not
three LSB. This fix is for completeness, bit 2 must be 0 in most if
not all cases because it works without it for the KSZ8794.

Signed-off-by: Pieter Van Trappen <pieter.van.trappen@cern.ch>
---
 drivers/net/dsa/microchip/ksz_common.c | 51 ++++++++++++++++++++++----
 net/dsa/tag_ksz.c                      |  2 +-
 2 files changed, 44 insertions(+), 9 deletions(-)

Comments

Arun Ramadoss Aug. 12, 2024, 10:45 a.m. UTC | #1 
Hi Pieter, 



@@ -141,7 +141,7 @@ static struct sk_buff *ksz8795_rcv(struct sk_buff
> *skb, struct net_device *dev)
>  {
>         u8 *tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;
> 
> -       return ksz_common_rcv(skb, dev, tag[0] & 7,
> KSZ_EGRESS_TAG_LEN);
> +       return ksz_common_rcv(skb, dev, tag[0] & 3,
> KSZ_EGRESS_TAG_LEN);
>  }

This change can be separate patch. since it is not related to
ksz87xx_dev_ops structure. Is it a fix or just good to have one. If it
is a fix then it should be point to net tree. 

> 
>  static const struct dsa_device_ops ksz8795_netdev_ops = {
> --
> 2.43.0
>
Pieter Aug. 12, 2024, 10:55 a.m. UTC | #2 
Hi Arun,

> @@ -141,7 +141,7 @@ static struct sk_buff *ksz8795_rcv(struct sk_buff
> > *skb, struct net_device *dev)
> >  {
> >         u8 *tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;
> >
> > -       return ksz_common_rcv(skb, dev, tag[0] & 7,
> > KSZ_EGRESS_TAG_LEN);
> > +       return ksz_common_rcv(skb, dev, tag[0] & 3,
> > KSZ_EGRESS_TAG_LEN);
> >  }
>
> This change can be separate patch. since it is not related to
> ksz87xx_dev_ops structure. Is it a fix or just good to have one. If it
> is a fix then it should be point to net tree.
>

It's a fix wrt to datasheet but in reality I can see from tests with
a KSZ8794 that bit 2 is always 0 so the bug doesn't manifest itself.

Please advise, keep it in net-next or make a separate patch for the
net tree?
Arun Ramadoss Aug. 12, 2024, 2:29 p.m. UTC | #3 
On Mon, 2024-08-12 at 12:55 +0200, Pieter wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> Hi Arun,
> 
> > @@ -141,7 +141,7 @@ static struct sk_buff *ksz8795_rcv(struct
> > sk_buff
> > > *skb, struct net_device *dev)
> > >  {
> > >         u8 *tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;
> > > 
> > > -       return ksz_common_rcv(skb, dev, tag[0] & 7,
> > > KSZ_EGRESS_TAG_LEN);
> > > +       return ksz_common_rcv(skb, dev, tag[0] & 3,
> > > KSZ_EGRESS_TAG_LEN);
> > >  }
> > 
> > This change can be separate patch. since it is not related to
> > ksz87xx_dev_ops structure. Is it a fix or just good to have one. If
> > it
> > is a fix then it should be point to net tree.
> > 
> 
> It's a fix wrt to datasheet but in reality I can see from tests with
> a KSZ8794 that bit 2 is always 0 so the bug doesn't manifest itself.
> 
> Please advise, keep it in net-next or make a separate patch for the
> net tree?

Then it could point to net-next. Keep it as 6th patch of this patch
set. Instead of 3 as magic number, it can be macro.
diff mbox series

Patch

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 3f3230d181d8..cd3991792b69 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -277,7 +277,7 @@  static const struct phylink_mac_ops ksz8_phylink_mac_ops = {
 	.mac_link_up	= ksz8_phylink_mac_link_up,
 };
 
-static const struct ksz_dev_ops ksz8_dev_ops = {
+static const struct ksz_dev_ops ksz88x3_dev_ops = {
 	.setup = ksz8_setup,
 	.get_port_addr = ksz8_get_port_addr,
 	.cfg_port_member = ksz8_cfg_port_member,
@@ -312,6 +312,41 @@  static const struct ksz_dev_ops ksz8_dev_ops = {
 	.pme_pwrite8 = ksz8_pme_pwrite8,
 };
 
+static const struct ksz_dev_ops ksz87xx_dev_ops = {
+	.setup = ksz8_setup,
+	.get_port_addr = ksz8_get_port_addr,
+	.cfg_port_member = ksz8_cfg_port_member,
+	.flush_dyn_mac_table = ksz8_flush_dyn_mac_table,
+	.port_setup = ksz8_port_setup,
+	.r_phy = ksz8_r_phy,
+	.w_phy = ksz8_w_phy,
+	.r_mib_cnt = ksz8_r_mib_cnt,
+	.r_mib_pkt = ksz8_r_mib_pkt,
+	.r_mib_stat64 = ksz_r_mib_stats64,
+	.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,
+	.vlan_add = ksz8_port_vlan_add,
+	.vlan_del = ksz8_port_vlan_del,
+	.mirror_add = ksz8_port_mirror_add,
+	.mirror_del = ksz8_port_mirror_del,
+	.get_caps = ksz8_get_caps,
+	.config_cpu_port = ksz8_config_cpu_port,
+	.enable_stp_addr = ksz8_enable_stp_addr,
+	.reset = ksz8_reset_switch,
+	.init = ksz8_switch_init,
+	.exit = ksz8_switch_exit,
+	.change_mtu = ksz8_change_mtu,
+	.pme_write8 = ksz8_pme_write8,
+	.pme_pread8 = ksz8_pme_pread8,
+	.pme_pwrite8 = ksz8_pme_pwrite8,
+};
+
 static void ksz9477_phylink_mac_link_up(struct phylink_config *config,
 					struct phy_device *phydev,
 					unsigned int mode,
@@ -1262,12 +1297,12 @@  const struct ksz_chip_data ksz_switch_chips[] = {
 		.dev_name = "KSZ8795",
 		.num_vlans = 4096,
 		.num_alus = 0,
-		.num_statics = 8,
+		.num_statics = 32,
 		.cpu_ports = 0x10,	/* can be configured as cpu port */
 		.port_cnt = 5,		/* total cpu and user ports */
 		.num_tx_queues = 4,
 		.num_ipms = 4,
-		.ops = &ksz8_dev_ops,
+		.ops = &ksz87xx_dev_ops,
 		.phylink_mac_ops = &ksz8_phylink_mac_ops,
 		.ksz87xx_eee_link_erratum = true,
 		.mib_names = ksz9477_mib_names,
@@ -1303,12 +1338,12 @@  const struct ksz_chip_data ksz_switch_chips[] = {
 		.dev_name = "KSZ8794",
 		.num_vlans = 4096,
 		.num_alus = 0,
-		.num_statics = 8,
+		.num_statics = 32,
 		.cpu_ports = 0x10,	/* can be configured as cpu port */
 		.port_cnt = 5,		/* total cpu and user ports */
 		.num_tx_queues = 4,
 		.num_ipms = 4,
-		.ops = &ksz8_dev_ops,
+		.ops = &ksz87xx_dev_ops,
 		.phylink_mac_ops = &ksz8_phylink_mac_ops,
 		.ksz87xx_eee_link_erratum = true,
 		.mib_names = ksz9477_mib_names,
@@ -1330,12 +1365,12 @@  const struct ksz_chip_data ksz_switch_chips[] = {
 		.dev_name = "KSZ8765",
 		.num_vlans = 4096,
 		.num_alus = 0,
-		.num_statics = 8,
+		.num_statics = 32,
 		.cpu_ports = 0x10,	/* can be configured as cpu port */
 		.port_cnt = 5,		/* total cpu and user ports */
 		.num_tx_queues = 4,
 		.num_ipms = 4,
-		.ops = &ksz8_dev_ops,
+		.ops = &ksz87xx_dev_ops,
 		.phylink_mac_ops = &ksz8_phylink_mac_ops,
 		.ksz87xx_eee_link_erratum = true,
 		.mib_names = ksz9477_mib_names,
@@ -1362,7 +1397,7 @@  const struct ksz_chip_data ksz_switch_chips[] = {
 		.port_cnt = 3,
 		.num_tx_queues = 4,
 		.num_ipms = 4,
-		.ops = &ksz8_dev_ops,
+		.ops = &ksz88x3_dev_ops,
 		.phylink_mac_ops = &ksz8830_phylink_mac_ops,
 		.mib_names = ksz88xx_mib_names,
 		.mib_cnt = ARRAY_SIZE(ksz88xx_mib_names),
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index ee7b272ab715..a0f9965de027 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -141,7 +141,7 @@  static struct sk_buff *ksz8795_rcv(struct sk_buff *skb, struct net_device *dev)
 {
 	u8 *tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;
 
-	return ksz_common_rcv(skb, dev, tag[0] & 7, KSZ_EGRESS_TAG_LEN);
+	return ksz_common_rcv(skb, dev, tag[0] & 3, KSZ_EGRESS_TAG_LEN);
 }
 
 static const struct dsa_device_ops ksz8795_netdev_ops = {