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

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: <20241106-crate-antihero-bc7b66037640@spud>
Date: Wed, 6 Nov 2024 18:38:26 +0000
From: Conor Dooley <conor@...nel.org>
To: Guenter Roeck <linux@...ck-us.net>
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 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).

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)