| Message ID | 20240424074242.519895-1-joychakr@google.com |
|---|---|
| Headers | show |
| Series | nvmem: Handle actual amount of data read/written by suppliers | expand |
On Wed, 24 Apr 2024 07:42:42 +0000 Joy Chakraborty <joychakr@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@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 > } > > 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,
On Wed, Apr 24, 2024 at 3:11 PM Andre Przywara <andre.przywara@arm.com> wrote: > > On Wed, 24 Apr 2024 07:42:42 +0000 > Joy Chakraborty <joychakr@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@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, >