linux-kernel - Re: [PATCH v3] mfd: Add support for Merrifield Basin Cove 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: <20190626111043.GJ9224@smile.fi.intel.com>
Date:   Wed, 26 Jun 2019 14:10:43 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] mfd: Add support for Merrifield Basin Cove PMIC

On Wed, Jun 26, 2019 at 11:17:27AM +0100, Lee Jones wrote:
> On Wed, 26 Jun 2019, Andy Shevchenko wrote:
> > On Mon, Jun 24, 2019 at 05:13:48PM +0100, Lee Jones wrote:
> > > On Wed, 12 Jun 2019, Andy Shevchenko wrote:

> > > > Add an MFD driver for Intel Merrifield Basin Cove PMIC.

> > > > +	for (i = 0; i < ARRAY_SIZE(irq_level2_resources); i++) {
> > > > +		ret = platform_get_irq(pdev, i);
> > > 
> > > If you already know the order, define the children's device IDs in the
> > > parent's shared header ('intel_soc_pmic_mrfld.h'?) and retreive them
> > > like:
> > > 
> > >   platform_get_irq(pdev->parent, <SUITABLE_DEFINED_ID>);
> > > 
> > > Then you can skip all of this platform device -> platform device hoop
> > > jumping.
> > 
> > The idea of MFD is to get children to be parent agnostic
> > (at least to some extent). What you are proposing here
> > seems like disadvantage from MFD philosophy. No?
> 
> Not at all.  The idea of MFD is to split up support for monolithic h/w
> such that they can be handled properly by their appropriate
> subsystems, and by extension, maintained by the associated subject
> matter experts.
> 
> Children are often aware of their parents (some siblings are even
> aware of each other!), and many expect and depend on the data-sets
> provided by their parents.

Yes, that's true and that's why I put wording "to some extent" above.

> For instance (this example may come to bite me in the behind, but),
> taken from this very patch, where is this consumed?
> 
>  platform_set_drvdata(pdev, pmic);

Yes. It's used in children. BUT. This structure covers several PMIC chips and
the children driver doesn't know which generation / version of PMIC is serving.

What you are proposing with the change is to strictly link the children driver
to PMIC gen X ver Y, while above example doesn't do that.

So, I'm not convinced it's a good change to have.

-- 
With Best Regards,
Andy Shevchenko