On Mon, Nov 18, 2024 at 3:27 AM Russell King (Oracle)
<linux@xxxxxxxxxxxxxxx> wrote:
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@xxxxxxxxx>
---
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@xxxxxxxxxxx>
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?
Another way to look at it is, if we convert this to
devm_platform_get_and_ioremap_resource(pdev, 1, &res);
it seems to only write to res if platform_get_resource succeeds and
otherwise doesn't care. The only real way to check if found is
!IS_ERR().
Actually the more appropriate function here is
devm_platform_ioremap_resource , which doesn't write to a struct
resource.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!