Hi Bartosz,
On Tue, 18 Aug 2020 15:46:36 +0200, Bartosz Golaszewski wrote:
> 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
> > (...)
> > @@ -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?
Definitely. I'll change the code that way and send v2 of the patch,
thank you for the suggestion.
> 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;
> > }
> >
--
Jean Delvare
SUSE L3 Support
