[PATCH v2 0/4] ksz9477 dsa switch driver improvements

[PATCH v2 0/4] ksz9477 dsa switch driver improvements

[Paul Barker]

These changes were made while debugging the ksz9477 driver for use on a
custom board which uses the ksz9893 switch supported by this driver. The
patches have been runtime tested on top of Linux 5.8.4, I couldn't
runtime test them on top of 5.9-rc3 due to unrelated issues. They have
been build tested on top of net-next.

Changes from v1:

  * Rebased onto net-next.

  * Dropped unnecessary `#include <linux/printk.h>`.

  * Instead of printing the phy mode in `ksz9477_port_setup()`, modify
    the existing print in `ksz9477_config_cpu_port()` to always produce
    output and to be more clear.

  * Include Reviewed-by tag from v1 series so it isn't lost (is this
    correct?).

Paul Barker (4):
  net: dsa: microchip: Make switch detection more informative
  net: dsa: microchip: Improve phy mode message
  net: dsa: microchip: Disable RGMII in-band status on KSZ9893
  net: dsa: microchip: Implement recommended reset timing

 drivers/net/dsa/microchip/ksz9477.c    | 25 ++++++++++++++++++++-----
 drivers/net/dsa/microchip/ksz_common.c |  3 ++-
 2 files changed, 22 insertions(+), 6 deletions(-)

-- 
2.28.0

[PATCH v2 1/4] net: dsa: microchip: Make switch detection more informative

[Paul Barker]

To make switch detection more informative print the result of the
ksz9477/ksz9893 compatibility check. With debug output enabled also
print the contents of the Chip ID registers as a 40-bit hex string.

As this detection is the first communication with the switch performed
by the driver, making it easy to see any errors here will help identify
issues with SPI data corruption or reset sequencing.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 drivers/net/dsa/microchip/ksz9477.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 3cb22d149813..df5ecd0261fa 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1426,10 +1426,12 @@ static int ksz9477_switch_detect(struct ksz_device *dev)
 	/* Default capability is gigabit capable. */
 	dev->features = GBIT_SUPPORT;
 
+	dev_dbg(dev->dev, "Switch detect: ID=%08x%02x\n", id32, data8);
 	id_hi = (u8)(id32 >> 16);
 	id_lo = (u8)(id32 >> 8);
 	if ((id_lo & 0xf) == 3) {
 		/* Chip is from KSZ9893 design. */
+		dev_info(dev->dev, "Found KSZ9893\n");
 		dev->features |= IS_9893;
 
 		/* Chip does not support gigabit. */
@@ -1438,6 +1440,7 @@ static int ksz9477_switch_detect(struct ksz_device *dev)
 		dev->mib_port_cnt = 3;
 		dev->phy_port_cnt = 2;
 	} else {
+		dev_info(dev->dev, "Found KSZ9477 or compatible\n");
 		/* Chip uses new XMII register definitions. */
 		dev->features |= NEW_XMII;
 
-- 
2.28.0

[PATCH v2 2/4] net: dsa: microchip: Improve phy mode message

[Paul Barker]

Always print the selected phy mode for the CPU port when using the
ksz9477 driver. If the phy mode was changed, also print the previous
mode to aid in debugging.

To make the message more clear, prefix it with the port number which it
applies to and improve the language a little.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 drivers/net/dsa/microchip/ksz9477.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index df5ecd0261fa..9513af057793 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1265,6 +1265,7 @@ static void ksz9477_config_cpu_port(struct dsa_switch *ds)
 	for (i = 0; i < dev->port_cnt; i++) {
 		if (dsa_is_cpu_port(ds, i) && (dev->cpu_ports & (1 << i))) {
 			phy_interface_t interface;
+			char *prev_msg, *prev_mode;
 
 			dev->cpu_port = i;
 			dev->host_mask = (1 << dev->cpu_port);
@@ -1277,11 +1278,19 @@ static void ksz9477_config_cpu_port(struct dsa_switch *ds)
 			interface = ksz9477_get_interface(dev, i);
 			if (!dev->interface)
 				dev->interface = interface;
-			if (interface && interface != dev->interface)
-				dev_info(dev->dev,
-					 "use %s instead of %s\n",
-					  phy_modes(dev->interface),
-					  phy_modes(interface));
+			if (interface && interface != dev->interface) {
+				prev_msg = " instead of ";
+				prev_mode = phy_modes(interface);
+			} else {
+				prev_msg = "";
+				prev_mode = "";
+			}
+			dev_info(dev->dev,
+				 "Port%d: using phy mode %s%s%s\n",
+				 i,
+				 phy_modes(dev->interface),
+				 prev_msg,
+				 prev_mode);
 
 			/* enable cpu port */
 			ksz9477_port_setup(dev, i, true);
-- 
2.28.0

[PATCH v2 3/4] net: dsa: microchip: Disable RGMII in-band status on KSZ9893

[Paul Barker]

We can't assume that the link partner supports the in-band status
reporting which is enabled by default on the KSZ9893 when using RGMII
for the upstream port.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/microchip/ksz9477.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 9513af057793..f379ea8242e0 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1235,6 +1235,9 @@ static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port)
 			if (dev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
 			    dev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
 				data8 |= PORT_RGMII_ID_EG_ENABLE;
+			/* On KSZ9893, disable RGMII in-band status support */
+			if (dev->features & IS_9893)
+				data8 &= ~PORT_MII_MAC_MODE;
 			p->phydev.speed = SPEED_1000;
 			break;
 		}
-- 
2.28.0

[PATCH v2 4/4] net: dsa: microchip: Implement recommended reset timing

[Paul Barker]

The datasheet for the ksz9893 and ksz9477 switches recommend waiting at
least 100us after the de-assertion of reset before trying to program the
device through any interface.

Also switch the existing msleep() call to usleep_range() as recommended
in Documentation/timers/timers-howto.rst. The 2ms range used here is
somewhat arbitrary, as long as the reset is asserted for at least 10ms
we should be ok.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 8d53b12d40a8..a31738662d95 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -400,8 +400,9 @@ int ksz_switch_register(struct ksz_device *dev,
 
 	if (dev->reset_gpio) {
 		gpiod_set_value_cansleep(dev->reset_gpio, 1);
-		mdelay(10);
+		usleep_range(10000, 12000);
 		gpiod_set_value_cansleep(dev->reset_gpio, 0);
+		usleep_range(100, 1000);
 	}
 
 	mutex_init(&dev->dev_mutex);
-- 
2.28.0

Re: [PATCH v2 2/4] net: dsa: microchip: Improve phy mode message

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4249 bytes --]

Hi Paul,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.9-rc4 next-20200903]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Paul-Barker/ksz9477-dsa-switch-driver-improvements/20200907-181434
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git f4d51dffc6c01a9e94650d95ce0104964f8ae822
config: mips-randconfig-s031-20200907 (attached as .config)
compiler: mips64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.2-191-g10164920-dirty
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> drivers/net/dsa/microchip/ksz9477.c:1283:43: sparse: sparse: incorrect type in assignment (different modifiers) @@     expected char *prev_mode @@     got char const * @@
>> drivers/net/dsa/microchip/ksz9477.c:1283:43: sparse:     expected char *prev_mode
>> drivers/net/dsa/microchip/ksz9477.c:1283:43: sparse:     got char const *

# https://github.com/0day-ci/linux/commit/1513424eb4a28841a6305ecbbf6a172b4a605014
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Paul-Barker/ksz9477-dsa-switch-driver-improvements/20200907-181434
git checkout 1513424eb4a28841a6305ecbbf6a172b4a605014
vim +1283 drivers/net/dsa/microchip/ksz9477.c

  1256	
  1257	static void ksz9477_config_cpu_port(struct dsa_switch *ds)
  1258	{
  1259		struct ksz_device *dev = ds->priv;
  1260		struct ksz_port *p;
  1261		int i;
  1262	
  1263		ds->num_ports = dev->port_cnt;
  1264	
  1265		for (i = 0; i < dev->port_cnt; i++) {
  1266			if (dsa_is_cpu_port(ds, i) && (dev->cpu_ports & (1 << i))) {
  1267				phy_interface_t interface;
  1268				char *prev_msg, *prev_mode;
  1269	
  1270				dev->cpu_port = i;
  1271				dev->host_mask = (1 << dev->cpu_port);
  1272				dev->port_mask |= dev->host_mask;
  1273	
  1274				/* Read from XMII register to determine host port
  1275				 * interface.  If set specifically in device tree
  1276				 * note the difference to help debugging.
  1277				 */
  1278				interface = ksz9477_get_interface(dev, i);
  1279				if (!dev->interface)
  1280					dev->interface = interface;
  1281				if (interface && interface != dev->interface) {
  1282					prev_msg = " instead of ";
> 1283					prev_mode = phy_modes(interface);
  1284				} else {
  1285					prev_msg = "";
  1286					prev_mode = "";
  1287				}
  1288				dev_info(dev->dev,
  1289					 "Port%d: using phy mode %s%s%s\n",
  1290					 i,
  1291					 phy_modes(dev->interface),
  1292					 prev_msg,
  1293					 prev_mode);
  1294	
  1295				/* enable cpu port */
  1296				ksz9477_port_setup(dev, i, true);
  1297				p = &dev->ports[dev->cpu_port];
  1298				p->vid_member = dev->port_mask;
  1299				p->on = 1;
  1300			}
  1301		}
  1302	
  1303		dev->member = dev->host_mask;
  1304	
  1305		for (i = 0; i < dev->mib_port_cnt; i++) {
  1306			if (i == dev->cpu_port)
  1307				continue;
  1308			p = &dev->ports[i];
  1309	
  1310			/* Initialize to non-zero so that ksz_cfg_port_member() will
  1311			 * be called.
  1312			 */
  1313			p->vid_member = (1 << i);
  1314			p->member = dev->port_mask;
  1315			ksz9477_port_stp_state_set(ds, i, BR_STATE_DISABLED);
  1316			p->on = 1;
  1317			if (i < dev->phy_port_cnt)
  1318				p->phy = 1;
  1319			if (dev->chip_id == 0x00947700 && i == 6) {
  1320				p->sgmii = 1;
  1321	
  1322				/* SGMII PHY detection code is not implemented yet. */
  1323				p->phy = 0;
  1324			}
  1325		}
  1326	}
  1327	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38894 bytes --]

Re: [PATCH v2 3/4] net: dsa: microchip: Disable RGMII in-band status on KSZ9893

[Florian Fainelli]

On 9/7/2020 3:12 AM, Paul Barker wrote:
> We can't assume that the link partner supports the in-band status
> reporting which is enabled by default on the KSZ9893 when using RGMII
> for the upstream port.
> 
> Signed-off-by: Paul Barker <pbarker@konsulko.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v2 4/4] net: dsa: microchip: Implement recommended reset timing

[Florian Fainelli]

On 9/7/2020 3:12 AM, Paul Barker wrote:
> The datasheet for the ksz9893 and ksz9477 switches recommend waiting at
> least 100us after the de-assertion of reset before trying to program the
> device through any interface.
> 
> Also switch the existing msleep() call to usleep_range() as recommended
> in Documentation/timers/timers-howto.rst. The 2ms range used here is
> somewhat arbitrary, as long as the reset is asserted for at least 10ms
> we should be ok.
> 
> Signed-off-by: Paul Barker <pbarker@konsulko.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v2 2/4] net: dsa: microchip: Improve phy mode message

[Florian Fainelli]

On 9/7/2020 3:12 AM, Paul Barker wrote:
> Always print the selected phy mode for the CPU port when using the
> ksz9477 driver. If the phy mode was changed, also print the previous
> mode to aid in debugging.
> 
> To make the message more clear, prefix it with the port number which it
> applies to and improve the language a little.
> 
> Signed-off-by: Paul Barker <pbarker@konsulko.com>

Once you fix the kbuild robot complaint about prev_mode not being a 
const char *:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v2 1/4] net: dsa: microchip: Make switch detection more informative

[Florian Fainelli]

On 9/7/2020 3:12 AM, Paul Barker wrote:
> To make switch detection more informative print the result of the
> ksz9477/ksz9893 compatibility check. With debug output enabled also
> print the contents of the Chip ID registers as a 40-bit hex string.
> 
> As this detection is the first communication with the switch performed
> by the driver, making it easy to see any errors here will help identify
> issues with SPI data corruption or reset sequencing.
> 
> Signed-off-by: Paul Barker <pbarker@konsulko.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian