mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Vasilij Strassheim <v.strassheim@linutronix.de>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Vladimir Oltean <olteanv@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	Russell King <linux@armlinux.org.uk>,
	 devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org,
	 Martin Kaistra <martin.kaistra@linutronix.de>,
	Benedikt Spranger <b.spranger@linutronix.de>
Subject: Re: [PATCH net-next v2 4/4] net: dsa: soce: Add basic support for SoC-e switch IP cores
Date: Wed, 16 Sep 2026 18:31:31 +0200	[thread overview]
Message-ID: <ffec9201db3d12944dbd350010560f6234262bbe.camel@linutronix.de> (raw)
In-Reply-To: <2346e6ee-4075-4d64-8e3e-8fbd1ce81cd6@lunn.ch>

On Tue, 2026-09-15 at 14:52 +0200, Andrew Lunn wrote:
> > Although mdio-mux-mmioreg might work, I have realized that it is not
> > suitable here.
> > 
> > According to the switch documentation, the bus selector occupies bits
> > 26:16 of the MDIO control register, while bit 0 is the transaction
> > start/busy bit. A partial write might trigger an operation before the
> > other fields have been updated.
> 
> Maybe. I can see at minimum it is a bit messy.  The mux would need to
> write the upper bits, but set the lower start/busy to 0. The MDIO
> driver would then need to read back the register, OR in the bits it
> wants to set, and set the start bit.
> 
> This start bit is pretty common, and generally, writes without it set
> are safe. But you need to test it on this particular hardware.
> 

Yes, it works.

> > Also accessing this register without checking the controller state
> > could interfere with an active or failed transaction and introduce
> > races (as noted by Netdev-Sashiko).
> 
> The mdio mux framework should take care of all the locking for you.
> Take a look a mdio_mux_read(). It takes the lock of the real MDIO bus
> controller, sets the mux, performs the read, and then releases the
> lock.
> 
> The only thing you need to be careful of is write must wait around for
> the write to complete before returning. Some MDIO bus implementations
> don't wait, they leave it running, and do a check the bus is idle
> before doing the next operation.
> 

I would like to do both. Check at the beginning and wait for the
operation to complete. However, this final wait should not last forever.
If a timeout occurs, something is completely broken and an error is
reported accordingly.

I still had concerns about the following call to set the mux with
mdio_mux_mmioreg_switch_fn() if the previous operation ended in such an
error and released the lock. However, in that case, we are already in a
faulty state and it can not get any worse. The following operation
checks the state and aborts anyway. An alternative switch_fn() with the
overhead of an extra driver is not worth it just to preserve a bitfield
in undefined error state.

So I will implement this as suggested at the beginning using
mdio-mux-mmioreg.

>        Andrew

Thanks,
Vasilij


  reply	other threads:[~2026-09-16 16:31 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-03 18:10 [PATCH net-next v2 0/4] net: dsa: Add SoC-e DSA driver Vasilij Strassheim
2026-09-03 18:10 ` [PATCH net-next v2 1/4] dt-bindings: vendor-prefixes: Add soce Vasilij Strassheim
2026-09-07  9:14   ` Krzysztof Kozlowski
2026-09-03 18:10 ` [PATCH net-next v2 2/4] dt-bindings: net: dsa: Add SoC-e SWIP switch Vasilij Strassheim
2026-09-07  9:21   ` Krzysztof Kozlowski
2026-09-07 14:01     ` Vasilij Strassheim
2026-09-07 18:56       ` Andrew Lunn
2026-09-08 10:16         ` Vasilij Strassheim
2026-09-08 10:29         ` Kurt Kanzenbach
2026-09-08  9:14       ` Krzysztof Kozlowski
2026-09-08 10:09         ` Vasilij Strassheim
2026-09-07 19:04   ` Andrew Lunn
2026-09-07 19:09     ` Andrew Lunn
2026-09-08 18:15     ` Vasilij Strassheim
2026-09-08 19:10       ` Andrew Lunn
2026-09-09 18:46         ` Vasilij Strassheim
2026-09-10 12:12           ` Andrew Lunn
2026-09-15 10:23             ` Vasilij Strassheim
2026-09-03 18:11 ` [PATCH net-next v2 3/4] net: dsa: Add tag handling for SoC-e switches Vasilij Strassheim
2026-09-09 12:12   ` netdev-bot+sashiko
2026-09-03 18:11 ` [PATCH net-next v2 4/4] net: dsa: soce: Add basic support for SoC-e switch IP cores Vasilij Strassheim
2026-09-07 19:28   ` Andrew Lunn
2026-09-08 18:44     ` Vasilij Strassheim
2026-09-08 19:20       ` Andrew Lunn
2026-09-09 19:28         ` Vasilij Strassheim
2026-09-10 12:18           ` Andrew Lunn
2026-09-15 10:48             ` Vasilij Strassheim
2026-09-15 12:52               ` Andrew Lunn
2026-09-16 16:31                 ` Vasilij Strassheim [this message]
2026-09-08  0:37   ` Andrew Lunn
2026-09-10 13:01     ` Vasilij Strassheim
2026-09-10 15:07       ` Andrew Lunn
2026-09-11 13:39         ` Vasilij Strassheim
2026-09-08  8:25   ` Kurt Kanzenbach
2026-09-08 10:12     ` Vasilij Strassheim
2026-09-09 12:12   ` netdev-bot+sashiko
2026-09-15 11:23     ` Vasilij Strassheim
2026-09-15 12:56       ` Andrew Lunn
2026-09-15 18:12         ` Vasilij Strassheim
2026-09-07  9:10 ` [PATCH net-next v2 0/4] net: dsa: Add SoC-e DSA driver Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ffec9201db3d12944dbd350010560f6234262bbe.camel@linutronix.de \
    --to=v.strassheim@linutronix.de \
    --cc=andrew@lunn.ch \
    --cc=b.spranger@linutronix.de \
    --cc=conor+dt@kernel.org \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=martin.kaistra@linutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®