All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcus Folkesson <marcus.folkesson@gmail.com>
To: Peter Rosin <peda@axentia.se>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>,
	Michael Hennerich <michael.hennerich@analog.com>,
	Bartosz Golaszewski <brgl@bgdev.pl>,
	Andi Shyti <andi.shyti@kernel.org>,
	Bartosz Golaszewski <brgl@kernel.org>,
	linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 2/5] i2c: mux: add support for per channel bus frequency
Date: Fri, 13 Feb 2026 11:59:05 +0100	[thread overview]
Message-ID: <aY8D-UJnmmGUl3lr@gmail.com> (raw)
In-Reply-To: <aY5KeH3Z1S3MswHI@gmail.com>

Hi Peter,

On Thu, Feb 12, 2026 at 10:47:36PM +0100, Marcus Folkesson wrote:
> Hi Peter!
> 
> Thank you so much for your thoughs!
> 
> On Thu, Feb 12, 2026 at 04:02:16PM +0100, Peter Rosin wrote:
> > Hi!
> > 
> 
> [...]
> 
> > > +static int i2c_mux_select_chan(struct i2c_adapter *adap, u32 chan_id)
> > > +{
> > > +	struct i2c_mux_priv *priv = adap->algo_data;
> > > +	struct i2c_mux_core *muxc = priv->muxc;
> > > +	struct i2c_adapter *parent = muxc->parent;
> > > +	struct i2c_adapter *root;
> > > +	int ret;
> > > +
> > > +	if (priv->adap.clock_hz && priv->adap.clock_hz != parent->clock_hz) {
> > > +		root = i2c_root_adapter(&adap->dev);
> > > +
> > > +		/* if we are parent-locked and the root adapter is our parent,
> > > +		 * we already have the lock we need. Otherwise take the bus lock for the root
> > > +		 * adapter before changing bus clock.
> > > +		 */
> > 
> > The assumptions made for the "otherwise" case is wrong, I think.
> 
> I think you are right.
> 
> > Consider e.g. the case where we are parent-locket, our parent is
> > another parent-locked mux and our grand-parent is the root adapter.
> > In that case we also have all the locks we need. Trying to grab
> > them again will be a deadlock.
> > 
> > The more correct approach is to do the parent walk in search of
> > the first ancestor mux that is not parent-locked and then call
> 
> Hrm, does it not apply for all mutex-locked ancestors?
> 
> Consider the following chain:
> Root - P1 - M1 - M2 - P2 - D1
> 
> P - Parent locked
> M - Mux locked
> D - Device
> 
> In this case we need to lock both M1 and M2, not just M2 ?
> I'm not completely sure though, I need to refresh myself on the code
> base.

Okay, after a good night's sleep, I think it should lock not the first
but topmost not parent-locked mux only, IOW, M1 in the example above.
Then the locking would be propageted up to the root adapter.

I will send out a v5 for this soon.

Best regards,
Marcus Folkesson


  reply	other threads:[~2026-02-13 10:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-28  9:54 [PATCH v4 0/5] I2C Mux per channel bus speed Marcus Folkesson
2026-01-28  9:54 ` [PATCH v4 1/5] i2c: core: add callback to change bus frequency Marcus Folkesson
2026-01-28  9:54 ` [PATCH v4 2/5] i2c: mux: add support for per channel " Marcus Folkesson
2026-02-12 15:02   ` Peter Rosin
2026-02-12 21:47     ` Marcus Folkesson
2026-02-13 10:59       ` Marcus Folkesson [this message]
2026-02-13 11:37       ` Peter Rosin
2026-02-13 15:53         ` Marcus Folkesson
2026-01-28  9:54 ` [PATCH v4 3/5] i2c: davinci: calculate bus freq from Hz instead of kHz Marcus Folkesson
2026-01-28  9:54 ` [PATCH v4 4/5] i2c: davinci: add support for setting bus frequency Marcus Folkesson
2026-01-28  9:54 ` [PATCH v4 5/5] docs: i2c: i2c-topology: add section about bus speed Marcus Folkesson
2026-02-12 13:49 ` [PATCH v4 0/5] I2C Mux per channel " Marcus Folkesson

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=aY8D-UJnmmGUl3lr@gmail.com \
    --to=marcus.folkesson@gmail.com \
    --cc=andi.shyti@kernel.org \
    --cc=brgl@bgdev.pl \
    --cc=brgl@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael.hennerich@analog.com \
    --cc=peda@axentia.se \
    --cc=wsa+renesas@sang-engineering.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.