Re: [PATCH v5 04/11] nvmem: Add a simple NVMEM framework for consumers

[Srinivas Kandagatla]

On 16/06/15 23:29, Stephen Boyd wrote:
> On 05/21/2015 09:43 AM, Srinivas Kandagatla wrote:
> > @@ -379,6 +380,351 @@ int nvmem_unregister(struct nvmem_device *nvmem)
> [...]
> > +
> > +	return nvmem;
> > +}
> > +
> > +static int __nvmem_device_put(struct nvmem_device *nvmem)
>
> Why does this return int? It's not used anywhere.
I can remove it.
> > +{
> > +	module_put(nvmem->owner);
> > +	mutex_lock(&nvmem_mutex);
> > +	nvmem->users--;
> > +	mutex_unlock(&nvmem_mutex);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
> > +{
> > +	struct nvmem_cell *cell = NULL;
> > +	struct nvmem_device *nvmem;
> > +
> > +	nvmem = __nvmem_device_get(NULL, &cell, cell_id);
> > +	if (IS_ERR(nvmem))
> > +		return (struct nvmem_cell *)nvmem;
>
> ERR_CAST?
Will fix it.
> > +
> > +
> > +	return cell;
> > +
> > +}
> > +
> > +static struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > +					    const char *name)
> > +{
> > +	struct device_node *cell_np, *nvmem_np;
> > +	struct nvmem_cell *cell;
> > +	struct nvmem_device *nvmem;
> > +	const __be32 *addr;
> > +	int rval, len, index;
> > +
> > +	index = of_property_match_string(np, "nvmem-cell-names", name);
> > +
> > +	cell_np = of_parse_phandle(np, "nvmem-cell", index);
> > +	if (!cell_np)
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	nvmem_np = of_get_next_parent(cell_np);
> > +	if (!nvmem_np)
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	nvmem = __nvmem_device_get(nvmem_np, NULL, NULL);
> > +	if (IS_ERR(nvmem))
> > +		return (struct nvmem_cell *)nvmem;
>
> ERR_CAST?
Will fix it.

> > +
> > +	addr = of_get_property(cell_np, "reg", &len);
> > +	if (!addr || (len < 2 * sizeof(int))) {
> > +		dev_err(&nvmem->dev, "of_i2c: invalid reg on %s\n",
>
> huh? of_i2c?
 :-)
Will fix it.
> > +
> > +	/* if it's not end on byte boundary */
> > +	if ((nbits + bit_offset) % BITS_PER_BYTE) {
> > +		/* setup the last byte with msb bits from nvmem */
> > +		rc = regmap_raw_read(nvmem->regmap,
> > +				    cell->offset + cell->bytes - 1, &v, 1);
> > +		*p |= GENMASK(7, (nbits + bit_offset) % BITS_PER_BYTE) & v;
> > +
> > +	}
> > +
> > +	return buf;
> > +}
> > +
> > +/**
> > + * nvmem_cell_write(): Write to a given nvmem cell
>
> This isn't kernel doc notation. It should be like
>
>      nvmem_cell_write - Write to a given nvmem cell
Will fix as suggested by Sascha as

foobar() - short function description of foobar

Looks correct according to Documentation/kernel-doc-nano-HOWTO.txt

> > + *
> > + * @cell: nvmem cell to be written.
> > + * @buf: Buffer to be written.
> > + * @len: length of buffer to be written to nvmem cell.
> > + *
> > + * The return value will be an length of bytes written or non zero on failure.
> > + */
> > +int nvmem_cell_write(struct nvmem_cell *cell, void *buf, ssize_t len)
> [..]
> > +
> >   static int nvmem_init(void)
> >   {
> >   	return class_register(&nvmem_class);
> > diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> > new file mode 100644
> > index 0000000..c3fa8c7
> > --- /dev/null
> > +++ b/include/linux/nvmem-consumer.h
> > @@ -0,0 +1,49 @@
> > +/*
> > + * nvmem framework consumer.
> > + *
> > + * Copyright (C) 2015 Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> > + * Copyright (C) 2013 Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx>
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2.  This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> > + */
> > +
> > +#ifndef _LINUX_NVMEM_CONSUMER_H
> > +#define _LINUX_NVMEM_CONSUMER_H
> > +
> > +/* consumer cookie */
> > +struct nvmem_cell;
> > +
> > +#if IS_ENABLED(CONFIG_NVMEM)
> > +
> > +/* Cell based interface */
> > +struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *name);
>
> We should probably forward declare struct device in this file too.
Yep I will do it.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/