Re: [PATCH v2 0/2] nvmem: fix a const-unsoundness in reg_write

[Link Mauve]

 On Thu, Jul 16, 2026 at 12:03:17PM +0300, Andy Shevchenko wrote:
> On Wed, Jul 15, 2026 at 09:55:16PM +0200, Link Mauve wrote:
> > This callback used to take a mutable void * for no reason, which causes
> > the compiler to be unaware that the val buffer should never be modified
> > by the callback.
> > 
> > This was found while drafting the nvmem-provider Rust abstraction.
> > 
> > Thanks to the guidance of Andy Shevchenko, this now introduces a new
> > callback and deprecates the existing one, with the goal of renaming the
> > new one into the old one once no user remains in the kernel.
> 
> You forgot to use --base. It's unclear against what should be this applied.
> I tried Linux Next (next-20260715), and it fails.
> 
> Yes, it applies against v7.2-rc3, but it means that this won't be applied on
> top of maintainer's tree (which has something already that you have to take
> into consideration).

Indeed I developed against Linus’s master, I’ll rebase on linux-next for
v3.

> 
> For the record, the first version of the series was no go as the first patch
> there breaks the things, like
> 
> drivers/nvmem/qfprom.c:446:22: error: incompatible function pointer types assigning to 'nvmem_reg_write_t' (aka 'int (*)(void *, unsigned int, const void *, unsigned long)') from 'int (void *, unsigned int, void *, size_t)' (aka 'int (void *, unsigned int, void *, unsigned long)') [-Wincompatible-function-pointer-types]
>   446 |                         econfig.reg_write = qfprom_reg_write;
>       |                                           ^ ~~~~~~~~~~~~~~~~
> 1 error generated.
> 
> This version doesn't have this issue (at least with my smoke build tests
> on x86_64).

Yup, I enabled the drivers but didn’t select COMPILE_TEST, so they
didn’t actually build and I assumed everything was correct…  This won’t
happen again.

> 
> Now, what catches me is that regmap_bulk_read() proto used for both cases in
> drivers/nvmem/apple-spmi-nvmem.c without any changes. Which makes me think
> that the approach can be done in a simpler way, id est converting users first
> to use const specifiers in their callbacks first. But this trick is done with
> using (void *) casting (?) which makes warning to disappear, which is
> interesting case. So I think the Apple driver should actually use proper
> protos and hence wrappers, otherwise it makes compiler blind, which is not
> good. TL;DR: you should fix the Apple driver (and might more if any of them
> use that dirty trick).

This series only affects the reg_write callback, not reg_read, so it’s
to be expected that regmap_bulk_read() keeps its void * parameter as it
will modify it, it’s regmap_bulk_write() which correctly takes const
void *.  The Apple driver completely removes all function pointer safety
by casting the function into void *, although that’s not due to const
incompatibility but due to the first argument being struct regmap *
instead of void *.

I’ve attached a patch fixing this particular issue, I will include it in
my v3 if that’s ok with you (or it could go as a different series, I
don’t care much).

I think I’ve found another bug in that driver, it says .max_register =
0xffff but .size = 0xffff, one or the other is probably off-by-one, and
I suspect .size should be 0x10000 instead.

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Thanks!

-- 
Link Mauve