diff mbox series

[net-next,1/1] net: dsa: microchip: rename ksz8 series files

Message ID 20240828102801.227588-1-vtpieter@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net-next,1/1] net: dsa: microchip: rename ksz8 series files | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
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: 16 this patch: 16
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 17 this patch: 17
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: 22 this patch: 22
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc fail Errors and warnings before: 0 this patch: 1
netdev/source_inline success Was 0 now: 0

Commit Message

Pieter Aug. 28, 2024, 10:27 a.m. UTC
From: Pieter Van Trappen <pieter.van.trappen@cern.ch>

The first KSZ8 series implementation was done for a KSZ8795 device but
since several other KSZ8 devices have been added. Rename these files
to adhere to the ksz8 naming convention as already used in most
functions and the existing ksz8.h; add an explanatory note.

In addition, remove one last register definition that is already part
of the ksz_common.c register structures.

Signed-off-by: Pieter Van Trappen <pieter.van.trappen@cern.ch>
---
 drivers/net/dsa/microchip/Kconfig                   |  4 ++--
 drivers/net/dsa/microchip/Makefile                  |  2 +-
 drivers/net/dsa/microchip/{ksz8795.c => ksz8.c}     |  4 ++--
 .../net/dsa/microchip/{ksz8795_reg.h => ksz8_reg.h} | 13 ++++++++-----
 4 files changed, 13 insertions(+), 10 deletions(-)
 rename drivers/net/dsa/microchip/{ksz8795.c => ksz8.c} (99%)
 rename drivers/net/dsa/microchip/{ksz8795_reg.h => ksz8_reg.h} (98%)


base-commit: e5899b60f52a7591cfc2a2dec3e83710975117d7

Comments

Arun Ramadoss Aug. 29, 2024, 5:35 a.m. UTC | #1
Hi Pieter, 

On Wed, 2024-08-28 at 12:27 +0200, vtpieter@gmail.com wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> From: Pieter Van Trappen <pieter.van.trappen@cern.ch>
> 
> The first KSZ8 series implementation was done for a KSZ8795 device
> but
> since several other KSZ8 devices have been added. Rename these files
> to adhere to the ksz8 naming convention as already used in most
> functions and the existing ksz8.h; add an explanatory note.

Refactoring the file name will better align what the implementation is.
But the file header/Kconfig should mentions what all the switches it
support. 
Because there are two switches KSZ8563 and KSZ8567 does not belong to
this Family. Instead it belongs to KSZ9477 family with only difference
they are not gigabit capable. 

The switch comes in KSZ8.c files are KSZ8863/KSZ8873,KSZ8895/KSZ8864,
KSZ8794/KSZ8795/KSZ8765.

> 
> In addition, remove one last register definition that is already part
> of the ksz_common.c register structures.

This should be separate patch.
>
Pieter Aug. 29, 2024, 3:56 p.m. UTC | #2
Hi Arun,

> Refactoring the file name will better align what the implementation is.
> But the file header/Kconfig should mentions what all the switches it
> support.
> Because there are two switches KSZ8563 and KSZ8567 does not belong to
> this Family. Instead it belongs to KSZ9477 family with only difference
> they are not gigabit capable.
>
> The switch comes in KSZ8.c files are KSZ8863/KSZ8873,KSZ8895/KSZ8864,
> KSZ8794/KSZ8795/KSZ8765.

Thanks, that makes sense - will do so. Looking now at chip_ids there's
(for me) some confusion regarding KSZ8830 mentions such as
KSZ8830_CHIP_ID which actually refers to the KSZ8863/73 switches.
Often, such as in ksz_is_ksz88x3, the KSZ88X3 naming is also used for
exactly these switches which is more intuitive but it doesn't help to
have two names for the same thing. Do you agree to remove KSZ8830
terms  in favor of KSZ88X3 as part of this patch series (will split)?

PS as far as I can see the KSZ8830 switch itself doesn't exist.

Added Oleksij and Tristram to Cc.

Cheers, Pieter
Tristram.Ha@microchip.com Aug. 29, 2024, 10:51 p.m. UTC | #3
> Subject: Re: [PATCH net-next 1/1] net: dsa: microchip: rename ksz8 series files
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content
> is safe
> 
> Hi Arun,
> 
> > Refactoring the file name will better align what the implementation is.
> > But the file header/Kconfig should mentions what all the switches it
> > support.
> > Because there are two switches KSZ8563 and KSZ8567 does not belong to
> > this Family. Instead it belongs to KSZ9477 family with only difference
> > they are not gigabit capable.
> >
> > The switch comes in KSZ8.c files are KSZ8863/KSZ8873,KSZ8895/KSZ8864,
> > KSZ8794/KSZ8795/KSZ8765.
> 
> Thanks, that makes sense - will do so. Looking now at chip_ids there's
> (for me) some confusion regarding KSZ8830 mentions such as
> KSZ8830_CHIP_ID which actually refers to the KSZ8863/73 switches.
> Often, such as in ksz_is_ksz88x3, the KSZ88X3 naming is also used for
> exactly these switches which is more intuitive but it doesn't help to
> have two names for the same thing. Do you agree to remove KSZ8830
> terms  in favor of KSZ88X3 as part of this patch series (will split)?
> 
> PS as far as I can see the KSZ8830 switch itself doesn't exist.

I think the name KSZ8830 was used because the hardware chip id is 0x30.
The switch names are indeed KSZ8863 and KSZ8873.  The switch family can
be called KSZ88X3.

The next switch family is KSZ8895 and KSZ8864.  Then come switch family
KSZ8795/KSZ8794/KSZ8765.

The next generation of switch family is KSZ9897 with many variations and
product names.  KSZ9477 is the switch with all of the features while the
rest have some features and ports disabled.

The KSZ9893/KSZ9563/KSZ8563 switch family shares most of the features of
KSZ9897 but has a slightly different internal design, so it has its own
bugs and issues.

LAN937X is the latest switch family that is based from KSZ9897.

Originally the intention was to have one driver for each switch family,
so there are ksz9477.c and ksz9477_reg.h and ksz8795.c and ksz8795_reg.h.
When KSZ8863 switch support was added the KSZ8863 related code was put in
ksz8795.c as some functions just needed minor changes to support the
switch.  Much later it was decided to have one common DSA driver to
support all KSZ switches, so most new code and register information were
put in ksz_common.c and ksz_common.h.
diff mbox series

Patch

diff --git a/drivers/net/dsa/microchip/Kconfig b/drivers/net/dsa/microchip/Kconfig
index c1b906c05a02..68a91afa9c22 100644
--- a/drivers/net/dsa/microchip/Kconfig
+++ b/drivers/net/dsa/microchip/Kconfig
@@ -1,6 +1,6 @@ 
 # SPDX-License-Identifier: GPL-2.0-only
 menuconfig NET_DSA_MICROCHIP_KSZ_COMMON
-	tristate "Microchip KSZ8795/KSZ9477/LAN937x series switch support"
+	tristate "Microchip KSZ8xxx/KSZ9477/LAN937x series switch support"
 	depends on NET_DSA
 	select NET_DSA_TAG_KSZ
 	select NET_DSA_TAG_NONE
@@ -8,7 +8,7 @@  menuconfig NET_DSA_MICROCHIP_KSZ_COMMON
 	select DCB
 	help
 	  This driver adds support for Microchip KSZ9477 series switch and
-	  KSZ8795/KSZ88x3 switch chips.
+	  KSZ8xxx e.g. KSZ8795 switch chips.
 
 config NET_DSA_MICROCHIP_KSZ9477_I2C
 	tristate "KSZ series I2C connected switch driver"
diff --git a/drivers/net/dsa/microchip/Makefile b/drivers/net/dsa/microchip/Makefile
index 1cfba1ec9355..9347cfb3d0b5 100644
--- a/drivers/net/dsa/microchip/Makefile
+++ b/drivers/net/dsa/microchip/Makefile
@@ -2,7 +2,7 @@ 
 obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ_COMMON)	+= ksz_switch.o
 ksz_switch-objs := ksz_common.o ksz_dcb.o
 ksz_switch-objs += ksz9477.o ksz9477_acl.o ksz9477_tc_flower.o
-ksz_switch-objs += ksz8795.o
+ksz_switch-objs += ksz8.o
 ksz_switch-objs += lan937x_main.o
 
 ifdef CONFIG_NET_DSA_MICROCHIP_KSZ_PTP
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8.c
similarity index 99%
rename from drivers/net/dsa/microchip/ksz8795.c
rename to drivers/net/dsa/microchip/ksz8.c
index aa09d89debf0..804085f3a0f0 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8.c
@@ -1,6 +1,6 @@ 
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Microchip KSZ8795 switch driver
+ * Microchip KSZ8XXX series switch driver
  *
  * Copyright (C) 2017 Microchip Technology Inc.
  *	Tristram Ha <Tristram.Ha@microchip.com>
@@ -23,7 +23,7 @@ 
 #include <linux/phylink.h>
 
 #include "ksz_common.h"
-#include "ksz8795_reg.h"
+#include "ksz8_reg.h"
 #include "ksz8.h"
 
 static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set)
diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h b/drivers/net/dsa/microchip/ksz8_reg.h
similarity index 98%
rename from drivers/net/dsa/microchip/ksz8795_reg.h
rename to drivers/net/dsa/microchip/ksz8_reg.h
index 69566a5d9cda..b2bc6727cdfb 100644
--- a/drivers/net/dsa/microchip/ksz8795_reg.h
+++ b/drivers/net/dsa/microchip/ksz8_reg.h
@@ -1,13 +1,18 @@ 
 /* SPDX-License-Identifier: GPL-2.0-or-later */
 /*
- * Microchip KSZ8795 register definitions
+ * Microchip KSZ8XXX series register definitions
+ *
+ * The base for these definitions is KSZ8795 but unless indicated
+ * differently by their prefix, they apply to all KSZ8 series
+ * devices. Registers and masks that do change are defined in
+ * dedicated structures in ksz_common.c.
  *
  * Copyright (c) 2017 Microchip Technology Inc.
  *	Tristram Ha <Tristram.Ha@microchip.com>
  */
 
-#ifndef __KSZ8795_REG_H
-#define __KSZ8795_REG_H
+#ifndef __KSZ8_REG_H
+#define __KSZ8_REG_H
 
 #define KS_PORT_M			0x1F
 
@@ -359,8 +364,6 @@ 
 #define REG_IND_DATA_1			0x77
 #define REG_IND_DATA_0			0x78
 
-#define REG_IND_DATA_PME_EEE_ACL	0xA0
-
 #define REG_INT_STATUS			0x7C
 #define REG_INT_ENABLE			0x7D