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/20240206042408.224138-1-joychakr@xxxxxxxxxx/
[2] https://lore.kernel.org/all/20240510082929.3792559-2-joychakr@xxxxxxxxxx/
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
