Messages in this thread | | | Date | Wed, 19 Oct 2011 14:32:58 +0100 | From | Mark Brown <> | Subject | Re: [PATCH 5/6] mfd: TPS65910: Fix tps65910_set_voltage |
| |
On Tue, Oct 18, 2011 at 01:26:27PM -0500, Kyle Manna wrote:
*Always* CC maintainers on patches.
> Previously tps65910_set_voltage() only selected from a fixed number of > voltages. Rename that function to tps65910_set_voltage_sel(). Do the > same for tps65911_set_voltage().
What is the issue being fixed here? This looks like a stylistic change rateher than a bug fix.
> Also add a tps65910_set_voltage that works with the regulator framework > and applies the correct voltage with apply_uv is set in the regulator's > constraints.
So this is adding support for a new chip? Whatever the answer it's clearly a distinct change from the above and should therefore be a separate patch.
> + /* Pick the nearest selector */ > + for (i = 0; i < tps65910_regs[id].table_len; i++) { > + new_uV = tps65910_regs[id].table[i] * 1000; > + > + if (new_uV >= min_uV && new_uV <= max_uV && > + (abs(new_uV - midpoint) < abs(selected_uV - midpoint))) { > + *selector = i; > + selected_uV = tps65910_regs[id].table[i] * 1000; > + } > + }
This looks wrong, the expected behaviour for the regulator API is that the driver will pick the minimum voltage within the range. Why is this being done?
| |