Dear Andrew Murray,

On Fri,  1 Mar 2013 12:23:36 +0000, Andrew Murray wrote:
> This patch factors out common implementations patterns to reduce overall 
> kernel
> code and provide a means for host bridge drivers to directly obtain struct
> resources from the DT's ranges property without relying on architecture 
> specific
> DT handling. This will make it easier to write archiecture independent host 
> bridge
> drivers and mitigate against further duplication of DT parsing code.
> 
> This patch can be used in the following way:
> 
>       struct of_pci_range_iter iter;
>       for_each_of_pci_range(&iter, np) {
> 
>               //directly access properties of the address range, e.g.:
>               //iter.pci_space, iter.pci_addr, iter.cpu_addr, iter.size or
>               //iter.flags
> 
>               //alternatively obtain a struct resource, e.g.:
>               //struct resource res;
>               //range_iter_fill_resource(iter, np, res);
>       }
> 
> Additionally the implementation takes care of adjacent ranges and merges them
> into a single range (as was the case with powerpc and microblaze).
> 
> The modifications to microblaze, mips and powerpc have not been tested.
> 
> v2:
>   This follows on from suggestions made by Grant Likely
>   (marc.info/?l=linux-kernel&m=136079602806328)
> 
> Signed-off-by: Andrew Murray <andrew.mur...@arm.com>
> Signed-off-by: Liviu Dudau <liviu.du...@arm.com>

Thanks, I've tested this successfully with the Marvell PCIe driver. I'm
about to send a new version of the Marvell PCIe patch set that includes
this RFC proposal.

I only made two small changes compared to your version, detailed below.


> +#define for_each_of_pci_range(iter, np) \
> +     for (; of_pci_process_ranges(iter, np);)

In the initial part of the loop, I added a memset() to initialize to
zero the "iter" structure. Otherwise, if you forget to do it before
calling of_pci_process_ranges(), it may crash (depending on the random
values present in the uninitialized structure).

> +#define range_iter_fill_resource(iter, np, res) \
> +     do { \
> +             res->flags = iter.flags; \
> +             res->start = iter.cpu_addr; \
> +             res->end = iter.cpu_addr + iter.size - 1; \
> +             res->parent = res->child = res->sibling = NULL; \
> +             res->name = np->full_name; \
> +     } while (0)

And here, I enclosed all the usage of the macro parameters in
parenthesis. Like (res)->flags instead of res->flags. If you don't do
that, then passing &foobar as the 'res' parameter causes some
compilation failure because &foobar->res is not valid, while
(&foobar)->res is.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to