Re: [PATCH v3 2/2] regulator: rk808: add dvs support

From: Chris Zhong
Date: Mon Dec 29 2014 - 22:07:42 EST



On 12/30/2014 01:25 AM, Mark Brown wrote:
On Mon, Dec 15, 2014 at 11:07:58AM +0800, Chris Zhong wrote:

+ sel <<= ffs(rdev->desc->vsel_mask) - 1;
+ sel |= old_sel & ~rdev->desc->vsel_mask;
+
+ ret = regmap_write(rdev->regmap, reg, sel);
+ if (ret)
+ return ret;
+
+ gpiod_set_value(gpio, !gpio_level);
So, this seems a bit odd. What we appear to be doing here is
alternating between the two different voltage setting registers which is
all well and good but makes me wonder why we're bothering - it's a bit
more work than just sticking with one. We do get...
you mean check the old_selector and selector? I think
_regulator_do_set_voltage have done it.

+ /*
+ * dvsok pin would be pull down when dvs1/2 pin changed, and
+ * it would be pull up once the voltage regulate complete.
+ * No need to wait dvsok signal when voltage falling.
+ */
...this but unless the voltage typically ramps much faster than spec
it's never clear to me that we're actually winning by polling the pin
instead of just dead reckoning the time, it's more work for the CPU to
poll the GPIO than to sleep after all.
Actually, it's slower than spec, so I think getting this dvsok pin state
is safer than delay.

One thing we can do with hardware like this is to program in a voltage
we're likely to want to switch to quickly and then use the GPIO to get
there. That can be a bit hard to arrange with the regulator API as it
currently stands since we don't exactly have an interface for it.

We can just check to see what the two values are current set to before
switching and skip the register write if it's the same (making things
faster since we're typically avoiding an I2C or SPI transaction by doing
that) but that's a bit meh. We can also try to do things like keep the
top voltage from the voltage ranges we're being given programmed which
for DVS typically ends up doing a reasonable job since governors often
like to jump straight to top speed when things get busy so that's one of
the common cases where we most want to change voltages as quickly as
possible.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/