Message-ID: <f6e9cc1a-bdd7-4231-844e-2d8c5c3be50f@roeck-us.net>
Date: Wed, 6 Nov 2024 10:19:19 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Conor Dooley <conor@...nel.org>
Cc: Cedric Encarnacion <cedricjustine.encarnacion@...log.com>,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-i2c@...r.kernel.org, linux-doc@...r.kernel.org,
 linux-hwmon@...r.kernel.org, Jean Delvare <jdelvare@...e.com>,
 Jonathan Corbet <corbet@....net>,
 Delphine CC Chiu <Delphine_CC_Chiu@...ynn.com>, Rob Herring
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Peter Yin <peteryin.openbmc@...il.com>,
 Noah Wang <noahwang.wang@...look.com>, Marek Vasut <marex@...x.de>,
 Lukas Wunner <lukas@...ner.de>
Subject: Re: [PATCH v2 1/2] dt-bindings: trivial-devices: add ltp8800

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 ...

> 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.

Thanks,
Guenter

Powered by blists - more mailing lists