Re: [PATCH V4 2/2] regulator: mt6323: Add support for MT6323 regulator
On Wed, 2016-02-03 at 12:29 +, Mark Brown wrote: > On Wed, Feb 03, 2016 at 01:39:02PM +0800, menghui lin wrote: > > On Tue, 2016-02-02 at 19:38 +, Mark Brown wrote: > > > > How does the driver know if it needs to change the mode (ie, how can it > > > tell if the current mode is inadequate) and surely if we can only change > > > in one direction this isn't terribly useful? > > > I think the datasheet of buck/ldo could provide information about power > > capability of each mode. The driver should adjust regulator mode per its > > device's power requirement. > > That's of no help for a consumer driver which doesn't know what > regulator is supplying it. > > > case 1: > > > We have a USB typeC micro-controller, which has two modes - standby and > > normal. It requires 1.8V and 3.3V to operate (both powers are always > > on). The device stays in standby mode when there is no cable in. When > > cable in, we got an interrupt and change device into normal mode. > > > The standby mode power consumption is quite small, so we would like the > > change mode of regulator into STANDBY to save more power. And we change > > into NORMAL when we receive cable-in interrupt. > > This seems like something that we ought to be doing via runtime PM > anyway which should be going through the suspend mode bindings, though > that would need some plumbing in. > > > case 2: > > > About buck regulator for CPU, it usually provides PWM mode, PWM/PFM Auto > > mode, PFM mode. I think it could map to FAST, NORMAL, IDLE mode > > respectively. Most of time we would use just normal mode. However, we > > would change regulator into PWM mode time to time to test buck output > > performance on the tested board. > > That's a test use that doesn't seem a good fit for upstream at all. Hi Mark, Thanks for your response. It seems more work is required for dynamic mode switch. For MT6323 regulator patch, we would remove mode-switch related code for now to provide basic regulator support first.
Re: [PATCH V4 2/2] regulator: mt6323: Add support for MT6323 regulator
On Tue, 2016-02-02 at 19:38 +, Mark Brown wrote: > On Fri, Jan 29, 2016 at 08:11:19PM +0800, menghui lin wrote: > > On Fri, 2016-01-29 at 12:27 +0100, Mark Brown wrote: > > > > None of this is answering my question - I know what the current API is, > > > describing it doesn't tell me about actual users or how they are able to > > > sensibly use the interface. Bear in mind that the definitions of the > > > various modes are all relative and what one device thinks is high usage > > > may be low usage for another device. > > > Assuming valid_modes_mask and initial_mode are specified, a possible > > way to modify regulator_set_mode() is to allow mode change only if the > > regulator is controlled exclusively by a certain consumer or the > > requested mode provides stronger power capability than current mode. > > Here I assume that power capability fast > normal > idle > standby. > > How does the driver know if it needs to change the mode (ie, how can it > tell if the current mode is inadequate) and surely if we can only change > in one direction this isn't terribly useful? Hi Mark, I think the datasheet of buck/ldo could provide information about power capability of each mode. The driver should adjust regulator mode per its device's power requirement. Below are some actual scenarios we have now. I provide them for your reference. If unfortunately below cases are not the actual users you ask, please kindly let me know more about your suggestion. Thank you. :) case 1: We have a USB typeC micro-controller, which has two modes - standby and normal. It requires 1.8V and 3.3V to operate (both powers are always on). The device stays in standby mode when there is no cable in. When cable in, we got an interrupt and change device into normal mode. The standby mode power consumption is quite small, so we would like the change mode of regulator into STANDBY to save more power. And we change into NORMAL when we receive cable-in interrupt. case 2: About buck regulator for CPU, it usually provides PWM mode, PWM/PFM Auto mode, PFM mode. I think it could map to FAST, NORMAL, IDLE mode respectively. Most of time we would use just normal mode. However, we would change regulator into PWM mode time to time to test buck output performance on the tested board.
Re: [PATCH V4 2/2] regulator: mt6323: Add support for MT6323 regulator
On Fri, 2016-01-29 at 12:27 +0100, Mark Brown wrote: > On Fri, Jan 29, 2016 at 05:52:14PM +0800, menghui lin wrote: > > On Fri, 2016-01-29 at 00:13 +0100, Mark Brown wrote: > > > > I'm not convinced this binding makes sense, how would a user of the API > > > (currently there are none in tree) know what the modes mean? It's a bit > > > different when the user is supplying configuration for a specific > > > regulator but this needs to be something that can be used by consumers. > > > > What are you actually trying to do with this? > > > In this patch, we want to support both normal/standby modes for mt6323 > > regulators due to mt6323 regulators support low power mode which > > provides better power efficiency for standby case. > > > We expect user of mt6323 regulators could dynamically change power mode > > by regulator_set_mode(). -EINVAL is returned if the given mode is not > > supported. > > > The regulator_set_mode() API looks very straightforward and possible > > modes are already defined in consumer.h. It looks like we don't have to > > list possible modes for mt6323 additionally in binding document. > > None of this is answering my question - I know what the current API is, > describing it doesn't tell me about actual users or how they are able to > sensibly use the interface. Bear in mind that the definitions of the > various modes are all relative and what one device thinks is high usage > may be low usage for another device. Assuming valid_modes_mask and initial_mode are specified, a possible way to modify regulator_set_mode() is to allow mode change only if the regulator is controlled exclusively by a certain consumer or the requested mode provides stronger power capability than current mode. Here I assume that power capability fast > normal > idle > standby.
Re: [PATCH V4 2/2] regulator: mt6323: Add support for MT6323 regulator
On Fri, 2016-01-29 at 00:13 +0100, Mark Brown wrote: > On Thu, Jan 28, 2016 at 07:13:48PM +0100, John Crispin wrote: > > > would the following two bindings be ok ? I would create patches to add them. > > > * regulator-allow-mode; or regulator-allow-change-mode; > > This seems redundant, if we have a list of valid modes presumably they > can be used - same idea as with voltage setting. > > > * regulator-modes = ; > > I'm not convinced this binding makes sense, how would a user of the API > (currently there are none in tree) know what the modes mean? It's a bit > different when the user is supplying configuration for a specific > regulator but this needs to be something that can be used by consumers. > > What are you actually trying to do with this? Hi Mark, Per documentation in regulator/consumer.h, I think user should be able to know the behavior of each mode(fast, normal, idle, standby). In this patch, we want to support both normal/standby modes for mt6323 regulators due to mt6323 regulators support low power mode which provides better power efficiency for standby case. We expect user of mt6323 regulators could dynamically change power mode by regulator_set_mode(). -EINVAL is returned if the given mode is not supported. The regulator_set_mode() API looks very straightforward and possible modes are already defined in consumer.h. It looks like we don't have to list possible modes for mt6323 additionally in binding document. MengHui
