| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <566FE5BD.4040101@linaro.org>
Date: Tue, 15 Dec 2015 10:04:45 +0000
From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To: Andrew Lunn <andrew@...n.ch>, GregKH <greg@...ah.com>,
maxime.ripard@...e-electrons.com, wsa@...-dreams.de,
broonie@...nel.org, vz@...ia.com
CC: afd@...com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/6] nvmem: Add backwards compatibility support for older
EEPROM drivers.
Below are few comments.
On 08/12/15 14:05, Andrew Lunn wrote:
> Older drivers made an 'eeprom' file available in the /sys device
> directory. Have the NVMEM core provide this to retain backwards
> compatibility.
>
> Signed-off-by: Andrew Lunn <andrew@...n.ch>
> ---
> drivers/nvmem/Kconfig | 7 ++++
> drivers/nvmem/core.c | 75 +++++++++++++++++++++++++++++++++++++++---
> include/linux/nvmem-provider.h | 10 ++++++
> 3 files changed, 88 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index bc4ea585b42e..b4e79ba7d502 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -13,6 +13,13 @@ menuconfig NVMEM
> If unsure, say no.
>
> if NVMEM
> +config NVMEM_COMPAT
> + bool "Enable /sys compatibility with old eeprom drivers"
> + help
> + Older EEPROM drivers, such as AT24, AT25, provide access to
> + the eeprom via a file called "eeprom" in /sys under the
> + device node. Enabling this option makes the NVMEM core
> + provide this file to retain backwards compatibility
>
Lets get rid of this Kconfig as Wolfram suggested.
We are already adding NVMEM_COMPAT in the nvmem_device structrure lets
move the flags into the struct nvmem_config and use the nvmem_register
api as it is.
nvmem_register() can decide what do do with that from there.
I would also prefer a warning if this flag is set, this is to deter any
new users.
Let me know your thoughts?
> config NVMEM_IMX_OCOTP
> tristate "i.MX6 On-Chip OTP Controller support"
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 4ccf03da6467..75a498f5e139 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -38,8 +38,13 @@ struct nvmem_device {
> int users;
> size_t size;
> bool read_only;
> + int flags;
> + struct bin_attribute eeprom;
> + struct device *base_dev;
> };
>
> +#define FLAG_COMPAT BIT(0)
> +
> struct nvmem_cell {
> const char *name;
> int offset;
> @@ -62,10 +67,16 @@ static ssize_t bin_attr_nvmem_read(struct file *filp, struct kobject *kobj,
> struct bin_attribute *attr,
> char *buf, loff_t pos, size_t count)
> {
> - struct device *dev = container_of(kobj, struct device, kobj);
> - struct nvmem_device *nvmem = to_nvmem_device(dev);
> + struct device *dev;
> + struct nvmem_device *nvmem;
> int rc;
>
> + if (attr->private)
> + dev = attr->private;
> + else
> + dev = container_of(kobj, struct device, kobj);
> + nvmem = to_nvmem_device(dev);
> +
> /* Stop the user from reading */
> if (pos >= nvmem->size)
> return 0;
> @@ -87,10 +98,16 @@ static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj,
> struct bin_attribute *attr,
> char *buf, loff_t pos, size_t count)
> {
> - struct device *dev = container_of(kobj, struct device, kobj);
> - struct nvmem_device *nvmem = to_nvmem_device(dev);
> + struct device *dev;
> + struct nvmem_device *nvmem;
> int rc;
>
> + if (attr->private)
> + dev = attr->private;
> + else
> + dev = container_of(kobj, struct device, kobj);
> + nvmem = to_nvmem_device(dev);
> +
> /* Stop the user from writing */
> if (pos >= nvmem->size)
> return 0;
> @@ -421,6 +438,53 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
> }
> EXPORT_SYMBOL_GPL(nvmem_register);
>
> +#if IS_ENABLED(CONFIG_NVMEM_COMPAT)
> +/**
> + * nvmem_register_compat() - Register a nvmem device for given nvmem_config.
> + * Also creates an binary entry in /sys/bus/nvmem/devices/dev-name/nvmem and
> + * an eeprom file in the drivers sys directory.
> + *
> + * @config: nvmem device configuration with which nvmem device is created.
> + * @dev: device structure of underlying device
> + *
> + * Return: Will be an ERR_PTR() on error or a valid pointer to nvmem_device
> + * on success.
> + */
> +
> +struct nvmem_device *nvmem_register_compat(const struct nvmem_config *config,
> + struct device *base_dev)
> +{
Possibly move most of it or some of it a local static function which
will be called from nvmem_register depending on the NVMEM_FLAG_COMPAT.
> + struct nvmem_device *nvmem;
> + int rval;
> +
> + nvmem = nvmem_register(config);
> + if (IS_ERR(nvmem))
> + return nvmem;
> +
> + if (nvmem->read_only)
> + nvmem->eeprom = bin_attr_ro_root_nvmem;
> + else
> + nvmem->eeprom = bin_attr_rw_root_nvmem;
> + nvmem->eeprom.attr.name = "eeprom";
> + nvmem->eeprom.size = nvmem->size;
> + nvmem->eeprom.private = &nvmem->dev;
> + nvmem->base_dev = base_dev;
> +
> + rval = device_create_bin_file(nvmem->base_dev, &nvmem->eeprom);
> + if (rval) {
> + dev_err(&nvmem->dev,
> + "Failed to create eeprom binary file %d\n", rval);
> + nvmem_unregister(nvmem);
> + return ERR_PTR(rval);
> + }
> +
> + nvmem->flags |= FLAG_COMPAT;
> +
> + return nvmem;
> +}
> +EXPORT_SYMBOL_GPL(nvmem_register_compat);
> +#endif /* CONFIG_NVMEM_COMPAT */
> +
> /**
> * nvmem_unregister() - Unregister previously registered nvmem device
> *
> @@ -437,6 +501,9 @@ int nvmem_unregister(struct nvmem_device *nvmem)
> }
> mutex_unlock(&nvmem_mutex);
>
> + if (nvmem->flags & FLAG_COMPAT)
> + device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
> +
> nvmem_device_remove_all_cells(nvmem);
> device_del(&nvmem->dev);
>
> diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
> index d24fefa0c11d..012030bd4495 100644
> --- a/include/linux/nvmem-provider.h
> +++ b/include/linux/nvmem-provider.h
> @@ -45,4 +45,14 @@ static inline int nvmem_unregister(struct nvmem_device *nvmem)
>
> #endif /* CONFIG_NVMEM */
>
> +#if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_NVMEM_COMPAT)
> +struct nvmem_device *nvmem_register_compat(const struct nvmem_config *config,
> + struct device *base_dev);
> +#else
> +static inline struct nvmem_device *
> +nvmem_register_compat(const struct nvmem_config *c, struct device *base_dev)
> +{
> + return ERR_PTR(-ENOSYS);
> +}
> +#endif /* CONFIG_NVMEM && CONFIG_NVMEM_COMPAT */
> #endif /* ifndef _LINUX_NVMEM_PROVIDER_H */
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists