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

[Drew Fustini <dfustini@xxxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxxx> 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