On Tue, May 1, 2012 at 4:57 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote: > On Sun, Mar 18, 2012 at 11:42 PM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote: >> It is changed from busn_res only version, because Bjorn found that version >> was not holding resource_lock. >> Even it may be ok for busn_res not holding resource_lock. >> It would be better to have it to be generic and use lock and we would >> use it for other resources. >> >> probe_resource() will try to find specified size or more in parent bus. >> If can not find current parent resource, and it will try to expand parents >> top. >> If still can not find that specified on top, it will try to reduce target size >> until find one. >> >> It will return 0, if it find any resource that it could use. >> >> Returned resource already register in the tree. >> >> So caller may still need call resource_replace to put real resource in >> the tree. >> >> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx> >> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> >> --- >> include/linux/ioport.h | 7 ++ >> kernel/resource.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++-- >> 2 files changed, 162 insertions(+), 5 deletions(-) >> >> diff --git a/include/linux/ioport.h b/include/linux/ioport.h >> index e885ba2..20a30df 100644 >> --- a/include/linux/ioport.h >> +++ b/include/linux/ioport.h >> @@ -156,6 +156,13 @@ extern int allocate_resource(struct resource *root, struct resource *new, >> resource_size_t, >> resource_size_t), >> void *alignf_data); >> +void resource_shrink_parents_top(struct resource *b_res, >> + long size, struct resource *parent_res); >> +struct device; >> +int probe_resource(struct resource *b_res, >> + struct device *dev, struct resource *busn_res, >> + resource_size_t needed_size, struct resource **p, >> + int skip_nr, int limit, char *name); > > This interface is a mess. I have a vague impression that this is > supposed to figure out whether a resource can be expanded, but why > does it need a struct device *? (I can read the code and see how you > use it, but it just doesn't fit in the resource abstraction.) for debug printing purpose. > Supplying one struct resource * makes sense, but you have three. A > char * name? What's skip_nr? This just doesn't make sense to me. name is for debug purpose too. skip_nr is for skipping. for example: parent [60-7e] when we try to probe for child buses, we need skip 60 as it is used already for pci devices. > > I think you need a simpler, more general interface. update_resource() > seems OK to me -- it's pretty straightforward and has an obvious > meaning. Maybe you can make a resource_expand() or something that > just expands a resource in both directions as far as possible (until > it hits a sibling or the parent limits). Then you would know the > maximum possible size, and you could use update_resource() to shrink > it again and give up anything you don't need. both directions may need more code. > > I spent most of the day merging the patches up to this point, and they > mostly make sense, but this one and the following ones are beyond my > ken, so I gave up. ok, let me check if i could simplify that code more. Thanks Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
