| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b2e63a28-7678-832e-f585-6d0f959d1b75@linaro.org>
Date: Tue, 22 Sep 2020 10:48:27 +0100
From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To: Vadym Kochan <vadym.kochan@...ision.eu>,
Rob Herring <robh+dt@...nel.org>, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH 1/3] nvmem: core: introduce cells parser
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
> Signed-off-by: Vadym Kochan <vadym.kochan@...ision.eu>
> ---
> 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 */
>
Powered by blists - more mailing lists