On Tue, Sep 22, 2020 at 10:48:27AM +0100, Srinivas Kandagatla wrote:
>
>
> On 15/09/2020 13:41, Vadym Kochan wrote:
> > Currently NVMEM core does not allow to register cells for an already
> > registered nvmem device and requires that this should be done before.
> > But there might a driver which needs to parse the nvmem device and then
> > register a cells table.
> >
> > Introduce nvmem_parser API which allows to register cells parser which
> > is called during nvmem device registration. During this stage the parser
> > can read the nvmem device and register the cells table.
> >
>
> Overall this approach looks okay to me!
>
> Rather than a binding for onie cell parser, I think we should add a generic
> bindings for parser something like:
>
>
> nvmem-cell-parser = "onie-tlv-cells";
>
>
> I also think the parser matching should be done based on this string which
> can remove
> 1. new DT node for onie parser.
> 2. works for both DT and non-DT setups.
>
> nvmem provider register will automatically parse this once it finds a
> matching parser or it will EPROBE_DEFER!
>
> Let me know if you think it works for you!
>
>
> --srini
Indeed, it sounds very good and fixes the early dependency that
parser should be registered first before nvmem provider.
I will try!
>
> > Signed-off-by: Vadym Kochan <vadym.kochan@xxxxxxxxxxx>
> > ---
> > drivers/nvmem/core.c | 89 ++++++++++++++++++++++++++++++++++
> > include/linux/nvmem-provider.h | 27 +++++++++++
> > 2 files changed, 116 insertions(+)
> >
> > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > index 6cd3edb2eaf6..82a96032bc3f 100644
> > --- a/drivers/nvmem/core.c
> > +++ b/drivers/nvmem/core.c
> > @@ -55,6 +55,14 @@ struct nvmem_cell {
> > struct list_head node;
> > };
> > +struct nvmem_parser {
> > + struct device_node *nvmem_of;
> > + struct kref refcnt;
> > + struct list_head head;
> > + nvmem_parse_t cells_parse;
> > + void *priv;
> > +};
> > +
> > static DEFINE_MUTEX(nvmem_mutex);
> > static DEFINE_IDA(nvmem_ida);
> > @@ -64,6 +72,9 @@ static LIST_HEAD(nvmem_cell_tables);
> > static DEFINE_MUTEX(nvmem_lookup_mutex);
> > static LIST_HEAD(nvmem_lookup_list);
> > +static DEFINE_MUTEX(nvmem_parser_mutex);
> > +static LIST_HEAD(nvmem_parser_list);
> > +
> > static BLOCKING_NOTIFIER_HEAD(nvmem_notifier);
> > static int nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
> > @@ -571,6 +582,30 @@ static int nvmem_add_cells_from_of(struct nvmem_device *nvmem)
> > return 0;
> > }
> > +static struct nvmem_parser *
> > +__nvmem_parser_find_of(const struct nvmem_device *nvmem)
> > +{
> > + struct nvmem_parser *parser;
> > +
> > + list_for_each_entry(parser, &nvmem_parser_list, head) {
> > + if (dev_of_node(nvmem->base_dev) == parser->nvmem_of)
> > + return parser;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +static void nvmem_cells_parse(struct nvmem_device *nvmem)
> > +{
> > + struct nvmem_parser *parser;
> > +
> > + mutex_lock(&nvmem_parser_mutex);
> > + parser = __nvmem_parser_find_of(nvmem);
> > + if (parser && parser->cells_parse)
> > + parser->cells_parse(parser->priv, nvmem);
> > + mutex_unlock(&nvmem_parser_mutex);
> > +}
> > +
> > /**
> > * nvmem_register() - Register a nvmem device for given nvmem_config.
> > * Also creates a binary entry in /sys/bus/nvmem/devices/dev-name/nvmem
> > @@ -674,6 +709,8 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
> > goto err_teardown_compat;
> > }
> > + nvmem_cells_parse(nvmem);
> > +
> > rval = nvmem_add_cells_from_table(nvmem);
> > if (rval)
> > goto err_remove_cells;
> > @@ -1630,6 +1667,58 @@ const char *nvmem_dev_name(struct nvmem_device *nvmem)
> > }
> > EXPORT_SYMBOL_GPL(nvmem_dev_name);
> > +struct nvmem_parser *
> > +nvmem_parser_register(const struct nvmem_parser_config *config)
> > +{
> > + struct device_node *nvmem_of;
> > + struct nvmem_parser *parser;
> > + int err;
> > +
> > + if (!config->cells_parse)
> > + return ERR_PTR(-EINVAL);
> > +
> > + if (!config->dev)
> > + return ERR_PTR(-EINVAL);
> > +
> > + nvmem_of = of_parse_phandle(dev_of_node(config->dev), "nvmem", 0);
> > + if (!nvmem_of)
> > + return ERR_PTR(-EINVAL);
> > +
> > + parser = kzalloc(sizeof(*parser), GFP_KERNEL);
> > + if (!parser) {
> > + err = -ENOMEM;
> > + goto err_alloc;
> > + }
> > +
> > + parser->cells_parse = config->cells_parse;
> > + /* parser->cells_remove = config->cells_remove; */
> > + parser->nvmem_of = nvmem_of;
> > + parser->priv = config->priv;
> > + kref_init(&parser->refcnt);
> > +
> > + mutex_lock(&nvmem_parser_mutex);
> > + list_add(&parser->head, &nvmem_parser_list);
> > + mutex_unlock(&nvmem_parser_mutex);
> > +
> > + return parser;
> > +
> > +err_alloc:
> > + of_node_put(nvmem_of);
> > +
> > + return ERR_PTR(err);
> > +}
> > +EXPORT_SYMBOL(nvmem_parser_register);
> > +
> > +void nvmem_parser_unregister(struct nvmem_parser *parser)
> > +{
> > + mutex_lock(&nvmem_parser_mutex);
> > + of_node_put(parser->nvmem_of);
> > + list_del(&parser->head);
> > + kfree(parser);
> > + mutex_unlock(&nvmem_parser_mutex);
> > +}
> > +EXPORT_SYMBOL(nvmem_parser_unregister);
> > +
> > static int __init nvmem_init(void)
> > {
> > return bus_register(&nvmem_bus_type);
> > diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
> > index 06409a6c40bc..854d0cf5234f 100644
> > --- a/include/linux/nvmem-provider.h
> > +++ b/include/linux/nvmem-provider.h
> > @@ -15,10 +15,13 @@
> > struct nvmem_device;
> > struct nvmem_cell_info;
> > +struct nvmem_cell_table;
> > +
> > typedef int (*nvmem_reg_read_t)(void *priv, unsigned int offset,
> > void *val, size_t bytes);
> > typedef int (*nvmem_reg_write_t)(void *priv, unsigned int offset,
> > void *val, size_t bytes);
> > +typedef void (*nvmem_parse_t)(void *priv, struct nvmem_device *nvmem);
> > enum nvmem_type {
> > NVMEM_TYPE_UNKNOWN = 0,
> > @@ -100,6 +103,12 @@ struct nvmem_cell_table {
> > struct list_head node;
> > };
> > +struct nvmem_parser_config {
> > + nvmem_parse_t cells_parse;
> > + void *priv;
> > + struct device *dev;
> > +};
> > +
> > #if IS_ENABLED(CONFIG_NVMEM)
> > struct nvmem_device *nvmem_register(const struct nvmem_config *cfg);
> > @@ -110,9 +119,19 @@ struct nvmem_device *devm_nvmem_register(struct device *dev,
> > int devm_nvmem_unregister(struct device *dev, struct nvmem_device *nvmem);
> > +int nvmem_cell_parser_register(const char *nvmem_name,
> > + const struct nvmem_config *cfg);
> > +void nvmem_cell_parser_unregister(const char *nvmem_name,
> > + const struct nvmem_config *cfg);
> > +
> > void nvmem_add_cell_table(struct nvmem_cell_table *table);
> > void nvmem_del_cell_table(struct nvmem_cell_table *table);
> > +struct nvmem_parser *
> > +nvmem_parser_register(const struct nvmem_parser_config *config);
> > +
> > +void nvmem_parser_unregister(struct nvmem_parser *parser);
> > +
> > #else
> > static inline struct nvmem_device *nvmem_register(const struct nvmem_config *c)
> > @@ -137,5 +156,13 @@ devm_nvmem_unregister(struct device *dev, struct nvmem_device *nvmem)
> > static inline void nvmem_add_cell_table(struct nvmem_cell_table *table) {}
> > static inline void nvmem_del_cell_table(struct nvmem_cell_table *table) {}
> > +static inline struct nvmem_parser *
> > +nvmem_parser_register(const struct nvmem_parser_config *config)
> > +{
> > + return -EOPNOTSUPP;
> > +}
> > +
> > +static inline void nvmem_parser_unregister(struct nvmem_parser *parser) {}
> > +
> > #endif /* CONFIG_NVMEM */
> > #endif /* ifndef _LINUX_NVMEM_PROVIDER_H */
> >
