diff mbox

[v4,10/14] regulator: of: Add support for parsing initial and suspend modes

Message ID 1415025649-8119-11-git-send-email-javier.martinez@collabora.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Javier Martinez Canillas Nov. 3, 2014, 2:40 p.m. UTC
Some regulators support their operating mode to be changed on startup
or by consumers when the system is running while others only support
their operating mode to be changed while the system has entered in a
suspend state.

The regulator Device Tree binding documents a set of properties to
configure the regulators operating modes from a FDT. This patch builds
on (40e20d6 regulator: of: Add support for parsing regulator_state for
suspend state) and adds support to parse those properties and fill the
regulator constraints so the regulator core can call the right suspend
handlers when the system enters into sleep.

The modes are defined in the Device Tree using the hardware specific
modes supported by the regulators. Regulator drivers have to define a
translation function that is used to map the hardware specific modes
to the standard ones.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

Changes in v4:
 - Parse the properties in the core and map using driver provided functions.
   Suggested by Mark Brown

Changes in v3:
 - Use the standard suspend states binding instead of custom properties.
   Suggested by Mark Brown

 drivers/regulator/of_regulator.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

Comments

Krzysztof Kozlowski Nov. 4, 2014, 10:41 a.m. UTC | #1
On pon, 2014-11-03 at 15:40 +0100, Javier Martinez Canillas wrote:
> Some regulators support their operating mode to be changed on startup
> or by consumers when the system is running while others only support
> their operating mode to be changed while the system has entered in a
> suspend state.
> 
> The regulator Device Tree binding documents a set of properties to
> configure the regulators operating modes from a FDT. This patch builds
> on (40e20d6 regulator: of: Add support for parsing regulator_state for
> suspend state) and adds support to parse those properties and fill the
> regulator constraints so the regulator core can call the right suspend
> handlers when the system enters into sleep.
> 
> The modes are defined in the Device Tree using the hardware specific
> modes supported by the regulators. Regulator drivers have to define a
> translation function that is used to map the hardware specific modes
> to the standard ones.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
> 
> Changes in v4:
>  - Parse the properties in the core and map using driver provided functions.
>    Suggested by Mark Brown
> 
> Changes in v3:
>  - Use the standard suspend states binding instead of custom properties.
>    Suggested by Mark Brown
> 
>  drivers/regulator/of_regulator.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
> index cbc1d71..cd65885 100644
> --- a/drivers/regulator/of_regulator.c
> +++ b/drivers/regulator/of_regulator.c
> @@ -25,7 +25,8 @@ static const char *const regulator_states[PM_SUSPEND_MAX + 1] = {
>  };
>  
>  static void of_get_regulation_constraints(struct device_node *np,
> -					struct regulator_init_data **init_data)
> +					struct regulator_init_data **init_data,
> +					const struct regulator_desc *desc)
>  {
>  	const __be32 *min_uV, *max_uV;
>  	struct regulation_constraints *constraints = &(*init_data)->constraints;
> @@ -81,6 +82,14 @@ static void of_get_regulation_constraints(struct device_node *np,
>  	if (!ret)
>  		constraints->enable_time = pval;
>  
> +	if (!of_property_read_u32(np, "regulator-initial-mode", &pval)) {
> +		if (desc && desc->map_modes)
> +			constraints->initial_mode = desc->map_modes(pval);
> +		else
> +			pr_warn("%s: failed to parse regulator-initial-mode\n",
> +				np->name);
> +	}
> +

Here's a hidden assumption that if driver does not provide map_modes
then any "regulator-initial-mode" property is not valid. Shouldn't this
be mentioned somewhere? Maybe in description of map_modes callback?

Best regards,
Krzysztof


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas Nov. 4, 2014, 11:07 a.m. UTC | #2
Hello Krzysztof,

On 11/04/2014 11:41 AM, Krzysztof Kozlowski wrote:
>> +	if (!of_property_read_u32(np, "regulator-initial-mode", &pval)) {
>> +		if (desc && desc->map_modes)
>> +			constraints->initial_mode = desc->map_modes(pval);
>> +		else
>> +			pr_warn("%s: failed to parse regulator-initial-mode\n",
>> +				np->name);
>> +	}
>> +
> 
> Here's a hidden assumption that if driver does not provide map_modes
> then any "regulator-initial-mode" property is not valid. Shouldn't this
> be mentioned somewhere? Maybe in description of map_modes callback?
> 

Well it can't be valid if the regulator core does not know how to map the
value in the property to one of the standard modes. The binding explains
that each PMIC has to define its hardware modes, I can also add a note
in the .map_modes. I guess no one will say no to more documentation :-)

Best regards,
Javier

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index cbc1d71..cd65885 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -25,7 +25,8 @@  static const char *const regulator_states[PM_SUSPEND_MAX + 1] = {
 };
 
 static void of_get_regulation_constraints(struct device_node *np,
-					struct regulator_init_data **init_data)
+					struct regulator_init_data **init_data,
+					const struct regulator_desc *desc)
 {
 	const __be32 *min_uV, *max_uV;
 	struct regulation_constraints *constraints = &(*init_data)->constraints;
@@ -81,6 +82,14 @@  static void of_get_regulation_constraints(struct device_node *np,
 	if (!ret)
 		constraints->enable_time = pval;
 
+	if (!of_property_read_u32(np, "regulator-initial-mode", &pval)) {
+		if (desc && desc->map_modes)
+			constraints->initial_mode = desc->map_modes(pval);
+		else
+			pr_warn("%s: failed to parse regulator-initial-mode\n",
+				np->name);
+	}
+
 	for (i = 0; i < ARRAY_SIZE(regulator_states); i++) {
 		switch (i) {
 		case PM_SUSPEND_MEM:
@@ -100,6 +109,15 @@  static void of_get_regulation_constraints(struct device_node *np,
 		if (!suspend_np || !suspend_state)
 			continue;
 
+		if (!of_property_read_u32(suspend_np, "regulator-mode",
+					  &pval)) {
+			if (desc && desc->map_modes)
+				suspend_state->mode = desc->map_modes(pval);
+			else
+				pr_warn("%s: failed to parse regulator-mode\n",
+					np->name);
+		}
+
 		if (of_property_read_bool(suspend_np,
 					"regulator-on-in-suspend"))
 			suspend_state->enabled = true;
@@ -140,7 +158,7 @@  struct regulator_init_data *of_get_regulator_init_data(struct device *dev,
 	if (!init_data)
 		return NULL; /* Out of memory? */
 
-	of_get_regulation_constraints(node, &init_data);
+	of_get_regulation_constraints(node, &init_data, desc);
 	return init_data;
 }
 EXPORT_SYMBOL_GPL(of_get_regulator_init_data);