On Thu, 2016-03-31 at 09:40 +0800, Henry Chen wrote: > On Wed, 2016-03-30 at 11:18 +0200, John Crispin wrote: > > Hi, > > > > small nitpick inline > > > > On 30/03/2016 09:25, Henry Chen wrote: > > > Some sub driver like RTC module need irq domain from parent to create > > > irq mapping when driver initialize. so move mt6397_irq_init() before > > > mfd_add_devices(). > > > > > > Signed-off-by: Henry Chen <henryc.c...@mediatek.com> > > > --- > > > This patch fixed the below warning based on "Linux kernel v4.6-rc1" > > > WARNING: CPU: 1 PID: 132 at kernel/mediatek/kernel/irq/irqdomain.c:471 > > > irq_create_mapping+0xc4/0xd0 > > > --- > > > drivers/mfd/mt6397-core.c | 37 ++++++++++++++++++++++--------------- > > > 1 file changed, 22 insertions(+), 15 deletions(-) > > > > > > diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c > > > index 8e8d932..a879223 100644 > > > --- a/drivers/mfd/mt6397-core.c > > > +++ b/drivers/mfd/mt6397-core.c > > > @@ -270,22 +270,36 @@ static int mt6397_probe(struct platform_device > > > *pdev) > > > goto fail_irq; > > > } > > > > > > + pmic->irq = platform_get_irq(pdev, 0); > > > + > > > switch (id & 0xff) { > > > case MT6323_CID_CODE: > > > - pmic->int_con[0] = MT6323_INT_CON0; > > > - pmic->int_con[1] = MT6323_INT_CON1; > > > - pmic->int_status[0] = MT6323_INT_STATUS0; > > > - pmic->int_status[1] = MT6323_INT_STATUS1; > > > + if (pmic->irq > 0) { > > > > should this not be > > > > if (pmic->irq >= 0) { > > > > i think the code before your patch was wrong as linux irqs start with 0. > > > > John > Hi John, > > Thanks, I will modify this.
Linux irq start from 1, 0 is invalid. I can't find the document saying this now, but you could see this from irq_create_mapping() in kernel/irq/irqdomain.c I think the code should have check return from platform_get_irq and handle -EPROBE_DEFER, but maybe it should be another patch? BTW, in this function, it is possible that pmic->irq_domain will be NULL in fail_irq error handling. We should check before calling irq_domain_remove. Joe.C