lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 3 Mar 2022 21:04:50 +0100 From: Javier Martinez Canillas <javierm@...hat.com> To: Jeremy Linton <jeremy.linton@....com>, Jakub Kicinski <kuba@...nel.org>, Florian Fainelli <f.fainelli@...il.com> Cc: Peter Robinson <pbrobinson@...il.com>, Doug Berger <opendmb@...il.com>, "David S. Miller" <davem@...emloft.net>, bcm-kernel-feedback-list@...adcom.com, netdev@...r.kernel.org Subject: Re: [PATCH] net: bcmgenet: Return not supported if we don't have a WoL IRQ Hello Jeremy, On 3/3/22 21:00, Jeremy Linton wrote: > Hi, > > On 2/23/22 16:48, Jakub Kicinski wrote: >> On Wed, 23 Feb 2022 09:54:26 -0800 Florian Fainelli wrote: >>>> I have no problems working with you to improve the driver, the problem >>>> I have is this is currently a regression in 5.17 so I would like to >>>> see something land, whether it's reverting the other patch, landing >>>> thing one or another straight forward fix and then maybe revisit as >>>> whole in 5.18. >>> >>> Understood and I won't require you or me to complete this investigating >>> before fixing the regression, this is just so we understand where it >>> stemmed from and possibly fix the IRQ layer if need be. Given what I >>> just wrote, do you think you can sprinkle debug prints throughout the >>> kernel to figure out whether enable_irq_wake() somehow messes up the >>> interrupt descriptor of interrupt and test that theory? We can do that >>> offline if you want. >> >> Let me mark v2 as Deferred for now, then. I'm not really sure if that's >> what's intended but we have 3 weeks or so until 5.17 is cut so we can >> afford a few days of investigating. >> >> I'm likely missing the point but sounds like the IRQ subsystem treats >> IRQ numbers as unsigned so if we pass a negative value "fun" is sort >> of expected. Isn't the problem that device somehow comes with wakeup >> capable being set already? Isn't it better to make sure device is not >> wake capable if there's no WoL irq instead of adding second check? >> >> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c >> index cfe09117fe6c..7dea44803beb 100644 >> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c >> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c >> @@ -4020,12 +4020,12 @@ static int bcmgenet_probe(struct platform_device *pdev) >> >> /* Request the WOL interrupt and advertise suspend if available */ >> priv->wol_irq_disabled = true; >> - if (priv->wol_irq > 0) { >> + if (priv->wol_irq > 0) >> err = devm_request_irq(&pdev->dev, priv->wol_irq, >> bcmgenet_wol_isr, 0, dev->name, priv); >> - if (!err) >> - device_set_wakeup_capable(&pdev->dev, 1); >> - } >> + else >> + err = -ENOENT; >> + device_set_wakeup_capable(&pdev->dev, !err); >> >> /* Set the needed headroom to account for any possible >> * features enabling/disabling at runtime >> > > > I duplicated the problem on rpi4/ACPI by moving to gcc12, so I have a/b > config that is close as I can achieve using gcc11 vs 12 and the one > built with gcc12 fails pretty consistently while the gcc11 works. > Did Peter's patch instead of this one help ? -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Powered by blists - more mailing lists