Re: [PATCH v7 3/6] mfd: Add support for the MediaTek MT6358 PMIC

[Lee Jones]

On Wed, 22 Jan 2020, Hsin-hsiung Wang wrote:

> Hi,
> 
> On Mon, 2019-12-16 at 15:17 +0000, Lee Jones wrote:
> > On Wed, 11 Dec 2019, Hsin-Hsiung Wang wrote:
> > 
> > > This adds support for the MediaTek MT6358 PMIC. This is a
> > > multifunction device with the following sub modules:
> > > 
> > > - Regulator
> > > - RTC
> > > - Codec
> > > - Interrupt
> > > 
> > > It is interfaced to the host controller using SPI interface
> > > by a proprietary hardware called PMIC wrapper or pwrap.
> > > MT6358 MFD is a child device of the pwrap.
> > > 
> > > Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@xxxxxxxxxxxx>
> > > ---
> > >  drivers/mfd/Makefile                 |   2 +-
> > >  drivers/mfd/mt6358-irq.c             | 224 ++++++++++++++++++++++++++++
> > >  drivers/mfd/mt6397-core.c            |  45 +++++-
> > >  include/linux/mfd/mt6358/core.h      | 158 ++++++++++++++++++++
> > >  include/linux/mfd/mt6358/registers.h | 282 +++++++++++++++++++++++++++++++++++
> > >  include/linux/mfd/mt6397/core.h      |   3 +
> > >  6 files changed, 712 insertions(+), 2 deletions(-)
> > >  create mode 100644 drivers/mfd/mt6358-irq.c
> > >  create mode 100644 include/linux/mfd/mt6358/core.h
> > >  create mode 100644 include/linux/mfd/mt6358/registers.h

[...]

> > > +int mt6358_irq_init(struct mt6397_chip *chip)
> > > +{
> > > +	int i, j, ret;
> > > +	struct pmic_irq_data *irqd;
> > > +
> > > +	irqd = devm_kzalloc(chip->dev, sizeof(struct pmic_irq_data *),
> > > +			    GFP_KERNEL);
> > > +	if (!irqd)
> > > +		return -ENOMEM;
> > > +
> > > +	chip->irq_data = irqd;
> > > +
> > > +	mutex_init(&chip->irqlock);
> > > +	irqd->top_int_status_reg = MT6358_TOP_INT_STATUS0;
> > > +	irqd->num_pmic_irqs = MT6358_IRQ_NR;
> > > +	irqd->num_top = ARRAY_SIZE(mt6358_ints);
> > > +
> > > +	irqd->enable_hwirq = devm_kcalloc(chip->dev,
> > > +					  irqd->num_pmic_irqs,
> > > +					  sizeof(bool),
> > 
> > This is fragile.  What if the type changes elsewhere?
> > 
> 
> Thanks for your comment.
> Do you mean using 'sizeof(*irqd->enable_hwirq)' instead of
> 'sizeof(bool)'?

Yes please.

-- 
Lee Jones [æçæ]
Linaro Services Technical Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog