Thread (3 messages) 3 messages, 3 authors, 2026-02-27

Re: [PATCH phy-next v2] phy: apple: apple: Use local variable for ioremap return value

From: Sven Peter <sven@kernel.org>
Date: 2026-02-15 12:07:44
Also in: asahi, linux-phy, lkml

On 15.02.26 09:02, Janne Grunau wrote:
The indirection through the resources array is unnecessarily complicated
and resuling in using IS_ERR() and PTR_ERR() on a valid address. A local
variable for the devm_ioremap_resource() return value is both easier to
read and matches expectations when reading code.

Reported-by: Dan Carpenter <redacted>
Closes: https://lore.kernel.org/asahi/aYXvX1bYOXtYCgfC@stanley.mountain/ (local)
Suggested-by: Vladimir Oltean <olteanv@gmail.com>
Fixes: 8e98ca1e74db ("phy: apple: Add Apple Type-C PHY")
Signed-off-by: Janne Grunau <j@jannau.net>
---
Reviewed-by: Sven Peter <sven@kernel.org>
quoted hunk ↗ jump to hunk
Changes in v2:
- Use a local variable instead of the complex indirection with the
   resources array
- Link to v1: https://lore.kernel.org/r/20260207-phy-apple-resource-err-ptr-v1-1-78735b07ed2d@jannau.net (local)
---
  drivers/phy/apple/atc.c | 8 +++++---
  1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/apple/atc.c b/drivers/phy/apple/atc.c
index dc867f368b68748ea953e594ad998d7f965d8d1d..64d0c3dba1cbb95f867d338da706225ee0bf79f7 100644
--- a/drivers/phy/apple/atc.c
+++ b/drivers/phy/apple/atc.c
@@ -2202,14 +2202,16 @@ static int atcphy_map_resources(struct platform_device *pdev, struct apple_atcph
  		{ "pipehandler", &atcphy->regs.pipehandler, NULL },
  	};
  	struct resource *res;
+	void __iomem *addr;
  
  	for (int i = 0; i < ARRAY_SIZE(resources); i++) {
  		res = platform_get_resource_byname(pdev, IORESOURCE_MEM, resources[i].name);
-		*resources[i].addr = devm_ioremap_resource(&pdev->dev, res);
-		if (IS_ERR(resources[i].addr))
-			return dev_err_probe(atcphy->dev, PTR_ERR(resources[i].addr),
+		addr = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(addr))
+			return dev_err_probe(atcphy->dev, PTR_ERR(addr),
  					     "Unable to map %s regs", resources[i].name);
  
+		*resources[i].addr = addr;
This is much easier to understand. I missed return PTR_ERR(..) error in 
the first version and introduced it originally due to the indirection as 
well.



Best,


Sven


Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help