Messages in this thread | | | Date | Thu, 22 Dec 2016 00:06:38 +0100 | From | Martin Kaiser <> | Subject | Re: [PATCH v2] rtc: imxdi: use the security violation interrupt |
| |
Hello Lucas,
thanks for taking the time to review my patch.
Thus wrote Lucas Stach (l.stach@pengutronix.de):
> > diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c > > index 67b56b8..ec6077a0 100644 > > --- a/drivers/rtc/rtc-imxdi.c > > +++ b/drivers/rtc/rtc-imxdi.c > > @@ -109,6 +109,7 @@ > > * @rtc: pointer to rtc struct > > * @ioaddr: IO registers pointer > > * @irq: dryice normal interrupt > > + * @sec_irq: dryice security violation interrupt
> This isn't used outside the probe routine, so doesn't need to be saved > in the driver data.
The same goes for imxdi->irq, I made this a local variable as well.
> > + rc = devm_request_irq(&pdev->dev, imxdi->sec_irq, dryice_sec_irq, > > + IRQF_SHARED, pdev->name, imxdi); > > + if (rc) { > > + dev_warn(&pdev->dev, "security violation interrupt not available.\n"); > > + /* this is not an error, see above */ > > + } > > + > Please just fold this into the "if (rc > 0)" path above.
Are you sure that this would be correct?
My understanding is that we should mask the interrupts by writing 0 into DIER and set the DryIce to a sane state in di_handle_state() before we install interrupt handlers.
Best regards, Martin
| |