Re: [RFC PATCH 0/5] Support ROHM BD79124 ADC/GPO

[Jonathan Cameron <jic23@kernel.org>]

On Sat, 1 Feb 2025 17:00:51 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> Hi Jonathan,
> 
> Thanks a ton for the help! :)
> 
> On 31/01/2025 19:08, Jonathan Cameron wrote:
> > On Fri, 31 Jan 2025 15:34:43 +0200
> > Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> >   
> >> Support ROHM BD79124 ADC.
> >>
> >> Quite usual stuff. 12-bit, 8-channel ADC with threshold monitoring.
> >>
> >> Except that:
> >>   - each ADC input pin can be configured as a general purpose output.
> >>   - manually starting an ADC conversion and reading the result would
> >>     require the I2C _master_ to do clock stretching(!) for the duration
> >>     of the conversion... Let's just say this is not well supported.
> >>   - IC supports 'autonomous measurement mode' and storing latest results
> >>     to the result registers. This mode is used by the driver due to the
> >>     "peculiar" I2C when doing manual reads.
> >>
> >> I sent this as an RFC because I implemented the pin purposing (GPO/ADC)
> >> using pinmux - which I've never done for upstream stuff before. Hence
> >> it's better to ask if this makes sense, or if there is better way to go.
> >> Anyways, resulted drivers spread to 3 subsystems (MFD, pinctrl and IIO)  
> > In principle nothing against pin mux for this.
> > There are other options though if pin mux ends up being too complex.
> > 
> > - provide ADC channels in the binding channel@x etc.
> > Anything else is freely available as a GPIO.
> > Normal GPIO bindings etc for those.
> > 
> > The channel bit is common on SoC ADC anyway where we don't want to
> > expose channels that aren't wired out.  
> 
> Thanks for the insight on how things are usually done :)
> 
> I think the only reason for having all the channels visible in IIO, 
> could be, if there was a need to provide a runtime configuration.
> 
> > For combined ADC GPIO chips we normally don't bother with an MFD.
> > Just host the gpio driver in the ADC one unless there is a strong
> > reasons someone will put this down for GPIO usage only.  
> 
> I don't really know about that. I don't like arguing, yet I seem to do 
> that all the time XD
> 
> I personally like using MFD and having smaller drivers in relevant 
> subsystems, because it tends to keep the drivers leaner - and allows 
> re-use of drivers when some of the hardware blocks are re-used. In some 
> cases this results (much) cleaner drivers.

I'm fully in agreement with MFD being useful, but for very simple
parts of a device it can be overkill. 
> 
> (Let's assume they did "new" ADC, and just dropped the GPO from it. With 
> the MFD the deal is to add new compatible, and have an MFD cell array 
> without the pinctrl/GPO matching this new device. And lets imagine they 
> later add this ADC to a PMIC. We add yet another MFD cell array for this 
> new device, with a cell for the regulators, power-supply and the ADC... 
> The same platform subdevice can be re-used to drive ADC (well, with 
> added register offsets)).
> 
> Allright. I believe you have more experience on this area than I do, but 
> I definitely think MFD has it's merits also for ADCs - they do tend to 
> put ADCs to all kinds of devices (like in PMICs after all, although 
> maybe not with 8 channels and less often without an accumulator).

It's a trade off.  Sometimes we just have a little code duplication
to the need for a more complex design.

Enjoy the rest of Fosdem

Jonathan