linux-kernel - Re: [RESEND PATCH v1 03/13] dt-bindings: mfd: ti,tps6594: Add TI TPS65224 PMIC

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7hfrxu6i0t.fsf@baylibre.com>
Date: Wed, 14 Feb 2024 10:02:42 -0800
From: Kevin Hilman <khilman@...nel.org>
To: Conor Dooley <conor@...nel.org>
Cc: Bhargav Raviprakash <bhargav.r@...s.com>, arnd@...db.de,
 broonie@...nel.org, conor+dt@...nel.org, devicetree@...r.kernel.org,
 gregkh@...uxfoundation.org, jpanis@...libre.com, kristo@...nel.org,
 krzysztof.kozlowski+dt@...aro.org, lee@...nel.org, lgirdwood@...il.com,
 linus.walleij@...aro.org, linux-arm-kernel@...ts.infradead.org,
 linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
 m.nirmaladevi@...s.com, nm@...com, robh+dt@...nel.org, vigneshr@...com
Subject: Re: [RESEND PATCH v1 03/13] dt-bindings: mfd: ti,tps6594: Add TI
 TPS65224 PMIC

Conor Dooley <conor@...nel.org> writes:

> On Wed, Feb 14, 2024 at 09:26:13AM -0800, Kevin Hilman wrote:
>> Conor Dooley <conor@...nel.org> writes:
>> > On Wed, Feb 14, 2024 at 03:01:06PM +0530, Bhargav Raviprakash wrote:
>> >> On Fri 2/9/2024 10:41 PM, Conor Dooley wrote:
>> >> > On Thu, Feb 08, 2024 at 04:23:33PM +0530, Bhargav Raviprakash wrote:
>> >> > > TPS65224 is a Power Management IC with 4 Buck regulators and 3 LDO
>> >> > > regulators, it includes additional features like GPIOs, watchdog, ESMs
>> >> > > (Error Signal Monitor), and PFSM (Pre-configurable Finite State Machine)
>> >> > > managing the state of the device.
>> >> > 
>> >> > > TPS6594 and TPS65224 have significant functional overlap.
>> >> > 
>> >> > What does "significant functional overlap" mean? Does one implement a
>> >> > compatible subset of the other? I assume the answer is no, given there
>> >> > seems to be some core looking registers at different addresses.
>> >> 
>> >> The intention behind “significant functional overlap” was meant to
>> >> indicate a lot of the features between TPS6594 and TPS65224 overlap,
>> >> while there are some features specific to TPS65224.
>> >> There is compatibility between the PMIC register maps, I2C, PFSM,
>> >> and other drivers even though there are some core registers at
>> >> different addresses.
>> >> 
>> >> Would it be more appropriate to say the 2 devices are compatible and have
>> >> sufficient feature overlap rather than significant functional overlap?
>> >
>> > If core registers are at different addresses, then it is unlikely that
>> > these devices are compatible.
>> 
>> That's not necessarily true.  Hardware designers can sometimes be
>> creative. :)
>
> Hence "unlikely" in my mail :)
>
>> > In this context, compatible means that existing software intended for
>> > the 6594 would run without modification on the 65224, although maybe
>> > only supporting a subset of features.  If that's not the case, then
>> > the devices are not compatible.
>> 
>> Compatible is a fuzzy term... so we need to get into the gray area.
>> 
>> What's going on here is that this new part is derivative in many
>> signifcant (but not all) ways from an existing similar part.  When
>> writing drivers for new, derivative parts, there's always a choice
>> between 1) extending the existing driver (using a new compatible string
>> & match table for the diffs) or 2) creating a new driver which will have
>> a bunch of duplicated code.
>> 
>> The first verion of this series[1] took the 2nd approach, but due to the
>> significant functional (and feature) overlap, the recommendation was
>> instead to take the "reuse" path to avoid signficant amounts of
>> duplicated code.
>> 
>> Of course, it's possible that while going down the "reuse" path, there
>> may be a point where creating a separate driver for some aspects might
>> make sense, but that needs to be justified.  Based on a quick glance of
>> what I see in this series so far (I have not done a detailed review),
>> the differences with the new device look to me like they can be handled
>> with chip-specific data in a match table.
>
> This is all nice information, but not really relevant here - this is a
> binding patch, not a driver one & the conversation stemmed from me
> making sure that a fallback compatible was not suitable.

hehe, oops. <blush>.  my fault for mixing the two together

Sorry for the noise.

Kevin