diff mbox

[v3,4/5] regulator: max77802: Parse regulator operating mode properties

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

Commit Message

Javier Martinez Canillas Oct. 20, 2014, 2:47 p.m. UTC
The max77802 PMIC regulators output can be configured in one of two
modes: Output ON (normal) and Output ON in Low Power Mode. Some of
the 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 parse
those properties and fills the regulator constraints so the regulator
core can call the suspend handlers when the system enters into sleep.

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

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

Changes since v1:
 - Use the static inline max77802_map_mode() function instead of a macro.
   Suggested by Mark Brown.

 drivers/regulator/max77802.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

Comments

Mark Brown Oct. 22, 2014, 4:48 p.m. UTC | #1
On Mon, Oct 20, 2014 at 04:47:51PM +0200, Javier Martinez Canillas wrote:

> +	char *states[PM_SUSPEND_MAX + 1] = {
> +		[PM_SUSPEND_MEM] = "regulator-state-mem",
> +		[PM_SUSPEND_MAX] = "regulator-state-disk",
> +	};

This still has the same problem as your previous patch with things not
being as joined up with the core as they should be.  What I'd like to
see is that the code to extract the number for each state is in the core
and then the drivers just provide a translation function to map from
this to the internal modes.
Javier Martinez Canillas Oct. 23, 2014, 9:28 a.m. UTC | #2
Hello Mark,

> On 22/10/2014, at 18:48, Mark Brown <broonie@kernel.org> wrote:
> 
>> On Mon, Oct 20, 2014 at 04:47:51PM +0200, Javier Martinez Canillas wrote:
>> 
>> +    char *states[PM_SUSPEND_MAX + 1] = {
>> +        [PM_SUSPEND_MEM] = "regulator-state-mem",
>> +        [PM_SUSPEND_MAX] = "regulator-state-disk",
>> +    };
> 
> This still has the same problem as your previous patch with things not
> being as joined up with the core as they should be.  What I'd like to
> see is that the code to extract the number for each state is in the core
> and then the drivers just provide a translation function to map from
> this to the internal modes.

Ok, I'll send a new version parsing it in the core and providing a translation function in the driver as you said.

However this is an implementation detail and should not change the DT bindings in the current version. Could you please let me know if you have any issues with the other patches from this series so I can address all of them when doing a re-spin?

Thanks a lot and 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
Mark Brown Oct. 24, 2014, 9:02 p.m. UTC | #3
On Thu, Oct 23, 2014 at 11:28:09AM +0200, Javier Martinez Canillas wrote:

Please fix your mailer to word wrap within paragraphs - you should know
this by now :/  I've reflowed for legibility.

> However this is an implementation detail and should not change the DT
> bindings in the current version. Could you please let me know if you
> have any issues with the other patches from this series so I can
> address all of them when doing a re-spin?

I can't recall anything but I'm not sure I looked at them in any detail.
Javier Martinez Canillas Oct. 26, 2014, 11:24 p.m. UTC | #4
Hello Mark,

On 10/24/2014 11:02 PM, Mark Brown wrote:
> On Thu, Oct 23, 2014 at 11:28:09AM +0200, Javier Martinez Canillas wrote:
> 
> Please fix your mailer to word wrap within paragraphs - you should know
> this by now :/  I've reflowed for legibility.
> 

Sorry about that, I was on holidays last week and sent you that email from
my phone. Next time I'll wait until I've access to a mailer that I'm sure
does the right thing before answering you.

>> However this is an implementation detail and should not change the DT
>> bindings in the current version. Could you please let me know if you
>> have any issues with the other patches from this series so I can
>> address all of them when doing a re-spin?
> 
> I can't recall anything but I'm not sure I looked at them in any detail.
> 

Ok, thanks a lot for the feedback.

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/max77802.c b/drivers/regulator/max77802.c
index 5839c45..2cbf980 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -518,6 +518,48 @@  static struct regulator_desc regulators[] = {
 };
 
 #ifdef CONFIG_OF
+
+static void max77802_parse_opmodes(struct device_node *np,
+				   struct regulation_constraints *cons)
+{
+	u32 pval;
+	int i;
+	char *states[PM_SUSPEND_MAX + 1] = {
+		[PM_SUSPEND_MEM] = "regulator-state-mem",
+		[PM_SUSPEND_MAX] = "regulator-state-disk",
+	};
+	struct regulator_state *state;
+	struct device_node *state_np;
+
+	if (!of_property_read_u32(np, "regulator-initial-mode", &pval))
+		cons->initial_mode = max77802_map_mode(pval);
+
+	for (i = 0; i < ARRAY_SIZE(states); i++) {
+		switch (i) {
+		case PM_SUSPEND_MEM:
+			state = &cons->state_mem;
+			break;
+		case PM_SUSPEND_MAX:
+			state = &cons->state_disk;
+			break;
+		default:
+			continue;
+		};
+
+		state_np = of_get_child_by_name(np, states[i]);
+		if (!state_np || !state)
+			continue;
+
+		if (!of_property_read_u32(np, "regulator-mode", &pval))
+			state->mode = max77802_map_mode(pval);
+
+		of_node_put(state_np);
+
+		state = NULL;
+		state_np = NULL;
+	};
+}
+
 static int max77802_pmic_dt_parse_pdata(struct platform_device *pdev,
 					struct max77686_platform_data *pdata)
 {
@@ -555,6 +597,8 @@  static int max77802_pmic_dt_parse_pdata(struct platform_device *pdev,
 		rdata[i].initdata = rmatch.init_data;
 		rdata[i].of_node = rmatch.of_node;
 		rdata[i].id = regulators[i].id;
+		max77802_parse_opmodes(rdata[i].of_node,
+				       &rdata[i].initdata->constraints);
 	}
 
 	pdata->regulators = rdata;