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... > + /* > + * 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. 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. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 473 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-rockchip/attachments/20141229/47006025/attachment.sig>
