Re: [PATCH can-next 2/5] can: m_can: m_can_isr(): handle device-specific interrupts

[Torin Cooper-Bennun <torin@maxiluxsystems.com>]

[-- Attachment #1: Type: text/plain, Size: 2027 bytes --]

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@pengutronix.de/

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]