Re: [PATCH 2/2] driver: workarroud resource request that conflist with errno PTR

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

On Wed, Jan 07, 2015 at 11:14:32AM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> > On Jan 6, 2015, at 8:44 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > 
> > On Tue, Jan 06, 2015 at 12:37:06PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >> broken since
> >> 
> >> commit ed6e965824303255cacc1c1a195d3684caa26bce
> >> Author:     Sascha Hauer <s.hauer@pengutronix.de>
> >> resource: Let dev_request_mem_region return an error pointer
> >> 
> >> Introduce dev_request_mem_region_err_null
> >> 
> >> only used on platform like at91 where the Ressource address collision with
> >> 
> >> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> >> ---
> >> arch/arm/mach-at91/at91sam926x_time.c |  6 +++---
> >> drivers/base/driver.c                 | 31 +++++++++++++++++++++++++++++--
> >> drivers/pinctrl/pinctrl-at91.c        |  6 +++---
> >> drivers/serial/atmel.c                |  6 +++---
> >> include/driver.h                      |  8 ++++++++
> >> 5 files changed, 46 insertions(+), 11 deletions(-)
> >> 
> >> diff --git a/arch/arm/mach-at91/at91sam926x_time.c b/arch/arm/mach-at91/at91sam926x_time.c
> >> index 789e1ec..cc7ad2f 100644
> >> --- a/arch/arm/mach-at91/at91sam926x_time.c
> >> +++ b/arch/arm/mach-at91/at91sam926x_time.c
> >> @@ -89,9 +89,9 @@ static int at91_pit_probe(struct device_d *dev)
> >> 		return ret;
> >> 	}
> >> 
> >> -	pit_base = dev_request_mem_region(dev, 0);
> >> -	if (IS_ERR(pit_base))
> >> -		return PTR_ERR(pit_base);
> >> +	pit_base = dev_request_mem_region_err_null(dev, 0);
> >> +	if (!pit_base)
> >> +		return -ENOENT;
> >> 
> >> 	pit_rate = clk_get_rate(clk) / 16;
> >> 
> >> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> >> index e0125a1..2375004 100644
> >> --- a/drivers/base/driver.c
> >> +++ b/drivers/base/driver.c
> >> @@ -242,8 +242,8 @@ int register_driver(struct driver_d *drv)
> >> }
> >> EXPORT_SYMBOL(register_driver);
> >> 
> >> -struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
> >> -				  int num)
> >> +static struct resource *__dev_get_resource(struct device_d *dev,
> >> +		unsigned long type, int num)
> >> {
> >> 	int i, n = 0;
> >> 
> >> @@ -256,6 +256,17 @@ struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
> >> 		}
> >> 	}
> >> 
> >> +	return NULL;
> >> +}
> >> +
> >> +struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
> >> +				  int num)
> >> +{
> >> +	struct resource *res = __dev_get_resource(dev, type, num);
> >> +
> >> +	if (res)
> >> +		return res;
> >> +
> >> 	return ERR_PTR(-ENOENT);
> >> }
> > 
> > No need to change dev_get_resource(), it returns a struct resource *
> > which will never conflict with an err ptr.
> 
> but as if no ressource is found it return ERR_PTR(-ENOENT)

Yes, and this is always correct. Only your
dev_request_mem_region_err_null() function must return NULL in this case
and not ERR_CAST(res) like dev_request_mem_region() does.

void __iomem *dev_request_mem_region_err_null(struct device_d *dev, int num)
{
       struct resource *res;

       res = dev_get_resource(dev, IORESOURCE_MEM, num);
       if (IS_ERR(res))
               return NULL;

       res = request_iomem_region(dev_name(dev), res->start, res->end);
       if (IS_ERR(res))
               return ERR_CAST(res);

       return (void __force __iomem *)res->start;
}
EXPORT_SYMBOL(dev_request_mem_region_err_null);

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox