From: Asmaa Mnebhi <email@example.com> To: Andrew Lunn <firstname.lastname@example.org> Cc: Linus Walleij <email@example.com>, Andy Shevchenko <firstname.lastname@example.org>, "open list:GPIO SUBSYSTEM" <email@example.com>, netdev <firstname.lastname@example.org>, linux-kernel <email@example.com>, ACPI Devel Maling List <firstname.lastname@example.org>, Jakub Kicinski <email@example.com>, Bartosz Golaszewski <firstname.lastname@example.org>, "David S. Miller" <email@example.com>, "Rafael J. Wysocki" <firstname.lastname@example.org>, David Thompson <email@example.com> Subject: RE: [PATCH v3 1/2] gpio: mlxbf2: Introduce IRQ support Date: Wed, 29 Sep 2021 19:14:48 +0000 [thread overview] Message-ID: <CH2PR12MB3895BD75A1BD0048A93B4A51D7A99@CH2PR12MB3895.namprd12.prod.outlook.com> (raw) In-Reply-To: <YVIXNEmhoMW7c1Sfirstname.lastname@example.org> > Asmaa>> Thank you very much for the detailed and clear explanation! > we only enable/support link up/down interrupts. QA has tested bringing > up/down the network interface +200 times in a loop. The micrel driver currently only uses two interrupts of the available 8. So it will be hard to trigger the problem with the current driver. Your best way to trigger it is going to bring the link down as soon as it goes up. So you get first a link up, and then a link down very shortly afterwards. There is however nothing stopping developers making use of the other interrupts. That will then increase the likelihood of problems. What does help you is that the interrupt register is clear on read. So the race condition window is small. Asmaa>> Hi Andrew, I had a meeting today with the HW folks to explain the problem at stake. The flow for this issue is like this: 1) PHY issues INT_N signal (active low level interrupt) 2) falling edge detected on the GPIO and transmitted to software 3) the first thing mlxbf2_gpio_irq_handler does is to clear the GPIO interrupt. However even if we clear the GPIO interrupt, the GPIO value itself will be low as long as the INT_N signal is low. The GPIO HW triggers the interrupt by detecting the falling edge of the GPIO pin. 4) mlxbf2_gpio_irq_handler triggers phy_interrupt which calls drv->handler_interrupt. handle_interrupt in our case = kszphy_handle_interrupt, which reads MII_KSZPHY_INTCS regs and hence clears all interrupts at once. - if no other interrupt happens within this time frame, INT_N goes back to 1 and the next interrupt will trigger another GPIO falling edge - if the interrupt happens after the MDIO read, then it is not a problem. The read would have already cleared the register and INT_N would go back to 1. So the new interrupt will trigger a new GPIO falling edge interrupt. Problem: - however, if there is a second interrupt right before or during the MDIO read of MII_KSZPHY_INTCS, it might not be detected by our GPIO HW. Anyways, the HW folks agreed that this is a problem since indeed they do not support LEVEL interrupts on the GPIOs at the moment. They suggested to read the GPIO pin value to check if it has returned to high in mlxbf2_gpio_irq_handler, then trigger the phy_interrupt handler. But I don't think it is a good workaround because there could be a chain of interrupts which hold the LEVEL low for a long time, and we don't want to be waiting too long in an interrupt handler routine. I would greatly appreciate some more feedback on what is the best way to deal With this in the upstreamed version of the driver. HW folks said they will fix this in future BlueField generations. > The software interrupt and handler is not registered based on the GPIO > interrupt but rather a HW interrupt which is common to all GPIO pins > (irrelevant here, but this is edge triggered): > ret = devm_request_irq(dev, irq, mlxbf2_gpio_irq_handler, > IRQF_SHARED, name, gs); IRQF_SHARED implied level. You cannot have a shared interrupt which is using edges. Andrew
next prev parent reply other threads:[~2021-09-29 19:14 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 2021-09-27 15:52 ` Asmaa Mnebhi 2021-09-27 19:10 ` Andrew Lunn 2021-09-29 19:14 ` Asmaa Mnebhi [this message] 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=CH2PR12MB3895BD75A1BD0048A93B4A51D7A99@CH2PR12MB3895.namprd12.prod.outlook.com \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='RE: [PATCH v3 1/2] gpio: mlxbf2: Introduce IRQ support' \ /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
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).