Messages in this thread | | | From | Andy Shevchenko <> | Date | Sat, 3 Jun 2017 16:26:08 +0300 | Subject | Re: [PATCH v5 7/8] mfd: intel_soc_pmic_bxtwc: Use chained irqs for second level irq chips |
| |
On Thu, Jun 1, 2017 at 1:37 AM, <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > > Whishkey cove PMIC has support to mask/unmask interrupts at two levels. > At first level we can mask/unmask interrupt domains like TMU, GPIO, ADC, > CHGR, BCU THERMAL and PWRBTN and at second level, it provides facility > to mask/unmask individual interrupts belong each of this domain. For > example, in case of TMU, at first level we have TMU interrupt domain, > and at second level we have two interrupts, wake alarm, system alarm that > belong to the TMU interrupt domain. > > Currently, in this driver all first level irqs are registered as part of > irq chip(bxtwc_regmap_irq_chip). By default, after you register the irq > chip from your driver, all irqs in that chip will masked and can only be > enabled if that irq is requested using request_irq call. This is the > default Linux irq behavior model. And whenever a dependent device that > belongs to PMIC requests only the second level irq and not explicitly > unmask the first level irq, then in essence the second level irq will > still be disabled. For example, if TMU device driver request wake_alarm > irq and not explicitly unmask TMU level 1 irq then according to the default > Linux irq model, wake_alarm irq will still be disabled. So the proper > solution to fix this issue is to use the chained irq chip concept. We > should chain all the second level chip irqs to the corresponding first > level irq. To do this, we need to create separate irq chips for every > group of second level irqs. > > In case of TMU, when adding second level irq chip, instead of using pmic > irq we should use the corresponding first level irq. So the following > code will change from
pmic -> PMIC irq -> IRQ irqs -> IRQs
These do apply to entire commit message.
> > ret = regmap_add_irq_chip(pmic->regmap, pmic->irq, ...) > > to, > > virq = regmap_irq_get_virq(&pmic->irq_chip_data, BXTWC_TMU_LVL1_IRQ); > > ret = regmap_add_irq_chip(pmic->regmap, virq, ...) > > In case of typec wcove driver, Since USBC IRQ is moved under charger level2 > irq chip. We should use charger irq chip(irq_chip_data_chgr) to get the USBC > virtual IRQ number.
typec wcove -> Whiskey Cove Type-C
> +enum bxtwc_irqs_tmu { > + BXTWC_TMU_IRQ = 0, > +};
> -enum bxtwc_irqs_tmu { > - BXTWC_TMU_IRQ = 0,
Leave it on the same place it will make patch a bit more cleaner.
> +static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = { > + REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06), > +};
> -static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = { > - REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
Ditto.
> +static int bxtwc_add_chained_irq_chip(struct intel_soc_pmic *pmic, > + struct regmap_irq_chip_data *pdata, > + int pirq, int irq_flags, > + const struct regmap_irq_chip *chip, > + struct regmap_irq_chip_data **data) > +{ > + int irq; > + > + irq = regmap_irq_get_virq(pdata, pirq); > + if (irq < 0) { > + dev_err(pmic->dev, "failed to get virtual interrupt:%d\n", irq);
Follow below pattern, i.e. "Failed to ..."
Moreover, it would be useful to see for which interrupt we are trying to get a virtual one. chip->name + pirq I guess would give us a thing.
> + return irq; > + } > + > + return devm_regmap_add_irq_chip(pmic->dev, pmic->regmap, irq, irq_flags, > + 0, chip, data); > +}
-- With Best Regards, Andy Shevchenko
| |