Received-SPF: pass (google.com: domain of linux-kernel+bounces-187880-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Thu, 23 May 2024 20:28:27 +0200
From: Horatiu Vultur <horatiu.vultur@microchip.com>
To: Andrew Lunn <andrew@lunn.ch>
CC: <hkallweit1@gmail.com>, <linux@armlinux.org.uk>, <davem@davemloft.net>,
	<edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net] net: micrel: Fix lan8841_config_intr after getting
 out of sleep mode
Message-ID: <20240523182827.rvavtqbsbhpcytiv@DEN-DL-M31836.microchip.com>
References: <20240523074226.3540332-1-horatiu.vultur@microchip.com>
 <f73778fe-ef01-44b4-9a8d-cd2a978c1a3d@lunn.ch>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
In-Reply-To: <f73778fe-ef01-44b4-9a8d-cd2a978c1a3d@lunn.ch>

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