Re: [PATCH v2 1/1] nvmem: Change return type of reg read/write to ssize_t
From: Joy Chakraborty
Date: Wed Apr 24 2024 - 06:19:02 EST
On Wed, Apr 24, 2024 at 3:11 PM Andre Przywara <andre.przywara@xxxxxxx> wrote:
>
> On Wed, 24 Apr 2024 07:42:42 +0000
> Joy Chakraborty <joychakr@xxxxxxxxxx> 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@xxxxxxxxxx>
>
> 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,
>