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

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Joe Perches <joe@perches.com>
Cc: Bartosz Golaszewski <brgl@bgdev.pl>,
	Jianqun Xu <jay.xu@rock-chips.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Sai Krishna Potthuri <lakshmi.sai.krishna.potthuri@xilinx.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-rockchip@lists.infradead.org" 
	<linux-rockchip@lists.infradead.org>,
	Bamvor Jian Zhang <bamv2005@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Gregory Clement <gregory.clement@bootlin.com>,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Patrice Chotard <patrice.chotard@foss.st.com>,
	Michal Simek <michal.simek@xilinx.com>,
	Andy Shevchenko <andy@kernel.org>
Subject: Re: [PATCH v1 14/19] pinctrl: st: Use temporary variable for struct device
Date: Mon, 8 Nov 2021 11:26:04 +0200	[thread overview]
Message-ID: <YYjtLGlSCDPM4UVL@smile.fi.intel.com> (raw)
In-Reply-To: <103d7321a2d18e44fb8c01483b1197766aff645a.camel@perches.com>

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@perches.com> 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