[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aQMwS5gyqVwYrGoS@smile.fi.intel.com>
Date: Thu, 30 Oct 2025 11:30:51 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Kees Cook <kees@...nel.org>, Mika Westerberg <westeri@...nel.org>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Manivannan Sadhasivam <mani@...nel.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Saravana Kannan <saravanak@...gle.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Andy Shevchenko <andy@...nel.org>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>,
	Srinivas Kandagatla <srini@...nel.org>,
	Liam Girdwood <lgirdwood@...il.com>,
	Mark Brown <broonie@...nel.org>, Jaroslav Kysela <perex@...ex.cz>,
	Takashi Iwai <tiwai@...e.com>,
	Alexey Klimov <alexey.klimov@...aro.org>,
	linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-gpio@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-sound@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v3 03/10] gpiolib: implement low-level, shared GPIO
 support
On Wed, Oct 29, 2025 at 04:57:02PM +0100, Bartosz Golaszewski wrote:
> On Wed, Oct 29, 2025 at 4:19 PM Andy Shevchenko
> <andriy.shevchenko@...el.com> wrote:
> > On Wed, Oct 29, 2025 at 01:39:34PM +0100, Bartosz Golaszewski wrote:
> > > On Wed, Oct 29, 2025 at 12:45 PM Andy Shevchenko
> > > <andriy.shevchenko@...el.com> wrote:
> > > > On Wed, Oct 29, 2025 at 12:20:39PM +0100, Bartosz Golaszewski wrote:
...
> > > > Besides strcmp_suffix() that already exists in OF core, there are also some
> > > > existing pieces that seems being repeated here (again). Can we reduce amount
> > > > of duplication?
> > >
> > > I'm afraid you need to be more specific here.
> >
> > You can simply browse the file, it's not long to find and think about it.
> > I'm _thinking_ that it's possible to improve the situation overall by
> > try our best of deduplicating (or rather not duplicating) things.
> 
> Sorry, but this is not how reviewing works. You can't just say: "I
> think this can be improved, go figure out what can and fix it, you can
> browse this file for reference". You need to specifically point out
> issues in code and propose alternatives.
Then consider this as a hint and not a review.
...
> > > > > +#if IS_ENABLED(CONFIG_OF)
> > > > > +static int gpio_shared_of_traverse(struct device_node *curr)
> > > > > +{
> > > >
> > > > I believe parts of this code may be resided somewhere in drivers/of/property.c
> > > > or nearby as it has the similar parsing routines.
> > >
> > > I don't think this is a good idea, I want to keep it within the
> > > confines of drivers/gpio/ and the use-case is so specific, there's
> > > really no point in putting parts of it under drivers/of/.
> > >
> > > If I could only iterate over all properties of an fwnode, I'd have
> > > skipped using OF-specific routines altogether.
> >
> > The problem is that every subsystem considers "it's not a good idea" or
> > "historical reasons" or other excuses. Since you are adding OF-specific
> > stuff that has something already done inside OF specific code, why to
> > spread it over the kernel by duplicating in another place(s)?
> 
> Well, point me to the things that have been done already and I'll see
> about reusing them.
I already gave a direction, but if you think the duplication is okay, up to
you. It can be addressed later, just a bit more of technical debt.
-- 
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists
 
