Re: [PATCH 2/2] regulator: Add driver for cros-ec-regulator

[Mark Brown]

On Wed, Jun 10, 2020 at 12:31:53PM +0800, Pi-Hsun Shih wrote:
> On Tue, Jun 9, 2020 at 7:19 PM Mark Brown <broonie@xxxxxxxxxx> wrote:
> > On Tue, Jun 09, 2020 at 03:59:55PM +0800, Pi-Hsun Shih wrote:

> > > +     int min_mV = DIV_ROUND_UP(min_uV, 1000);
> > > +     int max_mV = max_uV / 1000;
> > > +     struct ec_params_regulator_set_voltage cmd = {
> > > +             .index = data->index,
> > > +             .min_mv = min_mV,
> > > +             .max_mv = max_mV,
> > > +     };
> > > +
> > > +     if (min_mV > max_mV)
> > > +             return -EINVAL;

> > The core will do this for you.

> Since I'm doing DIV_ROUND_UP for the min_mV, so this may happen if the
> min_uV~max_uV range given by the core doesn't contain any value that
> can be represented exactly in mV.

This isn't clear from the code - I'd add a comment or move the division
to be next to the check.

Attachment: signature.asc
Description: PGP signature