[PATCH 0/2] nvmem: fixes for 6.16

[PATCH 0/2] nvmem: fixes for 6.16

[srini]

From: Srinivas Kandagatla <srini@kernel.org>

Hi Greg, 
Here are two nvmem fixes for 6.16, Could you queue
these for 6.16

Fixes include:
	- crc32 endianness conversion in u-boot-env
	- fix the mac address length in imx-ocotp provider

thanks,
Srini
.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Michael C. Pratt (1):
  nvmem: layouts: u-boot-env: remove crc32 endianness conversion

Steffen Bätz (1):
  nvmem: imx-ocotp: fix MAC address byte length

 drivers/nvmem/imx-ocotp-ele.c      | 5 ++++-
 drivers/nvmem/imx-ocotp.c          | 5 ++++-
 drivers/nvmem/layouts/u-boot-env.c | 6 +++---
 3 files changed, 11 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH 1/2] nvmem: layouts: u-boot-env: remove crc32 endianness conversion

[srini]

From: "Michael C. Pratt" <mcpratt@pm.me>

On 11 Oct 2022, it was reported that the crc32 verification
of the u-boot environment failed only on big-endian systems
for the u-boot-env nvmem layout driver with the following error.

  Invalid calculated CRC32: 0x88cd6f09 (expected: 0x096fcd88)

This problem has been present since the driver was introduced,
and before it was made into a layout driver.

The suggested fix at the time was to use further endianness
conversion macros in order to have both the stored and calculated
crc32 values to compare always represented in the system's endianness.
This was not accepted due to sparse warnings
and some disagreement on how to handle the situation.
Later on in a newer revision of the patch, it was proposed to use
cpu_to_le32() for both values to compare instead of le32_to_cpu()
and store the values as __le32 type to remove compilation errors.

The necessity of this is based on the assumption that the use of crc32()
requires endianness conversion because the algorithm uses little-endian,
however, this does not prove to be the case and the issue is unrelated.

Upon inspecting the current kernel code,
there already is an existing use of le32_to_cpu() in this driver,
which suggests there already is special handling for big-endian systems,
however, it is big-endian systems that have the problem.

This, being the only functional difference between architectures
in the driver combined with the fact that the suggested fix
was to use the exact same endianness conversion for the values
brings up the possibility that it was not necessary to begin with,
as the same endianness conversion for two values expected to be the same
is expected to be equivalent to no conversion at all.

After inspecting the u-boot environment of devices of both endianness
and trying to remove the existing endianness conversion,
the problem is resolved in an equivalent way as the other suggested fixes.

Ultimately, it seems that u-boot is agnostic to endianness
at least for the purpose of environment variables.
In other words, u-boot reads and writes the stored crc32 value
with the same endianness that the crc32 value is calculated with
in whichever endianness a certain architecture runs on.

Therefore, the u-boot-env driver does not need to convert endianness.
Remove the usage of endianness macros in the u-boot-env driver,
and change the type of local variables to maintain the same return type.

If there is a special situation in the case of endianness,
it would be a corner case and should be handled by a unique "compatible".

Even though it is not necessary to use endianness conversion macros here,
it may be useful to use them in the future for consistent error printing.

Fixes: d5542923f200 ("nvmem: add driver handling U-Boot environment variables")
Reported-by: INAGAKI Hiroshi <musashino.open@gmail.com>
Link: https://lore.kernel.org/all/20221011024928.1807-1-musashino.open@gmail.com
Cc: stable@vger.kernel.org # 6.12.x
Cc: stable@vger.kernel.org # 6.6.x: f4cf4e5: Revert "nvmem: add new config option"
Cc: stable@vger.kernel.org # 6.6.x: 7f38b70: of: device: Export of_device_make_bus_id()
Cc: stable@vger.kernel.org # 6.6.x: 4a1a402: nvmem: Move of_nvmem_layout_get_container() in another header
Cc: stable@vger.kernel.org # 6.6.x: fc29fd8: nvmem: core: Rework layouts to become regular devices
Cc: stable@vger.kernel.org # 6.6.x: 0331c61: nvmem: core: Expose cells through sysfs
Cc: stable@vger.kernel.org # 6.6.x: 401df0d: nvmem: layouts: refactor .add_cells() callback arguments
Cc: stable@vger.kernel.org # 6.6.x: 6d0ca4a: nvmem: layouts: store owner from modules with nvmem_layout_driver_register()
Cc: stable@vger.kernel.org # 6.6.x: 5f15811: nvmem: layouts: add U-Boot env layout
Cc: stable@vger.kernel.org # 6.6.x
Signed-off-by: Michael C. Pratt <mcpratt@pm.me>
Signed-off-by: Srinivas Kandagatla <srini@kernel.org>
---
 drivers/nvmem/layouts/u-boot-env.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/nvmem/layouts/u-boot-env.c b/drivers/nvmem/layouts/u-boot-env.c
index 436426d4e8f9..8571aac56295 100644
--- a/drivers/nvmem/layouts/u-boot-env.c
+++ b/drivers/nvmem/layouts/u-boot-env.c
@@ -92,7 +92,7 @@ int u_boot_env_parse(struct device *dev, struct nvmem_device *nvmem,
 	size_t crc32_data_offset;
 	size_t crc32_data_len;
 	size_t crc32_offset;
-	__le32 *crc32_addr;
+	uint32_t *crc32_addr;
 	size_t data_offset;
 	size_t data_len;
 	size_t dev_size;
@@ -143,8 +143,8 @@ int u_boot_env_parse(struct device *dev, struct nvmem_device *nvmem,
 		goto err_kfree;
 	}
 
-	crc32_addr = (__le32 *)(buf + crc32_offset);
-	crc32 = le32_to_cpu(*crc32_addr);
+	crc32_addr = (uint32_t *)(buf + crc32_offset);
+	crc32 = *crc32_addr;
 	crc32_data_len = dev_size - crc32_data_offset;
 	data_len = dev_size - data_offset;
 
-- 
2.43.0

Re: [PATCH 1/2] nvmem: layouts: u-boot-env: remove crc32 endianness conversion

[Greg KH]

On Sat, Jul 12, 2025 at 07:17:26PM +0100, srini@kernel.org wrote:
> From: "Michael C. Pratt" <mcpratt@pm.me>
> 
> On 11 Oct 2022, it was reported that the crc32 verification
> of the u-boot environment failed only on big-endian systems
> for the u-boot-env nvmem layout driver with the following error.
> 
>   Invalid calculated CRC32: 0x88cd6f09 (expected: 0x096fcd88)
> 
> This problem has been present since the driver was introduced,
> and before it was made into a layout driver.
> 
> The suggested fix at the time was to use further endianness
> conversion macros in order to have both the stored and calculated
> crc32 values to compare always represented in the system's endianness.
> This was not accepted due to sparse warnings
> and some disagreement on how to handle the situation.
> Later on in a newer revision of the patch, it was proposed to use
> cpu_to_le32() for both values to compare instead of le32_to_cpu()
> and store the values as __le32 type to remove compilation errors.
> 
> The necessity of this is based on the assumption that the use of crc32()
> requires endianness conversion because the algorithm uses little-endian,
> however, this does not prove to be the case and the issue is unrelated.
> 
> Upon inspecting the current kernel code,
> there already is an existing use of le32_to_cpu() in this driver,
> which suggests there already is special handling for big-endian systems,
> however, it is big-endian systems that have the problem.
> 
> This, being the only functional difference between architectures
> in the driver combined with the fact that the suggested fix
> was to use the exact same endianness conversion for the values
> brings up the possibility that it was not necessary to begin with,
> as the same endianness conversion for two values expected to be the same
> is expected to be equivalent to no conversion at all.
> 
> After inspecting the u-boot environment of devices of both endianness
> and trying to remove the existing endianness conversion,
> the problem is resolved in an equivalent way as the other suggested fixes.
> 
> Ultimately, it seems that u-boot is agnostic to endianness
> at least for the purpose of environment variables.
> In other words, u-boot reads and writes the stored crc32 value
> with the same endianness that the crc32 value is calculated with
> in whichever endianness a certain architecture runs on.
> 
> Therefore, the u-boot-env driver does not need to convert endianness.
> Remove the usage of endianness macros in the u-boot-env driver,
> and change the type of local variables to maintain the same return type.
> 
> If there is a special situation in the case of endianness,
> it would be a corner case and should be handled by a unique "compatible".
> 
> Even though it is not necessary to use endianness conversion macros here,
> it may be useful to use them in the future for consistent error printing.
> 
> Fixes: d5542923f200 ("nvmem: add driver handling U-Boot environment variables")

Note, this is a 6.1 commit id, but:

> Reported-by: INAGAKI Hiroshi <musashino.open@gmail.com>
> Link: https://lore.kernel.org/all/20221011024928.1807-1-musashino.open@gmail.com
> Cc: stable@vger.kernel.org # 6.12.x
> Cc: stable@vger.kernel.org # 6.6.x: f4cf4e5: Revert "nvmem: add new config option"
> Cc: stable@vger.kernel.org # 6.6.x: 7f38b70: of: device: Export of_device_make_bus_id()
> Cc: stable@vger.kernel.org # 6.6.x: 4a1a402: nvmem: Move of_nvmem_layout_get_container() in another header
> Cc: stable@vger.kernel.org # 6.6.x: fc29fd8: nvmem: core: Rework layouts to become regular devices
> Cc: stable@vger.kernel.org # 6.6.x: 0331c61: nvmem: core: Expose cells through sysfs
> Cc: stable@vger.kernel.org # 6.6.x: 401df0d: nvmem: layouts: refactor .add_cells() callback arguments
> Cc: stable@vger.kernel.org # 6.6.x: 6d0ca4a: nvmem: layouts: store owner from modules with nvmem_layout_driver_register()
> Cc: stable@vger.kernel.org # 6.6.x: 5f15811: nvmem: layouts: add U-Boot env layout
> Cc: stable@vger.kernel.org # 6.6.x

That's a load of (short) git ids for just 6.6.y?  What about 6.1.y?

thanks,

greg k-h

Re: [PATCH 1/2] nvmem: layouts: u-boot-env: remove crc32 endianness conversion

[Greg KH]

On Sun, Jul 13, 2025 at 05:41:45PM +0200, Greg KH wrote:
> On Sat, Jul 12, 2025 at 07:17:26PM +0100, srini@kernel.org wrote:
> > From: "Michael C. Pratt" <mcpratt@pm.me>
> > 
> > On 11 Oct 2022, it was reported that the crc32 verification
> > of the u-boot environment failed only on big-endian systems
> > for the u-boot-env nvmem layout driver with the following error.
> > 
> >   Invalid calculated CRC32: 0x88cd6f09 (expected: 0x096fcd88)
> > 
> > This problem has been present since the driver was introduced,
> > and before it was made into a layout driver.
> > 
> > The suggested fix at the time was to use further endianness
> > conversion macros in order to have both the stored and calculated
> > crc32 values to compare always represented in the system's endianness.
> > This was not accepted due to sparse warnings
> > and some disagreement on how to handle the situation.
> > Later on in a newer revision of the patch, it was proposed to use
> > cpu_to_le32() for both values to compare instead of le32_to_cpu()
> > and store the values as __le32 type to remove compilation errors.
> > 
> > The necessity of this is based on the assumption that the use of crc32()
> > requires endianness conversion because the algorithm uses little-endian,
> > however, this does not prove to be the case and the issue is unrelated.
> > 
> > Upon inspecting the current kernel code,
> > there already is an existing use of le32_to_cpu() in this driver,
> > which suggests there already is special handling for big-endian systems,
> > however, it is big-endian systems that have the problem.
> > 
> > This, being the only functional difference between architectures
> > in the driver combined with the fact that the suggested fix
> > was to use the exact same endianness conversion for the values
> > brings up the possibility that it was not necessary to begin with,
> > as the same endianness conversion for two values expected to be the same
> > is expected to be equivalent to no conversion at all.
> > 
> > After inspecting the u-boot environment of devices of both endianness
> > and trying to remove the existing endianness conversion,
> > the problem is resolved in an equivalent way as the other suggested fixes.
> > 
> > Ultimately, it seems that u-boot is agnostic to endianness
> > at least for the purpose of environment variables.
> > In other words, u-boot reads and writes the stored crc32 value
> > with the same endianness that the crc32 value is calculated with
> > in whichever endianness a certain architecture runs on.
> > 
> > Therefore, the u-boot-env driver does not need to convert endianness.
> > Remove the usage of endianness macros in the u-boot-env driver,
> > and change the type of local variables to maintain the same return type.
> > 
> > If there is a special situation in the case of endianness,
> > it would be a corner case and should be handled by a unique "compatible".
> > 
> > Even though it is not necessary to use endianness conversion macros here,
> > it may be useful to use them in the future for consistent error printing.
> > 
> > Fixes: d5542923f200 ("nvmem: add driver handling U-Boot environment variables")
> 
> Note, this is a 6.1 commit id, but:
> 
> > Reported-by: INAGAKI Hiroshi <musashino.open@gmail.com>
> > Link: https://lore.kernel.org/all/20221011024928.1807-1-musashino.open@gmail.com
> > Cc: stable@vger.kernel.org # 6.12.x
> > Cc: stable@vger.kernel.org # 6.6.x: f4cf4e5: Revert "nvmem: add new config option"
> > Cc: stable@vger.kernel.org # 6.6.x: 7f38b70: of: device: Export of_device_make_bus_id()
> > Cc: stable@vger.kernel.org # 6.6.x: 4a1a402: nvmem: Move of_nvmem_layout_get_container() in another header
> > Cc: stable@vger.kernel.org # 6.6.x: fc29fd8: nvmem: core: Rework layouts to become regular devices
> > Cc: stable@vger.kernel.org # 6.6.x: 0331c61: nvmem: core: Expose cells through sysfs
> > Cc: stable@vger.kernel.org # 6.6.x: 401df0d: nvmem: layouts: refactor .add_cells() callback arguments
> > Cc: stable@vger.kernel.org # 6.6.x: 6d0ca4a: nvmem: layouts: store owner from modules with nvmem_layout_driver_register()
> > Cc: stable@vger.kernel.org # 6.6.x: 5f15811: nvmem: layouts: add U-Boot env layout
> > Cc: stable@vger.kernel.org # 6.6.x
> 
> That's a load of (short) git ids for just 6.6.y?  What about 6.1.y?

And really, ALL of those commits are needed for this very tiny patch?
Reverting a config option?  sysfs apis being added?  Huh?

confused,

greg k-h

Re: [PATCH 1/2] nvmem: layouts: u-boot-env: remove crc32 endianness conversion

[Srinivas Kandagatla]

On 7/13/25 4:42 PM, Greg KH wrote:
> On Sun, Jul 13, 2025 at 05:41:45PM +0200, Greg KH wrote:
>> On Sat, Jul 12, 2025 at 07:17:26PM +0100, srini@kernel.org wrote:
>>> From: "Michael C. Pratt" <mcpratt@pm.me>
>>>
>>> On 11 Oct 2022, it was reported that the crc32 verification
>>> of the u-boot environment failed only on big-endian systems
>>> for the u-boot-env nvmem layout driver with the following error.
>>>
>>>   Invalid calculated CRC32: 0x88cd6f09 (expected: 0x096fcd88)
>>>
>>> This problem has been present since the driver was introduced,
>>> and before it was made into a layout driver.
>>>
>>> The suggested fix at the time was to use further endianness
>>> conversion macros in order to have both the stored and calculated
>>> crc32 values to compare always represented in the system's endianness.
>>> This was not accepted due to sparse warnings
>>> and some disagreement on how to handle the situation.
>>> Later on in a newer revision of the patch, it was proposed to use
>>> cpu_to_le32() for both values to compare instead of le32_to_cpu()
>>> and store the values as __le32 type to remove compilation errors.
>>>
>>> The necessity of this is based on the assumption that the use of crc32()
>>> requires endianness conversion because the algorithm uses little-endian,
>>> however, this does not prove to be the case and the issue is unrelated.
>>>
>>> Upon inspecting the current kernel code,
>>> there already is an existing use of le32_to_cpu() in this driver,
>>> which suggests there already is special handling for big-endian systems,
>>> however, it is big-endian systems that have the problem.
>>>
>>> This, being the only functional difference between architectures
>>> in the driver combined with the fact that the suggested fix
>>> was to use the exact same endianness conversion for the values
>>> brings up the possibility that it was not necessary to begin with,
>>> as the same endianness conversion for two values expected to be the same
>>> is expected to be equivalent to no conversion at all.
>>>
>>> After inspecting the u-boot environment of devices of both endianness
>>> and trying to remove the existing endianness conversion,
>>> the problem is resolved in an equivalent way as the other suggested fixes.
>>>
>>> Ultimately, it seems that u-boot is agnostic to endianness
>>> at least for the purpose of environment variables.
>>> In other words, u-boot reads and writes the stored crc32 value
>>> with the same endianness that the crc32 value is calculated with
>>> in whichever endianness a certain architecture runs on.
>>>
>>> Therefore, the u-boot-env driver does not need to convert endianness.
>>> Remove the usage of endianness macros in the u-boot-env driver,
>>> and change the type of local variables to maintain the same return type.
>>>
>>> If there is a special situation in the case of endianness,
>>> it would be a corner case and should be handled by a unique "compatible".
>>>
>>> Even though it is not necessary to use endianness conversion macros here,
>>> it may be useful to use them in the future for consistent error printing.
>>>
>>> Fixes: d5542923f200 ("nvmem: add driver handling U-Boot environment variables")
>>
>> Note, this is a 6.1 commit id, but:
>>
>>> Reported-by: INAGAKI Hiroshi <musashino.open@gmail.com>
>>> Link: https://lore.kernel.org/all/20221011024928.1807-1-musashino.open@gmail.com
>>> Cc: stable@vger.kernel.org # 6.12.x
>>> Cc: stable@vger.kernel.org # 6.6.x: f4cf4e5: Revert "nvmem: add new config option"
>>> Cc: stable@vger.kernel.org # 6.6.x: 7f38b70: of: device: Export of_device_make_bus_id()
>>> Cc: stable@vger.kernel.org # 6.6.x: 4a1a402: nvmem: Move of_nvmem_layout_get_container() in another header
>>> Cc: stable@vger.kernel.org # 6.6.x: fc29fd8: nvmem: core: Rework layouts to become regular devices
>>> Cc: stable@vger.kernel.org # 6.6.x: 0331c61: nvmem: core: Expose cells through sysfs
>>> Cc: stable@vger.kernel.org # 6.6.x: 401df0d: nvmem: layouts: refactor .add_cells() callback arguments
>>> Cc: stable@vger.kernel.org # 6.6.x: 6d0ca4a: nvmem: layouts: store owner from modules with nvmem_layout_driver_register()
>>> Cc: stable@vger.kernel.org # 6.6.x: 5f15811: nvmem: layouts: add U-Boot env layout
>>> Cc: stable@vger.kernel.org # 6.6.x
>>
>> That's a load of (short) git ids for just 6.6.y?  What about 6.1.y?
> 
> And really, ALL of those commits are needed for this very tiny patch?
May be not, AFAIU  Fixes: d5542923f200 ("nvmem: add driver handling
U-Boot environment variables") should be enough.

--srini
> Reverting a config option?  sysfs apis being added?  Huh?
> 
> confused,
> 
> greg k-h

Re: [PATCH 1/2] nvmem: layouts: u-boot-env: remove crc32 endianness conversion

[Greg KH]

On Tue, Jul 15, 2025 at 04:44:00PM +0100, Srinivas Kandagatla wrote:
> 
> 
> On 7/13/25 4:42 PM, Greg KH wrote:
> > On Sun, Jul 13, 2025 at 05:41:45PM +0200, Greg KH wrote:
> >> On Sat, Jul 12, 2025 at 07:17:26PM +0100, srini@kernel.org wrote:
> >>> From: "Michael C. Pratt" <mcpratt@pm.me>
> >>>
> >>> On 11 Oct 2022, it was reported that the crc32 verification
> >>> of the u-boot environment failed only on big-endian systems
> >>> for the u-boot-env nvmem layout driver with the following error.
> >>>
> >>>   Invalid calculated CRC32: 0x88cd6f09 (expected: 0x096fcd88)
> >>>
> >>> This problem has been present since the driver was introduced,
> >>> and before it was made into a layout driver.
> >>>
> >>> The suggested fix at the time was to use further endianness
> >>> conversion macros in order to have both the stored and calculated
> >>> crc32 values to compare always represented in the system's endianness.
> >>> This was not accepted due to sparse warnings
> >>> and some disagreement on how to handle the situation.
> >>> Later on in a newer revision of the patch, it was proposed to use
> >>> cpu_to_le32() for both values to compare instead of le32_to_cpu()
> >>> and store the values as __le32 type to remove compilation errors.
> >>>
> >>> The necessity of this is based on the assumption that the use of crc32()
> >>> requires endianness conversion because the algorithm uses little-endian,
> >>> however, this does not prove to be the case and the issue is unrelated.
> >>>
> >>> Upon inspecting the current kernel code,
> >>> there already is an existing use of le32_to_cpu() in this driver,
> >>> which suggests there already is special handling for big-endian systems,
> >>> however, it is big-endian systems that have the problem.
> >>>
> >>> This, being the only functional difference between architectures
> >>> in the driver combined with the fact that the suggested fix
> >>> was to use the exact same endianness conversion for the values
> >>> brings up the possibility that it was not necessary to begin with,
> >>> as the same endianness conversion for two values expected to be the same
> >>> is expected to be equivalent to no conversion at all.
> >>>
> >>> After inspecting the u-boot environment of devices of both endianness
> >>> and trying to remove the existing endianness conversion,
> >>> the problem is resolved in an equivalent way as the other suggested fixes.
> >>>
> >>> Ultimately, it seems that u-boot is agnostic to endianness
> >>> at least for the purpose of environment variables.
> >>> In other words, u-boot reads and writes the stored crc32 value
> >>> with the same endianness that the crc32 value is calculated with
> >>> in whichever endianness a certain architecture runs on.
> >>>
> >>> Therefore, the u-boot-env driver does not need to convert endianness.
> >>> Remove the usage of endianness macros in the u-boot-env driver,
> >>> and change the type of local variables to maintain the same return type.
> >>>
> >>> If there is a special situation in the case of endianness,
> >>> it would be a corner case and should be handled by a unique "compatible".
> >>>
> >>> Even though it is not necessary to use endianness conversion macros here,
> >>> it may be useful to use them in the future for consistent error printing.
> >>>
> >>> Fixes: d5542923f200 ("nvmem: add driver handling U-Boot environment variables")
> >>
> >> Note, this is a 6.1 commit id, but:
> >>
> >>> Reported-by: INAGAKI Hiroshi <musashino.open@gmail.com>
> >>> Link: https://lore.kernel.org/all/20221011024928.1807-1-musashino.open@gmail.com
> >>> Cc: stable@vger.kernel.org # 6.12.x
> >>> Cc: stable@vger.kernel.org # 6.6.x: f4cf4e5: Revert "nvmem: add new config option"
> >>> Cc: stable@vger.kernel.org # 6.6.x: 7f38b70: of: device: Export of_device_make_bus_id()
> >>> Cc: stable@vger.kernel.org # 6.6.x: 4a1a402: nvmem: Move of_nvmem_layout_get_container() in another header
> >>> Cc: stable@vger.kernel.org # 6.6.x: fc29fd8: nvmem: core: Rework layouts to become regular devices
> >>> Cc: stable@vger.kernel.org # 6.6.x: 0331c61: nvmem: core: Expose cells through sysfs
> >>> Cc: stable@vger.kernel.org # 6.6.x: 401df0d: nvmem: layouts: refactor .add_cells() callback arguments
> >>> Cc: stable@vger.kernel.org # 6.6.x: 6d0ca4a: nvmem: layouts: store owner from modules with nvmem_layout_driver_register()
> >>> Cc: stable@vger.kernel.org # 6.6.x: 5f15811: nvmem: layouts: add U-Boot env layout
> >>> Cc: stable@vger.kernel.org # 6.6.x
> >>
> >> That's a load of (short) git ids for just 6.6.y?  What about 6.1.y?
> > 
> > And really, ALL of those commits are needed for this very tiny patch?
> May be not, AFAIU  Fixes: d5542923f200 ("nvmem: add driver handling
> U-Boot environment variables") should be enough.

Great, can you fix this up and resend?

thanks,

greg k-h

Re: [PATCH 1/2] nvmem: layouts: u-boot-env: remove crc32 endianness conversion

[Michael Pratt]

Hi Greg and Srinivas,

Sorry for the delayed response.

On Sunday, July 13th, 2025 at 11:42, Greg KH <gregkh@linuxfoundation.org> wrote:

> 
> 
> On Sun, Jul 13, 2025 at 05:41:45PM +0200, Greg KH wrote:
> 
> > On Sat, Jul 12, 2025 at 07:17:26PM +0100, srini@kernel.org wrote:
> > 
> > > From: "Michael C. Pratt" mcpratt@pm.me
> > > 
> > > On 11 Oct 2022, it was reported that the crc32 verification
> > > of the u-boot environment failed only on big-endian systems
> > > for the u-boot-env nvmem layout driver with the following error.
> > > 
> > > Invalid calculated CRC32: 0x88cd6f09 (expected: 0x096fcd88)
> > > 
> > > This problem has been present since the driver was introduced,
> > > and before it was made into a layout driver.
> > > 
> > > The suggested fix at the time was to use further endianness
> > > conversion macros in order to have both the stored and calculated
> > > crc32 values to compare always represented in the system's endianness.
> > > This was not accepted due to sparse warnings
> > > and some disagreement on how to handle the situation.
> > > Later on in a newer revision of the patch, it was proposed to use
> > > cpu_to_le32() for both values to compare instead of le32_to_cpu()
> > > and store the values as __le32 type to remove compilation errors.
> > > 
> > > The necessity of this is based on the assumption that the use of crc32()
> > > requires endianness conversion because the algorithm uses little-endian,
> > > however, this does not prove to be the case and the issue is unrelated.
> > > 
> > > Upon inspecting the current kernel code,
> > > there already is an existing use of le32_to_cpu() in this driver,
> > > which suggests there already is special handling for big-endian systems,
> > > however, it is big-endian systems that have the problem.
> > > 
> > > This, being the only functional difference between architectures
> > > in the driver combined with the fact that the suggested fix
> > > was to use the exact same endianness conversion for the values
> > > brings up the possibility that it was not necessary to begin with,
> > > as the same endianness conversion for two values expected to be the same
> > > is expected to be equivalent to no conversion at all.
> > > 
> > > After inspecting the u-boot environment of devices of both endianness
> > > and trying to remove the existing endianness conversion,
> > > the problem is resolved in an equivalent way as the other suggested fixes.
> > > 
> > > Ultimately, it seems that u-boot is agnostic to endianness
> > > at least for the purpose of environment variables.
> > > In other words, u-boot reads and writes the stored crc32 value
> > > with the same endianness that the crc32 value is calculated with
> > > in whichever endianness a certain architecture runs on.
> > > 
> > > Therefore, the u-boot-env driver does not need to convert endianness.
> > > Remove the usage of endianness macros in the u-boot-env driver,
> > > and change the type of local variables to maintain the same return type.
> > > 
> > > If there is a special situation in the case of endianness,
> > > it would be a corner case and should be handled by a unique "compatible".
> > > 
> > > Even though it is not necessary to use endianness conversion macros here,
> > > it may be useful to use them in the future for consistent error printing.
> > > 
> > > Fixes: d5542923f200 ("nvmem: add driver handling U-Boot environment variables")
> > 
> > Note, this is a 6.1 commit id, but:
> > 
> > > Reported-by: INAGAKI Hiroshi musashino.open@gmail.com
> > > Link: https://lore.kernel.org/all/20221011024928.1807-1-musashino.open@gmail.com
> > > Cc: stable@vger.kernel.org # 6.12.x
> > > Cc: stable@vger.kernel.org # 6.6.x: f4cf4e5: Revert "nvmem: add new config option"
> > > Cc: stable@vger.kernel.org # 6.6.x: 7f38b70: of: device: Export of_device_make_bus_id()
> > > Cc: stable@vger.kernel.org # 6.6.x: 4a1a402: nvmem: Move of_nvmem_layout_get_container() in another header
> > > Cc: stable@vger.kernel.org # 6.6.x: fc29fd8: nvmem: core: Rework layouts to become regular devices
> > > Cc: stable@vger.kernel.org # 6.6.x: 0331c61: nvmem: core: Expose cells through sysfs
> > > Cc: stable@vger.kernel.org # 6.6.x: 401df0d: nvmem: layouts: refactor .add_cells() callback arguments
> > > Cc: stable@vger.kernel.org # 6.6.x: 6d0ca4a: nvmem: layouts: store owner from modules with nvmem_layout_driver_register()
> > > Cc: stable@vger.kernel.org # 6.6.x: 5f15811: nvmem: layouts: add U-Boot env layout
> > > Cc: stable@vger.kernel.org # 6.6.x
> > 
> > That's a load of (short) git ids for just 6.6.y? What about 6.1.y?
> 

Sorry for the short tags, I wrongly assumed that what Github provides
would not clobber with other commits.

> 
> And really, ALL of those commits are needed for this very tiny patch?

Yes... if we would like to backport to 6.6, (almost) all of them are necessary.
There was a lot of development between 6.6 and 6.12 in this area...

This is a long-standing problem since 6.1, but the code is now
completely rewritten into a different file, as a "layout driver"
instead of a "cell module" if that makes sense...

In order to backport to 6.6, we would have to backport
the rewriting of the code to the new layout driver form,
which is commit 5f1581128 ("nvmem: layouts: add U-Boot env layout").

Commit 5f1581128 depends on commit 401df0d4f, which strictly depends on
commit fc29fd821, and lightly depends (merge conflict) on commit 0331c6119.

Commit fc29fd821 strictly depends on both commit 4a1a40233 and 7f38b7004 for functionality.

I additionally included commit 6d0ca4a2a as it seems to improve function for all layout drivers,
and I additionally included commit f4cf4e5db simply because we also backport it,
not thinking that an extra one would be a problem.

In summary, the exact set of commits I presented for backporting is well tested,
but one or two are indeed not strictly necessary as you pointed out.

> Reverting a config option? sysfs apis being added? Huh?

If you prefer, you can skip backporting commits f4cf4e5db and 0331c6119,
although, skipping the latter would make you have to resolve the merge conflict.

If backporting to 6.6 is no longer appropriate in your opinion,
please at least backport to 6.12 which is very easy.

Perhaps this would be a better set of tags for example:

# 6.12.x
# 6.6.x: 7f38b7004: of: device: Export of_device_make_bus_id()
# 6.6.x: 4a1a40233: nvmem: Move of_nvmem_layout_get_container() in another header
# 6.6.x: fc29fd821: nvmem: core: Rework layouts to become regular devices
# 6.6.x: 401df0d4f: nvmem: layouts: refactor .add_cells() callback arguments
# 6.6.x: 6d0ca4a2a: nvmem: layouts: store owner from modules with nvmem_layout_driver_register()
# 6.6.x: 5f1581128: nvmem: layouts: add U-Boot env layout
# 6.6.x


> 
> confused,

Sorry for the confusion, thanks.

--
MCP

Re: [PATCH 1/2] nvmem: layouts: u-boot-env: remove crc32 endianness conversion

[Greg KH]

On Wed, Jul 16, 2025 at 09:24:10PM +0000, Michael Pratt wrote:
> Hi Greg and Srinivas,
> 
> Sorry for the delayed response.
> 
> On Sunday, July 13th, 2025 at 11:42, Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > 
> > 
> > On Sun, Jul 13, 2025 at 05:41:45PM +0200, Greg KH wrote:
> > 
> > > On Sat, Jul 12, 2025 at 07:17:26PM +0100, srini@kernel.org wrote:
> > > 
> > > > From: "Michael C. Pratt" mcpratt@pm.me
> > > > 
> > > > On 11 Oct 2022, it was reported that the crc32 verification
> > > > of the u-boot environment failed only on big-endian systems
> > > > for the u-boot-env nvmem layout driver with the following error.
> > > > 
> > > > Invalid calculated CRC32: 0x88cd6f09 (expected: 0x096fcd88)
> > > > 
> > > > This problem has been present since the driver was introduced,
> > > > and before it was made into a layout driver.
> > > > 
> > > > The suggested fix at the time was to use further endianness
> > > > conversion macros in order to have both the stored and calculated
> > > > crc32 values to compare always represented in the system's endianness.
> > > > This was not accepted due to sparse warnings
> > > > and some disagreement on how to handle the situation.
> > > > Later on in a newer revision of the patch, it was proposed to use
> > > > cpu_to_le32() for both values to compare instead of le32_to_cpu()
> > > > and store the values as __le32 type to remove compilation errors.
> > > > 
> > > > The necessity of this is based on the assumption that the use of crc32()
> > > > requires endianness conversion because the algorithm uses little-endian,
> > > > however, this does not prove to be the case and the issue is unrelated.
> > > > 
> > > > Upon inspecting the current kernel code,
> > > > there already is an existing use of le32_to_cpu() in this driver,
> > > > which suggests there already is special handling for big-endian systems,
> > > > however, it is big-endian systems that have the problem.
> > > > 
> > > > This, being the only functional difference between architectures
> > > > in the driver combined with the fact that the suggested fix
> > > > was to use the exact same endianness conversion for the values
> > > > brings up the possibility that it was not necessary to begin with,
> > > > as the same endianness conversion for two values expected to be the same
> > > > is expected to be equivalent to no conversion at all.
> > > > 
> > > > After inspecting the u-boot environment of devices of both endianness
> > > > and trying to remove the existing endianness conversion,
> > > > the problem is resolved in an equivalent way as the other suggested fixes.
> > > > 
> > > > Ultimately, it seems that u-boot is agnostic to endianness
> > > > at least for the purpose of environment variables.
> > > > In other words, u-boot reads and writes the stored crc32 value
> > > > with the same endianness that the crc32 value is calculated with
> > > > in whichever endianness a certain architecture runs on.
> > > > 
> > > > Therefore, the u-boot-env driver does not need to convert endianness.
> > > > Remove the usage of endianness macros in the u-boot-env driver,
> > > > and change the type of local variables to maintain the same return type.
> > > > 
> > > > If there is a special situation in the case of endianness,
> > > > it would be a corner case and should be handled by a unique "compatible".
> > > > 
> > > > Even though it is not necessary to use endianness conversion macros here,
> > > > it may be useful to use them in the future for consistent error printing.
> > > > 
> > > > Fixes: d5542923f200 ("nvmem: add driver handling U-Boot environment variables")
> > > 
> > > Note, this is a 6.1 commit id, but:
> > > 
> > > > Reported-by: INAGAKI Hiroshi musashino.open@gmail.com
> > > > Link: https://lore.kernel.org/all/20221011024928.1807-1-musashino.open@gmail.com
> > > > Cc: stable@vger.kernel.org # 6.12.x
> > > > Cc: stable@vger.kernel.org # 6.6.x: f4cf4e5: Revert "nvmem: add new config option"
> > > > Cc: stable@vger.kernel.org # 6.6.x: 7f38b70: of: device: Export of_device_make_bus_id()
> > > > Cc: stable@vger.kernel.org # 6.6.x: 4a1a402: nvmem: Move of_nvmem_layout_get_container() in another header
> > > > Cc: stable@vger.kernel.org # 6.6.x: fc29fd8: nvmem: core: Rework layouts to become regular devices
> > > > Cc: stable@vger.kernel.org # 6.6.x: 0331c61: nvmem: core: Expose cells through sysfs
> > > > Cc: stable@vger.kernel.org # 6.6.x: 401df0d: nvmem: layouts: refactor .add_cells() callback arguments
> > > > Cc: stable@vger.kernel.org # 6.6.x: 6d0ca4a: nvmem: layouts: store owner from modules with nvmem_layout_driver_register()
> > > > Cc: stable@vger.kernel.org # 6.6.x: 5f15811: nvmem: layouts: add U-Boot env layout
> > > > Cc: stable@vger.kernel.org # 6.6.x
> > > 
> > > That's a load of (short) git ids for just 6.6.y? What about 6.1.y?
> > 
> 
> Sorry for the short tags, I wrongly assumed that what Github provides
> would not clobber with other commits.
> 
> > 
> > And really, ALL of those commits are needed for this very tiny patch?
> 
> Yes... if we would like to backport to 6.6, (almost) all of them are necessary.
> There was a lot of development between 6.6 and 6.12 in this area...
> 
> This is a long-standing problem since 6.1, but the code is now
> completely rewritten into a different file, as a "layout driver"
> instead of a "cell module" if that makes sense...
> 
> In order to backport to 6.6, we would have to backport
> the rewriting of the code to the new layout driver form,
> which is commit 5f1581128 ("nvmem: layouts: add U-Boot env layout").
> 
> Commit 5f1581128 depends on commit 401df0d4f, which strictly depends on
> commit fc29fd821, and lightly depends (merge conflict) on commit 0331c6119.
> 
> Commit fc29fd821 strictly depends on both commit 4a1a40233 and 7f38b7004 for functionality.
> 
> I additionally included commit 6d0ca4a2a as it seems to improve function for all layout drivers,
> and I additionally included commit f4cf4e5db simply because we also backport it,
> not thinking that an extra one would be a problem.
> 
> In summary, the exact set of commits I presented for backporting is well tested,
> but one or two are indeed not strictly necessary as you pointed out.
> 
> > Reverting a config option? sysfs apis being added? Huh?
> 
> If you prefer, you can skip backporting commits f4cf4e5db and 0331c6119,
> although, skipping the latter would make you have to resolve the merge conflict.
> 
> If backporting to 6.6 is no longer appropriate in your opinion,
> please at least backport to 6.12 which is very easy.

I think that's already done, right?

If not, I'm still confused, can you send patch series for the different
stable branches showing what you want applied where?

thanks,

greg k-h

[PATCH 2/2] nvmem: imx-ocotp: fix MAC address byte length

[srini]

From: Steffen Bätz <steffen@innosonix.de>

The commit "13bcd440f2ff nvmem: core: verify cell's raw_len" caused an
extension of the "mac-address" cell from 6 to 8 bytes due to word_size
of 4 bytes. This led to a required byte swap of the full buffer length,
which caused truncation of the mac-address when read.

Previously, the mac-address was incorrectly truncated from
70:B3:D5:14:E9:0E to 00:00:70:B3:D5:14.

Fix the issue by swapping only the first 6 bytes to correctly pass the
mac-address to the upper layers.

Fixes: 13bcd440f2ff ("nvmem: core: verify cell's raw_len")
Cc: stable@vger.kernel.org
Signed-off-by: Steffen Bätz <steffen@innosonix.de>
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Signed-off-by: Srinivas Kandagatla <srini@kernel.org>
---
 drivers/nvmem/imx-ocotp-ele.c | 5 ++++-
 drivers/nvmem/imx-ocotp.c     | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/nvmem/imx-ocotp-ele.c b/drivers/nvmem/imx-ocotp-ele.c
index ca6dd71d8a2e..7807ec0e2d18 100644
--- a/drivers/nvmem/imx-ocotp-ele.c
+++ b/drivers/nvmem/imx-ocotp-ele.c
@@ -12,6 +12,7 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/if_ether.h>	/* ETH_ALEN */
 
 enum fuse_type {
 	FUSE_FSB = BIT(0),
@@ -118,9 +119,11 @@ static int imx_ocotp_cell_pp(void *context, const char *id, int index,
 	int i;
 
 	/* Deal with some post processing of nvmem cell data */
-	if (id && !strcmp(id, "mac-address"))
+	if (id && !strcmp(id, "mac-address")) {
+		bytes = min(bytes, ETH_ALEN);
 		for (i = 0; i < bytes / 2; i++)
 			swap(buf[i], buf[bytes - i - 1]);
+	}
 
 	return 0;
 }
diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 79dd4fda0329..7bf7656d4f96 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
+#include <linux/if_ether.h>	/* ETH_ALEN */
 
 #define IMX_OCOTP_OFFSET_B0W0		0x400 /* Offset from base address of the
 					       * OTP Bank0 Word0
@@ -227,9 +228,11 @@ static int imx_ocotp_cell_pp(void *context, const char *id, int index,
 	int i;
 
 	/* Deal with some post processing of nvmem cell data */
-	if (id && !strcmp(id, "mac-address"))
+	if (id && !strcmp(id, "mac-address")) {
+		bytes = min(bytes, ETH_ALEN);
 		for (i = 0; i < bytes / 2; i++)
 			swap(buf[i], buf[bytes - i - 1]);
+	}
 
 	return 0;
 }
-- 
2.43.0