netdev - Re: [PATCH net] net: micrel: Fix lan8841_config

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240523182827.rvavtqbsbhpcytiv@DEN-DL-M31836.microchip.com>
Date: Thu, 23 May 2024 20:28:27 +0200
From: Horatiu Vultur <horatiu.vultur@...rochip.com>
To: Andrew Lunn <andrew@...n.ch>
CC: <hkallweit1@...il.com>, <linux@...linux.org.uk>, <davem@...emloft.net>,
	<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net] net: micrel: Fix lan8841_config_intr after getting
 out of sleep mode

The 05/23/2024 16:32, Andrew Lunn wrote:

Hi Andrew,

> 
> On Thu, May 23, 2024 at 09:42:26AM +0200, Horatiu Vultur wrote:
> > When the interrupt is enabled, the function lan8841_config_intr tries to
> > clear any pending interrupts by reading the interrupt status, then
> > checks the return value for errors and then continue to enable the
> > interrupt. It has been seen that once the system gets out of sleep mode,
> > the interrupt status has the value 0x400 meaning that the PHY detected
> > that the link was in low power. That is correct value but the problem is
> > that the check is wrong.  We try to check for errors but we return an
> > error also in this case which is not an error. Therefore fix this by
> > returning only when there is an error.
> 
> Is the second case also broken in the same way?
> 
>         } else {
>                 err = phy_write(phydev, LAN8814_INTC, 0);
>                 if (err)
>                         return err;
> 
>                 err = phy_read(phydev, LAN8814_INTS);
>         }
> 
>         return err;
> 
> e.g. there was an outstanding interrupt as interrupts are
> disabled. This will cause the return value of the function to be not
> 0?

Yes, that is correct. In that case, it would return some positive number
which is the status, which is incorrect.
I will fix this in the new version.

> 
>         Andrew

-- 
/Horatiu