Re: [PATCH net] net: mdio-ipq4019: fix wrong NULL check

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Mon, Nov 18, 2024 at 06:26:27PM +0800, Jie Luo wrote:
> On 11/18/2024 5:28 AM, Rosen Penev wrote:
> > devm_ioremap_resource returns a PTR_ERR when it fails, not NULL. OTOH
> > this is conditionally set to either a PTR_ERR or a valid pointer. Use
> > !IS_ERR_OR_NULL to check if we can use this.
> > 
> > Fixes: 23a890d493 ("net: mdio: Add the reset function for IPQ MDIO driver")
> > 
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >   drivers/net/mdio/mdio-ipq4019.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c
> > index dd3ed2d6430b..859302b0d38c 100644
> > --- a/drivers/net/mdio/mdio-ipq4019.c
> > +++ b/drivers/net/mdio/mdio-ipq4019.c
> > @@ -256,7 +256,7 @@ static int ipq_mdio_reset(struct mii_bus *bus)
> >   	/* To indicate CMN_PLL that ethernet_ldo has been ready if platform resource 1
> >   	 * is specified in the device tree.
> >   	 */
> > -	if (priv->eth_ldo_rdy) {
> > +	if (!IS_ERR_OR_NULL(priv->eth_ldo_rdy)) {
> >   		val = readl(priv->eth_ldo_rdy);
> >   		val |= BIT(0);
> >   		writel(val, priv->eth_ldo_rdy);
> 
> Reviewed-by: Luo Jie <quic_luoj@quicinc.com>

Looking at the setup of this:

        /* The platform resource is provided on the chipset IPQ5018 */
        /* This resource is optional */
        res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
        if (res)
                priv->eth_ldo_rdy = devm_ioremap_resource(&pdev->dev, res);

While this is optional, surely the optional part is whether resource 1
is provided or not. If the resource is provided, but we fail to ioremap
it, isn't that an error which should be propagated? In that situation,
isn't the firmware saying "we have a second resource" but failing to
map it should be an error?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!