Re: [PATCH v1 14/19] pinctrl: st: Use temporary variable for struct device

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Sat, Nov 06, 2021 at 01:28:17AM -0700, Joe Perches wrote:
> On Sat, 2021-11-06 at 10:07 +0200, Andy Shevchenko wrote:
> > On Saturday, November 6, 2021, Joe Perches <joe@xxxxxxxxxxx> wrote:
> > > On Fri, 2021-11-05 at 14:42 +0200, Andy Shevchenko wrote:

...

> > > > -     new_map = devm_kcalloc(pctldev->dev,
> > > > -                             map_num, sizeof(*new_map), GFP_KERNEL);
> > > > +     new_map = devm_kcalloc(dev, map_num, sizeof(*new_map), GFP_KERNEL);
> > > 
> > > Are pctldev->dev and dev the same pointer?
> > 
> > Seems so.
> 
> OK.
> 
> > https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/core.c#L2015
> > 
> > > It seems they are not.
> > 
> > Can you elaborate, please?

> From code shape, you assign dev to info->dev rather than pctldev->dev

Yes. And they are the same. TBH these three drivers seem to be written by
copy'n'paste method where the first one, whichever it was, is simply messy
and buggy.

The extra redundant parameter (often struct platform_device) is passed to
zillions of functions when at the same time info structure already has pointer
to struct device is the easiest one to notice. And I believe so on, so on...

> I also believe this single 19 patch series would be better as
> multiple patch series.

I'm fine with either, but I would like to hear from Linus about what he wishes
as the maintainer. You know that we don't add code without users? So that's why
my motive to send it in full.

> IMO: the strarray variants introduction and use should be a separate
> patchset from the rest.

It will add unnecessary churn. Yeah, I have planned to send just that, but then
it took more and more cleanups and I have to stop at some point, the code there
is bad (historically or by other reasons, dunno).

-- 
With Best Regards,
Andy Shevchenko