diff mbox series

[v2,net-next] net: dsa: microchip: ksz9477: split half-duplex monitoring function

Message ID 20240808151421.636937-1-enguerrand.de-ribaucourt@savoirfairelinux.com (mailing list archive)
State Superseded
Commit c4e82c025b3f2561823b4ba7c5f112a2005f442b
Delegated to: Netdev Maintainers
Headers show
Series [v2,net-next] net: dsa: microchip: ksz9477: split half-duplex monitoring function | 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: 29 this patch: 29
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 4 maintainers not CCed: f.fainelli@gmail.com edumazet@google.com pabeni@redhat.com olteanv@gmail.com
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, 108 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
netdev/contest success net-next-2024-08-10--18-00 (tests: 707)

Commit Message

Enguerrand de Ribaucourt Aug. 8, 2024, 3:14 p.m. UTC
In order to respect the 80 columns limit, split the half-duplex
monitoring function in two.

This is just a styling change, no functional change.

Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>

---
v2:
 - added line breaks after return statements
 - removed Fixed-by: tag
v1: https://lore.kernel.org/netdev/20240708084934.131175-1-enguerrand.de-ribaucourt@savoirfairelinux.com/
---
 drivers/net/dsa/microchip/ksz9477.c | 91 +++++++++++++++++------------
 1 file changed, 54 insertions(+), 37 deletions(-)

Comments

Arun Ramadoss Aug. 8, 2024, 3:46 p.m. UTC | #1
On Thu, 2024-08-08 at 15:14 +0000, Enguerrand de Ribaucourt wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> In order to respect the 80 columns limit, split the half-duplex
> monitoring function in two.
> 
> This is just a styling change, no functional change.
> 
> Signed-off-by: Enguerrand de Ribaucourt <
> enguerrand.de-ribaucourt@savoirfairelinux.com>

Acked-by: Arun Ramadoss <arun.ramadoss@microchip.com>
patchwork-bot+netdevbpf@kernel.org Aug. 11, 2024, 4:21 p.m. UTC | #2
Hello:

This patch was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Thu,  8 Aug 2024 15:14:21 +0000 you wrote:
> In order to respect the 80 columns limit, split the half-duplex
> monitoring function in two.
> 
> This is just a styling change, no functional change.
> 
> Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>
> 
> [...]

Here is the summary with links:
  - [v2,net-next] net: dsa: microchip: ksz9477: split half-duplex monitoring function
    https://git.kernel.org/netdev/net-next/c/c4e82c025b3f

You are awesome, thank you!
Andrew Lunn Aug. 11, 2024, 4:22 p.m. UTC | #3
> -	ret = ksz_pread8(dev, port, REG_PORT_STATUS_0, &status);
> +	/* Errata DS80000754 recommends monitoring potential faults in
> +	 * half-duplex mode. The switch might not be able to communicate anymore
> +	 * in these states. If you see this message, please read the
> +	 * errata-sheet for more information:
> +	 * https://ww1.microchip.com/downloads/aemDocuments/documents
> +	 * /UNG/ProductDocuments/Errata/KSZ9477S-Errata-DS80000754.pdf

The 80 character rule is not sacrosanct. We allow strings which will
appear in the log to be longer than 80 because they are hard to grep
for otherwise. The 80 character limit is really about keeping the
indentation level low, encouraging lots of small functions which do
one thing, and does it well. This is exactly what you are doing here,
taking a big function and splitting it up into smaller functions with
less indentation. I see a URL as something which should not be split
like this, it should be allowed to be longer than 80 characters,
because i want it to be easy to copy/paste into a web browser.

With that fixed, please add a Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
diff mbox series

Patch

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 425e20daf1e9..1e2293aa00dc 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -427,54 +427,71 @@  void ksz9477_freeze_mib(struct ksz_device *dev, int port, bool freeze)
 	mutex_unlock(&p->mib.cnt_mutex);
 }
 
-int ksz9477_errata_monitor(struct ksz_device *dev, int port,
-			   u64 tx_late_col)
+static int ksz9477_half_duplex_monitor(struct ksz_device *dev, int port,
+				       u64 tx_late_col)
 {
+	u8 lue_ctrl;
 	u32 pmavbc;
-	u8 status;
 	u16 pqm;
 	int ret;
 
-	ret = ksz_pread8(dev, port, REG_PORT_STATUS_0, &status);
+	/* Errata DS80000754 recommends monitoring potential faults in
+	 * half-duplex mode. The switch might not be able to communicate anymore
+	 * in these states. If you see this message, please read the
+	 * errata-sheet for more information:
+	 * https://ww1.microchip.com/downloads/aemDocuments/documents
+	 * /UNG/ProductDocuments/Errata/KSZ9477S-Errata-DS80000754.pdf
+	 * To workaround this issue, half-duplex mode should be avoided.
+	 * A software reset could be implemented to recover from this state.
+	 */
+	dev_warn_once(dev->dev,
+		      "Half-duplex detected on port %d, transmission halt may occur\n",
+		      port);
+	if (tx_late_col != 0) {
+		/* Transmission halt with late collisions */
+		dev_crit_once(dev->dev,
+			      "TX late collisions detected, transmission may be halted on port %d\n",
+			      port);
+	}
+	ret = ksz_read8(dev, REG_SW_LUE_CTRL_0, &lue_ctrl);
 	if (ret)
 		return ret;
-	if (!(FIELD_GET(PORT_INTF_SPEED_MASK, status) == PORT_INTF_SPEED_NONE) &&
-	    !(status & PORT_INTF_FULL_DUPLEX)) {
-		/* Errata DS80000754 recommends monitoring potential faults in
-		 * half-duplex mode. The switch might not be able to communicate anymore
-		 * in these states.
-		 * If you see this message, please read the errata-sheet for more information:
-		 * https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/Errata/KSZ9477S-Errata-DS80000754.pdf
-		 * To workaround this issue, half-duplex mode should be avoided.
-		 * A software reset could be implemented to recover from this state.
-		 */
-		dev_warn_once(dev->dev,
-			      "Half-duplex detected on port %d, transmission halt may occur\n",
-			      port);
-		if (tx_late_col != 0) {
-			/* Transmission halt with late collisions */
-			dev_crit_once(dev->dev,
-				      "TX late collisions detected, transmission may be halted on port %d\n",
-				      port);
-		}
-		ret = ksz_read8(dev, REG_SW_LUE_CTRL_0, &status);
+	if (lue_ctrl & SW_VLAN_ENABLE) {
+		ret = ksz_pread16(dev, port, REG_PORT_QM_TX_CNT_0__4, &pqm);
 		if (ret)
 			return ret;
-		if (status & SW_VLAN_ENABLE) {
-			ret = ksz_pread16(dev, port, REG_PORT_QM_TX_CNT_0__4, &pqm);
-			if (ret)
-				return ret;
-			ret = ksz_read32(dev, REG_PMAVBC, &pmavbc);
-			if (ret)
-				return ret;
-			if ((FIELD_GET(PMAVBC_MASK, pmavbc) <= PMAVBC_MIN) ||
-			    (FIELD_GET(PORT_QM_TX_CNT_M, pqm) >= PORT_QM_TX_CNT_MAX)) {
-				/* Transmission halt with Half-Duplex and VLAN */
-				dev_crit_once(dev->dev,
-					      "resources out of limits, transmission may be halted\n");
-			}
+
+		ret = ksz_read32(dev, REG_PMAVBC, &pmavbc);
+		if (ret)
+			return ret;
+
+		if ((FIELD_GET(PMAVBC_MASK, pmavbc) <= PMAVBC_MIN) ||
+		    (FIELD_GET(PORT_QM_TX_CNT_M, pqm) >= PORT_QM_TX_CNT_MAX)) {
+			/* Transmission halt with Half-Duplex and VLAN */
+			dev_crit_once(dev->dev,
+				      "resources out of limits, transmission may be halted\n");
 		}
 	}
+
+	return ret;
+}
+
+int ksz9477_errata_monitor(struct ksz_device *dev, int port,
+			   u64 tx_late_col)
+{
+	u8 status;
+	int ret;
+
+	ret = ksz_pread8(dev, port, REG_PORT_STATUS_0, &status);
+	if (ret)
+		return ret;
+
+	if (!(FIELD_GET(PORT_INTF_SPEED_MASK, status)
+	      == PORT_INTF_SPEED_NONE) &&
+	    !(status & PORT_INTF_FULL_DUPLEX)) {
+		ret = ksz9477_half_duplex_monitor(dev, port, tx_late_col);
+	}
+
 	return ret;
 }