[PATCH v3 1/4] nvmem: Replace spaces with tab in documentation

[PATCH v3 1/4] nvmem: Replace spaces with tab in documentation

[Marek Vasut]

Replace two spaces with tab in the sysfs attribute documentation.
No functional change.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: linux-i2c@vger.kernel.org
---
V2: - New patch
V3: - No change
---
 Documentation/ABI/stable/sysfs-bus-nvmem | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem b/Documentation/ABI/stable/sysfs-bus-nvmem
index c399323f37de3..854bd11d72ac4 100644
--- a/Documentation/ABI/stable/sysfs-bus-nvmem
+++ b/Documentation/ABI/stable/sysfs-bus-nvmem
@@ -1,6 +1,6 @@
 What:		/sys/bus/nvmem/devices/.../nvmem
 Date:		July 2015
-KernelVersion:  4.2
+KernelVersion:	4.2
 Contact:	Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
 Description:
 		This file allows user to read/write the raw NVMEM contents.
-- 
2.43.0

[PATCH v3 2/4] nvmem: Document type attribute

[Marek Vasut]

Document a type attribute used by userspace to discern different types of
NVMEM devices. The implementation is already present, the ABI document is
missing, add it.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: linux-i2c@vger.kernel.org
---
V2: - New patch
V3: - Add RB from Alexandre
    - Add trailing full stop to the paragraph
---
 Documentation/ABI/stable/sysfs-bus-nvmem | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem b/Documentation/ABI/stable/sysfs-bus-nvmem
index 854bd11d72ac4..3f0a95250aa84 100644
--- a/Documentation/ABI/stable/sysfs-bus-nvmem
+++ b/Documentation/ABI/stable/sysfs-bus-nvmem
@@ -20,3 +20,14 @@ Description:
 		  ...
 		  *
 		  0001000
+
+What:		/sys/bus/nvmem/devices/.../type
+Date:		November 2018
+KernelVersion:	5.0
+Contact:	Alexandre Belloni <alexandre.belloni@bootlin.com>
+Description:
+		This read-only attribute allows user to read the NVMEM
+		device type. Supported types are "Unknown", "EEPROM",
+		"OTP", "Battery backed", "FRAM".
+		Note: This file is only present if CONFIG_NVMEM_SYSFS
+		is enabled.
-- 
2.43.0

[PATCH v3 3/4] nvmem: Use sysfs_emit() for type attribute

[Marek Vasut]

Use sysfs_emit() instead of sprintf() to follow best practice per
Documentation/filesystems/sysfs.rst
"
show() should only use sysfs_emit()...
"

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: linux-i2c@vger.kernel.org
---
V2: - New patch
V3: - No change
---
 drivers/nvmem/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index a31ea4275d5ae..5b3606114628b 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -179,7 +179,7 @@ static ssize_t type_show(struct device *dev,
 {
 	struct nvmem_device *nvmem = to_nvmem_device(dev);
 
-	return sprintf(buf, "%s\n", nvmem_type_str[nvmem->type]);
+	return sysfs_emit(buf, "%s\n", nvmem_type_str[nvmem->type]);
 }
 
 static DEVICE_ATTR_RO(type);
-- 
2.43.0

[PATCH v3 4/4] nvmem: core: Implement force_ro sysfs attribute

[Marek Vasut]

Implement "force_ro" sysfs attribute to allow users to set read-write
devices as read-only and back to read-write from userspace. The choice
of the name is based on MMC core 'force_ro' attribute.

This solves a situation where an AT24 I2C EEPROM with GPIO based nWP
signal may have to be occasionally updated. Such I2C EEPROM device is
usually set as read-only during most of the regular system operation,
but in case it has to be updated in a controlled manner, it could be
unlocked using this new "force_ro" sysfs attribute and then re-locked
again.

The "read-only" DT property and config->read_only configuration is
respected and is used to set default state of the device, read-only
or read-write, for devices which do implement .reg_write function.
For devices which do not implement .reg_write function, the device
is unconditionally read-only and the "force_ro" attribute is not
visible.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: linux-i2c@vger.kernel.org
---
V2: - Document the force_ro attribute
    - Use sysfs_emit()
V3: - Expand ABI documentation with what the force_ro attribute
      returns and what it can parse.
---
 Documentation/ABI/stable/sysfs-bus-nvmem | 17 +++++++++
 drivers/nvmem/core.c                     | 44 ++++++++++++++++++++++++
 drivers/nvmem/internals.h                |  1 +
 3 files changed, 62 insertions(+)

diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem b/Documentation/ABI/stable/sysfs-bus-nvmem
index 3f0a95250aa84..aa89adf18bc55 100644
--- a/Documentation/ABI/stable/sysfs-bus-nvmem
+++ b/Documentation/ABI/stable/sysfs-bus-nvmem
@@ -1,3 +1,20 @@
+What:		/sys/bus/nvmem/devices/.../force_ro
+Date:		June 2024
+KernelVersion:	6.11
+Contact:	Marek Vasut <marex@denx.de>
+Description:
+		This read/write attribute allows users to set read-write
+		devices as read-only and back to read-write from userspace.
+		This can be used to unlock and relock write-protection of
+		devices which are generally locked, except during sporadic
+		programming operation.
+		Read returns '0' or '1' for read-write or read-only modes
+		respectively.
+		Write parses one of 'YyTt1NnFf0', or [oO][NnFf] for "on"
+		and "off", i.e. what kstrbool() supports.
+		Note: This file is only present if CONFIG_NVMEM_SYSFS
+		is enabled.
+
 What:		/sys/bus/nvmem/devices/.../nvmem
 Date:		July 2015
 KernelVersion:	4.2
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 5b3606114628b..91ff5e2ce613c 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -184,7 +184,30 @@ static ssize_t type_show(struct device *dev,
 
 static DEVICE_ATTR_RO(type);
 
+static ssize_t force_ro_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	struct nvmem_device *nvmem = to_nvmem_device(dev);
+
+	return sysfs_emit(buf, "%d\n", nvmem->read_only);
+}
+
+static ssize_t force_ro_store(struct device *dev, struct device_attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct nvmem_device *nvmem = to_nvmem_device(dev);
+	int ret = kstrtobool(buf, &nvmem->read_only);
+
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(force_ro);
+
 static struct attribute *nvmem_attrs[] = {
+	&dev_attr_force_ro.attr,
 	&dev_attr_type.attr,
 	NULL,
 };
@@ -285,6 +308,25 @@ static umode_t nvmem_bin_attr_is_visible(struct kobject *kobj,
 	return nvmem_bin_attr_get_umode(nvmem);
 }
 
+static umode_t nvmem_attr_is_visible(struct kobject *kobj,
+				     struct attribute *attr, int i)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct nvmem_device *nvmem = to_nvmem_device(dev);
+
+	/*
+	 * If the device has no .reg_write operation, do not allow
+	 * configuration as read-write.
+	 * If the device is set as read-only by configuration, it
+	 * can be forced into read-write mode using the 'force_ro'
+	 * attribute.
+	 */
+	if (attr == &dev_attr_force_ro.attr && !nvmem->reg_write)
+		return 0;	/* Attribute not visible */
+
+	return attr->mode;
+}
+
 static struct nvmem_cell *nvmem_create_cell(struct nvmem_cell_entry *entry,
 					    const char *id, int index);
 
@@ -341,6 +383,7 @@ static const struct attribute_group nvmem_bin_group = {
 	.bin_attrs	= nvmem_bin_attributes,
 	.attrs		= nvmem_attrs,
 	.is_bin_visible = nvmem_bin_attr_is_visible,
+	.is_visible	= nvmem_attr_is_visible,
 };
 
 static const struct attribute_group *nvmem_dev_groups[] = {
@@ -945,6 +988,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 
 	nvmem->read_only = device_property_present(config->dev, "read-only") ||
 			   config->read_only || !nvmem->reg_write;
+	nvmem->default_read_only = nvmem->read_only;
 
 #ifdef CONFIG_NVMEM_SYSFS
 	nvmem->dev.groups = nvmem_dev_groups;
diff --git a/drivers/nvmem/internals.h b/drivers/nvmem/internals.h
index 18fed57270e5e..0667937ebb86b 100644
--- a/drivers/nvmem/internals.h
+++ b/drivers/nvmem/internals.h
@@ -16,6 +16,7 @@ struct nvmem_device {
 	int			id;
 	struct kref		refcnt;
 	size_t			size;
+	bool			default_read_only;
 	bool			read_only;
 	bool			root_only;
 	int			flags;
-- 
2.43.0

Re: [PATCH v3 4/4] nvmem: core: Implement force_ro sysfs attribute

[Srinivas Kandagatla]

On 26/06/2024 05:11, Marek Vasut wrote:
>   
>   static const struct attribute_group *nvmem_dev_groups[] = {
> @@ -945,6 +988,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>   
>   	nvmem->read_only = device_property_present(config->dev, "read-only") ||
>   			   config->read_only || !nvmem->reg_write;
> +	nvmem->default_read_only = nvmem->read_only;
>   
>   #ifdef CONFIG_NVMEM_SYSFS
>   	nvmem->dev.groups = nvmem_dev_groups;
> diff --git a/drivers/nvmem/internals.h b/drivers/nvmem/internals.h
> index 18fed57270e5e..0667937ebb86b 100644
> --- a/drivers/nvmem/internals.h
> +++ b/drivers/nvmem/internals.h
> @@ -16,6 +16,7 @@ struct nvmem_device {
>   	int			id;
>   	struct kref		refcnt;
>   	size_t			size;
> +	bool			default_read_only;


Its not very clear what is the need for this?


--srini
>   	bool			read_only;
>   	bool			root_only;
>   	int			flags;

Re: [PATCH v3 4/4] nvmem: core: Implement force_ro sysfs attribute

[Marek Vasut]

On 6/26/24 3:07 PM, Srinivas Kandagatla wrote:
> 
> 
> On 26/06/2024 05:11, Marek Vasut wrote:
>>   static const struct attribute_group *nvmem_dev_groups[] = {
>> @@ -945,6 +988,7 @@ struct nvmem_device *nvmem_register(const struct 
>> nvmem_config *config)
>>       nvmem->read_only = device_property_present(config->dev, 
>> "read-only") ||
>>                  config->read_only || !nvmem->reg_write;
>> +    nvmem->default_read_only = nvmem->read_only;
>>   #ifdef CONFIG_NVMEM_SYSFS
>>       nvmem->dev.groups = nvmem_dev_groups;
>> diff --git a/drivers/nvmem/internals.h b/drivers/nvmem/internals.h
>> index 18fed57270e5e..0667937ebb86b 100644
>> --- a/drivers/nvmem/internals.h
>> +++ b/drivers/nvmem/internals.h
>> @@ -16,6 +16,7 @@ struct nvmem_device {
>>       int            id;
>>       struct kref        refcnt;
>>       size_t            size;
>> +    bool            default_read_only;
> 
> 
> Its not very clear what is the need for this?

This field is used to discern devices which are by default read-write 
from ones which are by default read-only. Only the former can be 
switched between read-write and read-only mode ; the later cannot be 
switched to read-write mode .

Re: [PATCH v3 4/4] nvmem: core: Implement force_ro sysfs attribute

[Srinivas Kandagatla]

On 26/06/2024 17:07, Marek Vasut wrote:
> On 6/26/24 3:07 PM, Srinivas Kandagatla wrote:
>>
>>
>> On 26/06/2024 05:11, Marek Vasut wrote:
>>>   static const struct attribute_group *nvmem_dev_groups[] = {
>>> @@ -945,6 +988,7 @@ struct nvmem_device *nvmem_register(const struct 
>>> nvmem_config *config)
>>>       nvmem->read_only = device_property_present(config->dev, 
>>> "read-only") ||
>>>                  config->read_only || !nvmem->reg_write;
>>> +    nvmem->default_read_only = nvmem->read_only;
>>>   #ifdef CONFIG_NVMEM_SYSFS
>>>       nvmem->dev.groups = nvmem_dev_groups;
>>> diff --git a/drivers/nvmem/internals.h b/drivers/nvmem/internals.h
>>> index 18fed57270e5e..0667937ebb86b 100644
>>> --- a/drivers/nvmem/internals.h
>>> +++ b/drivers/nvmem/internals.h
>>> @@ -16,6 +16,7 @@ struct nvmem_device {
>>>       int            id;
>>>       struct kref        refcnt;
>>>       size_t            size;
>>> +    bool            default_read_only;
>>
>>
>> Its not very clear what is the need for this?
> 
> This field is used to discern devices which are by default read-write 
provider drivers or any drivers should not be accessing this struct 
directly.

> from ones which are by default read-only. Only the former can be 
Why should the drivers care about this?
If the read_only flag is set in the core, nvmem core should prohibit any 
writes for this provider.

--srini

> switched between read-write and read-only mode ; the later cannot be 
> switched to read-write mode .

Re: [PATCH v3 4/4] nvmem: core: Implement force_ro sysfs attribute

[Marek Vasut]

On 6/27/24 4:58 PM, Srinivas Kandagatla wrote:
> 
> 
> On 26/06/2024 17:07, Marek Vasut wrote:
>> On 6/26/24 3:07 PM, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 26/06/2024 05:11, Marek Vasut wrote:
>>>>   static const struct attribute_group *nvmem_dev_groups[] = {
>>>> @@ -945,6 +988,7 @@ struct nvmem_device *nvmem_register(const struct 
>>>> nvmem_config *config)
>>>>       nvmem->read_only = device_property_present(config->dev, 
>>>> "read-only") ||
>>>>                  config->read_only || !nvmem->reg_write;
>>>> +    nvmem->default_read_only = nvmem->read_only;
>>>>   #ifdef CONFIG_NVMEM_SYSFS
>>>>       nvmem->dev.groups = nvmem_dev_groups;
>>>> diff --git a/drivers/nvmem/internals.h b/drivers/nvmem/internals.h
>>>> index 18fed57270e5e..0667937ebb86b 100644
>>>> --- a/drivers/nvmem/internals.h
>>>> +++ b/drivers/nvmem/internals.h
>>>> @@ -16,6 +16,7 @@ struct nvmem_device {
>>>>       int            id;
>>>>       struct kref        refcnt;
>>>>       size_t            size;
>>>> +    bool            default_read_only;
>>>
>>>
>>> Its not very clear what is the need for this?
>>
>> This field is used to discern devices which are by default read-write 
> provider drivers or any drivers should not be accessing this struct 
> directly.
> 
>> from ones which are by default read-only. Only the former can be 
> Why should the drivers care about this?
> If the read_only flag is set in the core, nvmem core should prohibit any 
> writes for this provider.

I think this is a nvmem core patch , isn't it ?

Re: [PATCH v3 4/4] nvmem: core: Implement force_ro sysfs attribute

[Srinivas Kandagatla]

On 27/06/2024 16:55, Marek Vasut wrote:
> On 6/27/24 4:58 PM, Srinivas Kandagatla wrote:
>>
>>
>> On 26/06/2024 17:07, Marek Vasut wrote:
>>> On 6/26/24 3:07 PM, Srinivas Kandagatla wrote:
>>>>
>>>>
>>>> On 26/06/2024 05:11, Marek Vasut wrote:
>>>>>   static const struct attribute_group *nvmem_dev_groups[] = {
>>>>> @@ -945,6 +988,7 @@ struct nvmem_device *nvmem_register(const 
>>>>> struct nvmem_config *config)
>>>>>       nvmem->read_only = device_property_present(config->dev, 
>>>>> "read-only") ||
>>>>>                  config->read_only || !nvmem->reg_write;
>>>>> +    nvmem->default_read_only = nvmem->read_only;
>>>>>   #ifdef CONFIG_NVMEM_SYSFS
>>>>>       nvmem->dev.groups = nvmem_dev_groups;
>>>>> diff --git a/drivers/nvmem/internals.h b/drivers/nvmem/internals.h
>>>>> index 18fed57270e5e..0667937ebb86b 100644
>>>>> --- a/drivers/nvmem/internals.h
>>>>> +++ b/drivers/nvmem/internals.h
>>>>> @@ -16,6 +16,7 @@ struct nvmem_device {
>>>>>       int            id;
>>>>>       struct kref        refcnt;
>>>>>       size_t            size;
>>>>> +    bool            default_read_only;
>>>>
>>>>
>>>> Its not very clear what is the need for this?
>>>
>>> This field is used to discern devices which are by default read-write 
>> provider drivers or any drivers should not be accessing this struct 
>> directly.
>>
>>> from ones which are by default read-only. Only the former can be 
>> Why should the drivers care about this?
>> If the read_only flag is set in the core, nvmem core should prohibit 
>> any writes for this provider.
> 
> I think this is a nvmem core patch , isn't it ?
All that I see in this patch is setting default_read_only and no user 
for it. unless am looking at wrong patch.

--srin

Re: [PATCH v3 4/4] nvmem: core: Implement force_ro sysfs attribute

[Marek Vasut]

On 6/27/24 6:21 PM, Srinivas Kandagatla wrote:
> 
> 
> On 27/06/2024 16:55, Marek Vasut wrote:
>> On 6/27/24 4:58 PM, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 26/06/2024 17:07, Marek Vasut wrote:
>>>> On 6/26/24 3:07 PM, Srinivas Kandagatla wrote:
>>>>>
>>>>>
>>>>> On 26/06/2024 05:11, Marek Vasut wrote:
>>>>>>   static const struct attribute_group *nvmem_dev_groups[] = {
>>>>>> @@ -945,6 +988,7 @@ struct nvmem_device *nvmem_register(const 
>>>>>> struct nvmem_config *config)
>>>>>>       nvmem->read_only = device_property_present(config->dev, 
>>>>>> "read-only") ||
>>>>>>                  config->read_only || !nvmem->reg_write;
>>>>>> +    nvmem->default_read_only = nvmem->read_only;
>>>>>>   #ifdef CONFIG_NVMEM_SYSFS
>>>>>>       nvmem->dev.groups = nvmem_dev_groups;
>>>>>> diff --git a/drivers/nvmem/internals.h b/drivers/nvmem/internals.h
>>>>>> index 18fed57270e5e..0667937ebb86b 100644
>>>>>> --- a/drivers/nvmem/internals.h
>>>>>> +++ b/drivers/nvmem/internals.h
>>>>>> @@ -16,6 +16,7 @@ struct nvmem_device {
>>>>>>       int            id;
>>>>>>       struct kref        refcnt;
>>>>>>       size_t            size;
>>>>>> +    bool            default_read_only;
>>>>>
>>>>>
>>>>> Its not very clear what is the need for this?
>>>>
>>>> This field is used to discern devices which are by default read-write 
>>> provider drivers or any drivers should not be accessing this struct 
>>> directly.
>>>
>>>> from ones which are by default read-only. Only the former can be 
>>> Why should the drivers care about this?
>>> If the read_only flag is set in the core, nvmem core should prohibit 
>>> any writes for this provider.
>>
>> I think this is a nvmem core patch , isn't it ?
> All that I see in this patch is setting default_read_only and no user 
> for it. unless am looking at wrong patch.

Oh, now I understand, doh ... this is a remnant which should be removed. 
The force_ro attribute is now hidden for read-only devices outright in 
nvmem_attr_is_visible, so they cannot be switched to read-write mode.

Fixed in V4, thanks.