Re: [PATCH v2] pinctrl: mvebu: add pinctrl drivers for Dove and Kirkwood

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

On Thu, Jun 26, 2014 at 04:38:34PM +0200, Alexander Aring wrote:
> Hi,
> 
> > We could also return an error pointer instead of NULL. Given that not
> > many drivers check the return value anyway this shouldn't be hard to
> > change.
> > 
> 
> yeah, but a ERR_PTR(-ERRNO) is also some valid address which
> dev_request_mem_region could return. Likely in power of two addresses.
> But this is more unlikely than check on NULL. :-)
> 
> Anyway I would try to use the ERR_PTR solution.

So we can't use the last 524 bytes of the physical address space. I
think that's a risk we can take.

> 
> 
> I also see now because we print warning on resource conflicts Holger
> reports some resource conflict on his board. Maybe there is also some
> others board with resource conflicts but we didn't saw that because the
> debug print level.
> 
> I only add pr_warn for conflicts, but there are also some other debug
> prints on error handling, maybe we should change it to:
> 
> diff --git a/common/resource.c b/common/resource.c
> index 1ea2a75..e77805b 100644
> --- a/common/resource.c
> +++ b/common/resource.c
> @@ -44,7 +44,7 @@ struct resource *__request_region(struct resource *parent,
>         struct resource *r, *new;
>  
>         if (end < start) {
> -               debug("%s: request region 0x%08llx:0x%08llx: end < start\n",
> +               pr_warn("%s: request region 0x%08llx:0x%08llx: end < start\n",
>                                 __func__,
>                                 (unsigned long long)start,
>                                 (unsigned long long)end);
> @@ -53,7 +53,7 @@ struct resource *__request_region(struct resource *parent,
>  
>         /* outside parent resource? */
>         if (start < parent->start || end > parent->end) {
> -               debug("%s: 0x%08llx:0x%08llx outside parent resource 0x%08llx:0x%08llx\n",
> +               pr_warn("%s: 0x%08llx:0x%08llx outside parent resource 0x%08llx:0x%08llx\n",
>                                 __func__,
>                                 (unsigned long long)start,
>                                 (unsigned long long)end,
> 
> Should I prepare a patch for this? Maybe more users comes because there
> os something wrong with device probing, etc...

I don't think that's necessary. These debug prints are more for
debugging the resource code.

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