Re: [PATCH v4 2/2] regulator: rt6160: Add support for Richtek RT6160

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]

On Thu, May 27, 2021 at 11:14:17AM +0800, ChiYuan Huang wrote:

> I review the regulator_set_ramp_delay_regmap API.
> If seems I need to fill in the ramp_delay_table by the descend order.
> But this chip ramp delay table is designed the ascending value reg bit
> field [0 1 2 3] by
> the ascending order [1000 2500 5000 10000] uV/uS
> Even if I tried to filler in descending order, I also need a inverted operation.

I see... that really should be supportable, and I'd have expected
find_closest_bigger() to DTRT here, it's not obvious it's expecting
ordering.

> And I found the regulator_set_ramp_delay_regmap API has some logic error.
> From the include/linux/regulator/driver.h, the set_ramp_delay function says to
> set the less or equal one ramp delay value.
> But your logic will get the larger or equal one from the descending
> ramp delay table.

The code is correct here, the documentation should be fixed - with a
delay like this we should be erring on the side of delaying too long to
be safe.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]