[PATCH] mfd: palmas: Add DVFS mux setting

[PATCH] mfd: palmas: Add DVFS mux setting

[Andrew Chew]

There exists a PRIMARY_SECONDARY_PAD3 in the same register base as
PRIMARY_SECONDARY_PAD2, which controls the function of certain pins. Add
a property for this setting.

Signed-off-by: Andrew Chew <achew@nvidia.com>
---
 Documentation/devicetree/bindings/mfd/palmas.txt |  3 ++-
 drivers/mfd/palmas.c                             | 29 ++++++++++++++++++++++--
 include/linux/mfd/palmas.h                       | 12 +++++++++-
 3 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt b/Documentation/devicetree/bindings/mfd/palmas.txt
index 892537d..a718db5 100644
--- a/Documentation/devicetree/bindings/mfd/palmas.txt
+++ b/Documentation/devicetree/bindings/mfd/palmas.txt
@@ -24,7 +24,7 @@ and also the generic series names
 - interrupt-parent : The parent interrupt controller.
 
 Optional properties:
-  ti,mux-padX : set the pad register X (1-2) to the correct muxing for the
+  ti,mux-padX : set the pad register X (1-3) to the correct muxing for the
 		hardware, if not set will use muxing in OTP.
 
 Example:
@@ -38,6 +38,7 @@ palmas {
 
 	ti,mux-pad1 = <0>;
 	ti,mux-pad2 = <0>;
+	ti,mux-pad3 = <0>;
 
 	#address-cells = <1>;
 	#size-cells = <0>;
diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
index e4d1c70..b07b706 100644
--- a/drivers/mfd/palmas.c
+++ b/drivers/mfd/palmas.c
@@ -219,6 +219,12 @@ static void palmas_dt_to_pdata(struct i2c_client *i2c,
 		pdata->pad2 = prop;
 	}
 
+	ret = of_property_read_u32(node, "ti,mux-pad3", &prop);
+	if (!ret) {
+		pdata->mux_from_pdata = 1;
+		pdata->pad3 = prop;
+	}
+
 	/* The default for this register is all masked */
 	ret = of_property_read_u32(node, "ti,power-ctrl", &prop);
 	if (!ret)
@@ -404,9 +410,28 @@ no_irq:
 	if (!(reg & PALMAS_PRIMARY_SECONDARY_PAD2_GPIO_7_MASK))
 		palmas->gpio_muxed |= PALMAS_GPIO_7_MUXED;
 
-	dev_info(palmas->dev, "Muxing GPIO %x, PWM %x, LED %x\n",
+	addr = PALMAS_BASE_TO_REG(PALMAS_PU_PD_OD_BASE,
+			PALMAS_PRIMARY_SECONDARY_PAD3);
+
+	if (pdata->mux_from_pdata) {
+		reg = pdata->pad3;
+		ret = regmap_write(palmas->regmap[slave], addr, reg);
+		if (ret)
+			goto err_irq;
+	} else {
+		ret = regmap_read(palmas->regmap[slave], addr, &reg);
+		if (ret)
+			goto err_irq;
+	}
+
+	if (reg & PALMAS_PRIMARY_SECONDARY_PAD3_DVFS1)
+		palmas->dvfs_muxed |= PALMAS_DVFS1_MUXED;
+	if (reg & PALMAS_PRIMARY_SECONDARY_PAD3_DVFS2)
+		palmas->dvfs_muxed |= PALMAS_DVFS2_MUXED;
+
+	dev_info(palmas->dev, "Muxing GPIO %x, PWM %x, LED %x, DVFS %x\n",
 			palmas->gpio_muxed, palmas->pwm_muxed,
-			palmas->led_muxed);
+			palmas->led_muxed, palmas->dvfs_muxed);
 
 	reg = pdata->power_ctrl;
 
diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
index 1a8dd7a..e479107 100644
--- a/include/linux/mfd/palmas.h
+++ b/include/linux/mfd/palmas.h
@@ -84,6 +84,7 @@ struct palmas {
 	u8 gpio_muxed;
 	u8 led_muxed;
 	u8 pwm_muxed;
+	u8 dvfs_muxed;
 };
 
 struct palmas_gpadc_platform_data {
@@ -257,7 +258,7 @@ struct palmas_platform_data {
 	 * then the two value to load into the registers if true
 	 */
 	int mux_from_pdata;
-	u8 pad1, pad2;
+	u8 pad1, pad2, pad3;
 
 	struct palmas_pmic_platform_data *pmic_pdata;
 	struct palmas_gpadc_platform_data *gpadc_pdata;
@@ -436,6 +437,9 @@ enum usb_irq_events {
 #define PALMAS_PWM1_MUXED					(1 << 0)
 #define PALMAS_PWM2_MUXED					(1 << 1)
 
+#define PALMAS_DVFS1_MUXED					(1 << 0)
+#define PALMAS_DVFS2_MUXED					(1 << 1)
+
 /* helper macro to get correct slave number */
 #define PALMAS_BASE_TO_SLAVE(x)		((x >> 8) - 1)
 #define PALMAS_BASE_TO_REG(x, y)	((x & 0xff) + y)
@@ -1833,6 +1837,12 @@ enum usb_irq_events {
 #define PALMAS_PRIMARY_SECONDARY_PAD2_GPIO_4			0x01
 #define PALMAS_PRIMARY_SECONDARY_PAD2_GPIO_4_SHIFT		0
 
+/* Bit definitions for PRIMARY_SECONDARY_PAD3 */
+#define PALMAS_PRIMARY_SECONDARY_PAD3_DVFS2			0x02
+#define PALMAS_PRIMARY_SECONDARY_PAD3_DVFS2_SHIFT		1
+#define PALMAS_PRIMARY_SECONDARY_PAD3_DVFS1			0x01
+#define PALMAS_PRIMARY_SECONDARY_PAD3_DVFS1_SHIFT		0
+
 /* Bit definitions for I2C_SPI */
 #define PALMAS_I2C_SPI_I2C2OTP_EN				0x80
 #define PALMAS_I2C_SPI_I2C2OTP_EN_SHIFT				7
-- 
1.8.1.5

Re: [PATCH] mfd: palmas: Add DVFS mux setting

[Stephen Warren]

On 07/26/2013 02:41 PM, Andrew Chew wrote:
> There exists a PRIMARY_SECONDARY_PAD3 in the same register base as
> PRIMARY_SECONDARY_PAD2, which controls the function of certain pins. Add
> a property for this setting.

How does this interact with the pinctrl driver that Laxman just sent for
Palmas?

https://lkml.org/lkml/2013/7/26/141
[PATCH 0/2] pinctrl: palmas: add pincontrol driver

RE: [PATCH] mfd: palmas: Add DVFS mux setting

[Andrew Chew]

> On 07/26/2013 02:41 PM, Andrew Chew wrote:
> > There exists a PRIMARY_SECONDARY_PAD3 in the same register base as
> > PRIMARY_SECONDARY_PAD2, which controls the function of certain pins.
> > Add a property for this setting.
> 
> How does this interact with the pinctrl driver that Laxman just sent for
> Palmas?
> 
> https://lkml.org/lkml/2013/7/26/141
> [PATCH 0/2] pinctrl: palmas: add pincontrol driver

Thanks for pointing this out.  Given this:

+Optional properties:
+- ti,palams-enable-dvfs1: Enable DVFS1. Configure pins for DVFS1 mode.
+- ti,palams-enable-dvfs2: Enable DVFS2. Configure pins for DVFS2 mode.

I think his work already encompasses what my patch is supposed to do.

Abandoning this patch.

Re: [PATCH] mfd: palmas: Add DVFS mux setting

[Stephen Warren]

On 07/26/2013 03:28 PM, Andrew Chew wrote:
>> On 07/26/2013 02:41 PM, Andrew Chew wrote:
>>> There exists a PRIMARY_SECONDARY_PAD3 in the same register base as
>>> PRIMARY_SECONDARY_PAD2, which controls the function of certain pins.
>>> Add a property for this setting.
>>
>> How does this interact with the pinctrl driver that Laxman just sent for
>> Palmas?
>>
>> https://lkml.org/lkml/2013/7/26/141
>> [PATCH 0/2] pinctrl: palmas: add pincontrol driver
> 
> Thanks for pointing this out.  Given this:
> 
> +Optional properties:
> +- ti,palams-enable-dvfs1: Enable DVFS1. Configure pins for DVFS1 mode.
> +- ti,palams-enable-dvfs2: Enable DVFS2. Configure pins for DVFS2 mode.
> 
> I think his work already encompasses what my patch is supposed to do.
> 
> Abandoning this patch.

OK, that's simple!

Are the existing ti,mux-pad1/ti,mux-pad2 properties already in the
binding redundant with Laxman's pinctrl driver?

RE: [PATCH] mfd: palmas: Add DVFS mux setting

[Andrew Chew]

> >> How does this interact with the pinctrl driver that Laxman just sent
> >> for Palmas?
> >>
> >> https://lkml.org/lkml/2013/7/26/141
> >> [PATCH 0/2] pinctrl: palmas: add pincontrol driver
> >
> > Thanks for pointing this out.  Given this:
> >
> > +Optional properties:
> > +- ti,palams-enable-dvfs1: Enable DVFS1. Configure pins for DVFS1 mode.
> > +- ti,palams-enable-dvfs2: Enable DVFS2. Configure pins for DVFS2 mode.
> >
> > I think his work already encompasses what my patch is supposed to do.
> >
> > Abandoning this patch.
> 
> OK, that's simple!
> 
> Are the existing ti,mux-pad1/ti,mux-pad2 properties already in the binding
> redundant with Laxman's pinctrl driver?

In linux-next (where I based my work), yes, those two properties already exist,
and as far as I understand it, are redundant with Laxman's pinctrl driver.
I expect those properties will go away with Laxman's pinctrl driver.

Re: [PATCH] mfd: palmas: Add DVFS mux setting

[Stephen Warren]

On 07/26/2013 03:50 PM, Andrew Chew wrote:
>>>> How does this interact with the pinctrl driver that Laxman just sent
>>>> for Palmas?
>>>>
>>>> https://lkml.org/lkml/2013/7/26/141
>>>> [PATCH 0/2] pinctrl: palmas: add pincontrol driver
>>>
>>> Thanks for pointing this out.  Given this:
>>>
>>> +Optional properties:
>>> +- ti,palams-enable-dvfs1: Enable DVFS1. Configure pins for DVFS1 mode.
>>> +- ti,palams-enable-dvfs2: Enable DVFS2. Configure pins for DVFS2 mode.
>>>
>>> I think his work already encompasses what my patch is supposed to do.
>>>
>>> Abandoning this patch.
>>
>> OK, that's simple!
>>
>> Are the existing ti,mux-pad1/ti,mux-pad2 properties already in the binding
>> redundant with Laxman's pinctrl driver?
> 
> In linux-next (where I based my work), yes, those two properties already exist,
> and as far as I understand it, are redundant with Laxman's pinctrl driver.
> I expect those properties will go away with Laxman's pinctrl driver.

Except those properties have been there for many kernel revisions and
are an ABI and hence can't be removed, although I noticed that they got
renamed recently, and of course we aren't technically being strict about
this quite yet...

Re: the complete pinctrl driver: is anything outside the Palmas going to
need to reprogram the Palmas pinctrl HW at run-time? Are the functions
that can be routed to the pins just static configuration for PMIC
features, or might other generic (non-Palmas) drivers use those pins for
something? If not, perhaps it's be simpler to just add your ti,mux-pad3
property and be done.

RE: [PATCH] mfd: palmas: Add DVFS mux setting

[Andrew Chew]

> >>>> How does this interact with the pinctrl driver that Laxman just
> >>>> sent for Palmas?
> >>>>
> >>>> https://lkml.org/lkml/2013/7/26/141
> >>>> [PATCH 0/2] pinctrl: palmas: add pincontrol driver
> >>>
> >>> Thanks for pointing this out.  Given this:
> >>>
> >>> +Optional properties:
> >>> +- ti,palams-enable-dvfs1: Enable DVFS1. Configure pins for DVFS1
> mode.
> >>> +- ti,palams-enable-dvfs2: Enable DVFS2. Configure pins for DVFS2
> mode.
> >>>
> >>> I think his work already encompasses what my patch is supposed to do.
> >>>
> >>> Abandoning this patch.
> >>
> >> OK, that's simple!
> >>
> >> Are the existing ti,mux-pad1/ti,mux-pad2 properties already in the
> >> binding redundant with Laxman's pinctrl driver?
> >
> > In linux-next (where I based my work), yes, those two properties
> > already exist, and as far as I understand it, are redundant with Laxman's
> pinctrl driver.
> > I expect those properties will go away with Laxman's pinctrl driver.
> 
> Except those properties have been there for many kernel revisions and are
> an ABI and hence can't be removed, although I noticed that they got
> renamed recently, and of course we aren't technically being strict about this
> quite yet...
> 
> Re: the complete pinctrl driver: is anything outside the Palmas going to need
> to reprogram the Palmas pinctrl HW at run-time? Are the functions that can
> be routed to the pins just static configuration for PMIC features, or might
> other generic (non-Palmas) drivers use those pins for something? If not,
> perhaps it's be simpler to just add your ti,mux-pad3 property and be done.

I can imagine other projects wanting to do runtime muxing on those pins.
These pins can serve as GPIOs, or can be programmed for special functions.
For my particular scenario, I just need to statically set that particular mux
register (the power-on default value is not suitable for what we want to
do, namely to use the GPIO_6 pin as an actual GPIO pin).  If the existing
ti,mux-pad1 and ti,mux-pad2 properties are to stay, in the spirit of not changing
the existing ABI, then sure, we can make a case for adding the missing one
(ti,mux-pad3) for completeness.  In this case, if the palmas PMIC's pin
configuration can be statically set at start of day, one won't even need to
instantiate the palmas pinctrl driver at all, and with the addition of ti,mux-pad3,
the pin configuration control will actually be complete. 

Re: [PATCH] mfd: palmas: Add DVFS mux setting

[Laxman Dewangan]

On Saturday 27 July 2013 03:42 AM, Andrew Chew wrote:
>>>>>> How does this interact with the pinctrl driver that Laxman just
>>>>>> sent for Palmas?
>>>>>>
>>>>>> https://lkml.org/lkml/2013/7/26/141
>>>>>> [PATCH 0/2] pinctrl: palmas: add pincontrol driver
>>>>> Thanks for pointing this out.  Given this:
>>>>>
>>>>> +Optional properties:
>>>>> +- ti,palams-enable-dvfs1: Enable DVFS1. Configure pins for DVFS1
>> mode.
>>>>> +- ti,palams-enable-dvfs2: Enable DVFS2. Configure pins for DVFS2
>> mode.
>>>>> I think his work already encompasses what my patch is supposed to do.
>>>>>
>>>>> Abandoning this patch.
>>>> OK, that's simple!
>>>>
>>>> Are the existing ti,mux-pad1/ti,mux-pad2 properties already in the
>>>> binding redundant with Laxman's pinctrl driver?
>>> In linux-next (where I based my work), yes, those two properties
>>> already exist, and as far as I understand it, are redundant with Laxman's
>> pinctrl driver.
>>> I expect those properties will go away with Laxman's pinctrl driver.
>> Except those properties have been there for many kernel revisions and are
>> an ABI and hence can't be removed, although I noticed that they got
>> renamed recently, and of course we aren't technically being strict about this
>> quite yet...
>>
>> Re: the complete pinctrl driver: is anything outside the Palmas going to need
>> to reprogram the Palmas pinctrl HW at run-time? Are the functions that can
>> be routed to the pins just static configuration for PMIC features, or might
>> other generic (non-Palmas) drivers use those pins for something? If not,
>> perhaps it's be simpler to just add your ti,mux-pad3 property and be done.
> I can imagine other projects wanting to do runtime muxing on those pins.
> These pins can serve as GPIOs, or can be programmed for special functions.
> For my particular scenario, I just need to statically set that particular mux
> register (the power-on default value is not suitable for what we want to
> do, namely to use the GPIO_6 pin as an actual GPIO pin).  If the existing
> ti,mux-pad1 and ti,mux-pad2 properties are to stay, in the spirit of not changing
> the existing ABI, then sure, we can make a case for adding the missing one
> (ti,mux-pad3) for completeness.  In this case, if the palmas PMIC's pin
> configuration can be statically set at start of day, one won't even need to
> instantiate the palmas pinctrl driver at all, and with the addition of ti,mux-pad3,
> the pin configuration control will actually be complete.

In downstream, we are using it only during initialisation. There is no 
need to do it on runtime.

But main issue which I wanted to address by pinctrl driver is to:
- provide the abstraction.
- More configuration for open drain and pull up/down.

The way we are passing the mux_padx register value, it directly written 
into the register, And hence the is no abstraction and need to refer 
datasheet always to look the configuration. With the having proper 
pinmux driver, the initialisation is easy to handle and easy read from code.

Also on this apparach there is no configuration provided for the pull 
up/down and open drain. This will be available if we have pincontrol driver.

once we will have the pincontrol driver then mux pads are become redundant.

Re: [PATCH] mfd: palmas: Add DVFS mux setting

[Stephen Warren]

On 07/27/2013 03:55 AM, Laxman Dewangan wrote:
> On Saturday 27 July 2013 03:42 AM, Andrew Chew wrote:
>> I wrote:
>>> Andrew wrote:
>>>> [adding a third pinmux configuration property to Palmas's DT]
>>>
>>> How does this interact with the pinctrl driver that Laxman just
>>> sent for Palmas?
>>>
>>> https://lkml.org/lkml/2013/7/26/141
>>> [PATCH 0/2] pinctrl: palmas: add pincontrol driver
..
>> Abandoning this patch.
...
> once we will have the pincontrol driver then mux pads are become redundant.

OK. The driver should probably operate like this then:

* During probe(), parse the ti,mux-pad* parameters, if present, and
apply them. This is needed to maintain compatibility with old DTs that
may contain these properties.

* At the end of probe(), register the pinctrl driver. If standard
pinctrl properties are present in DT, these will then be applied. These
may override the values set by any ti,mux-pad* properties if they were
present.

Also, we should remove, or mark deprecated, the ti,mux-pad* properties
in the binding document when adding pinctrl support.

RE: [PATCH] mfd: palmas: Add DVFS mux setting

[Andrew Chew]

> On 07/27/2013 03:55 AM, Laxman Dewangan wrote:
> > On Saturday 27 July 2013 03:42 AM, Andrew Chew wrote:
> >> I wrote:
> >>> Andrew wrote:
> >>>> [adding a third pinmux configuration property to Palmas's DT]
> >>>
> >>> How does this interact with the pinctrl driver that Laxman just sent
> >>> for Palmas?
> >>>
> >>> https://lkml.org/lkml/2013/7/26/141
> >>> [PATCH 0/2] pinctrl: palmas: add pincontrol driver
> ..
> >> Abandoning this patch.
> ...
> > once we will have the pincontrol driver then mux pads are become
> redundant.
> 
> OK. The driver should probably operate like this then:
> 
> * During probe(), parse the ti,mux-pad* parameters, if present, and apply
> them. This is needed to maintain compatibility with old DTs that may contain
> these properties.
> 
> * At the end of probe(), register the pinctrl driver. If standard pinctrl
> properties are present in DT, these will then be applied. These may override
> the values set by any ti,mux-pad* properties if they were present.
> 
> Also, we should remove, or mark deprecated, the ti,mux-pad* properties in
> the binding document when adding pinctrl support.

Sounds reasonable to me.  The fate of my patch hasn't really been discussed, though.
Can we apply it, to make the ti,mux-pad* stuff complete?

Re: [PATCH] mfd: palmas: Add DVFS mux setting

[Stephen Warren]

On 07/30/2013 02:53 PM, Andrew Chew wrote:
>> On 07/27/2013 03:55 AM, Laxman Dewangan wrote:
>>> On Saturday 27 July 2013 03:42 AM, Andrew Chew wrote:
>>>> I wrote:
>>>>> Andrew wrote:
>>>>>> [adding a third pinmux configuration property to Palmas's DT]
>>>>>
>>>>> How does this interact with the pinctrl driver that Laxman just sent
>>>>> for Palmas?
>>>>>
>>>>> https://lkml.org/lkml/2013/7/26/141
>>>>> [PATCH 0/2] pinctrl: palmas: add pincontrol driver
>> ..
>>>> Abandoning this patch.
>> ...
>>> once we will have the pincontrol driver then mux pads are become
>> redundant.
>>
>> OK. The driver should probably operate like this then:
>>
>> * During probe(), parse the ti,mux-pad* parameters, if present, and apply
>> them. This is needed to maintain compatibility with old DTs that may contain
>> these properties.
>>
>> * At the end of probe(), register the pinctrl driver. If standard pinctrl
>> properties are present in DT, these will then be applied. These may override
>> the values set by any ti,mux-pad* properties if they were present.
>>
>> Also, we should remove, or mark deprecated, the ti,mux-pad* properties in
>> the binding document when adding pinctrl support.
> 
> Sounds reasonable to me.  The fate of my patch hasn't really been discussed, though.
> Can we apply it, to make the ti,mux-pad* stuff complete?

Oh, I thought you said you were dropping it. I don't see a need to apply
it if the correct approach is Laxman's pinctrl driver.