Re: regulator: ab8500-ext: Strange set_mode behavior when info->cfg->hwreqis set

From: Bengt JÃnsson
Date: Wed Apr 10 2013 - 05:01:19 EST


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
--
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/