This patch series facilitates compilation post the change in definition
of nvmem_reg_read_t and nvmem_reg_write_t callback in
https://lore.kernel.org/all/[email protected]/
Currently the nvmem core change is picked on
https://git.kernel.org/pub/scm/linux/kernel/git/srini/nvmem.git/log/?h=for-next
---
V1 Changes : Change read/write return type to ssize_t and handle
relevant logic changes
---
Joy Chakraborty (17):
hwmon: pmbus: adm1266: Change nvmem reg_read/write return type
media: i2c: ov2740: Change nvmem reg_read/write return type
media: i2c: video-i2c: Change nvmem reg_read/write return type
iio: pressure: bmp280: Change nvmem reg_read/write return type
misc: ds1682: Change nvmem reg_read/write return type
misc: eeprom: at24: Change nvmem reg_read/write return type
misc: eeprom: at25: Change nvmem reg_read/write return type
misc: eeprom: 93xx46: Change nvmem reg_read/write return type
misc: mchp_pci1xxxx: Change nvmem reg_read/write return type
mtd: core: Change nvmem reg_read/write return type
mtd: ubi: nvmem: Change nvmem reg_read/write return type
soc: atmel: sfr: Change nvmem reg_read/write return type
w1: slaves: w1_ds250x: Change nvmem reg_read/write return type
thunderbolt: switch: Change nvmem reg_read/write return type
thunderbolt: retimer: Change nvmem reg_read/write return type
soc: tegra: fuse: Change nvmem reg_read/write return type
rtc: Change nvmem reg_read/write return type
drivers/hwmon/pmbus/adm1266.c | 4 +-
drivers/iio/pressure/bmp280-core.c | 14 ++++---
drivers/media/i2c/ov2740.c | 6 +--
drivers/media/i2c/video-i2c.c | 9 +++--
drivers/misc/ds1682.c | 16 +++-----
drivers/misc/eeprom/at24.c | 10 +++--
drivers/misc/eeprom/at25.c | 11 +++---
drivers/misc/eeprom/eeprom_93xx46.c | 12 +++---
.../misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c | 16 ++++----
drivers/mtd/mtdcore.c | 18 ++++-----
drivers/mtd/ubi/nvmem.c | 6 +--
drivers/rtc/rtc-abx80x.c | 15 +++----
drivers/rtc/rtc-cmos.c | 8 ++--
drivers/rtc/rtc-ds1305.c | 18 ++++++---
drivers/rtc/rtc-ds1307.c | 22 +++++++----
drivers/rtc/rtc-ds1343.c | 18 ++++++---
drivers/rtc/rtc-ds1511.c | 12 +++---
drivers/rtc/rtc-ds1553.c | 14 ++++---
drivers/rtc/rtc-ds1685.c | 14 ++++---
drivers/rtc/rtc-ds1742.c | 14 ++++---
drivers/rtc/rtc-ds3232.c | 22 +++++++----
drivers/rtc/rtc-isl12026.c | 12 +++---
drivers/rtc/rtc-isl1208.c | 8 ++--
drivers/rtc/rtc-m48t59.c | 12 +++---
drivers/rtc/rtc-m48t86.c | 12 +++---
drivers/rtc/rtc-max31335.c | 18 ++++++---
drivers/rtc/rtc-meson.c | 18 ++++++---
drivers/rtc/rtc-omap.c | 12 +++---
drivers/rtc/rtc-pcf2127.c | 20 ++++++----
drivers/rtc/rtc-pcf85063.c | 20 +++++++---
drivers/rtc/rtc-pcf85363.c | 39 ++++++++++++-------
drivers/rtc/rtc-rp5c01.c | 14 ++++---
drivers/rtc/rtc-rv3028.c | 32 +++++++++------
drivers/rtc/rtc-rv3029c2.c | 20 +++++++---
drivers/rtc/rtc-rv3032.c | 24 ++++++++----
drivers/rtc/rtc-rv8803.c | 16 +++++---
drivers/rtc/rtc-rx8581.c | 39 ++++++++++++-------
drivers/rtc/rtc-stk17ta8.c | 14 ++++---
drivers/rtc/rtc-sun6i.c | 8 ++--
drivers/rtc/rtc-ti-k3.c | 16 +++++---
drivers/rtc/rtc-twl.c | 20 +++++++---
drivers/soc/atmel/sfr.c | 11 ++++--
drivers/soc/tegra/fuse/fuse-tegra.c | 6 +--
drivers/thunderbolt/retimer.c | 8 ++--
drivers/thunderbolt/switch.c | 8 ++--
drivers/w1/slaves/w1_ds250x.c | 4 +-
46 files changed, 408 insertions(+), 282 deletions(-)
--
2.45.1.467.gbab1589fc0-goog
Change nvmem read/write function definition return type to ssize_t.
Signed-off-by: Joy Chakraborty <[email protected]>
---
drivers/hwmon/pmbus/adm1266.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index 2c4d94cc8729..7eaab5a7b04c 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -375,7 +375,7 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)
return 0;
}
-static int adm1266_nvmem_read(void *priv, unsigned int offset, void *val, size_t bytes)
+static ssize_t adm1266_nvmem_read(void *priv, unsigned int offset, void *val, size_t bytes)
{
struct adm1266_data *data = priv;
int ret;
@@ -395,7 +395,7 @@ static int adm1266_nvmem_read(void *priv, unsigned int offset, void *val, size_t
memcpy(val, data->dev_mem + offset, bytes);
- return 0;
+ return bytes;
}
static int adm1266_config_nvmem(struct adm1266_data *data)
--
2.45.1.467.gbab1589fc0-goog
Change nvmem read/write function definition return type to ssize_t.
Signed-off-by: Joy Chakraborty <[email protected]>
---
drivers/media/i2c/ov2740.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
index c48dbcde9877..0101ab55a5ef 100644
--- a/drivers/media/i2c/ov2740.c
+++ b/drivers/media/i2c/ov2740.c
@@ -1212,8 +1212,8 @@ static void ov2740_remove(struct i2c_client *client)
pm_runtime_disable(&client->dev);
}
-static int ov2740_nvmem_read(void *priv, unsigned int off, void *val,
- size_t count)
+static ssize_t ov2740_nvmem_read(void *priv, unsigned int off, void *val,
+ size_t count)
{
struct nvm_data *nvm = priv;
struct device *dev = regmap_get_device(nvm->regmap);
@@ -1241,7 +1241,7 @@ static int ov2740_nvmem_read(void *priv, unsigned int off, void *val,
pm_runtime_put(dev);
exit:
v4l2_subdev_unlock_state(sd_state);
- return ret;
+ return ret < 0 ? ret : count;
}
static int ov2740_register_nvmem(struct i2c_client *client,
--
2.45.1.467.gbab1589fc0-goog
Change nvmem read/write function definition return type to ssize_t.
Signed-off-by: Joy Chakraborty <[email protected]>
---
drivers/misc/ds1682.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/misc/ds1682.c b/drivers/misc/ds1682.c
index 5f8dcd0e3848..953341666ddb 100644
--- a/drivers/misc/ds1682.c
+++ b/drivers/misc/ds1682.c
@@ -198,26 +198,22 @@ static const struct bin_attribute ds1682_eeprom_attr = {
.write = ds1682_eeprom_write,
};
-static int ds1682_nvmem_read(void *priv, unsigned int offset, void *val,
- size_t bytes)
+static ssize_t ds1682_nvmem_read(void *priv, unsigned int offset, void *val,
+ size_t bytes)
{
struct i2c_client *client = priv;
- int ret;
- ret = i2c_smbus_read_i2c_block_data(client, DS1682_REG_EEPROM + offset,
+ return i2c_smbus_read_i2c_block_data(client, DS1682_REG_EEPROM + offset,
bytes, val);
- return ret < 0 ? ret : 0;
}
-static int ds1682_nvmem_write(void *priv, unsigned int offset, void *val,
- size_t bytes)
+static ssize_t ds1682_nvmem_write(void *priv, unsigned int offset, void *val,
+ size_t bytes)
{
struct i2c_client *client = priv;
- int ret;
- ret = i2c_smbus_write_i2c_block_data(client, DS1682_REG_EEPROM + offset,
+ return i2c_smbus_write_i2c_block_data(client, DS1682_REG_EEPROM + offset,
bytes, val);
- return ret < 0 ? ret : 0;
}
/*
--
2.45.1.467.gbab1589fc0-goog
Change nvmem read/write function definition return type to ssize_t.
Signed-off-by: Joy Chakraborty <[email protected]>
---
drivers/misc/eeprom/at25.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
index 595ceb9a7126..73d60f80aea8 100644
--- a/drivers/misc/eeprom/at25.c
+++ b/drivers/misc/eeprom/at25.c
@@ -74,8 +74,8 @@ struct at25_data {
#define io_limit PAGE_SIZE /* bytes */
-static int at25_ee_read(void *priv, unsigned int offset,
- void *val, size_t count)
+static ssize_t at25_ee_read(void *priv, unsigned int offset,
+ void *val, size_t count)
{
struct at25_data *at25 = priv;
char *buf = val;
@@ -147,7 +147,7 @@ static int at25_ee_read(void *priv, unsigned int offset,
dev_dbg(&at25->spi->dev, "read %zu bytes at %d\n",
count, offset);
- return 0;
+ return count;
}
/* Read extra registers as ID or serial number */
@@ -195,10 +195,11 @@ static struct attribute *sernum_attrs[] = {
};
ATTRIBUTE_GROUPS(sernum);
-static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
+static ssize_t at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
{
struct at25_data *at25 = priv;
size_t maxsz = spi_max_transfer_size(at25->spi);
+ size_t bytes_written = count;
const char *buf = val;
int status = 0;
unsigned buf_size;
@@ -313,7 +314,7 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
mutex_unlock(&at25->lock);
kfree(bounce);
- return status;
+ return status < 0 ? status : bytes_written;
}
/*-------------------------------------------------------------------------*/
--
2.45.1.467.gbab1589fc0-goog
Change nvmem read/write function definition return type to ssize_t.
Signed-off-by: Joy Chakraborty <[email protected]>
---
drivers/misc/eeprom/at24.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 4bd4f32bcdab..0e8d92d6ab1e 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -424,8 +424,9 @@ static ssize_t at24_regmap_write(struct at24_data *at24, const char *buf,
return -ETIMEDOUT;
}
-static int at24_read(void *priv, unsigned int off, void *val, size_t count)
+static ssize_t at24_read(void *priv, unsigned int off, void *val, size_t count)
{
+ size_t bytes_read = count;
struct at24_data *at24;
struct device *dev;
char *buf = val;
@@ -465,11 +466,12 @@ static int at24_read(void *priv, unsigned int off, void *val, size_t count)
if (unlikely(at24->read_post))
at24->read_post(off, buf, i);
- return 0;
+ return bytes_read;
}
-static int at24_write(void *priv, unsigned int off, void *val, size_t count)
+static ssize_t at24_write(void *priv, unsigned int off, void *val, size_t count)
{
+ size_t bytes_written = count;
struct at24_data *at24;
struct device *dev;
char *buf = val;
@@ -509,7 +511,7 @@ static int at24_write(void *priv, unsigned int off, void *val, size_t count)
pm_runtime_put(dev);
- return 0;
+ return bytes_written;
}
static int at24_make_dummy_client(struct at24_data *at24, unsigned int index,
--
2.45.1.467.gbab1589fc0-goog
Change nvmem read/write function definition return type to ssize_t.
Signed-off-by: Joy Chakraborty <[email protected]>
---
drivers/misc/eeprom/eeprom_93xx46.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
index 45c8ae0db8f9..5d618a13fe5a 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -79,10 +79,11 @@ static inline bool has_quirk_extra_read_cycle(struct eeprom_93xx46_dev *edev)
return edev->pdata->quirks & EEPROM_93XX46_QUIRK_EXTRA_READ_CYCLE;
}
-static int eeprom_93xx46_read(void *priv, unsigned int off,
- void *val, size_t count)
+static ssize_t eeprom_93xx46_read(void *priv, unsigned int off,
+ void *val, size_t count)
{
struct eeprom_93xx46_dev *edev = priv;
+ size_t bytes_read = count;
char *buf = val;
int err = 0;
int bits;
@@ -158,7 +159,7 @@ static int eeprom_93xx46_read(void *priv, unsigned int off,
mutex_unlock(&edev->lock);
- return err;
+ return err < 0 ? err : bytes_read;
}
static int eeprom_93xx46_ew(struct eeprom_93xx46_dev *edev, int is_on)
@@ -258,12 +259,13 @@ eeprom_93xx46_write_word(struct eeprom_93xx46_dev *edev,
return ret;
}
-static int eeprom_93xx46_write(void *priv, unsigned int off,
+static ssize_t eeprom_93xx46_write(void *priv, unsigned int off,
void *val, size_t count)
{
struct eeprom_93xx46_dev *edev = priv;
char *buf = val;
int i, ret, step = 1;
+ size_t bytes_written = count;
if (unlikely(off >= edev->size))
return -EFBIG;
@@ -304,7 +306,7 @@ static int eeprom_93xx46_write(void *priv, unsigned int off,
/* erase/write disable */
eeprom_93xx46_ew(edev, 0);
- return ret;
+ return ret < 0 ? ret : bytes_written;
}
static int eeprom_93xx46_eral(struct eeprom_93xx46_dev *edev)
--
2.45.1.467.gbab1589fc0-goog
Change nvmem read/write function definition return type to ssize_t.
Signed-off-by: Joy Chakraborty <[email protected]>
---
.../misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c
index 16695cb5e69c..817382c342d3 100644
--- a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c
+++ b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c
@@ -117,8 +117,8 @@ static bool is_eeprom_responsive(struct pci1xxxx_otp_eeprom_device *priv)
return true;
}
-static int pci1xxxx_eeprom_read(void *priv_t, unsigned int off,
- void *buf_t, size_t count)
+static ssize_t pci1xxxx_eeprom_read(void *priv_t, unsigned int off,
+ void *buf_t, size_t count)
{
struct pci1xxxx_otp_eeprom_device *priv = priv_t;
void __iomem *rb = priv->reg_base;
@@ -159,8 +159,8 @@ static int pci1xxxx_eeprom_read(void *priv_t, unsigned int off,
return ret;
}
-static int pci1xxxx_eeprom_write(void *priv_t, unsigned int off,
- void *value_t, size_t count)
+static ssize_t pci1xxxx_eeprom_write(void *priv_t, unsigned int off,
+ void *value_t, size_t count)
{
struct pci1xxxx_otp_eeprom_device *priv = priv_t;
void __iomem *rb = priv->reg_base;
@@ -214,8 +214,8 @@ static void otp_device_set_address(struct pci1xxxx_otp_eeprom_device *priv,
writew(hi, priv->reg_base + MMAP_OTP_OFFSET(OTP_ADDR_HIGH_OFFSET));
}
-static int pci1xxxx_otp_read(void *priv_t, unsigned int off,
- void *buf_t, size_t count)
+static ssize_t pci1xxxx_otp_read(void *priv_t, unsigned int off,
+ void *buf_t, size_t count)
{
struct pci1xxxx_otp_eeprom_device *priv = priv_t;
void __iomem *rb = priv->reg_base;
@@ -264,8 +264,8 @@ static int pci1xxxx_otp_read(void *priv_t, unsigned int off,
return ret;
}
-static int pci1xxxx_otp_write(void *priv_t, unsigned int off,
- void *value_t, size_t count)
+static ssize_t pci1xxxx_otp_write(void *priv_t, unsigned int off,
+ void *value_t, size_t count)
{
struct pci1xxxx_otp_eeprom_device *priv = priv_t;
void __iomem *rb = priv->reg_base;
--
2.45.1.467.gbab1589fc0-goog
On 05/06/2024 18:59, Joy Chakraborty wrote:
> Currently the nvmem core change is picked on
> https://git.kernel.org/pub/scm/linux/kernel/git/srini/nvmem.git/log/?h=for-next
This patch is reverted due to next build failures.
Please resend the series with this included.
--srini
Change nvmem read/write function definition return type to ssize_t.
Signed-off-by: Joy Chakraborty <[email protected]>
---
drivers/media/i2c/video-i2c.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
index 56dbe07a1c99..2bfc221b5104 100644
--- a/drivers/media/i2c/video-i2c.c
+++ b/drivers/media/i2c/video-i2c.c
@@ -151,12 +151,15 @@ struct video_i2c_chip {
int (*hwmon_init)(struct video_i2c_data *data);
};
-static int mlx90640_nvram_read(void *priv, unsigned int offset, void *val,
- size_t bytes)
+static ssize_t mlx90640_nvram_read(void *priv, unsigned int offset, void *val,
+ size_t bytes)
{
struct video_i2c_data *data = priv;
+ int ret;
+
+ ret = regmap_bulk_read(data->regmap, MLX90640_EEPROM_START_ADDR + offset, val, bytes);
- return regmap_bulk_read(data->regmap, MLX90640_EEPROM_START_ADDR + offset, val, bytes);
+ return ret < 0 ? ret : bytes;
}
static struct nvmem_config mlx90640_nvram_config = {
--
2.45.1.467.gbab1589fc0-goog
Change nvmem read/write function definition return type to ssize_t.
Signed-off-by: Joy Chakraborty <[email protected]>
---
drivers/iio/pressure/bmp280-core.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 09f53d987c7d..8d5aeabfa297 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -1465,10 +1465,11 @@ static const int bmp580_odr_table[][2] = {
static const int bmp580_nvmem_addrs[] = { 0x20, 0x21, 0x22 };
-static int bmp580_nvmem_read(void *priv, unsigned int offset, void *val,
- size_t bytes)
+static ssize_t bmp580_nvmem_read(void *priv, unsigned int offset, void *val,
+ size_t bytes)
{
struct bmp280_data *data = priv;
+ size_t bytes_read = bytes;
u16 *dst = val;
int ret, addr;
@@ -1518,13 +1519,14 @@ static int bmp580_nvmem_read(void *priv, unsigned int offset, void *val,
mutex_unlock(&data->lock);
pm_runtime_mark_last_busy(data->dev);
pm_runtime_put_autosuspend(data->dev);
- return ret;
+ return ret < 0 ? ret : bytes_read;
}
-static int bmp580_nvmem_write(void *priv, unsigned int offset, void *val,
- size_t bytes)
+static ssize_t bmp580_nvmem_write(void *priv, unsigned int offset, void *val,
+ size_t bytes)
{
struct bmp280_data *data = priv;
+ size_t bytes_written = bytes;
u16 *buf = val;
int ret, addr;
@@ -1582,7 +1584,7 @@ static int bmp580_nvmem_write(void *priv, unsigned int offset, void *val,
mutex_unlock(&data->lock);
pm_runtime_mark_last_busy(data->dev);
pm_runtime_put_autosuspend(data->dev);
- return ret;
+ return ret < 0 ? ret : bytes_written;
}
static int bmp580_preinit(struct bmp280_data *data)
--
2.45.1.467.gbab1589fc0-goog
On 6/5/24 10:59, Joy Chakraborty wrote:
> Change nvmem read/write function definition return type to ssize_t.
>
> Signed-off-by: Joy Chakraborty <[email protected]>
> ---
> drivers/misc/ds1682.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/misc/ds1682.c b/drivers/misc/ds1682.c
> index 5f8dcd0e3848..953341666ddb 100644
> --- a/drivers/misc/ds1682.c
> +++ b/drivers/misc/ds1682.c
> @@ -198,26 +198,22 @@ static const struct bin_attribute ds1682_eeprom_attr = {
> .write = ds1682_eeprom_write,
> };
>
> -static int ds1682_nvmem_read(void *priv, unsigned int offset, void *val,
> - size_t bytes)
> +static ssize_t ds1682_nvmem_read(void *priv, unsigned int offset, void *val,
> + size_t bytes)
> {
> struct i2c_client *client = priv;
> - int ret;
>
> - ret = i2c_smbus_read_i2c_block_data(client, DS1682_REG_EEPROM + offset,
> + return i2c_smbus_read_i2c_block_data(client, DS1682_REG_EEPROM + offset,
> bytes, val);
> - return ret < 0 ? ret : 0;
> }
>
> -static int ds1682_nvmem_write(void *priv, unsigned int offset, void *val,
> - size_t bytes)
> +static ssize_t ds1682_nvmem_write(void *priv, unsigned int offset, void *val,
> + size_t bytes)
> {
> struct i2c_client *client = priv;
> - int ret;
>
> - ret = i2c_smbus_write_i2c_block_data(client, DS1682_REG_EEPROM + offset,
> + return i2c_smbus_write_i2c_block_data(client, DS1682_REG_EEPROM + offset,
> bytes, val);
i2c_smbus_write_i2c_block_data() does not return the number of bytes written.
It returns either 0 or an error code.
Guenter
On 6/5/24 10:59, Joy Chakraborty wrote:
> Change nvmem read/write function definition return type to ssize_t.
>
> Signed-off-by: Joy Chakraborty <[email protected]>
> ---
> drivers/hwmon/pmbus/adm1266.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index 2c4d94cc8729..7eaab5a7b04c 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
> @@ -375,7 +375,7 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)
> return 0;
> }
>
> -static int adm1266_nvmem_read(void *priv, unsigned int offset, void *val, size_t bytes)
> +static ssize_t adm1266_nvmem_read(void *priv, unsigned int offset, void *val, size_t bytes)
> {
> struct adm1266_data *data = priv;
> int ret;
> @@ -395,7 +395,7 @@ static int adm1266_nvmem_read(void *priv, unsigned int offset, void *val, size_t
>
> memcpy(val, data->dev_mem + offset, bytes);
>
> - return 0;
> + return bytes;
> }
>
> static int adm1266_config_nvmem(struct adm1266_data *data)
The series doesn't explain what a driver is supposed to do if it
only transfers part of the data but not all of it due to an error,
or because the request exceeded the size of the media.
For example, this driver still returns an error code if it successfully
transferred some data but not all of it, or if more data was requested
than is available.
I didn't check other drivers, but I would assume that many of them
have the same or a similar problem.
Guenter
On Wed, Jun 05, 2024 at 05:59:45PM +0000, Joy Chakraborty wrote:
> Change nvmem read/write function definition return type to ssize_t.
>
> Signed-off-by: Joy Chakraborty <[email protected]>
> ---
> drivers/hwmon/pmbus/adm1266.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index 2c4d94cc8729..7eaab5a7b04c 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
> @@ -375,7 +375,7 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)
> return 0;
> }
>
> -static int adm1266_nvmem_read(void *priv, unsigned int offset, void *val, size_t bytes)
> +static ssize_t adm1266_nvmem_read(void *priv, unsigned int offset, void *val, size_t bytes)
> {
> struct adm1266_data *data = priv;
> int ret;
> @@ -395,7 +395,7 @@ static int adm1266_nvmem_read(void *priv, unsigned int offset, void *val, size_t
>
> memcpy(val, data->dev_mem + offset, bytes);
>
> - return 0;
> + return bytes;
> }
This breaks the build so it's not allowed. The way to do it is to:
1) add a new pointer which takes a ssize_t
2) convert everything to the new pointer
3) Rename the new pointer to the old name
regards,
dan carpenter
On Wed, Jun 05, 2024 at 05:59:51PM +0000, Joy Chakraborty wrote:
> @@ -195,10 +195,11 @@ static struct attribute *sernum_attrs[] = {
> };
> ATTRIBUTE_GROUPS(sernum);
>
> -static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
> +static ssize_t at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
> {
> struct at25_data *at25 = priv;
> size_t maxsz = spi_max_transfer_size(at25->spi);
> + size_t bytes_written = count;
> const char *buf = val;
> int status = 0;
> unsigned buf_size;
> @@ -313,7 +314,7 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
> mutex_unlock(&at25->lock);
>
> kfree(bounce);
> - return status;
> + return status < 0 ? status : bytes_written;
> }
So the original bug was that rmem_read() is returning positive values
on success instead of zero[1]. That started a discussion about partial
reads which resulted in changing the API to support partial reads[2].
That patchset broke the build. This patchset is trying to fix the
build breakage.
[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
The bug in rmem_read() is still not fixed. That needs to be fixed as
a stand alone patch. We can discuss re-writing the API separately.
These functions are used internally and exported to the user through
sysfs via bin_attr_nvmem_read/write(). For internal users partial reads
should be treated as failure. What are we supposed to do with a partial
read? I don't think anyone has asked for partial reads to be supported
from sysfs either except Greg was wondering about it while reading the
code.
Currently, a lot of drivers return -EINVAL for partial read/writes but
some return success. It is a bit messy. But this patchset doesn't
really improve anything. In at24_read() we check if it's going to be a
partial read and return -EINVAL. Below we report a partial read as a
full read. It's just a more complicated way of doing exactly what we
were doing before.
drivers/misc/eeprom/at25.c
198 static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
199 {
200 struct at25_data *at25 = priv;
201 size_t maxsz = spi_max_transfer_size(at25->spi);
New: size_t bytes_written = count;
^^^^^^^^^^^^^^^^^^^^^
This is not the number of bytes written.
202 const char *buf = val;
203 int status = 0;
204 unsigned buf_size;
205 u8 *bounce;
206
207 if (unlikely(off >= at25->chip.byte_len))
208 return -EFBIG;
209 if ((off + count) > at25->chip.byte_len)
210 count = at25->chip.byte_len - off;
^^^^^
This is.
211 if (unlikely(!count))
212 return -EINVAL;
213
214 /* Temp buffer starts with command and address */
215 buf_size = at25->chip.page_size;
216 if (buf_size > io_limit)
217 buf_size = io_limit;
218 bounce = kmalloc(buf_size + at25->addrlen + 1, GFP_KERNEL);
219 if (!bounce)
220 return -ENOMEM;
221
regards,
dan carpenter
On Thu, Jun 6, 2024 at 2:48 AM Guenter Roeck <[email protected]> wrote:
>
> On 6/5/24 10:59, Joy Chakraborty wrote:
> > Change nvmem read/write function definition return type to ssize_t.
> >
> > Signed-off-by: Joy Chakraborty <[email protected]>
> > ---
> > drivers/misc/ds1682.c | 16 ++++++----------
> > 1 file changed, 6 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/misc/ds1682.c b/drivers/misc/ds1682.c
> > index 5f8dcd0e3848..953341666ddb 100644
> > --- a/drivers/misc/ds1682.c
> > +++ b/drivers/misc/ds1682.c
> > @@ -198,26 +198,22 @@ static const struct bin_attribute ds1682_eeprom_attr = {
> > .write = ds1682_eeprom_write,
> > };
> >
> > -static int ds1682_nvmem_read(void *priv, unsigned int offset, void *val,
> > - size_t bytes)
> > +static ssize_t ds1682_nvmem_read(void *priv, unsigned int offset, void *val,
> > + size_t bytes)
> > {
> > struct i2c_client *client = priv;
> > - int ret;
> >
> > - ret = i2c_smbus_read_i2c_block_data(client, DS1682_REG_EEPROM + offset,
> > + return i2c_smbus_read_i2c_block_data(client, DS1682_REG_EEPROM + offset,
> > bytes, val);
> > - return ret < 0 ? ret : 0;
> > }
> >
> > -static int ds1682_nvmem_write(void *priv, unsigned int offset, void *val,
> > - size_t bytes)
> > +static ssize_t ds1682_nvmem_write(void *priv, unsigned int offset, void *val,
> > + size_t bytes)
> > {
> > struct i2c_client *client = priv;
> > - int ret;
> >
> > - ret = i2c_smbus_write_i2c_block_data(client, DS1682_REG_EEPROM + offset,
> > + return i2c_smbus_write_i2c_block_data(client, DS1682_REG_EEPROM + offset,
> > bytes, val);
>
> i2c_smbus_write_i2c_block_data() does not return the number of bytes written.
> It returns either 0 or an error code.
>
Ack, I see only i2c_smbus_read_i2c_block_data() returns the number of
bytes read . Will fix it next revision.
> Guenter
>
On Thu, Jun 6, 2024 at 2:11 PM Dan Carpenter <[email protected]> wrote:
>
> On Wed, Jun 05, 2024 at 05:59:51PM +0000, Joy Chakraborty wrote:
> > @@ -195,10 +195,11 @@ static struct attribute *sernum_attrs[] = {
> > };
> > ATTRIBUTE_GROUPS(sernum);
> >
> > -static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
> > +static ssize_t at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
> > {
> > struct at25_data *at25 = priv;
> > size_t maxsz = spi_max_transfer_size(at25->spi);
> > + size_t bytes_written = count;
> > const char *buf = val;
> > int status = 0;
> > unsigned buf_size;
> > @@ -313,7 +314,7 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
> > mutex_unlock(&at25->lock);
> >
> > kfree(bounce);
> > - return status;
> > + return status < 0 ? status : bytes_written;
> > }
>
> So the original bug was that rmem_read() is returning positive values
> on success instead of zero[1]. That started a discussion about partial
> reads which resulted in changing the API to support partial reads[2].
> That patchset broke the build. This patchset is trying to fix the
> build breakage.
>
> [1] https://lore.kernel.org/all/[email protected]/
> [2] https://lore.kernel.org/all/[email protected]/
>
> The bug in rmem_read() is still not fixed. That needs to be fixed as
> a stand alone patch. We can discuss re-writing the API separately.
>
True, fixing the return type would fix that as well is what I thought
but maybe yes we need to fix that separately as well.
> These functions are used internally and exported to the user through
> sysfs via bin_attr_nvmem_read/write(). For internal users partial reads
> should be treated as failure. What are we supposed to do with a partial
> read? I don't think anyone has asked for partial reads to be supported
> from sysfs either except Greg was wondering about it while reading the
> code.
>
> Currently, a lot of drivers return -EINVAL for partial read/writes but
> some return success. It is a bit messy. But this patchset doesn't
> really improve anything. In at24_read() we check if it's going to be a
> partial read and return -EINVAL. Below we report a partial read as a
> full read. It's just a more complicated way of doing exactly what we
> were doing before.
Currently what drivers return is up to their interpretation of int
return type, there are a few drivers which also return the number of
bytes written/read already like
drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c .
The objective of the patch was to handle partial reads and errors at
the nvmem core and instead of leaving it up to each nvmem provider by
providing a better return value to nvmem providers.
Regarding drivers/misc/eeprom/at25.c which you pointed below, that is
a problem in my code change. I missed that count was modified later on
and should initialize bytes_written to the new value of count, will
fix that when I come up with the new patch.
I agree that it does not improve anything for a lot of nvmem providers
for example the ones which call into other reg_map_read/write apis
which do not return the number of bytes read/written but it does help
us do better error handling at the nvmem core layer for nvmem
providers who can return the valid number of bytes read/written.
Please let me know if you have any other suggestions on how to handle
this better.
Thanks
Joy
>
> drivers/misc/eeprom/at25.c
> 198 static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
> 199 {
> 200 struct at25_data *at25 = priv;
> 201 size_t maxsz = spi_max_transfer_size(at25->spi);
> New: size_t bytes_written = count;
> ^^^^^^^^^^^^^^^^^^^^^
> This is not the number of bytes written.
>
> 202 const char *buf = val;
> 203 int status = 0;
> 204 unsigned buf_size;
> 205 u8 *bounce;
> 206
> 207 if (unlikely(off >= at25->chip.byte_len))
> 208 return -EFBIG;
> 209 if ((off + count) > at25->chip.byte_len)
> 210 count = at25->chip.byte_len - off;
> ^^^^^
> This is.
>
> 211 if (unlikely(!count))
> 212 return -EINVAL;
> 213
> 214 /* Temp buffer starts with command and address */
> 215 buf_size = at25->chip.page_size;
> 216 if (buf_size > io_limit)
> 217 buf_size = io_limit;
> 218 bounce = kmalloc(buf_size + at25->addrlen + 1, GFP_KERNEL);
> 219 if (!bounce)
> 220 return -ENOMEM;
> 221
>
> regards,
> dan carpenter
On Thu, Jun 6, 2024 at 2:59 AM Guenter Roeck <[email protected]> wrote:
>
> On 6/5/24 10:59, Joy Chakraborty wrote:
> > Change nvmem read/write function definition return type to ssize_t.
> >
> > Signed-off-by: Joy Chakraborty <[email protected]>
> > ---
> > drivers/hwmon/pmbus/adm1266.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> > index 2c4d94cc8729..7eaab5a7b04c 100644
> > --- a/drivers/hwmon/pmbus/adm1266.c
> > +++ b/drivers/hwmon/pmbus/adm1266.c
> > @@ -375,7 +375,7 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)
> > return 0;
> > }
> >
> > -static int adm1266_nvmem_read(void *priv, unsigned int offset, void *val, size_t bytes)
> > +static ssize_t adm1266_nvmem_read(void *priv, unsigned int offset, void *val, size_t bytes)
> > {
> > struct adm1266_data *data = priv;
> > int ret;
> > @@ -395,7 +395,7 @@ static int adm1266_nvmem_read(void *priv, unsigned int offset, void *val, size_t
> >
> > memcpy(val, data->dev_mem + offset, bytes);
> >
> > - return 0;
> > + return bytes;
> > }
> >
> > static int adm1266_config_nvmem(struct adm1266_data *data)
>
> The series doesn't explain what a driver is supposed to do if it
> only transfers part of the data but not all of it due to an error,
> or because the request exceeded the size of the media.
>
This patch series is actually a follow up on
https://lore.kernel.org/all/[email protected]/
which has now been reverted . I shall try to collate it and send it
again with a better explanation.
> For example, this driver still returns an error code if it successfully
> transferred some data but not all of it, or if more data was requested
> than is available.
>
> I didn't check other drivers, but I would assume that many of them
> have the same or a similar problem.
>
> Guenter
>
On Thu, Jun 06, 2024 at 03:12:03PM +0530, Joy Chakraborty wrote:
> > These functions are used internally and exported to the user through
> > sysfs via bin_attr_nvmem_read/write(). For internal users partial reads
> > should be treated as failure. What are we supposed to do with a partial
> > read? I don't think anyone has asked for partial reads to be supported
> > from sysfs either except Greg was wondering about it while reading the
> > code.
> >
> > Currently, a lot of drivers return -EINVAL for partial read/writes but
> > some return success. It is a bit messy. But this patchset doesn't
> > really improve anything. In at24_read() we check if it's going to be a
> > partial read and return -EINVAL. Below we report a partial read as a
> > full read. It's just a more complicated way of doing exactly what we
> > were doing before.
>
> Currently what drivers return is up to their interpretation of int
> return type, there are a few drivers which also return the number of
> bytes written/read already like
> drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c .
Returning non-zero is a bug. It won't break bin_attr_nvmem_read/write()
but it will break other places like nvmem_access_with_keepouts(),
__nvmem_cell_read() and nvmem_cell_prepare_write_buffer() where all
non-zero returns from nvmem_reg_read() are treated as an error.
> The objective of the patch was to handle partial reads and errors at
> the nvmem core and instead of leaving it up to each nvmem provider by
> providing a better return value to nvmem providers.
>
> Regarding drivers/misc/eeprom/at25.c which you pointed below, that is
> a problem in my code change. I missed that count was modified later on
> and should initialize bytes_written to the new value of count, will
> fix that when I come up with the new patch.
>
> I agree that it does not improve anything for a lot of nvmem providers
> for example the ones which call into other reg_map_read/write apis
> which do not return the number of bytes read/written but it does help
> us do better error handling at the nvmem core layer for nvmem
> providers who can return the valid number of bytes read/written.
If we're going to support partial writes, then it needs to be done all
the way. We need to audit functions like at24_read() and remove the
-EINVAL lines.
440 if (off + count > at24->byte_len)
441 return -EINVAL;
It should be:
if (off + count > at24->byte_len)
count = at24->byte_len - off;
Some drivers handle writing zero bytes as -EINVAL and some return 0.
Those changes could be done before we change the API.
You updated nvmem_access_with_keepouts() to handle negative returns but
not zero returns so it could lead to a forever loop.
regards,
dan carpenter
On Wed, Jun 05, 2024 at 05:59:50PM +0000, Joy Chakraborty wrote:
> Change nvmem read/write function definition return type to ssize_t.
>
> Signed-off-by: Joy Chakraborty <[email protected]>
> ---
> drivers/misc/eeprom/at24.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 4bd4f32bcdab..0e8d92d6ab1e 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -424,8 +424,9 @@ static ssize_t at24_regmap_write(struct at24_data *at24, const char *buf,
> return -ETIMEDOUT;
> }
>
> -static int at24_read(void *priv, unsigned int off, void *val, size_t count)
> +static ssize_t at24_read(void *priv, unsigned int off, void *val, size_t count)
> {
> + size_t bytes_read = count;
> struct at24_data *at24;
> struct device *dev;
> char *buf = val;
> @@ -465,11 +466,12 @@ static int at24_read(void *priv, unsigned int off, void *val, size_t count)
> if (unlikely(at24->read_post))
> at24->read_post(off, buf, i);
>
> - return 0;
> + return bytes_read;
> }
This is called like:
574 ret = at24_read(at24, 2, &val, 1);
575 if (ret || val != 11)
576 return;
So this breaks the driver.
regards,
dan carpenter
On Thu, Jun 6, 2024 at 3:41 PM Dan Carpenter <[email protected]> wrote:
>
> On Thu, Jun 06, 2024 at 03:12:03PM +0530, Joy Chakraborty wrote:
> > > These functions are used internally and exported to the user through
> > > sysfs via bin_attr_nvmem_read/write(). For internal users partial reads
> > > should be treated as failure. What are we supposed to do with a partial
> > > read? I don't think anyone has asked for partial reads to be supported
> > > from sysfs either except Greg was wondering about it while reading the
> > > code.
> > >
> > > Currently, a lot of drivers return -EINVAL for partial read/writes but
> > > some return success. It is a bit messy. But this patchset doesn't
> > > really improve anything. In at24_read() we check if it's going to be a
> > > partial read and return -EINVAL. Below we report a partial read as a
> > > full read. It's just a more complicated way of doing exactly what we
> > > were doing before.
> >
> > Currently what drivers return is up to their interpretation of int
> > return type, there are a few drivers which also return the number of
> > bytes written/read already like
> > drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c .
>
> Returning non-zero is a bug. It won't break bin_attr_nvmem_read/write()
> but it will break other places like nvmem_access_with_keepouts(),
> __nvmem_cell_read() and nvmem_cell_prepare_write_buffer() where all
> non-zero returns from nvmem_reg_read() are treated as an error.
>
Yes, I will resend the patch to fix that.
> > The objective of the patch was to handle partial reads and errors at
> > the nvmem core and instead of leaving it up to each nvmem provider by
> > providing a better return value to nvmem providers.
> >
> > Regarding drivers/misc/eeprom/at25.c which you pointed below, that is
> > a problem in my code change. I missed that count was modified later on
> > and should initialize bytes_written to the new value of count, will
> > fix that when I come up with the new patch.
> >
> > I agree that it does not improve anything for a lot of nvmem providers
> > for example the ones which call into other reg_map_read/write apis
> > which do not return the number of bytes read/written but it does help
> > us do better error handling at the nvmem core layer for nvmem
> > providers who can return the valid number of bytes read/written.
>
> If we're going to support partial writes, then it needs to be done all
> the way. We need to audit functions like at24_read() and remove the
> -EINVAL lines.
>
> 440 if (off + count > at24->byte_len)
> 441 return -EINVAL;
>
> It should be:
>
> if (off + count > at24->byte_len)
> count = at24->byte_len - off;
>
> Some drivers handle writing zero bytes as -EINVAL and some return 0.
> Those changes could be done before we change the API.
>
Sure, we can do it in a phased manner like you suggested in another
reply by creating new pointers and slowly moving each driver to the
new pointer and then deprecating the old one.
> You updated nvmem_access_with_keepouts() to handle negative returns but
> not zero returns so it could lead to a forever loop.
>
Yes, that is a possible case. Will rework it.
> regards,
> dan carpenter
>
Thanks
Joy
On 06/06/2024 09:41, Dan Carpenter wrote:
> So the original bug was that rmem_read() is returning positive values
> on success instead of zero[1]. That started a discussion about partial
> reads which resulted in changing the API to support partial reads[2].
> That patchset broke the build. This patchset is trying to fix the
> build breakage.
>
> [1]https://lore.kernel.org/all/[email protected]/
> [2]https://lore.kernel.org/all/[email protected]/
>
> The bug in rmem_read() is still not fixed. That needs to be fixed as
> a stand alone patch. We can discuss re-writing the API separately.
I agree with Dan, Lets fix the rmem_read and start working on the API
rework in parallel.
Am happy to pick the [1].
--srini