| Message ID | 20190809103235.16338-4-tbogendoerfer@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Use MFD framework for SGI IOC3 drivers | expand |
On 09/08/2019 11:32, Thomas Bogendoerfer wrote: > nvmem_device_find provides a way to search for nvmem devices with > the help of a match function simlair to bus_find_device. > > Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de> > --- > drivers/nvmem/core.c | 62 ++++++++++++++++++++++-------------------- > include/linux/nvmem-consumer.h | 9 ++++++ > 2 files changed, 41 insertions(+), 30 deletions(-) Have you considered using nvmem_register_notifier() ? --srini
On Tue, 13 Aug 2019 10:40:34 +0100 Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote: > > > On 09/08/2019 11:32, Thomas Bogendoerfer wrote: > > nvmem_device_find provides a way to search for nvmem devices with > > the help of a match function simlair to bus_find_device. > > > > Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de> > > --- > > drivers/nvmem/core.c | 62 ++++++++++++++++++++++-------------------- > > include/linux/nvmem-consumer.h | 9 ++++++ > > 2 files changed, 41 insertions(+), 30 deletions(-) > > Have you considered using nvmem_register_notifier() ? yes, that was the first idea. But then I realized I need to build up a private database of information already present in nvmem bus. So I looked for a way to retrieve it from there. Unfortunately I couldn't use bus_find_device directly, because nvmem_bus_type and struct nvmem_device is hidden. So I refactured the lookup code and added a more universal lookup function, which fits my needs and should be usable for more. Thomas.
On 14/08/2019 12:46, Thomas Bogendoerfer wrote: > On Tue, 13 Aug 2019 10:40:34 +0100 > Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote: > >> >> >> On 09/08/2019 11:32, Thomas Bogendoerfer wrote: >>> nvmem_device_find provides a way to search for nvmem devices with >>> the help of a match function simlair to bus_find_device. >>> >>> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de> >>> --- >>> drivers/nvmem/core.c | 62 ++++++++++++++++++++++-------------------- >>> include/linux/nvmem-consumer.h | 9 ++++++ >>> 2 files changed, 41 insertions(+), 30 deletions(-) >> >> Have you considered using nvmem_register_notifier() ? > > yes, that was the first idea. But then I realized I need to build up > a private database of information already present in nvmem bus. So I > looked for a way to retrieve it from there. Unfortunately I couldn't > use bus_find_device directly, because nvmem_bus_type and struct nvmem_device > is hidden. So I refactured the lookup code and added a more universal > lookup function, which fits my needs and should be usable for more. I see your point. overall the patch as it is look good, but recently we added more generic lookups for DT node, looks like part of your patch is un-doing generic device name lookup. DT node match lookup is in https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/log/?h=generic_lookup_helpers of_nvmem_match and nvmem_match_name are duplicating the code here. Looks like its possible to use generic lookups along with custom match by splitting __nvmem_device_get() to two functions, one for lookup and other for refcounting. Other missing bit is adding this api to documentation in ./Documentation/driver-api/nvmem.rst thanks, srini > > Thomas. >
On Wed, Aug 14, 2019 at 01:52:49PM +0100, Srinivas Kandagatla wrote: > On 14/08/2019 12:46, Thomas Bogendoerfer wrote: > >On Tue, 13 Aug 2019 10:40:34 +0100 > >Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote: > >>On 09/08/2019 11:32, Thomas Bogendoerfer wrote: > >>>nvmem_device_find provides a way to search for nvmem devices with > >>>the help of a match function simlair to bus_find_device. > >>> > >>>Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de> > >>>--- > >>> drivers/nvmem/core.c | 62 ++++++++++++++++++++++-------------------- > >>> include/linux/nvmem-consumer.h | 9 ++++++ > >>> 2 files changed, 41 insertions(+), 30 deletions(-) > >> > >>Have you considered using nvmem_register_notifier() ? > > > >yes, that was the first idea. But then I realized I need to build up > >a private database of information already present in nvmem bus. So I > >looked for a way to retrieve it from there. Unfortunately I couldn't > >use bus_find_device directly, because nvmem_bus_type and struct nvmem_device > >is hidden. So I refactured the lookup code and added a more universal > >lookup function, which fits my needs and should be usable for more. > I see your point. > > overall the patch as it is look good, but recently we added more generic > lookups for DT node, looks like part of your patch is un-doing generic > device name lookup. > > DT node match lookup is in https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/log/?h=generic_lookup_helpers these patches are not in Linus tree, yet. I guess they will show up in 5.4. No idea how to deal with it right now, do you ? > Other missing bit is adding this api to documentation in > ./Documentation/driver-api/nvmem.rst ok, will do. Thomas.
On 16/08/2019 15:09, Thomas Bogendoerfer wrote: > On Wed, Aug 14, 2019 at 01:52:49PM +0100, Srinivas Kandagatla wrote: >> On 14/08/2019 12:46, Thomas Bogendoerfer wrote: >>> On Tue, 13 Aug 2019 10:40:34 +0100 >>> Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote: >>>> On 09/08/2019 11:32, Thomas Bogendoerfer wrote: >>>>> nvmem_device_find provides a way to search for nvmem devices with >>>>> the help of a match function simlair to bus_find_device. >>>>> >>>>> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de> >>>>> --- >>>>> drivers/nvmem/core.c | 62 ++++++++++++++++++++++-------------------- >>>>> include/linux/nvmem-consumer.h | 9 ++++++ >>>>> 2 files changed, 41 insertions(+), 30 deletions(-) >>>> >>>> Have you considered using nvmem_register_notifier() ? >>> >>> yes, that was the first idea. But then I realized I need to build up >>> a private database of information already present in nvmem bus. So I >>> looked for a way to retrieve it from there. Unfortunately I couldn't >>> use bus_find_device directly, because nvmem_bus_type and struct nvmem_device >>> is hidden. So I refactured the lookup code and added a more universal >>> lookup function, which fits my needs and should be usable for more. >> I see your point. >> >> overall the patch as it is look good, but recently we added more generic >> lookups for DT node, looks like part of your patch is un-doing generic >> device name lookup. >> >> DT node match lookup is in https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/log/?h=generic_lookup_helpers > > these patches are not in Linus tree, yet. I guess they will show up > in 5.4. No idea how to deal with it right now, do you ? All these patches are due to go in next merge window, You should base your patch on top of linux-next. thanks, srini > >> Other missing bit is adding this api to documentation in >> ./Documentation/driver-api/nvmem.rst > > ok, will do. > > Thomas. >
On Mon, Aug 19, 2019 at 05:03:42PM +0100, Srinivas Kandagatla wrote: > >>On 14/08/2019 12:46, Thomas Bogendoerfer wrote: > >these patches are not in Linus tree, yet. I guess they will show up > >in 5.4. No idea how to deal with it right now, do you ? > All these patches are due to go in next merge window, > You should base your patch on top of linux-next. that depends, which maintainer will merge this series. Right now it doesn't look like this series will make it into 5.4 as there is still no sign form the W1 maintainer. My idea is to break out the 5.4 parts and submit it. So I'll rebase the nvmem patch to linux-next and send it. Hope it will be ok, if the user of the new function will show up in 5.5. Thomas.
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c index ac5d945be88a..e591ba54758f 100644 --- a/drivers/nvmem/core.c +++ b/drivers/nvmem/core.c @@ -76,36 +76,18 @@ static struct bus_type nvmem_bus_type = { .name = "nvmem", }; +#if IS_ENABLED(CONFIG_OF) static int of_nvmem_match(struct device *dev, const void *nvmem_np) { return dev->of_node == nvmem_np; } +#endif -static struct nvmem_device *of_nvmem_find(struct device_node *nvmem_np) +static int nvmem_match_name(struct device *dev, const void *data) { - struct device *d; - - if (!nvmem_np) - return NULL; - - d = bus_find_device(&nvmem_bus_type, NULL, nvmem_np, of_nvmem_match); - - if (!d) - return NULL; + const char *name = data; - return to_nvmem_device(d); -} - -static struct nvmem_device *nvmem_find(const char *name) -{ - struct device *d; - - d = bus_find_device_by_name(&nvmem_bus_type, NULL, name); - - if (!d) - return NULL; - - return to_nvmem_device(d); + return sysfs_streq(name, dev_name(dev)); } static void nvmem_cell_drop(struct nvmem_cell *cell) @@ -537,13 +519,16 @@ int devm_nvmem_unregister(struct device *dev, struct nvmem_device *nvmem) } EXPORT_SYMBOL(devm_nvmem_unregister); -static struct nvmem_device *__nvmem_device_get(struct device_node *np, - const char *nvmem_name) +static struct nvmem_device *__nvmem_device_get(void *data, + int (*match)(struct device *dev, const void *data)) { struct nvmem_device *nvmem = NULL; + struct device *dev; mutex_lock(&nvmem_mutex); - nvmem = np ? of_nvmem_find(np) : nvmem_find(nvmem_name); + dev = bus_find_device(&nvmem_bus_type, NULL, data, match); + if (dev) + nvmem = to_nvmem_device(dev); mutex_unlock(&nvmem_mutex); if (!nvmem) return ERR_PTR(-EPROBE_DEFER); @@ -592,7 +577,7 @@ struct nvmem_device *of_nvmem_device_get(struct device_node *np, const char *id) if (!nvmem_np) return ERR_PTR(-ENOENT); - return __nvmem_device_get(nvmem_np, NULL); + return __nvmem_device_get(nvmem_np, of_nvmem_match); } EXPORT_SYMBOL_GPL(of_nvmem_device_get); #endif @@ -618,10 +603,26 @@ struct nvmem_device *nvmem_device_get(struct device *dev, const char *dev_name) } - return __nvmem_device_get(NULL, dev_name); + return __nvmem_device_get((void *)dev_name, nvmem_match_name); } EXPORT_SYMBOL_GPL(nvmem_device_get); +/** + * nvmem_device_find() - Find nvmem device with matching function + * + * @data: Data to pass to match function + * @match: Callback function to check device + * + * Return: ERR_PTR() on error or a valid pointer to a struct nvmem_device + * on success. + */ +struct nvmem_device *nvmem_device_find(void *data, + int (*match)(struct device *dev, const void *data)) +{ + return __nvmem_device_get(data, match); +} +EXPORT_SYMBOL_GPL(nvmem_device_find); + static int devm_nvmem_device_match(struct device *dev, void *res, void *data) { struct nvmem_device **nvmem = res; @@ -715,7 +716,8 @@ nvmem_cell_get_from_lookup(struct device *dev, const char *con_id) if ((strcmp(lookup->dev_id, dev_id) == 0) && (strcmp(lookup->con_id, con_id) == 0)) { /* This is the right entry. */ - nvmem = __nvmem_device_get(NULL, lookup->nvmem_name); + nvmem = __nvmem_device_get((void *)lookup->nvmem_name, + nvmem_match_name); if (IS_ERR(nvmem)) { /* Provider may not be registered yet. */ cell = ERR_CAST(nvmem); @@ -785,7 +787,7 @@ struct nvmem_cell *of_nvmem_cell_get(struct device_node *np, const char *id) if (!nvmem_np) return ERR_PTR(-EINVAL); - nvmem = __nvmem_device_get(nvmem_np, NULL); + nvmem = __nvmem_device_get(nvmem_np, of_nvmem_match); of_node_put(nvmem_np); if (IS_ERR(nvmem)) return ERR_CAST(nvmem); diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h index 8f8be5b00060..02dc4aa992b2 100644 --- a/include/linux/nvmem-consumer.h +++ b/include/linux/nvmem-consumer.h @@ -89,6 +89,9 @@ void nvmem_del_cell_lookups(struct nvmem_cell_lookup *entries, int nvmem_register_notifier(struct notifier_block *nb); int nvmem_unregister_notifier(struct notifier_block *nb); +struct nvmem_device *nvmem_device_find(void *data, + int (*match)(struct device *dev, const void *data)); + #else static inline struct nvmem_cell *nvmem_cell_get(struct device *dev, @@ -204,6 +207,12 @@ static inline int nvmem_unregister_notifier(struct notifier_block *nb) return -EOPNOTSUPP; } +static inline struct nvmem_device *nvmem_device_find(void *data, + int (*match)(struct device *dev, const void *data)) +{ + return NULL; +} + #endif /* CONFIG_NVMEM */ #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
nvmem_device_find provides a way to search for nvmem devices with the help of a match function simlair to bus_find_device. Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de> --- drivers/nvmem/core.c | 62 ++++++++++++++++++++++-------------------- include/linux/nvmem-consumer.h | 9 ++++++ 2 files changed, 41 insertions(+), 30 deletions(-)