On Tue, May 19, 2026 at 12:47:31PM +0200, Thomas Weißschuh wrote:
> On Tue, May 19, 2026 at 12:20:14PM +0200, Gregor Herburger wrote:
> > On Tue, May 19, 2026 at 11:14:28AM +0200, Thomas Weißschuh wrote:
> > > On Fri, May 08, 2026 at 04:42:45PM +0200, Gregor Herburger wrote:
> > > > +config NVMEM_RASPBERRYPI_OTP
> > > > + tristate "Raspberry Pi OTP support"
> > > > + depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST &&
> > > > !RASPBERRYPI_FIRMWARE)
> > >
> > > The '&& !RASPBERRYPI_FIRMWARE' clause looks weird, is it really necessary?
> >
> > Yes it does looks weird but I think it is necessary. Without this it would
> > be
> > possible to build RASPBERRYPI_FIRMWARE=m and NVMEM_RASPBERRYPI_OTP=y which
> > results in linker errors.
>
> Fair enough. I would prefer the solution below, though.
> It would cleanly solve the issues also for other (future) drivers.
>
> diff --git a/include/soc/bcm2835/raspberrypi-firmware.h
> b/include/soc/bcm2835/raspberrypi-firmware.h
> index 17595a96e90b..0efd479ffced 100644
> --- a/include/soc/bcm2835/raspberrypi-firmware.h
> +++ b/include/soc/bcm2835/raspberrypi-firmware.h
> @@ -188,7 +188,7 @@ struct rpi_otp_driver_data {
> int size;
> };
>
> -#if IS_ENABLED(CONFIG_RASPBERRYPI_FIRMWARE)
> +#if IS_REACHABLE(CONFIG_RASPBERRYPI_FIRMWARE)
> int rpi_firmware_property(struct rpi_firmware *fw,
> u32 tag, void *data, size_t len);
> int rpi_firmware_property_list(struct rpi_firmware *fw,
>
> (...)
>
Yes this should work. Will change it in the next version.
> > > > +static int rpi_otp_read(void *context, unsigned int offset, void *buf,
> > > > size_t bytes)
> > > > +{
> > > > + struct rpi_otp_priv *priv = context;
> > > > + struct rpi_otp_header *fwbuf;
> > > > + u32 count;
> > > > + int ret;
> > > > +
> > > > + if (!IS_ALIGNED(offset, 4) || !IS_ALIGNED(bytes, 4))
> > > > + return -EINVAL;
> > >
> > > Isn't this already enforced by the nvmem core?
> >
> > Only for sysfs access through bin_attr_nvmem_read/bin_attr_nvmem_write. But
> > there is an in-kernel API nvmem_device_read/nvmem_device_write which does
> > not
> > have alignment checks. So I added the check to be more defensive here.
>
> The other drivers don't seem to check this explicitly. It looks like an
> accident waiting to happen.
Indeed, I will have a look at nvmem core and see if I can find a proper solution
for this.
Gregor
