Hi, On Mon, 2020-07-27 at 16:48 +0100, Lee Jones wrote: > On Thu, 23 Jul 2020, Hsin-Hsiung Wang wrote: > > > This patch refines the interrupt related code to support new chips. > > Refines in what way? > > What makes this better? > Thanks for the comment. I will add more information into comment message based on my below explanation. > > Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@xxxxxxxxxxxx> > > --- > > drivers/mfd/mt6358-irq.c | 65 ++++++++++++++++++++++++----------------- > > include/linux/mfd/mt6358/core.h | 8 ++--- > > 2 files changed, 41 insertions(+), 32 deletions(-) > > > > diff --git a/drivers/mfd/mt6358-irq.c b/drivers/mfd/mt6358-irq.c > > index db734f2..4b094e5 100644 > > --- a/drivers/mfd/mt6358-irq.c > > +++ b/drivers/mfd/mt6358-irq.c > > @@ -13,7 +13,9 @@ > > #include <linux/platform_device.h> > > #include <linux/regmap.h> > > > > -static struct irq_top_t mt6358_ints[] = { > > +#define MTK_PMIC_REG_WIDTH 16 > > + > > +static const struct irq_top_t mt6358_ints[] = { > > MT6358_TOP_GEN(BUCK), > > MT6358_TOP_GEN(LDO), > > MT6358_TOP_GEN(PSC), > > @@ -24,6 +26,13 @@ static struct irq_top_t mt6358_ints[] = { > > MT6358_TOP_GEN(MISC), > > }; > > > > +static struct pmic_irq_data mt6358_irqd = { > > + .num_top = ARRAY_SIZE(mt6358_ints), > > + .num_pmic_irqs = MT6358_IRQ_NR, > > + .top_int_status_reg = MT6358_TOP_INT_STATUS0, > > + .pmic_ints = mt6358_ints, > > +}; > > Dynamically assigned driver data is usually preferred. > > Why have you gone static? > Do you consider the memory allocation? Below modification is to assign necessary data dynamically and the code will become longer with more chips if we assign every member of the structure. @@ -180,17 +190,18 @@ int mt6358_irq_init(struct mt6397_chip *chip) int i, j, ret; struct pmic_irq_data *irqd; - irqd = devm_kzalloc(chip->dev, sizeof(*irqd), GFP_KERNEL); - if (!irqd) - return -ENOMEM; + switch (chip->chip_id) { + case MT6358_CHIP_ID: + chip->irq_data = &mt6358_irqd; + break; - chip->irq_data = irqd; + default: + dev_err(chip->dev, "unsupported chip: 0x%x \n", chip->chip_id); + return -ENODEV; + } 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 = chip->irq_data; irqd->enable_hwirq = devm_kcalloc(chip->dev, irqd->num_pmic_irqs, sizeof(*irqd->enable_hwirq),