[PATCH] nvmem: core: eeprom: at24: Handle EEPROM with both read-only and wp-gpios

[PATCH] nvmem: core: eeprom: at24: Handle EEPROM with both read-only and wp-gpios

[Marek Vasut]

Handle special-case of AT24 EEPROM described in DT, which contains both
"read-only" and "wp-gpios" properties. Interpret this configuration as
default read-only, but with the possibility of unlock via force_ro sysfs
attribute.

For this to work, the nvmem core must keep the "nvmem" sysfs bin attr as
read-write, any writes into the bin attr which occur while the device is
read-only are blocked in bin_attr_nvmem_write(). The AT24 EEPROM driver
must not mark the EEPROM as read-only if both "read-only" and "wp-gpios"
DT properties are present.

The updated behavior can be tested as follows:

Current content:
"
$ hexdump -C /sys/bus/nvmem/devices/logging7/nvmem
00000000  66 6f 6f 0a ff ff ff ff
"

Write into default-read-only device:
"
$ echo bar > /sys/bus/nvmem/devices/logging7/nvmem
bash: echo: write error: Operation not permitted
$ cat /sys/bus/nvmem/devices/logging7/force_ro
1
"

Unlock and write into device:
"
$ echo 0 > /sys/bus/nvmem/devices/logging7/force_ro
$ cat /sys/bus/nvmem/devices/logging7/force_ro
0

$ echo bar > /sys/bus/nvmem/devices/logging7/nvmem
$ hexdump -C /sys/bus/nvmem/devices/logging7/nvmem
00000000  62 61 72 0a ff ff ff ff
"

Relock and write into device, fails because device is read-only again:
"
$ echo 1 > /sys/bus/nvmem/devices/logging7/force_ro
$ echo baz > /sys/bus/nvmem/devices/logging7/nvmem
bash: echo: write error: Operation not permitted

$ hexdump -C /sys/bus/nvmem/devices/logging7/nvmem
00000000  62 61 72 0a ff ff ff ff
"

Signed-off-by: Marek Vasut <marex@nabladev.com>
---
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bartosz Golaszewski <brgl@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Srinivas Kandagatla <srini@kernel.org>
Cc: linux-i2c@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/misc/eeprom/at24.c | 3 ++-
 drivers/nvmem/core.c       | 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 0200288d3a7ab..9ef00b0ce83d2 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -627,7 +627,8 @@ static int at24_probe(struct i2c_client *client)
 		page_size = 1;
 
 	flags = cdata->flags;
-	if (device_property_present(dev, "read-only"))
+	if (device_property_present(dev, "read-only") &&
+	    !device_property_present(dev, "wp-gpios"))
 		flags |= AT24_FLAG_READONLY;
 	if (device_property_present(dev, "no-read-rollover"))
 		flags |= AT24_FLAG_NO_RDROL;
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 311cb2e5a5c02..c0eedfe8d706c 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -282,6 +282,9 @@ static umode_t nvmem_bin_attr_get_umode(struct nvmem_device *nvmem)
 	if (!nvmem->root_only)
 		mode |= 0044;
 
+	if (nvmem->read_only && nvmem->reg_write)
+		mode |= 0200;
+
 	if (!nvmem->read_only)
 		mode |= 0200;
 
-- 
2.53.0

Re: [PATCH] nvmem: core: eeprom: at24: Handle EEPROM with both read-only and wp-gpios

[Bartosz Golaszewski]

On Tue, Apr 21, 2026 at 4:08 PM Marek Vasut <marex@nabladev.com> wrote:
>
> Handle special-case of AT24 EEPROM described in DT, which contains both
> "read-only" and "wp-gpios" properties. Interpret this configuration as
> default read-only, but with the possibility of unlock via force_ro sysfs
> attribute.
>

Patch looks ok code-wise but does this really make sense? If an EEPROM
is read-only, we should forbid writes in the kernel. Which board uses
it? Can we simply remove the read-only flag from DT?

Admittedly: the DT bindings do allow it as read-only and wp-gpios are
not mutually exclusive but I think it's more of an accidental omission
than a planned feature.

Bart

Re: [PATCH] nvmem: core: eeprom: at24: Handle EEPROM with both read-only and wp-gpios

[Marek Vasut]

On 4/22/26 11:07 AM, Bartosz Golaszewski wrote:
> On Tue, Apr 21, 2026 at 4:08 PM Marek Vasut <marex@nabladev.com> wrote:
>>
>> Handle special-case of AT24 EEPROM described in DT, which contains both
>> "read-only" and "wp-gpios" properties. Interpret this configuration as
>> default read-only, but with the possibility of unlock via force_ro sysfs
>> attribute.
>>
> 
> Patch looks ok code-wise but does this really make sense? If an EEPROM
> is read-only, we should forbid writes in the kernel. Which board uses
> it? Can we simply remove the read-only flag from DT?

Currently I am not aware of any upstream users, I plan to introduce one 
once this patch or some for of it lands.

I have is an ID EEPROM which I would like to be able to program under 
special circumstances (hence the wp-gpios control) , but it should be by 
default read-only .

If I remove the read-only, then by default the EEPROM is read-write, 
which is undesired. If I remote wp-gpios then I loose access to the 
force_ro attribute which controls the nWP GPIO from userspace, which is 
undesired.

So I think defining this special-case where wp-gpios and read-only are 
used together as default-read-only is sensible.

> Admittedly: the DT bindings do allow it as read-only and wp-gpios are
> not mutually exclusive but I think it's more of an accidental omission
> than a planned feature.
I think it is currently an undefined behavior, and this patch defines it.

Re: [PATCH] nvmem: core: eeprom: at24: Handle EEPROM with both read-only and wp-gpios

[Marek Vasut]

On 4/22/26 1:33 PM, Marek Vasut wrote:
> On 4/22/26 11:07 AM, Bartosz Golaszewski wrote:
>> On Tue, Apr 21, 2026 at 4:08 PM Marek Vasut <marex@nabladev.com> wrote:
>>>
>>> Handle special-case of AT24 EEPROM described in DT, which contains both
>>> "read-only" and "wp-gpios" properties. Interpret this configuration as
>>> default read-only, but with the possibility of unlock via force_ro sysfs
>>> attribute.
>>>
>>
>> Patch looks ok code-wise but does this really make sense? If an EEPROM
>> is read-only, we should forbid writes in the kernel. Which board uses
>> it? Can we simply remove the read-only flag from DT?
> 
> Currently I am not aware of any upstream users, I plan to introduce one 
> once this patch or some for of it lands.

I have to amend my statement, I would also like to adjust an already 
upstream DT to make use of this default-read-only functionality now.

I would however like to get go/no-go on this patch before I roll out the 
DT patches.

> I have is an ID EEPROM which I would like to be able to program under 
> special circumstances (hence the wp-gpios control) , but it should be by 
> default read-only .
> 
> If I remove the read-only, then by default the EEPROM is read-write, 
> which is undesired. If I remote wp-gpios then I loose access to the 
> force_ro attribute which controls the nWP GPIO from userspace, which is 
> undesired.
> 
> So I think defining this special-case where wp-gpios and read-only are 
> used together as default-read-only is sensible.
> 
>> Admittedly: the DT bindings do allow it as read-only and wp-gpios are
>> not mutually exclusive but I think it's more of an accidental omission
>> than a planned feature.
> I think it is currently an undefined behavior, and this patch defines it.

Also, this default-read-only behavior is effectively the same behavior 
like the eMMC HW BOOT partitions have, they are also default read-only, 
but can be switched and written to by setting their force_ro sysfs 
attribute.

Re: [PATCH] nvmem: core: eeprom: at24: Handle EEPROM with both read-only and wp-gpios

[Bartosz Golaszewski]

On Wed, Apr 22, 2026 at 7:01 PM Marek Vasut <marex@nabladev.com> wrote:
>
> On 4/22/26 1:33 PM, Marek Vasut wrote:
> > On 4/22/26 11:07 AM, Bartosz Golaszewski wrote:
> >> On Tue, Apr 21, 2026 at 4:08 PM Marek Vasut <marex@nabladev.com> wrote:
> >>>
> >>> Handle special-case of AT24 EEPROM described in DT, which contains both
> >>> "read-only" and "wp-gpios" properties. Interpret this configuration as
> >>> default read-only, but with the possibility of unlock via force_ro sysfs
> >>> attribute.
> >>>
> >>
> >> Patch looks ok code-wise but does this really make sense? If an EEPROM
> >> is read-only, we should forbid writes in the kernel. Which board uses
> >> it? Can we simply remove the read-only flag from DT?
> >
> > Currently I am not aware of any upstream users, I plan to introduce one
> > once this patch or some for of it lands.
>
> I have to amend my statement, I would also like to adjust an already
> upstream DT to make use of this default-read-only functionality now.
>
> I would however like to get go/no-go on this patch before I roll out the
> DT patches.
>

Yes, go ahead.

> > I have is an ID EEPROM which I would like to be able to program under
> > special circumstances (hence the wp-gpios control) , but it should be by
> > default read-only .
> >
> > If I remove the read-only, then by default the EEPROM is read-write,
> > which is undesired. If I remote wp-gpios then I loose access to the
> > force_ro attribute which controls the nWP GPIO from userspace, which is
> > undesired.
> >
> > So I think defining this special-case where wp-gpios and read-only are
> > used together as default-read-only is sensible.
> >
> >> Admittedly: the DT bindings do allow it as read-only and wp-gpios are
> >> not mutually exclusive but I think it's more of an accidental omission
> >> than a planned feature.
> > I think it is currently an undefined behavior, and this patch defines it.
>
> Also, this default-read-only behavior is effectively the same behavior
> like the eMMC HW BOOT partitions have, they are also default read-only,
> but can be switched and written to by setting their force_ro sysfs
> attribute.

I see. Ok, please send a v2.

Bart

Re: [PATCH] nvmem: core: eeprom: at24: Handle EEPROM with both read-only and wp-gpios

[Marek Vasut]

On 4/23/26 9:42 AM, Bartosz Golaszewski wrote:
> On Wed, Apr 22, 2026 at 7:01 PM Marek Vasut <marex@nabladev.com> wrote:
>>
>> On 4/22/26 1:33 PM, Marek Vasut wrote:
>>> On 4/22/26 11:07 AM, Bartosz Golaszewski wrote:
>>>> On Tue, Apr 21, 2026 at 4:08 PM Marek Vasut <marex@nabladev.com> wrote:
>>>>>
>>>>> Handle special-case of AT24 EEPROM described in DT, which contains both
>>>>> "read-only" and "wp-gpios" properties. Interpret this configuration as
>>>>> default read-only, but with the possibility of unlock via force_ro sysfs
>>>>> attribute.
>>>>>
>>>>
>>>> Patch looks ok code-wise but does this really make sense? If an EEPROM
>>>> is read-only, we should forbid writes in the kernel. Which board uses
>>>> it? Can we simply remove the read-only flag from DT?
>>>
>>> Currently I am not aware of any upstream users, I plan to introduce one
>>> once this patch or some for of it lands.
>>
>> I have to amend my statement, I would also like to adjust an already
>> upstream DT to make use of this default-read-only functionality now.
>>
>> I would however like to get go/no-go on this patch before I roll out the
>> DT patches.
>>
> 
> Yes, go ahead.
> 
>>> I have is an ID EEPROM which I would like to be able to program under
>>> special circumstances (hence the wp-gpios control) , but it should be by
>>> default read-only .
>>>
>>> If I remove the read-only, then by default the EEPROM is read-write,
>>> which is undesired. If I remote wp-gpios then I loose access to the
>>> force_ro attribute which controls the nWP GPIO from userspace, which is
>>> undesired.
>>>
>>> So I think defining this special-case where wp-gpios and read-only are
>>> used together as default-read-only is sensible.
>>>
>>>> Admittedly: the DT bindings do allow it as read-only and wp-gpios are
>>>> not mutually exclusive but I think it's more of an accidental omission
>>>> than a planned feature.
>>> I think it is currently an undefined behavior, and this patch defines it.
>>
>> Also, this default-read-only behavior is effectively the same behavior
>> like the eMMC HW BOOT partitions have, they are also default read-only,
>> but can be switched and written to by setting their force_ro sysfs
>> attribute.
> 
> I see. Ok, please send a v2.
Does this patch require any changes ?

I will be sending the DT changes separately.

Re: [PATCH] nvmem: core: eeprom: at24: Handle EEPROM with both read-only and wp-gpios

[Bartosz Golaszewski]

On Thu, Apr 23, 2026 at 2:04 PM Marek Vasut <marex@nabladev.com> wrote:
>
> >
> > I see. Ok, please send a v2.
> Does this patch require any changes ?
>
> I will be sending the DT changes separately.

Sashiko is saying this:
https://sashiko.dev/#/patchset/20260421140755.54222-1-marex%40nabladev.com

Shouldn't we report the device as read-only in sysfs unless it was
"unlocked" with force_ro?

Bart

Re: [PATCH] nvmem: core: eeprom: at24: Handle EEPROM with both read-only and wp-gpios

[Marek Vasut]

On 4/23/26 2:17 PM, Bartosz Golaszewski wrote:
> On Thu, Apr 23, 2026 at 2:04 PM Marek Vasut <marex@nabladev.com> wrote:
>>
>>>
>>> I see. Ok, please send a v2.
>> Does this patch require any changes ?
>>
>> I will be sending the DT changes separately.
> 
> Sashiko is saying this:
> https://sashiko.dev/#/patchset/20260421140755.54222-1-marex%40nabladev.com

What does this mean ?

> Shouldn't we report the device as read-only in sysfs unless it was
> "unlocked" with force_ro?
This would be ideal, but I did not find a way to toggle the "nvmem" bin 
attr permissions at runtime. Is that even possible ?

Re: [PATCH] nvmem: core: eeprom: at24: Handle EEPROM with both read-only and wp-gpios

[Bartosz Golaszewski]

On Thu, Apr 23, 2026 at 4:06 PM Marek Vasut <marex@nabladev.com> wrote:
>
> On 4/23/26 2:17 PM, Bartosz Golaszewski wrote:
> > On Thu, Apr 23, 2026 at 2:04 PM Marek Vasut <marex@nabladev.com> wrote:
> >>
> >>>
> >>> I see. Ok, please send a v2.
> >> Does this patch require any changes ?
> >>
> >> I will be sending the DT changes separately.
> >
> > Sashiko is saying this:
> > https://sashiko.dev/#/patchset/20260421140755.54222-1-marex%40nabladev.com
>
> What does this mean ?
>
> > Shouldn't we report the device as read-only in sysfs unless it was
> > "unlocked" with force_ro?
> This would be ideal, but I did not find a way to toggle the "nvmem" bin
> attr permissions at runtime. Is that even possible ?

Right, it seems like it's set once and can't be changed (Greg: correct
me if I'm wrong).

Ok, nevermind the comment then. Maybe just split the changes into
nvmem and at24 changes and I can take both with an Ack from Srini.

Bart

Re: [PATCH] nvmem: core: eeprom: at24: Handle EEPROM with both read-only and wp-gpios

[Marek Vasut]

On 4/23/26 4:19 PM, Bartosz Golaszewski wrote:
> On Thu, Apr 23, 2026 at 4:06 PM Marek Vasut <marex@nabladev.com> wrote:
>>
>> On 4/23/26 2:17 PM, Bartosz Golaszewski wrote:
>>> On Thu, Apr 23, 2026 at 2:04 PM Marek Vasut <marex@nabladev.com> wrote:
>>>>
>>>>>
>>>>> I see. Ok, please send a v2.
>>>> Does this patch require any changes ?
>>>>
>>>> I will be sending the DT changes separately.
>>>
>>> Sashiko is saying this:
>>> https://sashiko.dev/#/patchset/20260421140755.54222-1-marex%40nabladev.com
>>
>> What does this mean ?
>>
>>> Shouldn't we report the device as read-only in sysfs unless it was
>>> "unlocked" with force_ro?
>> This would be ideal, but I did not find a way to toggle the "nvmem" bin
>> attr permissions at runtime. Is that even possible ?
> 
> Right, it seems like it's set once and can't be changed (Greg: correct
> me if I'm wrong).
> 
> Ok, nevermind the comment then. Maybe just split the changes into
> nvmem and at24 changes and I can take both with an Ack from Srini.
I have two more ideas I would like to run past you ... how about either:

- If wp-gpios is present, set the device as default RO after boot, and
   let force_ro sysfs attribute toggle the protection of the device back
   and forth afterward. This would however change the userspace facing
   behavior slightly, because right now, with wp-gpios present in DT, the
   device is default RW.

- Introduce new DT property, wp-gpios-default-read-only or
   default-read-only or some such, to indicate the device should be in
   read-only mode by default. That would mitigate the downside of the
   aforementioned point, but would require a new DT property.

Thoughts ?

Re: [PATCH] nvmem: core: eeprom: at24: Handle EEPROM with both read-only and wp-gpios

[Bartosz Golaszewski]

On Thu, Apr 23, 2026 at 9:15 PM Marek Vasut <marex@nabladev.com> wrote:
>
> On 4/23/26 4:19 PM, Bartosz Golaszewski wrote:
> > On Thu, Apr 23, 2026 at 4:06 PM Marek Vasut <marex@nabladev.com> wrote:
> >>
> >> On 4/23/26 2:17 PM, Bartosz Golaszewski wrote:
> >>> On Thu, Apr 23, 2026 at 2:04 PM Marek Vasut <marex@nabladev.com> wrote:
> >>>>
> >>>>>
> >>>>> I see. Ok, please send a v2.
> >>>> Does this patch require any changes ?
> >>>>
> >>>> I will be sending the DT changes separately.
> >>>
> >>> Sashiko is saying this:
> >>> https://sashiko.dev/#/patchset/20260421140755.54222-1-marex%40nabladev.com
> >>
> >> What does this mean ?
> >>
> >>> Shouldn't we report the device as read-only in sysfs unless it was
> >>> "unlocked" with force_ro?
> >> This would be ideal, but I did not find a way to toggle the "nvmem" bin
> >> attr permissions at runtime. Is that even possible ?
> >
> > Right, it seems like it's set once and can't be changed (Greg: correct
> > me if I'm wrong).
> >
> > Ok, nevermind the comment then. Maybe just split the changes into
> > nvmem and at24 changes and I can take both with an Ack from Srini.
> I have two more ideas I would like to run past you ... how about either:
>
> - If wp-gpios is present, set the device as default RO after boot, and

Isn't this already what happens though? nvmem core requests the GPIO
as output-high and drives it low only when it's doing the writing.

>    let force_ro sysfs attribute toggle the protection of the device back
>    and forth afterward. This would however change the userspace facing
>    behavior slightly, because right now, with wp-gpios present in DT, the
>    device is default RW.
>

Or do you mean just the file permissions?

If the latter, then that does sound logically sound but yeah, it's
asking for a regression report. :)

> - Introduce new DT property, wp-gpios-default-read-only or
>    default-read-only or some such, to indicate the device should be in
>    read-only mode by default. That would mitigate the downside of the
>    aforementioned point, but would require a new DT property.
>
> Thoughts ?

I like "default-read-only" and it both allows to introduce this
behavior and not break existing users. Let's loop in DT maintainers
and see. I'd just like to clarify if we're talking about sysfs
permissions or GPIO behavior here.

Bart

Re: [PATCH] nvmem: core: eeprom: at24: Handle EEPROM with both read-only and wp-gpios

[Marek Vasut]

On 4/24/26 10:12 AM, Bartosz Golaszewski wrote:
> On Thu, Apr 23, 2026 at 9:15 PM Marek Vasut <marex@nabladev.com> wrote:
>>
>> On 4/23/26 4:19 PM, Bartosz Golaszewski wrote:
>>> On Thu, Apr 23, 2026 at 4:06 PM Marek Vasut <marex@nabladev.com> wrote:
>>>>
>>>> On 4/23/26 2:17 PM, Bartosz Golaszewski wrote:
>>>>> On Thu, Apr 23, 2026 at 2:04 PM Marek Vasut <marex@nabladev.com> wrote:
>>>>>>
>>>>>>>
>>>>>>> I see. Ok, please send a v2.
>>>>>> Does this patch require any changes ?
>>>>>>
>>>>>> I will be sending the DT changes separately.
>>>>>
>>>>> Sashiko is saying this:
>>>>> https://sashiko.dev/#/patchset/20260421140755.54222-1-marex%40nabladev.com
>>>>
>>>> What does this mean ?
>>>>
>>>>> Shouldn't we report the device as read-only in sysfs unless it was
>>>>> "unlocked" with force_ro?
>>>> This would be ideal, but I did not find a way to toggle the "nvmem" bin
>>>> attr permissions at runtime. Is that even possible ?
>>>
>>> Right, it seems like it's set once and can't be changed (Greg: correct
>>> me if I'm wrong).
>>>
>>> Ok, nevermind the comment then. Maybe just split the changes into
>>> nvmem and at24 changes and I can take both with an Ack from Srini.
>> I have two more ideas I would like to run past you ... how about either:
>>
>> - If wp-gpios is present, set the device as default RO after boot, and
> 
> Isn't this already what happens though? nvmem core requests the GPIO
> as output-high and drives it low only when it's doing the writing.

It isn't, the default is read-write . But I now submitted a V2:

nvmem: core: Default to read-only if wp-gpios present

>>     let force_ro sysfs attribute toggle the protection of the device back
>>     and forth afterward. This would however change the userspace facing
>>     behavior slightly, because right now, with wp-gpios present in DT, the
>>     device is default RW.
>>
> 
> Or do you mean just the file permissions?
> 
> If the latter, then that does sound logically sound but yeah, it's
> asking for a regression report. :)

Please see "nvmem: core: Default to read-only if wp-gpios present" that 
I just submitted.

>> - Introduce new DT property, wp-gpios-default-read-only or
>>     default-read-only or some such, to indicate the device should be in
>>     read-only mode by default. That would mitigate the downside of the
>>     aforementioned point, but would require a new DT property.
>>
>> Thoughts ?
> 
> I like "default-read-only" and it both allows to introduce this
> behavior and not break existing users. Let's loop in DT maintainers
> and see. I'd just like to clarify if we're talking about sysfs
> permissions or GPIO behavior here.
I think we might not need this, please see above.