lkml.org 
[lkml]   [2024]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [RESEND PATCH v1 03/13] dt-bindings: mfd: ti,tps6594: Add TI TPS65224 PMIC
    Date
    Conor Dooley <conor@kernel.org> writes:

    > On Wed, Feb 14, 2024 at 09:26:13AM -0800, Kevin Hilman wrote:
    >> Conor Dooley <conor@kernel.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

    \
     
     \ /
      Last update: 2024-05-27 15:04    [W:5.377 / U:0.228 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site