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

From: Lee Jones
Date: Mon Feb 03 2020 - 02:46:58 EST


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