| Message ID | 1496147943-25822-7-git-send-email-kgunda@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
On 05/30, Kiran Gunda wrote: > From: Abhijeet Dharmapurikar <adharmap@codeaurora.org> > > irq_enable is called when the device resumes. Note that the > irq_enable is called regardless of whether the interrupt was > marked enabled/disabled in the descriptor or whether it was > masked/unmasked at the controller while resuming. > > The current driver unconditionally clears the interrupt in its > irq_enable callback. This is dangerous as any interrupts that > happen right before the resume could be missed. > Remove the irq_enable callback and use mask/unmask instead. > > Also remove struct pmic_arb_irq_spec as it serves no real purpose. > It is used only in the translate function and the code is much > cleaner without it. Also a separate patch for that part.... > > Signed-off-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org> > Signed-off-by: Kiran Gunda <kgunda@codeaurora.org> > --- > drivers/spmi/spmi-pmic-arb.c | 29 +++-------------------------- > 1 file changed, 3 insertions(+), 26 deletions(-) > > diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c > index 0a5728c..bc03737 100644 > --- a/drivers/spmi/spmi-pmic-arb.c > +++ b/drivers/spmi/spmi-pmic-arb.c > @@ -588,17 +588,6 @@ static void qpnpint_irq_unmask(struct irq_data *d) > qpnpint_spmi_write(d, QPNPINT_REG_EN_CLR, &data, 1); > } > > -static void qpnpint_irq_enable(struct irq_data *d) > -{ > - u8 irq = d->hwirq >> 8; > - u8 data; > - > - qpnpint_irq_unmask(d); > - > - data = BIT(irq); > - qpnpint_spmi_write(d, QPNPINT_REG_LATCHED_CLR, &data, 1); > -} > - > static int qpnpint_irq_set_type(struct irq_data *d, unsigned int flow_type) > { > struct spmi_pmic_arb_qpnpint_type type; > @@ -647,7 +636,6 @@ static int qpnpint_get_irqchip_state(struct irq_data *d, > > static struct irq_chip pmic_arb_irqchip = { > .name = "pmic_arb", > - .irq_enable = qpnpint_irq_enable, > .irq_ack = qpnpint_irq_ack, > .irq_mask = qpnpint_irq_mask, > .irq_unmask = qpnpint_irq_unmask, This looks ok.
On 2017-05-31 07:30, Stephen Boyd wrote: > On 05/30, Kiran Gunda wrote: >> From: Abhijeet Dharmapurikar <adharmap@codeaurora.org> >> >> irq_enable is called when the device resumes. Note that the >> irq_enable is called regardless of whether the interrupt was >> marked enabled/disabled in the descriptor or whether it was >> masked/unmasked at the controller while resuming. >> >> The current driver unconditionally clears the interrupt in its >> irq_enable callback. This is dangerous as any interrupts that >> happen right before the resume could be missed. >> Remove the irq_enable callback and use mask/unmask instead. >> >> Also remove struct pmic_arb_irq_spec as it serves no real purpose. >> It is used only in the translate function and the code is much >> cleaner without it. > > Also a separate patch for that part.... > Sure. will split it in to different patch or move it in the https://patchwork.kernel.org/patch/9754511/ patch as per your comment. >> >> Signed-off-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org> >> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org> >> --- >> drivers/spmi/spmi-pmic-arb.c | 29 +++-------------------------- >> 1 file changed, 3 insertions(+), 26 deletions(-) >> >> diff --git a/drivers/spmi/spmi-pmic-arb.c >> b/drivers/spmi/spmi-pmic-arb.c >> index 0a5728c..bc03737 100644 >> --- a/drivers/spmi/spmi-pmic-arb.c >> +++ b/drivers/spmi/spmi-pmic-arb.c >> @@ -588,17 +588,6 @@ static void qpnpint_irq_unmask(struct irq_data >> *d) >> qpnpint_spmi_write(d, QPNPINT_REG_EN_CLR, &data, 1); >> } >> >> -static void qpnpint_irq_enable(struct irq_data *d) >> -{ >> - u8 irq = d->hwirq >> 8; >> - u8 data; >> - >> - qpnpint_irq_unmask(d); >> - >> - data = BIT(irq); >> - qpnpint_spmi_write(d, QPNPINT_REG_LATCHED_CLR, &data, 1); >> -} >> - >> static int qpnpint_irq_set_type(struct irq_data *d, unsigned int >> flow_type) >> { >> struct spmi_pmic_arb_qpnpint_type type; >> @@ -647,7 +636,6 @@ static int qpnpint_get_irqchip_state(struct >> irq_data *d, >> >> static struct irq_chip pmic_arb_irqchip = { >> .name = "pmic_arb", >> - .irq_enable = qpnpint_irq_enable, >> .irq_ack = qpnpint_irq_ack, >> .irq_mask = qpnpint_irq_mask, >> .irq_unmask = qpnpint_irq_unmask, > > This looks ok. Thanks ! -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c index 0a5728c..bc03737 100644 --- a/drivers/spmi/spmi-pmic-arb.c +++ b/drivers/spmi/spmi-pmic-arb.c @@ -588,17 +588,6 @@ static void qpnpint_irq_unmask(struct irq_data *d) qpnpint_spmi_write(d, QPNPINT_REG_EN_CLR, &data, 1); } -static void qpnpint_irq_enable(struct irq_data *d) -{ - u8 irq = d->hwirq >> 8; - u8 data; - - qpnpint_irq_unmask(d); - - data = BIT(irq); - qpnpint_spmi_write(d, QPNPINT_REG_LATCHED_CLR, &data, 1); -} - static int qpnpint_irq_set_type(struct irq_data *d, unsigned int flow_type) { struct spmi_pmic_arb_qpnpint_type type; @@ -647,7 +636,6 @@ static int qpnpint_get_irqchip_state(struct irq_data *d, static struct irq_chip pmic_arb_irqchip = { .name = "pmic_arb", - .irq_enable = qpnpint_irq_enable, .irq_ack = qpnpint_irq_ack, .irq_mask = qpnpint_irq_mask, .irq_unmask = qpnpint_irq_unmask, @@ -657,12 +645,6 @@ static int qpnpint_get_irqchip_state(struct irq_data *d, | IRQCHIP_SKIP_SET_WAKE, }; -struct spmi_pmic_arb_irq_spec { - unsigned slave:4; - unsigned per:8; - unsigned irq:3; -}; - static int qpnpint_irq_domain_dt_translate(struct irq_domain *d, struct device_node *controller, const u32 *intspec, @@ -671,7 +653,6 @@ static int qpnpint_irq_domain_dt_translate(struct irq_domain *d, unsigned int *out_type) { struct spmi_pmic_arb *pa = d->host_data; - struct spmi_pmic_arb_irq_spec spec; int rc; u8 apid; @@ -686,10 +667,6 @@ static int qpnpint_irq_domain_dt_translate(struct irq_domain *d, if (intspec[0] > 0xF || intspec[1] > 0xFF || intspec[2] > 0x7) return -EINVAL; - spec.slave = intspec[0]; - spec.per = intspec[1]; - spec.irq = intspec[2]; - rc = pa->ver_ops->ppid_to_apid(pa, intspec[0], (intspec[1] << 8), &apid); if (rc < 0) { @@ -705,9 +682,9 @@ static int qpnpint_irq_domain_dt_translate(struct irq_domain *d, if (apid < pa->min_apid) pa->min_apid = apid; - *out_hwirq = spec.slave << 24 - | spec.per << 16 - | spec.irq << 8 + *out_hwirq = (intspec[0] & 0xF) << 24 + | (intspec[1] & 0xFF) << 16 + | (intspec[2] & 0x7) << 8 | apid; *out_type = intspec[3] & IRQ_TYPE_SENSE_MASK;