Re: [PATCH v2 2/2] hwmon: ltc4282: add support for the LTC4282 chip

From: "Nuno Sá" <noname.nuno@gmail.com>
To: Guenter Roeck <linux@roeck-us.net>, Andy Shevchenko <andy@kernel.org>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	nuno.sa@analog.com,  linux-hwmon@vger.kernel.org,
	devicetree@vger.kernel.org,  linux-doc@vger.kernel.org,
	Jean Delvare <jdelvare@suse.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	 Conor Dooley <conor+dt@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Bartosz Golaszewski <brgl@bgdev.pl>
Subject: Re: [PATCH v2 2/2] hwmon: ltc4282: add support for the LTC4282 chip
Date: Fri, 01 Dec 2023 17:29:59 +0100	[thread overview]
Message-ID: <986fb7dc2a34602fa9c2d57a7a3e06a71cfdc0a0.camel@gmail.com> (raw)
In-Reply-To: <7dc3f137-6073-4262-afb5-439d024bbbd2@roeck-us.net>

On Fri, 2023-12-01 at 08:04 -0800, Guenter Roeck wrote:
> On 12/1/23 07:47, Andy Shevchenko wrote:
> > On Fri, Dec 01, 2023 at 04:24:35PM +0100, Nuno Sá wrote:
> > > On Fri, 2023-12-01 at 14:40 +0100, Linus Walleij wrote:
> > 
> > ...
> > 
> > > Yes, that is the only thing we have. Meaning that there is no hw setting to set
> > > the
> > > pins to open drain. Open drain is what they are. That is why I'm not seeing the
> > > point
> > > in having PIN_CONFIG_DRIVE_OPEN_DRAIN implemented.
> > 
> > At least you have to implement error for PUSH_PULL mode and other modes,
> > so from the (core) software point of view the user should be able to ask for
> > anything and get an answer from the certain driver that "hey, i do support OD",
> > or "hey, push-pull can't be supported with this hw".
> > 
> 
> It seems to me that this is heading towards a mfd driver. I don't feel comfortable
> with all that gpio specific code in the hwmon subsystem.
> 
> Maybe I should request that all hwmon chips with gpio support must be implemented
> as mfd drivers. I'll have to think about that.
> 
> Guenter
> 

Hopefully you don't ask that already for this driver...

I mean, I guess It could be done but having this is as mfd will make it more
difficult to handle the pins. Because if we look at the bindings we can see that
these pins might only be relevant to the hwmon driver and not even be used as gpios.
But being an mfd, I don't think there's any requirement like one of the drivers can
only exist if the other is also there. So making the gpio driver depend on the hwmon
would be weird. I might be overcomplicating but things can get messy if we want to
make it right in the gpio driver:

* If the hwmon is not reachable then we could just assume we can use the 4 pins as
gpios freely.
* If the hwmon is reachable then we need some kind of syncup to make sure we dont use
a pin as gpio if the hwmon driver wants to use it. Things can get even more shady if
we think one driver being a module and the other being built-in.

Or maybe we can also just have an array property in the gpio driver to explicitly set
the pins we want as gpios. Then if the user messes up and we have a config overlaps
one of the driver fails probing...

Or maybe Andy's auxiliary bus suggestion might be more suitable (not really familiar
with it).

- Nuno Sá