All of
 help / color / mirror / Atom feed
From: Manivannan Sadhasivam <>
To: Marc Kleine-Budde <>
Cc:, Thomas Kopp <>
Subject: Re: [PATCH] can: mcp251xfd: mcp251xfd_open(): request IRQ as shared
Date: Tue, 27 Jul 2021 14:40:03 +0530	[thread overview]
Message-ID: <20210727091003.GC33931@thinkpad> (raw)
In-Reply-To: <>

On Tue, Jul 27, 2021 at 09:46:17AM +0200, Marc Kleine-Budde wrote:
> On 27.07.2021 12:47:17, Manivannan Sadhasivam wrote:
> > On Sat, Jul 24, 2021 at 10:52:13PM +0200, Marc Kleine-Budde wrote:
> > > The driver's IRQ handler supports shared IRQs, so request a shared IRQ
> > > handler.
> > 
> > I don't see any issue with the idea but I'd like to understand the
> > requirement for it.
> Hardware designers might come up with strange ideas, so better be
> prepared for this. :)
> But seriously, there's a group of people trying to bring the mcp251xfd
> driver to work on ACPI based systems. Having a shared mcp251xfd IRQ
> handler will (hopefully) help them during debugging.

Oh, that's very interesting! ACPI on a ECU :)

> I've written the IRQ handler to properly only return IRQ_HANDLED if
> there really was an interrupt, this means it should be capable running
> as a shared IRQ handler. I've tested it and it works. So let the driver
> request a shared IRQ handler.

I had one more look at it and looks fine to me. The driver authors tend to
return IRQ_HANDLED even during error cases (especially for devices sitting on
buses like i2c, spi) but I hate it. Why would anyone want to enable the
interrupt for the device if it can't be communicated in the ISR?

But you did return IRQ_NONE for error cases, so fine.

> > Usually the IRQ lines are shared when multiple devices use them
> > physically. For instance, a MFD device using a single GPIO for all of its
> > functions. But I don't see any sort of requirement like that here.
> Indeed there is really no good reason to do so, but it works. For
> testing I've connected 2 mcp2518fd to the same IRQ line and the kernel
> runs both IRQ handlers an interrupt is triggered. If course this brings
> the overhead of an additional SPI transfer per IRQ, but it works.
> > Making the IRQ lines shared will only induce latency IMO.
> ACK - you will not get better performance compared to separate IRQ lines
> :) But if you don't use shared interrupts this change doesn't make the
> driver or system any slower or induce latency.

Ack. Feel free to add,

Reviewed-by: Manivannan Sadhasivam <>


> regards,
> Marc
> -- 
> Pengutronix e.K.                 | Marc Kleine-Budde           |
> Embedded Linux                   |  |
> Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

      reply	other threads:[~2021-07-27  9:10 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-24 20:52 [PATCH] can: mcp251xfd: mcp251xfd_open(): request IRQ as shared Marc Kleine-Budde
2021-07-27  7:17 ` Manivannan Sadhasivam
2021-07-27  7:46   ` Marc Kleine-Budde
2021-07-27  9:10     ` Manivannan Sadhasivam [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210727091003.GC33931@thinkpad \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.