Messages in this thread |  | | Date | Mon, 8 Jun 2015 14:03:51 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] genalloc: add support of multiple gen_pools per device |
| |
On Fri, 5 Jun 2015 02:35:30 +0300 Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> wrote:
> > > If there's a pattern, it is "subsytem-identifier_operation-on-it", so > > > > devm_gen_pool_named_create > > dev_gen_pool_named_get > > of_named_gen_pool_get > > > > ie: it's big-endian. The name starts out with the most significant > > thing (subsystem identification) and fields in order of decreasing > > significance. > > > > Anyway, please have a think about it ;) > > What would be your opinion on the following naming proposal: > > devm_gen_pool_create() --> devm_gen_pool_create(), > devm_gen_pool_create_named() > > No changes, and "named" flavour of a new gen_pool create interface goes > to the suffix position. > > dev_get_gen_pool() --> dev_gen_pool_get(), > dev_gen_pool_get_named() > > And the last one > > of_get_named_gen_pool() --> of_gen_pool_get() > > Here "named" reminds of need to provide a phandle name, not gen_pool > name as above, to avoid confusion I would propose to drop it. > > If it is okay from your point of view, I'll send another non-functional > patch against linux-next to rename existing interfaces.
Sounds good.
> >> +/** > >> + * dev_get_gen_pool_named - Obtain the gen_pool (if any) for a device > >> + * @dev: device to retrieve the gen_pool from > >> + * @name: name of a gen_pool, addresses a particular gen_pool from device > >> + * > >> + * Returns the gen_pool for the device if one is present, or NULL. > >> + */ > >> +struct gen_pool *dev_get_gen_pool_named(struct device *dev, const char *name) > >> +{ > >> + struct gen_pool **p = devres_find(dev, devm_gen_pool_release, > >> + dev_gen_pool_match, (void *)name); > >> + > >> + if (!p) > >> + return NULL; > >> + return *p; > >> +} > >> +EXPORT_SYMBOL_GPL(dev_get_gen_pool_named); > > > > But we didn't do anything to prevent duplicated names. > > Like with MTD OF partitions used as a template technically it is > possible to register several gen_pools under the same name, when > requested the first found one is returned to a client, correctness of > one to one mapping is offloaded to the register party, for instance if > of_get_named_gen_pool() is supposed to be used, then DTS description is > expected to be consistent.
Well, if we go this way then your "first found one" becomes part of the dev_get_gen_pool_named() interface definition and should be documented and maintained. Most-recently-registered or least-recently-registered?
But either way, does the interface make sense? Or it is an error condition? If it's an error condition then we should check for it rather than silently ignoring it.
> Is it good enough or better to add a name uniqueness check? In my > opinion the latter case may require to change error return value of > devm_gen_pool_create() from NULL to ERR_PTR().
It sounds that way.
|  |