On 24-06-14, Sascha Hauer wrote:
> On Thu, Jun 13, 2024 at 03:15:29PM +0200, Marco Felsch wrote:
> > Expose the nvmem cells via cdevs which is our equivalent to the Linux
> > sysfs exposure. This allows the easier user queries for board code and
> > shell. Keep the Linux function name scheme for
> > nvmem_populate_sysfs_cells() to reduce the diff for nvmem_register()
> > function.
> >
> > Signed-off-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx>
> > ---
> > v2:
> > - fix error handling path during nvmem_register
> > - drop useless list_empty()
> >
> > drivers/nvmem/core.c | 101 +++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 101 insertions(+)
> >
> > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > index ee7d4c6301c3..8e91d9c0fc8a 100644
> > --- a/drivers/nvmem/core.c
> > +++ b/drivers/nvmem/core.c
> > @@ -44,6 +44,8 @@ struct nvmem_cell_entry {
> > struct device_node *np;
> > struct nvmem_device *nvmem;
> > struct list_head node;
> > +
> > + struct cdev cdev;
> > };
> >
> > struct nvmem_cell {
> > @@ -144,6 +146,101 @@ static struct nvmem_device *of_nvmem_find(struct device_node *nvmem_np)
> > return NULL;
> > }
> >
> > +static struct nvmem_cell *nvmem_create_cell(struct nvmem_cell_entry *entry,
> > + const char *id, int index);
> > +
> > +static ssize_t nvmem_cell_cdev_read(struct cdev *cdev, void *buf, size_t count,
> > + loff_t offset, unsigned long flags)
> > +{
> > + struct nvmem_cell_entry *entry;
> > + struct nvmem_cell *cell = NULL;
> > + size_t cell_sz, read_len;
> > + void *content;
> > +
> > + entry = container_of(cdev, struct nvmem_cell_entry, cdev);
> > + cell = nvmem_create_cell(entry, entry->name, 0);
> > + if (IS_ERR(cell))
> > + return PTR_ERR(cell);
> > +
> > + content = nvmem_cell_read(cell, &cell_sz);
> > + if (IS_ERR(content)) {
> > + read_len = PTR_ERR(content);
> > + goto destroy_cell;
> > + }
> > +
> > + read_len = min_t(unsigned int, cell_sz - offset, count);
> > + memcpy(buf, content + offset, read_len);
> > + kfree(content);
> > +
> > +destroy_cell:
> > + kfree_const(cell->id);
> > + kfree(cell);
> > +
> > + return read_len;
> > +}
> > +
> > +static ssize_t nvmem_cell_cdev_write(struct cdev *cdev, const void *buf, size_t count,
> > + loff_t offset, unsigned long flags)
> > +{
> > + struct nvmem_cell_entry *entry;
> > + struct nvmem_cell *cell;
> > + int ret;
> > +
> > + entry = container_of(cdev, struct nvmem_cell_entry, cdev);
> > +
> > + if (!entry->nvmem->reg_write)
> > + return -EPERM;
> > +
> > + if (offset >= entry->bytes)
> > + return -EFBIG;
> > +
> > + if (offset + count > entry->bytes)
> > + count = entry->bytes - offset;
> > +
> > + cell = nvmem_create_cell(entry, entry->name, 0);
> > + if (IS_ERR(cell))
> > + return PTR_ERR(cell);
> > +
> > + if (!cell)
> > + return -EINVAL;
> > +
> > + ret = nvmem_cell_write(cell, buf, count);
> > +
> > + kfree_const(cell->id);
> > + kfree(cell);
> > +
> > + return ret;
> > +}
> > +
> > +static struct cdev_operations nvmem_cell_chrdev_ops = {
> > + .read = nvmem_cell_cdev_read,
> > + .write = nvmem_cell_cdev_write,
> > +};
> > +
> > +static int nvmem_populate_sysfs_cells(struct nvmem_device *nvmem)
> > +{
> > + struct device *dev = &nvmem->dev;
> > + struct nvmem_cell_entry *entry;
> > +
> > + list_for_each_entry(entry, &nvmem->cells, node) {
> > + struct cdev *cdev;
> > + int ret;
> > +
> > + cdev = &entry->cdev;
> > + cdev->name = xasprintf("%s.%s", dev_name(dev),
> > + kbasename(entry->name));
> > + cdev->ops = &nvmem_cell_chrdev_ops;
> > + cdev->dev = dev;
> > + cdev->size = entry->bytes;
> > +
> > + ret = devfs_create(cdev);
> > + if (ret)
> > + return ret;
>
> You might fail here without unregistering the already registered cells.
Argh.. you're right. I will give it a 3th spin. Thanks for the review.
Regards,
Marco
> Furthermore you free the nvmem device which likely leads to memory
> corruptions further down. Your choices are:
>
> - Do not fail here, but only print a warning when devfs_create() fails,
> or:
> - properly unregister what you registered beforehand.
>
> Sascha
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
>
