Messages in this thread | | | Subject | Re: [PATCH v3 3/4] nvmem: core: Add support for keepout regions | From | Srinivas Kandagatla <> | Date | Thu, 29 Oct 2020 12:08:38 +0000 |
| |
Thanks Evan for doing this,
On 29/10/2020 00:28, Evan Green wrote: > Introduce support into the nvmem core for arrays of register ranges > that should not result in actual device access. For these regions a > constant byte (repeated) is returned instead on read, and writes are > quietly ignored and returned as successful. > > This is useful for instance if certain efuse regions are protected > from access by Linux because they contain secret info to another part > of the system (like an integrated modem). > > Signed-off-by: Evan Green <evgreen@chromium.org>
Overall the patch looks good for me. I have applied just this patch for more testing in next!
I can pick up 1/4 and 4/4 once Rob's Ack/Reviews the patch!
thanks, srini > --- > > Changes in v3: > - Use min()/max() macros instead of defining my own (Doug) > - Comment changes to indicate sorting (Doug) > - Add function to validate keepouts are proper (Doug) > > Changes in v2: > - Introduced keepout regions into the core (Srini) > > drivers/nvmem/core.c | 153 ++++++++++++++++++++++++++++++++- > include/linux/nvmem-provider.h | 17 ++++ > 2 files changed, 166 insertions(+), 4 deletions(-) > > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c > index a09ff8409f600..177f5bf27c6d5 100644 > --- a/drivers/nvmem/core.c > +++ b/drivers/nvmem/core.c > @@ -34,6 +34,8 @@ struct nvmem_device { > struct bin_attribute eeprom; > struct device *base_dev; > struct list_head cells; > + const struct nvmem_keepout *keepout; > + unsigned int nkeepout; > nvmem_reg_read_t reg_read; > nvmem_reg_write_t reg_write; > struct gpio_desc *wp_gpio; > @@ -66,8 +68,8 @@ static LIST_HEAD(nvmem_lookup_list); > > static BLOCKING_NOTIFIER_HEAD(nvmem_notifier); > > -static int nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset, > - void *val, size_t bytes) > +static int __nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset, > + void *val, size_t bytes) > { > if (nvmem->reg_read) > return nvmem->reg_read(nvmem->priv, offset, val, bytes); > @@ -75,8 +77,8 @@ static int nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset, > return -EINVAL; > } > > -static int nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset, > - void *val, size_t bytes) > +static int __nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset, > + void *val, size_t bytes) > { > int ret; > > @@ -90,6 +92,88 @@ static int nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset, > return -EINVAL; > } > > +static int nvmem_access_with_keepouts(struct nvmem_device *nvmem, > + unsigned int offset, void *val, > + size_t bytes, int write) > +{ > + > + unsigned int end = offset + bytes; > + unsigned int kend, ksize; > + const struct nvmem_keepout *keepout = nvmem->keepout; > + const struct nvmem_keepout *keepoutend = keepout + nvmem->nkeepout; > + int rc; > + > + /* > + * Skip all keepouts before the range being accessed. > + * Keepouts are sorted. > + */ > + while ((keepout < keepoutend) && (keepout->end <= offset)) > + keepout++; > + > + while ((offset < end) && (keepout < keepoutend)) { > + /* Access the valid portion before the keepout. */ > + if (offset < keepout->start) { > + kend = min(end, keepout->start); > + ksize = kend - offset; > + if (write) > + rc = __nvmem_reg_write(nvmem, offset, val, ksize); > + else > + rc = __nvmem_reg_read(nvmem, offset, val, ksize); > + > + if (rc) > + return rc; > + > + offset += ksize; > + val += ksize; > + } > + > + /* > + * Now we're aligned to the start of this keepout zone. Go > + * through it. > + */ > + kend = min(end, keepout->end); > + ksize = kend - offset; > + if (!write) > + memset(val, keepout->value, ksize); > + > + val += ksize; > + offset += ksize; > + keepout++; > + } > + > + /* > + * If we ran out of keepouts but there's still stuff to do, send it > + * down directly > + */ > + if (offset < end) { > + ksize = end - offset; > + if (write) > + return __nvmem_reg_write(nvmem, offset, val, ksize); > + else > + return __nvmem_reg_read(nvmem, offset, val, ksize); > + } > + > + return 0; > +} > + > +static int nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset, > + void *val, size_t bytes) > +{ > + if (!nvmem->nkeepout) > + return __nvmem_reg_read(nvmem, offset, val, bytes); > + > + return nvmem_access_with_keepouts(nvmem, offset, val, bytes, false); > +} > + > +static int nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset, > + void *val, size_t bytes) > +{ > + if (!nvmem->nkeepout) > + return __nvmem_reg_write(nvmem, offset, val, bytes); > + > + return nvmem_access_with_keepouts(nvmem, offset, val, bytes, true); > +} > + > #ifdef CONFIG_NVMEM_SYSFS > static const char * const nvmem_type_str[] = { > [NVMEM_TYPE_UNKNOWN] = "Unknown", > @@ -533,6 +617,59 @@ nvmem_find_cell_by_name(struct nvmem_device *nvmem, const char *cell_id) > return cell; > } > > +static int nvmem_validate_keepouts(struct nvmem_device *nvmem) > +{ > + unsigned int cur = 0; > + const struct nvmem_keepout *keepout = nvmem->keepout; > + const struct nvmem_keepout *keepoutend = keepout + nvmem->nkeepout; > + > + while (keepout < keepoutend) { > + /* Ensure keepouts are sorted and don't overlap. */ > + if (keepout->start < cur) { > + dev_err(&nvmem->dev, > + "Keepout regions aren't sorted or overlap.\n"); > + > + return -ERANGE; > + } > + > + if (keepout->end < keepout->start) { > + dev_err(&nvmem->dev, > + "Invalid keepout region.\n"); > + > + return -EINVAL; > + } > + > + /* > + * Validate keepouts (and holes between) don't violate > + * word_size constraints. > + */ > + if ((keepout->end - keepout->start < nvmem->word_size) || > + ((keepout->start != cur) && > + (keepout->start - cur < nvmem->word_size))) { > + > + dev_err(&nvmem->dev, > + "Keepout regions violate word_size constraints.\n"); > + > + return -ERANGE; > + } > + > + /* Validate keepouts don't violate stride (alignment). */ > + if (!IS_ALIGNED(keepout->start, nvmem->stride) || > + !IS_ALIGNED(keepout->end, nvmem->stride)) { > + > + dev_err(&nvmem->dev, > + "Keepout regions violate stride.\n"); > + > + return -EINVAL; > + } > + > + cur = keepout->end; > + keepout++; > + } > + > + return 0; > +} > + > static int nvmem_add_cells_from_of(struct nvmem_device *nvmem) > { > struct device_node *parent, *child; > @@ -647,6 +784,8 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) > nvmem->type = config->type; > nvmem->reg_read = config->reg_read; > nvmem->reg_write = config->reg_write; > + nvmem->keepout = config->keepout; > + nvmem->nkeepout = config->nkeepout; > if (!config->no_of_node) > nvmem->dev.of_node = config->dev->of_node; > > @@ -671,6 +810,12 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) > nvmem->dev.groups = nvmem_dev_groups; > #endif > > + if (nvmem->nkeepout) { > + rval = nvmem_validate_keepouts(nvmem); > + if (rval) > + goto err_put_device; > + } > + > dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name); > > rval = device_register(&nvmem->dev); > diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h > index 06409a6c40bcb..e162b757b6d54 100644 > --- a/include/linux/nvmem-provider.h > +++ b/include/linux/nvmem-provider.h > @@ -30,6 +30,19 @@ enum nvmem_type { > #define NVMEM_DEVID_NONE (-1) > #define NVMEM_DEVID_AUTO (-2) > > +/** > + * struct nvmem_keepout - NVMEM register keepout range. > + * > + * @start: The first byte offset to avoid. > + * @end: One beyond the last byte offset to avoid. > + * @value: The byte to fill reads with for this region. > + */ > +struct nvmem_keepout { > + unsigned int start; > + unsigned int end; > + unsigned char value; > +}; > + > /** > * struct nvmem_config - NVMEM device configuration > * > @@ -39,6 +52,8 @@ enum nvmem_type { > * @owner: Pointer to exporter module. Used for refcounting. > * @cells: Optional array of pre-defined NVMEM cells. > * @ncells: Number of elements in cells. > + * @keepout: Optional array of keepout ranges (sorted ascending by start). > + * @nkeepout: Number of elements in the keepout array. > * @type: Type of the nvmem storage > * @read_only: Device is read-only. > * @root_only: Device is accessibly to root only. > @@ -66,6 +81,8 @@ struct nvmem_config { > struct gpio_desc *wp_gpio; > const struct nvmem_cell_info *cells; > int ncells; > + const struct nvmem_keepout *keepout; > + unsigned int nkeepout; > enum nvmem_type type; > bool read_only; > bool root_only; >
| |