Re: [PATCH v3 0/8] pinctrl: Add T-Head TH1520 SoC pin controllers

From: Drew Fustini <dfustini@tenstorrent.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Drew Fustini <drew@pdp7.com>, Guo Ren <guoren@kernel.org>,
	Fu Wei <wefu@redhat.com>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Emil Renner Berthing <emil.renner.berthing@canonical.com>,
	Thomas Bonnefille <thomas.bonnefille@bootlin.com>,
	linux-riscv@lists.infradead.org, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 0/8] pinctrl: Add T-Head TH1520 SoC pin controllers
Date: Wed, 2 Oct 2024 17:02:42 -0700	[thread overview]
Message-ID: <Zv3fIuMUhukmOWQJ@x1> (raw)
In-Reply-To: <CACRpkdZRg+k=N42EA3+3c4Er=DHf2Q1aVzzCM0OQuEx7xWMAvw@mail.gmail.com>

On Wed, Oct 02, 2024 at 10:46:41PM +0200, Linus Walleij wrote:
> On Wed, Oct 2, 2024 at 8:35 PM Drew Fustini <dfustini@tenstorrent.com> wrote:
> 
> > > Then I merged that into my "devel" branch for v6.13.
> >
> > Thanks for taking this. Will that also end up in linux-next eventually?
> 
> Yes next -next.
> 
> > I'm working on a TH1520 Ethernet driver which depends on the pinctrl
> > driver. Andrew Lunn replied to me that all the dependencies need to be
> > in linux-next [1].
> 
> Well compile-time dependencies for sure, run-time dependencies
> we are usually a bit lax with as long as we know they will
> get there eventually.
> 
> > > I think I'll make a stab at using guarded mutexes etc and see what
> > > you think about it!
> >
> > Do you mean using scoped_guard() for thp->mutex in
> > th1520_pinctrl_dt_node_to_map()?
> 
> For all mutex and spinlocks in the driver.

The thp->lock spinlock is already using scoped_guard() everywhere.

I will post a patch that adds guard() for the thp->mutex like this:

diff --git a/drivers/pinctrl/pinctrl-th1520.c b/drivers/pinctrl/pinctrl-th1520.c
index 1bb78b212fd5..b7c2d998e9e7 100644
--- a/drivers/pinctrl/pinctrl-th1520.c
+++ b/drivers/pinctrl/pinctrl-th1520.c
@@ -444,8 +444,8 @@ static int th1520_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
                return -ENOMEM;

        nmaps = 0;
-       mutex_lock(&thp->mutex);
-       for_each_available_child_of_node(np, child) {
+       guard(mutex)(&thp->mutex);
+       for_each_available_child_of_node_scoped(np, child) {
                unsigned int rollback = nmaps;
                enum th1520_muxtype muxtype;
                struct property *prop;
@@ -530,7 +530,6 @@ static int th1520_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,

        *maps = map;
        *num_maps = nmaps;
-       mutex_unlock(&thp->mutex);
        return 0;

 free_configs:
@@ -538,7 +537,6 @@ static int th1520_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
 put_child:
        of_node_put(child);
        th1520_pinctrl_dt_free_map(pctldev, map, nmaps);
-       mutex_unlock(&thp->mutex);
        return ret;
 }

--
Thanks,
Drew
