Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH net 1/1] net: dsa: qca: ar9331: reorder MDIO write sequence
@ 2021-08-03  6:37 Oleksij Rempel
  2021-08-03  9:49 ` Florian Fainelli
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Oleksij Rempel @ 2021-08-03  6:37 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, Russell King
  Cc: Oleksij Rempel, Pengutronix Kernel Team, netdev, linux-kernel,
	linux-mips

In case of this switch we work with 32bit registers on top of 16bit
bus. Some registers (for example access to forwarding database) have
trigger bit on the first 16bit half of request and the result +
configuration of request in the second half. Without this patch, we would
trigger database operation and overwrite result in one run.

To make it work properly, we should do the second part of transfer
before the first one is done.

So far, this rule seems to work for all registers on this switch.

Fixes: ec6698c272de ("net: dsa: add support for Atheros AR9331 built-in switch")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/qca/ar9331.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/qca/ar9331.c b/drivers/net/dsa/qca/ar9331.c
index ca2ad77b71f1..6686192e1883 100644
--- a/drivers/net/dsa/qca/ar9331.c
+++ b/drivers/net/dsa/qca/ar9331.c
@@ -837,16 +837,24 @@ static int ar9331_mdio_write(void *ctx, u32 reg, u32 val)
 		return 0;
 	}
 
-	ret = __ar9331_mdio_write(sbus, AR9331_SW_MDIO_PHY_MODE_REG, reg, val);
+	/* In case of this switch we work with 32bit registers on top of 16bit
+	 * bus. Some registers (for example access to forwarding database) have
+	 * trigger bit on the first 16bit half of request, the result and
+	 * configuration of request in the second half.
+	 * To make it work properly, we should do the second part of transfer
+	 * before the first one is done.
+	 */
+	ret = __ar9331_mdio_write(sbus, AR9331_SW_MDIO_PHY_MODE_REG, reg + 2,
+				  val >> 16);
 	if (ret < 0)
 		goto error;
 
-	ret = __ar9331_mdio_write(sbus, AR9331_SW_MDIO_PHY_MODE_REG, reg + 2,
-				  val >> 16);
+	ret = __ar9331_mdio_write(sbus, AR9331_SW_MDIO_PHY_MODE_REG, reg, val);
 	if (ret < 0)
 		goto error;
 
 	return 0;
+
 error:
 	dev_err_ratelimited(&sbus->dev, "Bus error. Failed to write register.\n");
 	return ret;
-- 
2.30.2


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net 1/1] net: dsa: qca: ar9331: reorder MDIO write sequence
  2021-08-03  6:37 [PATCH net 1/1] net: dsa: qca: ar9331: reorder MDIO write sequence Oleksij Rempel
@ 2021-08-03  9:49 ` Florian Fainelli
  2021-08-03  9:53 ` Vladimir Oltean
  2021-08-03 21:40 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Florian Fainelli @ 2021-08-03  9:49 UTC (permalink / raw)
  To: Oleksij Rempel, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, Russell King
  Cc: Pengutronix Kernel Team, netdev, linux-kernel, linux-mips



On 8/2/2021 11:37 PM, Oleksij Rempel wrote:
> In case of this switch we work with 32bit registers on top of 16bit
> bus. Some registers (for example access to forwarding database) have
> trigger bit on the first 16bit half of request and the result +
> configuration of request in the second half. Without this patch, we would
> trigger database operation and overwrite result in one run.
> 
> To make it work properly, we should do the second part of transfer
> before the first one is done.
> 
> So far, this rule seems to work for all registers on this switch.
> 
> Fixes: ec6698c272de ("net: dsa: add support for Atheros AR9331 built-in switch")
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net 1/1] net: dsa: qca: ar9331: reorder MDIO write sequence
  2021-08-03  6:37 [PATCH net 1/1] net: dsa: qca: ar9331: reorder MDIO write sequence Oleksij Rempel
  2021-08-03  9:49 ` Florian Fainelli
@ 2021-08-03  9:53 ` Vladimir Oltean
  2021-08-03 21:40 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Vladimir Oltean @ 2021-08-03  9:53 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
	Jakub Kicinski, Russell King, Pengutronix Kernel Team, netdev,
	linux-kernel, linux-mips

On Tue, Aug 03, 2021 at 08:37:46AM +0200, Oleksij Rempel wrote:
> In case of this switch we work with 32bit registers on top of 16bit
> bus. Some registers (for example access to forwarding database) have
> trigger bit on the first 16bit half of request and the result +
> configuration of request in the second half. Without this patch, we would
> trigger database operation and overwrite result in one run.
> 
> To make it work properly, we should do the second part of transfer
> before the first one is done.
> 
> So far, this rule seems to work for all registers on this switch.
> 
> Fixes: ec6698c272de ("net: dsa: add support for Atheros AR9331 built-in switch")
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> ---

I don't really like to chase a patch to make sure my review tags get
propagated, but anyway:

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

https://patchwork.kernel.org/project/netdevbpf/patch/20210802131037.32326-2-o.rempel@pengutronix.de/

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net 1/1] net: dsa: qca: ar9331: reorder MDIO write sequence
  2021-08-03  6:37 [PATCH net 1/1] net: dsa: qca: ar9331: reorder MDIO write sequence Oleksij Rempel
  2021-08-03  9:49 ` Florian Fainelli
  2021-08-03  9:53 ` Vladimir Oltean
@ 2021-08-03 21:40 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-08-03 21:40 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: andrew, vivien.didelot, f.fainelli, olteanv, davem, kuba, linux,
	kernel, netdev, linux-kernel, linux-mips

Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Tue,  3 Aug 2021 08:37:46 +0200 you wrote:
> In case of this switch we work with 32bit registers on top of 16bit
> bus. Some registers (for example access to forwarding database) have
> trigger bit on the first 16bit half of request and the result +
> configuration of request in the second half. Without this patch, we would
> trigger database operation and overwrite result in one run.
> 
> To make it work properly, we should do the second part of transfer
> before the first one is done.
> 
> [...]

Here is the summary with links:
  - [net,1/1] net: dsa: qca: ar9331: reorder MDIO write sequence
    https://git.kernel.org/netdev/net/c/d1a58c013a58

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



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-08-03 21:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-03  6:37 [PATCH net 1/1] net: dsa: qca: ar9331: reorder MDIO write sequence Oleksij Rempel
2021-08-03  9:49 ` Florian Fainelli
2021-08-03  9:53 ` Vladimir Oltean
2021-08-03 21:40 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).