Re: [PATCH] driver: have dev_platform_ioremap_resource return error pointer

[Sascha Hauer <s.hauer@pengutronix.de>]

On Mon, Apr 14, 2025 at 12:10:54PM +0200, Ahmad Fatoum wrote:
> Hello Sascha,
> 
> On 4/14/25 11:07, Sascha Hauer wrote:
> > On Mon, Apr 14, 2025 at 08:23:03AM +0200, Ahmad Fatoum wrote:
> >> From: Ahmad Fatoum <a.fatoum@barebox.org>
> >>
> >> devm_request_mem_region doesn't return NULL on error on Linux and thus
> >> it should neither in barebox. Fix this.
> >>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >>  drivers/base/driver.c  | 2 ++
> >>  include/linux/device.h | 2 +-
> >>  2 files changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> >> index edba037fa2dd..4e361a96c9ee 100644
> >> --- a/drivers/base/driver.c
> >> +++ b/drivers/base/driver.c
> >> @@ -587,6 +587,8 @@ void __iomem *dev_request_mem_region(struct device *dev, int num)
> >>  	struct resource *res;
> >>  
> >>  	res = dev_request_mem_resource(dev, num);
> >> +	if (!IS_ERR(res) && WARN_ON(IS_ERR_VALUE(res->start)))
> >> +		return IOMEM_ERR_PTR(res->start);
> > 
> > Which code can set res->start to an error value?
> 
> It can end up being an error value, because there is actual a MMIO
> region at a high address. In that case, I'd rather we return an
> error, so dev_request_mem_resource would need to be used instead.

Ok, so you don't expect code to intentionally set the resource start to
an error value, but instead the resource start actually is in a range
that is interpreted as an error value (i.e. >= 0x100000000 - MAX_ERRNO
aka the last page in the 32bit address space).

Can we add a comment here to clarify this?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |