linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Asmaa Mnebhi <asmaa@nvidia.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	netdev <netdev@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	Jakub Kicinski <kuba@kernel.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	David Thompson <davthompson@nvidia.com>
Subject: Re: [PATCH v3 1/2] gpio: mlxbf2: Introduce IRQ support
Date: Mon, 27 Sep 2021 16:56:35 +0200	[thread overview]
Message-ID: <YVHbo/cJcHzxUk+d@lunn.ch> (raw)
In-Reply-To: <CH2PR12MB389585F7D5EFE5E2453593DBD7A79@CH2PR12MB3895.namprd12.prod.outlook.com>

On Mon, Sep 27, 2021 at 02:19:45PM +0000, Asmaa Mnebhi wrote:
> 
> > The BlueField GPIO HW only support Edge interrupts.
> 
> O.K. So please remove all level support from this driver,
> and return -EINVAL if requested to do level.
> This also means, you cannot use interrupts with the
> Ethernet PHY. The PHY is using level interrupts.
> 
> Why not? The HW folks said it is alright because they
> Do some internal conversion of PHY signal and we have tested
> This extensively.

So the PHY is level based. The PHY is combing multiple interrupt
sources into one external interrupt. If any of those internal
interrupt sources are active, the external interrupt is active. If
there are multiple active sources at once, the interrupt stays low,
until they are all cleared. This means there is not an edge per
interrupt. There is one edge when the first internal source occurs,
and no more edges, even if there are more internal interrupts.

The general flow in the PHY interrupt handler is to read the interrupt
status register, which tells you which internal interrupts have
fired. You then address these internal interrupts one by one. This can
take some time, MDIO is a slow bus etc. While handling these interrupt
sources, it could be another internal interrupt source triggers. This
new internal interrupt source keeps the external interrupt active. But
there has not been an edge, since the interrupt handler is still
clearing the sources which caused the first interrupt. With level
interrupts, this is not an issue. When the interrupt handler exits,
the interrupt is re-enabled. Since it is still active, due to the
unhandled internal interrupt sources, the level interrupt immediately
fires again. the handler then sees this new interrupt and handles
it. At that point the level interrupt goes inactive.

Now think about what happens if you are using an edge interrupt
controller with a level interrupt. You get the first edge, and call
the interrupt handler. And then there are no more edges, despite there
being more interrupts. You not only loose the new interrupt, you never
see any more interrupts. You PHY link can go up and down, it can try
to report being over temperature, that it has detected power from the
peer, cable tests have passed, etc. But since there is no edge, there
is never an interrupt.

So you say it has been extensively tested. Has it been extensively
tested with multiple internal interrupt sources at the same time? And
with slight timing variations, so that you trigger this race
condition? It is not going to happen very often, but when it does, it
is going to be very bad.

	Andrew


  parent reply	other threads:[~2021-09-27 14:56 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-23 20:22 [PATCH v3 0/2] gpio: mlxbf2: Introduce proper interrupt handling Asmaa Mnebhi
2021-09-23 20:22 ` [PATCH v3 1/2] gpio: mlxbf2: Introduce IRQ support Asmaa Mnebhi
2021-09-24 11:46   ` Andrew Lunn
2021-09-24 23:48     ` Linus Walleij
2021-09-27 14:04       ` Asmaa Mnebhi
2021-09-27 14:08         ` Andrew Lunn
2021-09-27 14:19           ` Asmaa Mnebhi
2021-09-27 14:26             ` Asmaa Mnebhi
2021-09-27 14:56             ` Andrew Lunn [this message]
2021-09-27 15:52               ` Asmaa Mnebhi
2021-09-27 19:10                 ` Andrew Lunn
2021-09-29 19:14                   ` Asmaa Mnebhi
2021-09-28 15:02               ` Asmaa Mnebhi
2021-09-29 20:24                 ` Andrew Lunn
2021-10-08 14:47                   ` Asmaa Mnebhi
2021-09-23 20:22 ` [PATCH v3 2/2] net: mellanox: mlxbf_gige: Replace non-standard interrupt handling Asmaa Mnebhi
  -- strict thread matches above, loose matches on Subject: below --
2021-09-23 20:18 [PATCH v3 0/2] gpio: mlxbf2: Introduce proper " Asmaa Mnebhi
2021-09-23 20:18 ` [PATCH v3 1/2] gpio: mlxbf2: Introduce IRQ support Asmaa Mnebhi

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=YVHbo/cJcHzxUk+d@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=andy.shevchenko@gmail.com \
    --cc=asmaa@nvidia.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=davem@davemloft.net \
    --cc=davthompson@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).