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.