Re: [PATCH 3/8] nvmem: Add nvmem_cell_get_optional and devm_nvmem_cell_get_optional

[Alban]

On Tue, 15 Jan 2019 12:40:53 +0000
Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> wrote:

> On 06/01/2019 19:28, Alban Bedel wrote:
> > Add helper functions to make the driver code simpler when a cell is
> > optional. Using these functions just return NULL when the cell doesn't
> > exists or if nvmem is disabled.
> > 
> > Signed-off-by: Alban Bedel<albeu@xxxxxxx>
> > ---
> >   drivers/nvmem/core.c           | 48 ++++++++++++++++++++++++++++++++++
> >   include/linux/nvmem-consumer.h | 16 ++++++++++++
> >   2 files changed, 64 insertions(+)
> > 
> > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > index f8c43da6f2ca..8e1b52559467 100644
> > --- a/drivers/nvmem/core.c
> > +++ b/drivers/nvmem/core.c
> > @@ -1083,6 +1083,30 @@ struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *id)
> >   }
> >   EXPORT_SYMBOL_GPL(nvmem_cell_get);
> >   
> > +/**
> > + * nvmem_cell_get_optional() - Get an optional nvmem cell of device from
> > + * a given id.
> > + *
> > + * @dev: Device that requests the nvmem cell.
> > + * @cell_id: nvmem cell name to get.
> > + *
> > + * Return: Will be NULL if no cell with the given name is defined,
> > + * an ERR_PTR() on error or a valid pointer to a struct nvmem_cell.
> > + * The nvmem_cell will be freed by the nvmem_cell_put().
> > + */
> > +struct nvmem_cell *nvmem_cell_get_optional(struct device *dev,
> > +					   const char *cell_id)
> > +{
> > +	struct nvmem_cell *cell;
> > +
> > +	cell = nvmem_cell_get(dev, cell_id);
> > +	if (IS_ERR(cell) && PTR_ERR(cell) == -ENOENT)
> > +		return NULL;  
> 
> What is the real use-case here, it does not make sense to me to add this 
> additional call just to return NULL when cell is not found!

It also return NULL when nvmem is not compiled in. I quiet like such
convenience functions as they make the driver code much simpler and
the intent explicit. It replace:

	data->cell = devm_nvmem_cell_get(dev, "my-cell");
	if (IS_ERR(data->cell) {
		if (PTR_ERR(data->cell) == -ENOENT ||
		    PTR_ERR(data->cell) == -EOPNOTSUPP)
			data->cell = NULL;
		else
			return PTR_ERR(data->cell);
	}

with:

	data->cell = dev_nvmem_cell_get_optional(dev, "my-cell");
	if (IS_ERR(cell))
		return PTR_ERR(data->cell);

It's your call if you find that useful or not.

Alban

Attachment: pgpyVLfl61WqH.pgp
Description: OpenPGP digital signature