Re: [PATCH v2 3/7] eeprom: at24: add regmap-based write function

[Bartosz Golaszewski <brgl@xxxxxxxx>]

2017-11-16 21:26 GMT+01:00 Heiner Kallweit <hkallweit1@xxxxxxxxx>:
> Add a regmap-based write function.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
> ---
> v2:
> - rebased
> ---
>  drivers/misc/eeprom/at24.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 1411fa029..c0c59575e 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -525,6 +525,22 @@ static ssize_t at24_eeprom_write_smbus_byte(struct at24_data *at24,
>         return -ETIMEDOUT;
>  }
>
> +static ssize_t at24_regmap_write(struct at24_data *at24, const char *buf,
> +                                unsigned int offset, size_t count)
> +{
> +       unsigned long timeout, write_time;
> +       struct regmap *regmap;
> +
> +       regmap = at24_translate_offset(at24, &offset)->regmap;
> +       count = at24_adjust_write_count(at24, offset, count);
> +
> +       loop_until_timeout(timeout, write_time)
> +               if (!regmap_bulk_write(regmap, offset, buf, count))
> +                       return count;
> +

Please do this:

int rv;

rv = regmap_bulk_write(...);
if (rv == 0)
    return count;

I'd like to stay as readable as possible and making the return value
type visible is always good.

Also: it would be nice if you could keep the dev_dbg() call from the
original write function.

Thanks,
Bartosz