linux-kernel - Re: [PATCH v6 03/12] pinctrl: Add MAX7360 pinctrl driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z_ahLqrMASFXbG5Q@smile.fi.intel.com>
Date: Wed, 9 Apr 2025 19:32:46 +0300
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Mathieu Dubois-Briand <mathieu.dubois-briand@...tlin.com>
Cc: Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Kamel Bouhara <kamel.bouhara@...tlin.com>,
	Linus Walleij <linus.walleij@...aro.org>,
	Bartosz Golaszewski <brgl@...ev.pl>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Uwe Kleine-König <ukleinek@...nel.org>,
	Michael Walle <mwalle@...nel.org>, Mark Brown <broonie@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Danilo Krummrich <dakr@...nel.org>, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
	linux-input@...r.kernel.org, linux-pwm@...r.kernel.org,
	Grégory Clement <gregory.clement@...tlin.com>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v6 03/12] pinctrl: Add MAX7360 pinctrl driver

On Wed, Apr 09, 2025 at 05:03:02PM +0200, Mathieu Dubois-Briand wrote:
> On Wed Apr 9, 2025 at 4:55 PM CEST, Mathieu Dubois-Briand wrote:
> > Add driver for Maxim Integrated MAX7360 pinctrl on the PORT pins. Pins
> > can be used either for GPIO, PWM or rotary encoder functionalities.

...

The all the rest of the driver LGTM, but the below.

> > +	device_set_of_node_from_dev(dev, dev->parent);
> 
> Ok, so this goes a bit against what I said I was going to do on my
> previous series, let me explain why. Same reasoning applies for both
> uses, in PWM and pinctrl drivers.
> 
> With my previous experiments, I came to the conclusion that:
> - Either we should use device_set_of_node_from_dev() as I do here.
> - Or we should add more subnodes in the device tree binding.

> - Also, copying the fwnode with device_set_node() was not possible, as
>   the kernel would then try to apply pinctrl on both the parent and
>   child device.

Hmm... I need to refresh my memory with the old discussions. Can you point out
to the problem statement with that approach?

> I previously said the second solution was probably the way to go, but I
> changed my mind for two reasons.
> 
> First having more subnodes in the device tree was already rejected in
> the past in the reviews of the dt-bindings patch. This do makes sense as
> it would be describing device internals (which should not be made in
> DT), just to ease one specific software implementation (which should
> also be avoided). So I believe this change would again be rejected.
> https://lore.kernel.org/lkml/58c80c2a-2532-4bc5-9c9f-52480b3af52a@kernel.org/
> 
> But the the second reason is, doing
> 'git grep "device_set_of_node_from_dev.*parent"', I found several
> drivers using device_set_of_node_from_dev() for a similar need. Some of
> these uses are also for MFD child devices:
> - gpio-adp5585.c / pwm-adp5585.c,
> - pwm-ntxec.c,
> - max77620-regulator.c / max77620_thermal.c.
> 
> So, based on this, I believe using device_set_of_node_from_dev() in
> these two drivers is the way to go.

The problem with this solution is that, It's OF-centric. Which shouldn't be
done in a new code (and I don't see impediments to avoid it). Yes, it does
the right thing for the case, but only on OF systems. Note, fwnode is a list
of maximum of two entries (yeah, designed like that right now), can you utilise
that somehow?

-- 
With Best Regards,
Andy Shevchenko