[PATCH v1 net-next 0/2] clean up ocelot_reset() routine

[PATCH v1 net-next 0/2] clean up ocelot_reset() routine

[Colin Foster]

ocelot_reset() will soon be exported to a common library to be used by
the ocelot_ext system. This will make error values from regmap calls
possible, so they must be checked. Additionally, readx_poll_timeout()
can be substituted for the custom loop, as a simple cleanup.

I don't have hardware to verify this set directly, but there shouldn't
be any functional changes.

v2:
    Fix 64-bit compiler warning (1/2) (kernel test robot)
    Remove unnecessary variable assignment (2/2)
    Add Reviewed tag (1/2)

Colin Foster (2):
  net: mscc: ocelot: utilize readx_poll_timeout() for chip reset
  net: mscc: ocelot: check return values of writes during reset

 drivers/net/ethernet/mscc/ocelot_vsc7514.c | 46 ++++++++++++++++------
 1 file changed, 33 insertions(+), 13 deletions(-)

-- 
2.25.1

[PATCH v2 net-next 1/2] net: mscc: ocelot: utilize readx_poll_timeout() for chip reset

[Colin Foster]

Clean up the reset code by utilizing readx_poll_timeout instead of a custom
loop.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reported-by: kernel test robot <lkp@intel.com>
---

v2:
    Remove IS_ERR_VALUE macro compile error for 64-bit
    Add Reviewed and Reported tags

---
 drivers/net/ethernet/mscc/ocelot_vsc7514.c | 32 ++++++++++++++++------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index ae42bbba5747..3fb9183c1159 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -6,6 +6,7 @@
  */
 #include <linux/dsa/ocelot.h>
 #include <linux/interrupt.h>
+#include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/of_net.h>
 #include <linux/netdevice.h>
@@ -25,6 +26,9 @@
 #define VSC7514_VCAP_POLICER_BASE			128
 #define VSC7514_VCAP_POLICER_MAX			191
 
+#define MEM_INIT_SLEEP_US				1000
+#define MEM_INIT_TIMEOUT_US				100000
+
 static const u32 *ocelot_regmap[TARGET_MAX] = {
 	[ANA] = vsc7514_ana_regmap,
 	[QS] = vsc7514_qs_regmap,
@@ -191,22 +195,32 @@ static const struct of_device_id mscc_ocelot_match[] = {
 };
 MODULE_DEVICE_TABLE(of, mscc_ocelot_match);
 
+static int ocelot_mem_init_status(struct ocelot *ocelot)
+{
+	unsigned int val;
+	int err;
+
+	err = regmap_field_read(ocelot->regfields[SYS_RESET_CFG_MEM_INIT],
+				&val);
+
+	return err ?: val;
+}
+
 static int ocelot_reset(struct ocelot *ocelot)
 {
-	int retries = 100;
+	int err;
 	u32 val;
 
 	regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_INIT], 1);
 	regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1);
 
-	do {
-		msleep(1);
-		regmap_field_read(ocelot->regfields[SYS_RESET_CFG_MEM_INIT],
-				  &val);
-	} while (val && --retries);
-
-	if (!retries)
-		return -ETIMEDOUT;
+	/* MEM_INIT is a self-clearing bit. Wait for it to be cleared (should be
+	 * 100us) before enabling the switch core.
+	 */
+	err = readx_poll_timeout(ocelot_mem_init_status, ocelot, val, !val,
+				 MEM_INIT_SLEEP_US, MEM_INIT_TIMEOUT_US);
+	if (err)
+		return err;
 
 	regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1);
 	regmap_field_write(ocelot->regfields[SYS_RESET_CFG_CORE_ENA], 1);
-- 
2.25.1

[PATCH v2 net-next 2/2] net: mscc: ocelot: check return values of writes during reset

[Colin Foster]

The ocelot_reset() function utilizes regmap_field_write() but wasn't
checking return values. While this won't cause issues for the current MMIO
regmaps, it could be an issue for externally controlled interfaces.

Add checks for these return values.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---

v2:
    Change "err = ...; return err" to just "return ..."

---
 drivers/net/ethernet/mscc/ocelot_vsc7514.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index 3fb9183c1159..6f22aea08a64 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -211,8 +211,13 @@ static int ocelot_reset(struct ocelot *ocelot)
 	int err;
 	u32 val;
 
-	regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_INIT], 1);
-	regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1);
+	err = regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_INIT], 1);
+	if (err)
+		return err;
+
+	err = regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1);
+	if (err)
+		return err;
 
 	/* MEM_INIT is a self-clearing bit. Wait for it to be cleared (should be
 	 * 100us) before enabling the switch core.
@@ -222,10 +227,11 @@ static int ocelot_reset(struct ocelot *ocelot)
 	if (err)
 		return err;
 
-	regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1);
-	regmap_field_write(ocelot->regfields[SYS_RESET_CFG_CORE_ENA], 1);
+	err = regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1);
+	if (err)
+		return err;
 
-	return 0;
+	return regmap_field_write(ocelot->regfields[SYS_RESET_CFG_CORE_ENA], 1);
 }
 
 /* Watermark encode
-- 
2.25.1

Re: [PATCH v1 net-next 0/2] clean up ocelot_reset() routine

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Sat, 17 Sep 2022 10:51:25 -0700 you wrote:
> ocelot_reset() will soon be exported to a common library to be used by
> the ocelot_ext system. This will make error values from regmap calls
> possible, so they must be checked. Additionally, readx_poll_timeout()
> can be substituted for the custom loop, as a simple cleanup.
> 
> I don't have hardware to verify this set directly, but there shouldn't
> be any functional changes.
> 
> [...]

Here is the summary with links:
  - [v2,net-next,1/2] net: mscc: ocelot: utilize readx_poll_timeout() for chip reset
    https://git.kernel.org/netdev/net-next/c/21bb08cd2cda
  - [v2,net-next,2/2] net: mscc: ocelot: check return values of writes during reset
    https://git.kernel.org/netdev/net-next/c/fa1d90b048c2

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

[PATCH v1 net-next 0/2] clean up ocelot_reset() routine

[Colin Foster]

ocelot_reset() will soon be exported to a common library to be used by
the ocelot_ext system. This will make error values from regmap calls
possible, so they must be checked. Additionally, readx_poll_timeout()
can be substituted for the custom loop, as a simple cleanup.

I don't have hardware to verify this set directly, but there shouldn't
be any functional changes.

Colin Foster (2):
  net: mscc: ocelot: utilize readx_poll_timeout() for chip reset
  net: mscc: ocelot: check return values of writes during reset

 drivers/net/ethernet/mscc/ocelot_vsc7514.c | 48 ++++++++++++++++------
 1 file changed, 35 insertions(+), 13 deletions(-)

-- 
2.25.1