Re: Barebox x86 IDE support

[Michel Stam <michel@xxxxxxxxxxx>]

Hello Sascha,

I will repost the patches separately as you requested.

With regard to your comment below, this was somewhat intentional, as I 
understood:
> struct resource iomem_resource = {
>         .start = 0,
>         .end = 0xffffffff,
>         .name = "iomem",
>         .children = LIST_HEAD_INIT(iomem_resource.children),
> };
as "I/O and memory resource". This is also why I check on the resource 
type in request_region( ):
>         /*
>          * We keep the list of child resources ordered which helps
>          * us searching for conflicts here.
>          */
>         list_for_each_entry(r, &parent->children, sibling) {
>                 if (end < r->start)
>                         goto ok;
>                 if (start > r->end)
>                         continue;
>
>                 if (type != resource_type(r->parent))
>                         continue;
I retain one resource tree that way, also for if people decide to add 
DMA or IRQ's in the future. The iteration function skips the resource if 
the types do not match. To me the discussion seems the difference 
between one linked list for every resource type (== more work if a new 
type is added), or a single linked list with a type field (new resource 
type == new integer constant, no more).

I would like to hear your thoughts on that.

to be continued after the posting.

Cheers,

Michel
On 03/25/2014 09:18 AM, Sascha Hauer wrote:
> Hi Michel,
>
> On Mon, Mar 24, 2014 at 10:37:48AM +0100, Stam, Michel [FINT] wrote:
> > Hello maintainers,
> >
> > I was wondering if one of you has had time to verify these patches and apply them to trunk?
> Could you send the patches as a series so that it's easier to comment on
> the patches on the list?
>
> There are some things to comment on, I think the most important one is
> this:
>
> > +/*
> > + * request an io region inside the io space
> > + */
> > +struct resource *request_io_region(const char *name,
> > +		resource_size_t start, resource_size_t end,int type)
> > +{
> > +	return request_region(&iomem_resource, name, start, end, type);
> > +}
> You request here from the iomem resource, but ioports are a completely
> separate resource, so you have to use a new toplevel resource like
> this:
>
> /* The root resource for the whole io space */
> struct resource io_resource = {
> 	.start = 0,
> 	.end = 0xffffffff,
> 	.name = "ioport",
> 	.children = LIST_HEAD_INIT(io_resource.children),
> };
>
> /*
>   * request an io region inside the io space
>   */
> struct resource *request_io_region(const char *name,
> 		resource_size_t start, resource_size_t end)
> {
> 	return request_region(&io_resource, name, start, end, IORESOURCE_IO);
> }
>
> The 'type' argument to request_io_region is unnecessary since the
> function name already implies it, right?
>
> Sascha

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

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