Re: [PATCH v2 1/2] dt-bindings: trivial-devices: add ltp8800

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3576 bytes --]

On Wed, Nov 06, 2024 at 10:19:19AM -0800, Guenter Roeck wrote:
> On 11/6/24 08:54, Conor Dooley wrote:
> > On Wed, Nov 06, 2024 at 08:43:54AM -0800, Guenter Roeck wrote:
> > > On 11/6/24 08:11, Conor Dooley wrote:
> > > > On Wed, Nov 06, 2024 at 04:06:02PM +0000, Conor Dooley wrote:
> > > > > On Tue, Nov 05, 2024 at 08:34:01PM -0800, Guenter Roeck wrote:
> > > > > > On 11/5/24 19:09, Cedric Encarnacion wrote:
> > > > > > > Add Analog Devices LTP8800-1A, LTP8800-2, and LTP8800-4A DC/DC μModule
> > > > > > > regulator.
> > > > > 
> > > > > A single compatible for 3 devices is highly suspect. What is
> > > > > different between these devices?
> > > > 
> > > > Additionally, looking at one of the datasheets, this has several inputs
> > > > that could be controlled by a GPIO, a clock input and several supply
> > > > inputs. It also has a regulator output. I don't think it is suitable for
> > > > trivial-devices.yaml.
> > > > 
> > > 
> > > All PMBus devices are by definition regulators with input and output voltages.
> > > After all, PMBus stands for "Power Management Bus". Some of them are listed
> > > in trivial devices, some are not. Is that a general guidance, or in other
> > > words should I (we) automatically reject patches adding PMBus devices
> > > to the trivial devices file ?
> > 
> > Personally I like what Jonathan does for iio devices, where he requires
> > input supplies to be documented, which in turns means they can't go into
> > trivial-devices.yaml. I wanted to add an input supply option to
> > trivial-devices.yaml but ?Rob? was not a fan.
> 
> I may be missing something, but doesn't every chip have an input supply ?
> granted, PMBus chips often have more than one, but still ...

Yeah, that's why I wanted to permit a supply in trivial-devices, because
I bet 99% of devices in there have a supply. IIRC the problem was that
there wasn't a good "generic" name for one. I don't think it was a "you
cannot do this" but a "you need to come up with a name for that supply
that works generically" and I couldn't.

> > In this case it would need a dedicated binding to document the regulator
> > child node and permit things like regulator-always-on or for any
> > consumers of the regulator to exist. I suppose that probably applies to
> > all pmbus bindings?
> 
> Yes. There may be a few exceptions, for example if a fan controller is
> modeled as PMBus device, but that is rare. From a driver perspective,
> exposing regulator nodes is optional, though.
> 
> > In this case, there seems to be an input "sync" clock that may need to
> > be enabled, which is another nail in the coffin for
> > trivial-devices.yaml.
> 
> I really don't know if it is a good idea to expose such data. That clock can
> be connected to ground. It is only necessary in power-sharing configurations,
> and requires all chips to use the same clock. I'd assume it to be a fixed clock
> in pretty much all circumstances. The frequency needs to be configured into
> the chip, but that needs to be done during board manufacturing because it
> determines the switching frequency. Writing wrong data into the chip may
> render the board unusable or even destroy it (I destroyed several PMBus chips
> myself while playing with such parameters on evaluation boards). Maybe there
> is some use case where changing the configuration is necessary, but I am not
> in favor of exposing it due to the risk involved.

I figured it'd be fixed, but that doesn't mean it can't have an enable
(or a supply of its own).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]