linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
To: Vadym Kochan <vadym.kochan@plvision.eu>,
	Rob Herring <robh+dt@kernel.org>,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 1/3] nvmem: core: introduce cells parser
Date: Tue, 22 Sep 2020 10:48:27 +0100	[thread overview]
Message-ID: <b2e63a28-7678-832e-f585-6d0f959d1b75@linaro.org> (raw)
In-Reply-To: <20200915124116.7196-2-vadym.kochan@plvision.eu>



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@plvision.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 */
> 

  reply	other threads:[~2020-09-22  9:48 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-15 12:41 [PATCH 0/3] nvmem: add ONIE NVMEM cells provider Vadym Kochan
2020-09-15 12:41 ` [PATCH 1/3] nvmem: core: introduce cells parser Vadym Kochan
2020-09-22  9:48   ` Srinivas Kandagatla [this message]
2020-09-22 11:10     ` Vadym Kochan
2020-09-15 12:41 ` [PATCH 2/3] nvmem: add ONIE nvmem " Vadym Kochan
2020-09-15 12:41 ` [PATCH 3/3] dt-bindings: nvmem: add description for ONIE " Vadym Kochan
2020-09-21 23:56 ` [PATCH 0/3] nvmem: add ONIE NVMEM cells provider Vadym Kochan
2020-09-22  9:48   ` Srinivas Kandagatla
2020-09-22 10:04     ` Vadym Kochan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b2e63a28-7678-832e-f585-6d0f959d1b75@linaro.org \
    --to=srinivas.kandagatla@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=vadym.kochan@plvision.eu \
    --subject='Re: [PATCH 1/3] nvmem: core: introduce cells parser' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).