Re: [PATCH v12 7/7] nvmem: core: Expose cells through sysfs

Srinivas Kandagatla
2023-10-09T09:48:24Z

On 05/10/2023 16:59, Miquel Raynal wrote:
> , int index);
> +
> +static ssize_t nvmem_cell_attr_read(struct file *filp, struct kobject *kobj,
> + struct bin_attribute *attr, char *buf,
> + loff_t pos, size_t count)
> +{
> + struct nvmem_cell_entry *entry;
> + struct nvmem_cell *cell = NULL;
> + size_t cell_sz, read_len;
> + void *content;
> +
> + entry = attr->private;
> + cell = nvmem_create_cell(entry, entry->name, 0);
> + if (IS_ERR(cell))
> + return PTR_ERR(cell);
> +
> + if (!cell)
> + return -EINVAL;
> +
> + 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 - pos, count);
> + memcpy(buf, content + pos, read_len);
> + kfree(content);
> +
> +destroy_cell:
> + kfree_const(cell->id);
> + kfree(cell);
> +
> + return read_len;
> +}
> +
> /* default read/write permissions */
> static struct bin_attribute bin_attr_rw_nvmem = {
> .attr = {
> @@ -320,11 +357,21 @@ static const struct attribute_group nvmem_bin_group = {
> .is_bin_visible = nvmem_bin_attr_is_visible,
> };
>
> +/* Cell attributes will be dynamically allocated */
> +static struct attribute_group nvmem_cells_group = {
> + .name = "cells",
> +};
> +
> static const struct attribute_group *nvmem_dev_groups[] = {
> &nvmem_bin_group,
> NULL,
> };
>
> +static const struct attribute_group *nvmem_cells_groups[] = {
> + &nvmem_cells_group,
> + NULL,
> +};
> +
> static struct bin_attribute bin_attr_nvmem_eeprom_compat = {
> .attr = {
> .name = "eeprom",
> @@ -380,6 +427,68 @@ static void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
> device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
> }
>
> +static int nvmem_populate_sysfs_cells(struct nvmem_device *nvmem)
> +{
> + struct bin_attribute **cells_attrs, *attrs;
> + struct nvmem_cell_entry *entry;
> + unsigned int ncells = 0, i = 0;
> + int ret = 0;
> +
> + mutex_lock(&nvmem_mutex);
> +
> + if (list_empty(&nvmem->cells) || nvmem->sysfs_cells_populated) {
> + nvmem_cells_group.bin_attrs = NULL;
> + goto unlock_mutex;
> + }
> +
> + /* Allocate an array of attributes with a sentinel */
> + ncells = list_count_nodes(&nvmem->cells);
> + cells_attrs = devm_kcalloc(&nvmem->dev, ncells + 1,
> + sizeof(struct bin_attribute *), GFP_KERNEL);
> + if (!cells_attrs) {
> + ret = -ENOMEM;
> + goto unlock_mutex;
> + }
> +
> + attrs = devm_kcalloc(&nvmem->dev, ncells, sizeof(struct bin_attribute), GFP_KERNEL);
> + if (!attrs) {
> + ret = -ENOMEM;
> + goto unlock_mutex;
> + }
> +
> + /* Initialize each attribute to take the name and size of the cell */
> + list_for_each_entry(entry, &nvmem->cells, node) {
> + sysfs_bin_attr_init(&attrs[i]);
> + attrs[i].attr.name = devm_kasprintf(&nvmem->dev, GFP_KERNEL,
> + "%s@%x", entry->name,
> + entry->offset);
> + attrs[i].attr.mode = 0444;
> + attrs[i].size = entry->bytes;
> + attrs[i].read = &nvmem_cell_attr_read;
> + attrs[i].private = entry;
> + if (!attrs[i].attr.name) {
> + ret = -ENOMEM;
> + goto unlock_mutex;
> + }
> +
> + cells_attrs[i] = &attrs[i];
> + i++;
> + }
> +
> + nvmem_cells_group.bin_attrs = cells_attrs;
> +
> + ret = devm_device_add_groups(&nvmem->dev, nvmem_cells_groups);
> + if (ret)
> + goto unlock_mutex;

This is going to create groups after the nvmem device is added, isn't
this going to be problem with user space notifications?

--srini
> +
> + nvmem->sysfs_cells_populated = true;
> +
> +unlock_mutex:
> + mutex_unlock(&nvmem_mutex);
> +
> + return ret;