From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965260AbcBQKUK (ORCPT ); Wed, 17 Feb 2016 05:20:10 -0500 Received: from mail-wm0-f51.google.com ([74.125.82.51]:35374 "EHLO mail-wm0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933984AbcBQKRh (ORCPT ); Wed, 17 Feb 2016 05:17:37 -0500 Subject: Re: [PATCHv4 2/7] nvmem: Add backwards compatibility support for older EEPROM drivers. To: Andrew Lunn , GregKH References: <1455666097-9115-1-git-send-email-andrew@lunn.ch> <1455666097-9115-3-git-send-email-andrew@lunn.ch> Cc: maxime.ripard@free-electrons.com, wsa@the-dreams.de, broonie@kernel.org, vz@mleia.com, fd@ti.com, linux-kernel@vger.kernel.org, pantelis.antoniou@konsulko.com, bgolaszewski@baylibre.com From: Srinivas Kandagatla Message-ID: <56C448BE.30303@linaro.org> Date: Wed, 17 Feb 2016 10:17:34 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <1455666097-9115-3-git-send-email-andrew@lunn.ch> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 16/02/16 23:41, 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 > --- > v4: Add lockdep support > --- > drivers/nvmem/core.c | 84 ++++++++++++++++++++++++++++++++++++++---- > include/linux/nvmem-provider.h | 4 +- > 2 files changed, 79 insertions(+), 9 deletions(-) > > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c > index 4ccf03da6467..9ad1c2cf75ac 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; Any reason why should this base_dev be any different to the dev in the nvmem_config? Should we just not reuse dev? unless am missing something obvious. Other parts of the patch looks good to me. > }; > > +#define FLAG_COMPAT BIT(0) > + > struct nvmem_cell { > const char *name; > int offset; > @@ -56,16 +61,26 @@ static DEFINE_IDA(nvmem_ida); > static LIST_HEAD(nvmem_cells); > static DEFINE_MUTEX(nvmem_cells_mutex); > > +#ifdef CONFIG_DEBUG_LOCK_ALLOC > +static struct lock_class_key eeprom_lock_key; > +#endif > + > #define to_nvmem_device(d) container_of(d, struct nvmem_device, dev) > > 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 +102,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; > @@ -341,6 +362,43 @@ err: > return rval; > } > > +/* > + * nvmem_setup_compat() - Create an additional binary entry in > + * drivers sys directory, to be backwards compatible with the older > + * drivers/misc/eeprom drivers. > + */ > +static int nvmem_setup_compat(struct nvmem_device *nvmem, > + const struct nvmem_config *config) > +{ > + int rval; > + > + if (!config->base_dev) > + return -EINVAL; > + > + 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; > +#ifdef CONFIG_DEBUG_LOCK_ALLOC > + nvmem->eeprom.attr.key = &eeprom_lock_key; > +#endif > + nvmem->eeprom.private = &nvmem->dev; > + nvmem->base_dev = config->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); > + return rval; > + } > + > + nvmem->flags |= FLAG_COMPAT; > + > + return 0; > +} > + > /** > * nvmem_register() - Register a nvmem device for given nvmem_config. > * Also creates an binary entry in /sys/bus/nvmem/devices/dev-name/nvmem > @@ -408,16 +466,23 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) > dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name); > > rval = device_add(&nvmem->dev); > - if (rval) { > - ida_simple_remove(&nvmem_ida, nvmem->id); > - kfree(nvmem); > - return ERR_PTR(rval); > + if (rval) > + goto out; > + > + if (config->compat) { > + rval = nvmem_setup_compat(nvmem, config); > + if (rval) > + goto out; > } > > if (config->cells) > nvmem_add_cells(nvmem, config); > > return nvmem; > +out: > + ida_simple_remove(&nvmem_ida, nvmem->id); > + kfree(nvmem); > + return ERR_PTR(rval); > } > EXPORT_SYMBOL_GPL(nvmem_register); > > @@ -437,6 +502,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..a4fcc90b0f20 100644 > --- a/include/linux/nvmem-provider.h > +++ b/include/linux/nvmem-provider.h > @@ -24,6 +24,9 @@ struct nvmem_config { > int ncells; > bool read_only; > bool root_only; > + /* To be only used by old driver/misc/eeprom drivers */ > + bool compat; > + struct device *base_dev; > }; > > #if IS_ENABLED(CONFIG_NVMEM) > @@ -44,5 +47,4 @@ static inline int nvmem_unregister(struct nvmem_device *nvmem) > } > > #endif /* CONFIG_NVMEM */ > - > #endif /* ifndef _LINUX_NVMEM_PROVIDER_H */ >