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

[Lee Jones <lee.jones@linaro.org>]

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.
> > > 
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > > - updated copyright year to be 2019
> > > - rebased on top of latest vanilla rc
> > > 
> > >  drivers/mfd/Kconfig                      |  11 ++
> > >  drivers/mfd/Makefile                     |   1 +
> > >  drivers/mfd/intel_soc_pmic_mrfld.c       | 157 +++++++++++++++++++++++
> > >  include/linux/mfd/intel_soc_pmic_mrfld.h |  81 ++++++++++++
> > >  4 files changed, 250 insertions(+)
> > >  create mode 100644 drivers/mfd/intel_soc_pmic_mrfld.c
> > >  create mode 100644 include/linux/mfd/intel_soc_pmic_mrfld.h
> > 
> > [...]
> > 
> > > +static int bcove_probe(struct platform_device *pdev)
> > > +{
> > > +	struct device *dev = &pdev->dev;
> > > +	struct intel_soc_pmic *pmic;
> > > +	unsigned int i;
> > > +	int ret;
> > > +
> > > +	pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL);
> > > +	if (!pmic)
> > > +		return -ENOMEM;
> > > +
> > > +	platform_set_drvdata(pdev, pmic);
> > > +	pmic->dev = &pdev->dev;
> > > +
> > > +	pmic->regmap = devm_regmap_init(dev, NULL, pmic, &bcove_regmap_config);
> > > +	if (IS_ERR(pmic->regmap))
> > > +		return PTR_ERR(pmic->regmap);
> > > +
> > > +	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.

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);

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog