Hi,
I see below code && comments in enable() function:
/*
* To satisfy both HW high power request and SW request, the regulator
* must be on in high power.
*/
if (info->cfg && info->cfg->hwreq)
*regval = info->update_val_hp;
I'm not very clear about the comment and the code.
1) Does that mean the device does not allow set REGULATOR_MODE_IDLE when
info->cfg->hwreq is set?
current code looks strange because when info->cfg->hwreq is set:
set_mode() allows set REGULATOR_MODE_IDLE and get_mode() returns the status
is REGULATOR_MODE_IDLE. However, current code actually write info->update_val_hp
to the register (which means it is in REGULATOR_MODE_NORMAL mode).
If the device does not allow set REGULATOR_MODE_IDLE when info->cfg->hwreq is
set, we probably needs to return error in set REGULATOR_MODE_IDLE case.
Or 2) Does above comment mean the device needs set to REGULATOR_MODE_NORMAL
when the regulator is switching from off to on? Which means it allows setting
the regulator to REGULATOR_MODE_IDLE if the regulator is already on.
If this is the case, we cannot call enable() in set_mode() because current code
in set_mode() will write to register only when the regulator is already on.
Calling enable() always write info->update_val_hp to the register.
Thus it should just call abx500_mask_and_set_register_interruptible() directly
to update the register.
comments?
Regards,
Axel
On 04/09/2013 05:01 PM, Axel Lin wrote:
> Hi,
>
> I see below code && comments in enable() function:
>
> /*
> * To satisfy both HW high power request and SW request, the regulator
> * must be on in high power.
> */
> if (info->cfg && info->cfg->hwreq)
> *regval = info->update_val_hp;
>
>
> I'm not very clear about the comment and the code.
>
> 1) Does that mean the device does not allow set REGULATOR_MODE_IDLE when
> info->cfg->hwreq is set?
Yes.
> current code looks strange because when info->cfg->hwreq is set:
> set_mode() allows set REGULATOR_MODE_IDLE and get_mode() returns the status
> is REGULATOR_MODE_IDLE. However, current code actually write info->update_val_hp
> to the register (which means it is in REGULATOR_MODE_NORMAL mode).
>
> If the device does not allow set REGULATOR_MODE_IDLE when info->cfg->hwreq is
> set, we probably needs to return error in set REGULATOR_MODE_IDLE case.
I understand that this requires some explanation.
The ab8500 hardware supports a special hardwarerequest mode where the
regulator can be automatically activated in HP mode by a HW signal. The
same two register bits are used for setting this mode or to control the
regulator between off/LP mode/HP mode:
- 0b00 = off,
- 0b01 = HP (high power) mode,
- 0b10 = HW request,
- 0b11 = LP (low power) mode.
So these external regulators are designed to be controlled by either HW
request OR control from software. In practice, we had a case where we
wanted to combine SW and HW requests and fall-back to HW request when
there is no SW request dynamically. This is what the code aims to do: if
the regulator is configured to support HW request, it may be set to HP
at any time by HW signals. If a SW request is put on the regulator from
the regulator framework, it must go to HP mode to satisfy requests from
both HW and SW, LP mode is not sufficient.
I designed the code so that the regulator framework does not know about
the HW request mode, as if this was handled by the hardware itself. In
some cases, when the regulator framework requests LP mode and there is
no HW request, we will set HP mode anyway. There is no way to avoid
itfor the external regulators.
>
> Or 2) Does above comment mean the device needs set to REGULATOR_MODE_NORMAL
> when the regulator is switching from off to on? Which means it allows setting
> the regulator to REGULATOR_MODE_IDLE if the regulator is already on.
>
> If this is the case, we cannot call enable() in set_mode() because current code
> in set_mode() will write to register only when the regulator is already on.
> Calling enable() always write info->update_val_hp to the register.
> Thus it should just call abx500_mask_and_set_register_interruptible() directly
> to update the register.
I am not sure what you mean here. Maybe above comments answer your question.
>
> comments?
>
> Regards,
> Axel
>
I hope this clarifies. Regards,
Bengt