Re: [PATCH v3] mfd: Add support for Merrifield Basin Cove PMIC

[Andy Shevchenko]

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