lkml.org 
[lkml]   [2011]   [Mar]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [Qualcomm PM8921 MFD v2 2/6] mfd: pm8xxx: Add irq support
On Tue, 8 Mar 2011, Abhijeet Dharmapurikar wrote:
> Thomas Gleixner wrote:
> > On Mon, 7 Mar 2011, adharmap@codeaurora.org wrote:
> > > + if (pdata->irq_pdata) {
> >
> > So if pdata->irq_pdata == NULL you return success. Is that correct ?
>
> Yes. The board configuration may choose not to use pmic interrupts.

Ok.

> > Also please return early on (!pdata->irq_pdata) and avoid that extra
> > indent level for the real code path.
>
> I did not do that because there are other subdevices that I will be adding in
> the later patches. I cannot return early. well I will change it for this
> patch.

Maybe splitting out the various init subsections into different
functions which are called from here might be a good thing.

> > > +static void pm8xxx_irq_ack(struct irq_data *d)
> > > +{
> > > + const struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
> > > + unsigned int pmirq = d->irq - chip->irq_base;
> > > + u8 block, config;
> > > +
> > > + block = pmirq / 8;
> > > +
> > > + config = PM_IRQF_WRITE | chip->config[pmirq] | PM_IRQF_CLR;
> > > + /* Keep the mask */
> > > + if (!(chip->irqs_allowed[block] & (1 << (pmirq % 8))))
> > > + config |= PM_IRQF_MASK_FE | PM_IRQF_MASK_RE;
> >
> > What's the point of this exercise? ack is called before mask and it
>
> The register design is such that we cannot only clear the interrupt. One has
> to write to the trigger bits while clearing it. Now trigger bits define
> whether the interrupt is masked or unmasked. If unmasked they define whether
> the interrupt rising/falling/level high/level low triggered.
> So the code remembers which interrupts are masked and for them it clears and
> rewrite the masked status in trigger bits. For unmasked ones it clears and
> writes to the trigger bits essentially configuring them same way as it was
> before. That is why the if satement to check interrupt was masked earlier,
> chip->irqs_allowed[] maintains which interrupt are unmasked.
>
> > ack is called before mask and it
> > should never be called when the interrupt is masked.
>
> I didnt quite understand this comment. handle_level_irq calls mask_ack which
> masks the interrupt and then acks it. In this case the ack is called after the

Indeed, sorry. So the right way to deal with that is to provide a
mask_ack() callback which does it in the correct order for your
HW. That way you avoid all the local state storage.

Thanks,

tglx


\
 
 \ /
  Last update: 2011-03-10 11:35    [W:0.086 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site