On Wed, May 26, 2021 at 05:18:33PM +0200, Marc Kleine-Budde wrote:
> On 26.05.2021 17:07:05, Marc Kleine-Budde wrote:
> > On 26.05.2021 13:47:44, Torin Cooper-Bennun wrote:
> > > + if (!ir) {
> > > + /* Handle device-specific interrupts */
> > > + if (cdev->ops->handle_dev_interrupts)
> > > + irq_ret = cdev->ops->handle_dev_interrupts(cdev, false);
> > > + return irq_ret;
> > > + }
> > > +
> > > + /* ACK M_CAN interrupts */
> > > + m_can_write(cdev, M_CAN_IR, ir);
> > >
> > > + /* ACK device-specific interrupts */
> > > if (cdev->ops->handle_dev_interrupts)
> > > cdev->ops->handle_dev_interrupts(cdev, true);
> >
> > Why do you call a 2nd time the handle_dev_interrupts() callback?
>
> I see, clear and no clear. Why are these two separate operations?
As discussed here,
https://lore.kernel.org/linux-can/20210514141012.3ehw4tosog3lreq4@xxxxxxxxxxxxxx/
you previously recommended adding a 2nd parameter the callback to
indicate that we only want to clear the device IR, not read it, in order
to save an SPI transaction when M_CAN core interrupts were already
handled.
However, quoting from your previous mail,
> On 26.05.2021 13:47:44, Torin Cooper-Bennun wrote:
> > Device-specific interrupts are handled, if no M_CAN core interrupts were
> > handled in the ISR call.
>
> In case there are both core and device specific interrupts the kernel
> IRQ handler will call the ISR a 2nd time - should be OK.
this implies I should leave device IR totally alone unless no M_CAN
interrupts are asserted, and do the following only:
| ir = m_can_read(cdev, M_CAN_IR);
|
| if (!ir) {
| /* Handle device-specific interrupts */
| if (cdev->ops->handle_dev_interrupts)
| irq_ret = cdev->ops->handle_dev_interrupts(cdev, false);
| return irq_ret;
| }
|
| /* ACK M_CAN interrupts */
| m_can_write(cdev, M_CAN_IR, ir);
I can probably also kill off the clear_only parameter.
--
Regards,
Torin Cooper-Bennun
Software Engineer | maxiluxsystems.com
Attachment:
signature.asc
Description: PGP signature
