[PATCH v2 1/1] nvmem: Change return type of reg read/write to ssize_t

Joy Chakraborty joychakr at google.com
Wed Apr 24 03:18:32 PDT 2024


On Wed, Apr 24, 2024 at 3:11 PM Andre Przywara <andre.przywara at arm.com> wrote:
>
> On Wed, 24 Apr 2024 07:42:42 +0000
> Joy Chakraborty <joychakr at google.com> wrote:
>
> > Change return type of reg_read() and reg_write() callback to ssize_t for
> > nvmem suppliers to return number of bytes read/written to the nvmem core.
> >
> > Currently nvmem core assumes the amount of data read/written is equal
> > to what it has requested from the supplier, this return code facilitates
> > better error handling in the nvmem core.
> >
> > Signed-off-by: Joy Chakraborty <joychakr at google.com>
>
> There are two problems in the sunxi driver:
>
> > ---
> >  drivers/nvmem/apple-efuses.c        |  7 +--
> >  drivers/nvmem/bcm-ocotp.c           | 12 ++---
> >  drivers/nvmem/brcm_nvram.c          | 10 ++--
> >  drivers/nvmem/core.c                | 83 +++++++++++++----------------
> >  drivers/nvmem/imx-iim.c             |  6 +--
> >  drivers/nvmem/imx-ocotp-ele.c       |  4 +-
> >  drivers/nvmem/imx-ocotp-scu.c       | 12 ++---
> >  drivers/nvmem/imx-ocotp.c           | 10 ++--
> >  drivers/nvmem/jz4780-efuse.c        |  7 +--
> >  drivers/nvmem/lan9662-otpc.c        | 12 ++---
> >  drivers/nvmem/layerscape-sfp.c      | 11 ++--
> >  drivers/nvmem/lpc18xx_eeprom.c      | 14 ++---
> >  drivers/nvmem/lpc18xx_otp.c         |  6 +--
> >  drivers/nvmem/meson-efuse.c         | 22 +++++---
> >  drivers/nvmem/meson-mx-efuse.c      |  6 +--
> >  drivers/nvmem/microchip-otpc.c      |  6 +--
> >  drivers/nvmem/mtk-efuse.c           |  6 +--
> >  drivers/nvmem/mxs-ocotp.c           |  7 +--
> >  drivers/nvmem/nintendo-otp.c        |  6 +--
> >  drivers/nvmem/qcom-spmi-sdam.c      | 12 ++---
> >  drivers/nvmem/qfprom.c              | 14 ++---
> >  drivers/nvmem/qoriq-efuse.c         |  6 +--
> >  drivers/nvmem/rave-sp-eeprom.c      | 18 +++----
> >  drivers/nvmem/rmem.c                |  4 +-
> >  drivers/nvmem/rockchip-efuse.c      | 19 +++----
> >  drivers/nvmem/rockchip-otp.c        | 19 +++----
> >  drivers/nvmem/sc27xx-efuse.c        |  3 +-
> >  drivers/nvmem/sec-qfprom.c          |  4 +-
> >  drivers/nvmem/snvs_lpgpr.c          | 17 +++---
> >  drivers/nvmem/sprd-efuse.c          |  8 +--
> >  drivers/nvmem/stm32-bsec-optee-ta.c | 12 ++---
> >  drivers/nvmem/stm32-bsec-optee-ta.h | 20 +++----
> >  drivers/nvmem/stm32-romem.c         | 26 ++++-----
> >  drivers/nvmem/sunplus-ocotp.c       |  4 +-
> >  drivers/nvmem/sunxi_sid.c           | 15 +++---
> >  drivers/nvmem/u-boot-env.c          |  6 +--
> >  drivers/nvmem/uniphier-efuse.c      |  6 +--
> >  drivers/nvmem/vf610-ocotp.c         |  7 +--
> >  drivers/nvmem/zynqmp_nvmem.c        | 13 ++---
> >  include/linux/nvmem-provider.h      |  4 +-
> >  40 files changed, 253 insertions(+), 231 deletions(-)
>
> [ ... ]
>
> >
> > diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
> > index ba14a76208ab..0133263d2adb 100644
> > --- a/drivers/nvmem/sunxi_sid.c
> > +++ b/drivers/nvmem/sunxi_sid.c
> > @@ -36,8 +36,8 @@ struct sunxi_sid {
> >       u32                     value_offset;
> >  };
> >
> > -static int sunxi_sid_read(void *context, unsigned int offset,
> > -                       void *val, size_t bytes)
> > +static ssize_t sunxi_sid_read(void *context, unsigned int offset,
> > +                           void *val, size_t bytes)
> >  {
> >       struct sunxi_sid *sid = context;
> >       u32 word;
> > @@ -56,7 +56,7 @@ static int sunxi_sid_read(void *context, unsigned int offset,
>
>         (adding more context here)
>
> >
> >       val += round_down(bytes, 4);
> >       offset += round_down(bytes, 4);
> >       bytes = bytes % 4;
> >
> >       if (!bytes)
> >               return 0;
> >
> >       /* Handle any trailing bytes */
> >       word = readl_relaxed(sid->base + sid->value_offset + offset);
> >       memcpy(val, &word, bytes);
> >
> > -     return 0;
> > +     return bytes;
>
> So this is only the code path in case the read request was not 4 byte
> aligned, so the "return 0;" above must also be changed. But please note
> that the bytes parameter is changed, so we either need to save that, or
> derive the amount read from something else.
>
> Cheers,
> Andre

Ack, Missed the return logic above.
Will fix this next version by saving bytes read in another variable.
Thanks for reviewing.

Thanks
Joy

>
> >  }
> >
> >  static int sun8i_sid_register_readout(const struct sunxi_sid *sid,
> > @@ -90,10 +90,11 @@ static int sun8i_sid_register_readout(const struct sunxi_sid *sid,
> >   * to be not reliable at all.
> >   * Read by the registers instead.
> >   */
> > -static int sun8i_sid_read_by_reg(void *context, unsigned int offset,
> > -                              void *val, size_t bytes)
> > +static ssize_t sun8i_sid_read_by_reg(void *context, unsigned int offset,
> > +                                  void *val, size_t bytes)
> >  {
> >       struct sunxi_sid *sid = context;
> > +     size_t bytes_read = bytes;
> >       u32 word;
> >       int ret;
> >
> > @@ -109,7 +110,7 @@ static int sun8i_sid_read_by_reg(void *context, unsigned int offset,
> >       }
> >
> >       if (!bytes)
> > -             return 0;
> > +             return bytes_read;
> >
> >       /* Handle any trailing bytes */
> >       ret = sun8i_sid_register_readout(sid, offset, &word);
> > @@ -118,7 +119,7 @@ static int sun8i_sid_read_by_reg(void *context, unsigned int offset,
> >
> >       memcpy(val, &word, bytes);
> >
> > -     return 0;
> > +     return bytes_read;
> >  }
> >
> >  static int sunxi_sid_probe(struct platform_device *pdev)
> > diff --git a/drivers/nvmem/u-boot-env.c b/drivers/nvmem/u-boot-env.c
> > index befbab156cda..2288a2891bb2 100644
> > --- a/drivers/nvmem/u-boot-env.c
> > +++ b/drivers/nvmem/u-boot-env.c
> > @@ -47,8 +47,8 @@ struct u_boot_env_image_broadcom {
> >       DECLARE_FLEX_ARRAY(uint8_t, data);
> >  } __packed;
> >
> > -static int u_boot_env_read(void *context, unsigned int offset, void *val,
> > -                        size_t bytes)
> > +static ssize_t u_boot_env_read(void *context, unsigned int offset, void *val,
> > +                            size_t bytes)
> >  {
> >       struct u_boot_env *priv = context;
> >       struct device *dev = priv->dev;
> > @@ -66,7 +66,7 @@ static int u_boot_env_read(void *context, unsigned int offset, void *val,
> >               return -EIO;
> >       }
> >
> > -     return 0;
> > +     return bytes_read;
> >  }
> >
> >  static int u_boot_env_read_post_process_ethaddr(void *context, const char *id, int index,
> > diff --git a/drivers/nvmem/uniphier-efuse.c b/drivers/nvmem/uniphier-efuse.c
> > index 6ad3295d3195..a6c28e03adc2 100644
> > --- a/drivers/nvmem/uniphier-efuse.c
> > +++ b/drivers/nvmem/uniphier-efuse.c
> > @@ -16,8 +16,8 @@ struct uniphier_efuse_priv {
> >       void __iomem *base;
> >  };
> >
> > -static int uniphier_reg_read(void *context,
> > -                          unsigned int reg, void *_val, size_t bytes)
> > +static ssize_t uniphier_reg_read(void *context,
> > +                              unsigned int reg, void *_val, size_t bytes)
> >  {
> >       struct uniphier_efuse_priv *priv = context;
> >       u8 *val = _val;
> > @@ -26,7 +26,7 @@ static int uniphier_reg_read(void *context,
> >       for (offs = 0; offs < bytes; offs += sizeof(u8))
> >               *val++ = readb(priv->base + reg + offs);
> >
> > -     return 0;
> > +     return bytes;
> >  }
> >
> >  static int uniphier_efuse_probe(struct platform_device *pdev)
> > diff --git a/drivers/nvmem/vf610-ocotp.c b/drivers/nvmem/vf610-ocotp.c
> > index ee9c61ae727d..4e2bdb38305d 100644
> > --- a/drivers/nvmem/vf610-ocotp.c
> > +++ b/drivers/nvmem/vf610-ocotp.c
> > @@ -143,11 +143,12 @@ static int vf610_get_fuse_address(int base_addr_offset)
> >       return -EINVAL;
> >  }
> >
> > -static int vf610_ocotp_read(void *context, unsigned int offset,
> > -                     void *val, size_t bytes)
> > +static ssize_t vf610_ocotp_read(void *context, unsigned int offset,
> > +                             void *val, size_t bytes)
> >  {
> >       struct vf610_ocotp *ocotp = context;
> >       void __iomem *base = ocotp->base;
> > +     size_t bytes_read = bytes;
> >       u32 reg, *buf = val;
> >       int fuse_addr;
> >       int ret;
> > @@ -193,7 +194,7 @@ static int vf610_ocotp_read(void *context, unsigned int offset,
> >               offset += 4;
> >       }
> >
> > -     return 0;
> > +     return bytes_read;
> >  }
> >
> >  static struct nvmem_config ocotp_config = {
> > diff --git a/drivers/nvmem/zynqmp_nvmem.c b/drivers/nvmem/zynqmp_nvmem.c
> > index 8682adaacd69..1502d4998159 100644
> > --- a/drivers/nvmem/zynqmp_nvmem.c
> > +++ b/drivers/nvmem/zynqmp_nvmem.c
> > @@ -56,8 +56,8 @@ struct xilinx_efuse {
> >       u32 pufuserfuse;
> >  };
> >
> > -static int zynqmp_efuse_access(void *context, unsigned int offset,
> > -                            void *val, size_t bytes, enum efuse_access flag,
> > +static ssize_t zynqmp_efuse_access(void *context, unsigned int offset,
> > +                                void *val, size_t bytes, enum efuse_access flag,
> >                              unsigned int pufflag)
> >  {
> >       struct device *dev = context;
> > @@ -140,10 +140,10 @@ static int zynqmp_efuse_access(void *context, unsigned int offset,
> >       dma_free_coherent(dev, sizeof(struct xilinx_efuse),
> >                         efuse, dma_addr);
> >
> > -     return ret;
> > +     return ret < 0 ? ret : bytes;
> >  }
> >
> > -static int zynqmp_nvmem_read(void *context, unsigned int offset, void *val, size_t bytes)
> > +static ssize_t zynqmp_nvmem_read(void *context, unsigned int offset, void *val, size_t bytes)
> >  {
> >       struct device *dev = context;
> >       int ret;
> > @@ -166,6 +166,7 @@ static int zynqmp_nvmem_read(void *context, unsigned int offset, void *val, size
> >
> >               dev_dbg(dev, "Read chipid val %x %x\n", idcode, version);
> >               *(int *)val = version & SILICON_REVISION_MASK;
> > +             ret = SOC_VER_SIZE;
> >               break;
> >       /* Efuse offset starts from 0xc */
> >       case EFUSE_START_OFFSET ... EFUSE_END_OFFSET:
> > @@ -182,8 +183,8 @@ static int zynqmp_nvmem_read(void *context, unsigned int offset, void *val, size
> >       return ret;
> >  }
> >
> > -static int zynqmp_nvmem_write(void *context,
> > -                           unsigned int offset, void *val, size_t bytes)
> > +static ssize_t zynqmp_nvmem_write(void *context,
> > +                               unsigned int offset, void *val, size_t bytes)
> >  {
> >       int pufflag = 0;
> >
> > diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
> > index 3ebeaa0ded00..f7e83a59aa2f 100644
> > --- a/include/linux/nvmem-provider.h
> > +++ b/include/linux/nvmem-provider.h
> > @@ -16,9 +16,9 @@
> >  #include <linux/gpio/consumer.h>
> >
> >  struct nvmem_device;
> > -typedef int (*nvmem_reg_read_t)(void *priv, unsigned int offset,
> > +typedef ssize_t (*nvmem_reg_read_t)(void *priv, unsigned int offset,
> >                               void *val, size_t bytes);
> > -typedef int (*nvmem_reg_write_t)(void *priv, unsigned int offset,
> > +typedef ssize_t (*nvmem_reg_write_t)(void *priv, unsigned int offset,
> >                                void *val, size_t bytes);
> >  /* used for vendor specific post processing of cell data */
> >  typedef int (*nvmem_cell_post_process_t)(void *priv, const char *id, int index,
>



More information about the linux-arm-kernel mailing list