On Fri, Aug 7, 2020 at 4:19 PM Jean Delvare <jdelvare@xxxxxxx> wrote:
>
> The elegant code in at24_read() has the drawback that we now need
> to make a copy of all parameters to pass them to the post-processing
> callback function if there is one. Rewrite the loop in such a way that
> the parameters are not modified, so saving them is no longer needed.
>
> Signed-off-by: Jean Delvare <jdelvare@xxxxxxx>
> Cc: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
> This has the drawback of creating an asymetry with at24_write(), so
> I'm not 100% if we want to apply this. If anyone has a better idea,
> please let me know.
>
> drivers/misc/eeprom/at24.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> --- linux-5.7.orig/drivers/misc/eeprom/at24.c 2020-08-07 14:23:39.882191500 +0200
> +++ linux-5.7/drivers/misc/eeprom/at24.c 2020-08-07 14:28:39.039360687 +0200
> @@ -423,10 +423,7 @@ static int at24_read(void *priv, unsigne
> struct at24_data *at24;
> struct device *dev;
> char *buf = val;
> - int ret;
> - unsigned int orig_off = off;
> - char *orig_buf = buf;
> - size_t orig_count = count;
> + int i, ret;
>
> at24 = priv;
> dev = at24_base_client_dev(at24);
> @@ -449,15 +446,15 @@ static int at24_read(void *priv, unsigne
> */
> mutex_lock(&at24->lock);
>
> + i = 0;
Hi Jean,
I think doing:
for (i = 0; count; i += ret, count -= ret)
would be even more elegant, don't you think?
Bartosz
> while (count) {
> - ret = at24_regmap_read(at24, buf, off, count);
> + ret = at24_regmap_read(at24, buf + i, off + i, count);
> if (ret < 0) {
> mutex_unlock(&at24->lock);
> pm_runtime_put(dev);
> return ret;
> }
> - buf += ret;
> - off += ret;
> + i += ret;
> count -= ret;
> }
>
> @@ -466,7 +463,7 @@ static int at24_read(void *priv, unsigne
> pm_runtime_put(dev);
>
> if (unlikely(at24->read_post))
> - at24->read_post(orig_off, orig_buf, orig_count);
> + at24->read_post(off, buf, i);
>
> return 0;
> }
>
> --
> Jean Delvare
> SUSE L3 Support
