[PATCH 1/4] ARM: dts: exynos5250-snow: add pinctrl for i2c-arbitrator

[PATCH 1/4] ARM: dts: exynos5250-snow: add pinctrl for i2c-arbitrator

[Sachin Kamat]

From: Doug Anderson <dianders@chromium.org>

Added i2c-arbitrator pinctrl node to Snow board.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 arch/arm/boot/dts/exynos5250-snow.dts |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
index 1ce1088..32715b3 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -39,6 +39,22 @@
 		};
 	};
 
+	pinctrl@13400000 {
+		arb_their_claim: arb-their-claim {
+			samsung,pins = "gpe0-4";
+			samsung,pin-function = <0>;
+			samsung,pin-pud = <3>;
+			samsung,pin-drv = <0>;
+		};
+
+		arb_our_claim: arb-our-claim {
+			samsung,pins = "gpf0-3";
+			samsung,pin-function = <1>;
+			samsung,pin-pud = <0>;
+			samsung,pin-drv = <0>;
+		};
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys";
 
@@ -65,6 +81,9 @@
 		wait-retry-us = <3000>;
 		wait-free-us = <50000>;
 
+		pinctrl-names = "default";
+		pinctrl-0 = <&arb_our_claim &arb_their_claim>;
+
 		/* Use ID 104 as a hint that we're on physical bus 4 */
 		i2c_104: i2c@0 {
 			reg = <0>;
-- 
1.7.9.5

[PATCH 2/4] ARM: dts: exynos5250-snow: add pinctrl for EC irq

[Sachin Kamat]

From: Doug Anderson <dianders@chromium.org>

Added pinctrl node for embedded controller (EC) IRQ
on Snow board.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 arch/arm/boot/dts/exynos5250-snow.dts |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
index 32715b3..469c85d 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -25,6 +25,13 @@
 	};
 
 	pinctrl@11400000 {
+		ec_irq: ec-irq {
+			samsung,pins = "gpx1-6";
+			samsung,pin-function = <0>;
+			samsung,pin-pud = <0>;
+			samsung,pin-drv = <0>;
+		};
+
 		sd3_clk: sd3-clk {
 			samsung,pin-drv = <0>;
 		};
@@ -101,6 +108,8 @@
 				reg = <0x1e>;
 				interrupts = <6 0>;
 				interrupt-parent = <&gpx1>;
+				pinctrl-names = "default";
+				pinctrl-0 = <&ec_irq>;
 				wakeup-source;
 
 				keyboard-controller {
-- 
1.7.9.5

[PATCH 3/4] ARM: dts: exynos5250-snow: add tps65090 power regulator

[Sachin Kamat]

From: Doug Anderson <dianders@chromium.org>

Added TPS65090 regulator related nodes to Snow board.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 arch/arm/boot/dts/exynos5250-snow.dts |   90 +++++++++++++++++++++++++++++++++
 1 file changed, 90 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
index 469c85d..a38fd18 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -44,6 +44,13 @@
 		sd3_bus4: sd3-bus-width4 {
 			samsung,pin-drv = <0>;
 		};
+
+		tps65090_irq: tps65090-irq {
+			samsung,pins = "gpx2-6";
+			samsung,pin-function = <0>;
+			samsung,pin-pud = <0>;
+			samsung,pin-drv = <0>;
+		};
 	};
 
 	pinctrl@13400000 {
@@ -75,6 +82,12 @@
 		};
 	};
 
+	vbat: vbat-fixed-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vbat-supply";
+		regulator-boot-on;
+	};
+
 	i2c-arbitrator {
 		compatible = "i2c-arb-gpio-challenge";
 		#address-cells = <1>;
@@ -201,6 +214,83 @@
 							0x070c0069>;	/* LEFT */
 				};
 			};
+
+			power-regulator {
+				compatible = "ti,tps65090";
+				reg = <0x48>;
+
+				/*
+				 * Config irq to disable internal pulls
+				 * even though we run in polling mode.
+				 */
+				pinctrl-names = "default";
+				pinctrl-0 = <&tps65090_irq>;
+
+				vsys1-supply = <&vbat>;
+				vsys2-supply = <&vbat>;
+				vsys3-supply = <&vbat>;
+				infet1-supply = <&vbat>;
+				infet2-supply = <&vbat>;
+				infet3-supply = <&vbat>;
+				infet4-supply = <&vbat>;
+				infet5-supply = <&vbat>;
+				infet6-supply = <&vbat>;
+				infet7-supply = <&vbat>;
+				vsys-l1-supply = <&vbat>;
+				vsys-l2-supply = <&vbat>;
+
+				regulators {
+					dcdc1 {
+						ti,enable-ext-control;
+					};
+					dcdc2 {
+						ti,enable-ext-control;
+					};
+					dcdc3 {
+						ti,enable-ext-control;
+					};
+					fet1 {
+						regulator-name = "vcd_led";
+						ti,overcurrent-wait = <3>;
+					};
+					tps65090_fet2: fet2 {
+						regulator-name = "video_mid";
+						regulator-always-on;
+						ti,overcurrent-wait = <3>;
+					};
+					fet3 {
+						regulator-name = "wwan_r";
+						regulator-always-on;
+						ti,overcurrent-wait = <3>;
+					};
+					fet4 {
+						regulator-name = "sdcard";
+						ti,overcurrent-wait = <3>;
+					};
+					fet5 {
+						regulator-name = "camout";
+						regulator-always-on;
+						ti,overcurrent-wait = <3>;
+					};
+					fet6 {
+						regulator-name = "lcd_vdd";
+						ti,overcurrent-wait = <3>;
+					};
+					tps65090_fet7: fet7 {
+						regulator-name = "video_mid_1a";
+						regulator-always-on;
+						ti,overcurrent-wait = <3>;
+					};
+					ldo1 {
+					};
+					ldo2 {
+					};
+				};
+
+				charger {
+					compatible = "ti,tps65090-charger";
+				};
+			};
 		};
 	};
 
-- 
1.7.9.5

[PATCH 4/4] ARM: dts: exynos5250-snow: add ptn3460 node

[Sachin Kamat]

From: Doug Anderson <dianders@chromium.org>

Added nodes for ptn3460 driver to Snow board.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 arch/arm/boot/dts/exynos5250-snow.dts |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
index a38fd18..52ae335 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -32,6 +32,13 @@
 			samsung,pin-drv = <0>;
 		};
 
+		ptn3460_gpios: ptn3460-gpios {
+			samsung,pins = "gpy2-5", "gpx1-5";
+			samsung,pin-function = <1>;
+			samsung,pin-pud = <0>;
+			samsung,pin-drv = <0>;
+		};
+
 		sd3_clk: sd3-clk {
 			samsung,pin-drv = <0>;
 		};
@@ -324,4 +331,16 @@
 			clock-frequency = <24000000>;
 		};
 	};
+
+	i2c_7: i2c@12CD0000 {
+		ptn3460-bridge@20 {
+			compatible = "nxp,ptn3460";
+			reg = <0x20>;
+			powerdown-gpio = <&gpy2 5 0>;
+			reset-gpio = <&gpx1 5 0>;
+			edid-emulation = <5>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&ptn3460_gpios>;
+		};
+	};
 };
-- 
1.7.9.5

Re: [PATCH 1/4] ARM: dts: exynos5250-snow: add pinctrl for i2c-arbitrator

[Doug Anderson]

Sachin,

On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> From: Doug Anderson <dianders@chromium.org>

I probably wouldn't have bothered giving me authorship since this
isn't exactly a clean patch from the chromium tree (you pulled the
proper pieces yourself, did the commit message yourself, etc).  ...but
I appreciate the thought and as far as I know setting the "author" in
cases like this is a bit of a judgement call...

The Signed-off-by is certainly correct.  ;)

>
> Added i2c-arbitrator pinctrl node to Snow board.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  arch/arm/boot/dts/exynos5250-snow.dts |   19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)

This matches what's in our tree and and is what people are using, so:

Reviewed-by: Doug Anderson <dianders@chromium.org>


> diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
> index 1ce1088..32715b3 100644
> --- a/arch/arm/boot/dts/exynos5250-snow.dts
> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
> @@ -39,6 +39,22 @@
>                 };
>         };
>
> +       pinctrl@13400000 {
> +               arb_their_claim: arb-their-claim {
> +                       samsung,pins = "gpe0-4";
> +                       samsung,pin-function = <0>;
> +                       samsung,pin-pud = <3>;
> +                       samsung,pin-drv = <0>;
> +               };
> +
> +               arb_our_claim: arb-our-claim {
> +                       samsung,pins = "gpf0-3";
> +                       samsung,pin-function = <1>;
> +                       samsung,pin-pud = <0>;
> +                       samsung,pin-drv = <0>;
> +               };

It's odd to me that one of these has a pullup but not the other, but I
think that's because the arbitration lines ended up using some other
signals that were originally hooked up for other usage.  Certainly the
pullups / pulldowns match what's in our tree and also match what we
had in the original shipping 3.4 kernel.

Re: [PATCH 2/4] ARM: dts: exynos5250-snow: add pinctrl for EC irq

[Doug Anderson]

Sachin,

On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> From: Doug Anderson <dianders@chromium.org>
>
> Added pinctrl node for embedded controller (EC) IRQ
> on Snow board.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  arch/arm/boot/dts/exynos5250-snow.dts |    9 +++++++++
>  1 file changed, 9 insertions(+)

Reviewed-by: Doug Anderson <dianders@chromium.org>

Re: [PATCH 1/4] ARM: dts: exynos5250-snow: add pinctrl for i2c-arbitrator

[Tomasz Figa]

Hi Doug,

On 15.04.2014 00:30, Doug Anderson wrote:
> Sachin,
>
> On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>> From: Doug Anderson <dianders@chromium.org>
>
> I probably wouldn't have bothered giving me authorship since this
> isn't exactly a clean patch from the chromium tree (you pulled the
> proper pieces yourself, did the commit message yourself, etc).  ...but
> I appreciate the thought and as far as I know setting the "author" in
> cases like this is a bit of a judgement call...
>
> The Signed-off-by is certainly correct.  ;)
>
>>
>> Added i2c-arbitrator pinctrl node to Snow board.
>>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> ---
>>   arch/arm/boot/dts/exynos5250-snow.dts |   19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>
> This matches what's in our tree and and is what people are using, so:
>
> Reviewed-by: Doug Anderson <dianders@chromium.org>
>
>
>> diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
>> index 1ce1088..32715b3 100644
>> --- a/arch/arm/boot/dts/exynos5250-snow.dts
>> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
>> @@ -39,6 +39,22 @@
>>                  };
>>          };
>>
>> +       pinctrl@13400000 {
>> +               arb_their_claim: arb-their-claim {
>> +                       samsung,pins = "gpe0-4";
>> +                       samsung,pin-function = <0>;
>> +                       samsung,pin-pud = <3>;
>> +                       samsung,pin-drv = <0>;
>> +               };
>> +
>> +               arb_our_claim: arb-our-claim {
>> +                       samsung,pins = "gpf0-3";
>> +                       samsung,pin-function = <1>;
>> +                       samsung,pin-pud = <0>;
>> +                       samsung,pin-drv = <0>;
>> +               };
>
> It's odd to me that one of these has a pullup but not the other, but I
> think that's because the arbitration lines ended up using some other
> signals that were originally hooked up for other usage.  Certainly the
> pullups / pulldowns match what's in our tree and also match what we
> had in the original shipping 3.4 kernel.

Just a wild guess, but probably the input needs a pull-up, while 
obviously the output doesn't. I don't have much idea about the 
arbitration thing happening on snow, so I can't say much about this 
series. (Maybe description of patch 1/4 should be saying a bit more 
about the meaning of this?)

Best regards,
Tomasz

Re: [PATCH 3/4] ARM: dts: exynos5250-snow: add tps65090 power regulator

[Doug Anderson]

Sachin,

On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> From: Doug Anderson <dianders@chromium.org>
>
> Added TPS65090 regulator related nodes to Snow board.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  arch/arm/boot/dts/exynos5250-snow.dts |   90 +++++++++++++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
> index 469c85d..a38fd18 100644
> --- a/arch/arm/boot/dts/exynos5250-snow.dts
> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
> @@ -44,6 +44,13 @@
>                 sd3_bus4: sd3-bus-width4 {
>                         samsung,pin-drv = <0>;
>                 };
> +
> +               tps65090_irq: tps65090-irq {
> +                       samsung,pins = "gpx2-6";
> +                       samsung,pin-function = <0>;
> +                       samsung,pin-pud = <0>;
> +                       samsung,pin-drv = <0>;
> +               };
>         };
>
>         pinctrl@13400000 {
> @@ -75,6 +82,12 @@
>                 };
>         };
>
> +       vbat: vbat-fixed-regulator {
> +               compatible = "regulator-fixed";
> +               regulator-name = "vbat-supply";
> +               regulator-boot-on;
> +       };
> +
>         i2c-arbitrator {
>                 compatible = "i2c-arb-gpio-challenge";
>                 #address-cells = <1>;
> @@ -201,6 +214,83 @@
>                                                         0x070c0069>;    /* LEFT */
>                                 };
>                         };
> +
> +                       power-regulator {
> +                               compatible = "ti,tps65090";
> +                               reg = <0x48>;
> +
> +                               /*
> +                                * Config irq to disable internal pulls
> +                                * even though we run in polling mode.

This comment refers to a feature that hasn't yet been sent upstream.
...but it's a good reminder and I can send it upstream now.  You can
see it at:

* https://chromium-review.googlesource.com/62630
* https://chromium-review.googlesource.com/175653

Since this is only a comment I guess it's OK to refer to a feature
that hasn't yet landed (?).


> +                                */
> +                               pinctrl-names = "default";
> +                               pinctrl-0 = <&tps65090_irq>;
> +
> +                               vsys1-supply = <&vbat>;
> +                               vsys2-supply = <&vbat>;
> +                               vsys3-supply = <&vbat>;
> +                               infet1-supply = <&vbat>;
> +                               infet2-supply = <&vbat>;
> +                               infet3-supply = <&vbat>;
> +                               infet4-supply = <&vbat>;
> +                               infet5-supply = <&vbat>;
> +                               infet6-supply = <&vbat>;
> +                               infet7-supply = <&vbat>;
> +                               vsys-l1-supply = <&vbat>;
> +                               vsys-l2-supply = <&vbat>;
> +
> +                               regulators {
> +                                       dcdc1 {
> +                                               ti,enable-ext-control;
> +                                       };
> +                                       dcdc2 {
> +                                               ti,enable-ext-control;
> +                                       };
> +                                       dcdc3 {
> +                                               ti,enable-ext-control;
> +                                       };
> +                                       fet1 {
> +                                               regulator-name = "vcd_led";
> +                                               ti,overcurrent-wait = <3>;

This is also referring to something that hasn't been sent upstream yet.  See:
* https://chromium-review.googlesource.com/184607

> +                                       };
> +                                       tps65090_fet2: fet2 {
> +                                               regulator-name = "video_mid";
> +                                               regulator-always-on;
> +                                               ti,overcurrent-wait = <3>;
> +                                       };
> +                                       fet3 {
> +                                               regulator-name = "wwan_r";
> +                                               regulator-always-on;
> +                                               ti,overcurrent-wait = <3>;
> +                                       };
> +                                       fet4 {
> +                                               regulator-name = "sdcard";
> +                                               ti,overcurrent-wait = <3>;
> +                                       };
> +                                       fet5 {
> +                                               regulator-name = "camout";
> +                                               regulator-always-on;
> +                                               ti,overcurrent-wait = <3>;
> +                                       };
> +                                       fet6 {
> +                                               regulator-name = "lcd_vdd";
> +                                               ti,overcurrent-wait = <3>;
> +                                       };
> +                                       tps65090_fet7: fet7 {
> +                                               regulator-name = "video_mid_1a";
> +                                               regulator-always-on;
> +                                               ti,overcurrent-wait = <3>;
> +                                       };
> +                                       ldo1 {
> +                                       };
> +                                       ldo2 {
> +                                       };
> +                               };
> +
> +                               charger {
> +                                       compatible = "ti,tps65090-charger";
> +                               };

I'm nearly certain that the charger will be pretty unhappy without the
polling mode change mentioned above.

> +                       };
>                 };
>         };
>

-Doug

Re: [PATCH 1/4] ARM: dts: exynos5250-snow: add pinctrl for i2c-arbitrator

[Doug Anderson]

Tomasz,

On Mon, Apr 14, 2014 at 3:38 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Doug,
>
>
> On 15.04.2014 00:30, Doug Anderson wrote:
>>
>> Sachin,
>>
>> On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat <sachin.kamat@linaro.org>
>> wrote:
>>>
>>> From: Doug Anderson <dianders@chromium.org>
>>
>>
>> I probably wouldn't have bothered giving me authorship since this
>> isn't exactly a clean patch from the chromium tree (you pulled the
>> proper pieces yourself, did the commit message yourself, etc).  ...but
>> I appreciate the thought and as far as I know setting the "author" in
>> cases like this is a bit of a judgement call...
>>
>> The Signed-off-by is certainly correct.  ;)
>>
>>>
>>> Added i2c-arbitrator pinctrl node to Snow board.
>>>
>>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>>> ---
>>>   arch/arm/boot/dts/exynos5250-snow.dts |   19 +++++++++++++++++++
>>>   1 file changed, 19 insertions(+)
>>
>>
>> This matches what's in our tree and and is what people are using, so:
>>
>> Reviewed-by: Doug Anderson <dianders@chromium.org>
>>
>>
>>> diff --git a/arch/arm/boot/dts/exynos5250-snow.dts
>>> b/arch/arm/boot/dts/exynos5250-snow.dts
>>> index 1ce1088..32715b3 100644
>>> --- a/arch/arm/boot/dts/exynos5250-snow.dts
>>> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
>>> @@ -39,6 +39,22 @@
>>>                  };
>>>          };
>>>
>>> +       pinctrl@13400000 {
>>> +               arb_their_claim: arb-their-claim {
>>> +                       samsung,pins = "gpe0-4";
>>> +                       samsung,pin-function = <0>;
>>> +                       samsung,pin-pud = <3>;
>>> +                       samsung,pin-drv = <0>;
>>> +               };
>>> +
>>> +               arb_our_claim: arb-our-claim {
>>> +                       samsung,pins = "gpf0-3";
>>> +                       samsung,pin-function = <1>;
>>> +                       samsung,pin-pud = <0>;
>>> +                       samsung,pin-drv = <0>;
>>> +               };
>>
>>
>> It's odd to me that one of these has a pullup but not the other, but I
>> think that's because the arbitration lines ended up using some other
>> signals that were originally hooked up for other usage.  Certainly the
>> pullups / pulldowns match what's in our tree and also match what we
>> had in the original shipping 3.4 kernel.
>
>
> Just a wild guess, but probably the input needs a pull-up, while obviously
> the output doesn't. I don't have much idea about the arbitration thing
> happening on snow, so I can't say much about this series. (Maybe description
> of patch 1/4 should be saying a bit more about the meaning of this?)

Right, of course.  I'm not sure quite what I was thinking.  I think I
was getting confused since these go through level converters and my
brain was in open drain mode.  ...but looking at this again this looks
reasonable.

I think the whole discussion of arbitration was from a long time ago.
I think it's fairly well documented in the "i2c-arb-gpio-challenge"
driver.

Basically it looks like Sachin is getting pinctrl stuff matched up
properly for the device tree that's upstream.

-Doug

Re: [PATCH 4/4] ARM: dts: exynos5250-snow: add ptn3460 node

[Olof Johansson]

Hi Sachin,

On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> From: Doug Anderson <dianders@chromium.org>
>
> Added nodes for ptn3460 driver to Snow board.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>

I'm curious, how are you testing this code on the upstream kernel
before you post the patches?

Especially since Doug was referring to some of these containing parts
that is needed for drivers that are not yet upstreamed.


-Olof

Re: [PATCH 4/4] ARM: dts: exynos5250-snow: add ptn3460 node

[Doug Anderson]

Sachin,

On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> From: Doug Anderson <dianders@chromium.org>

For this patch in particular giving me authorship is pretty tenuous.
I don't think I was even the author of the majority of the code in the
chromium tree.


> Added nodes for ptn3460 driver to Snow board.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  arch/arm/boot/dts/exynos5250-snow.dts |   19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)

I don't see any problems here but really the graphics people are the
ones that should be commenting.  Added a few folks to the CC list.

For the general GPIOs / pinctrl stuff, though:

Reviewed-by: Doug Anderson <dianders@chromium.org>


...and as Olof points out, please let us know how this was all tested.

Re: [PATCH 3/4] ARM: dts: exynos5250-snow: add tps65090 power regulator

[Sachin Kamat]

Hi Doug,

On 15 April 2014 04:13, Doug Anderson <dianders@chromium.org> wrote:
> Sachin,
>
> On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>> From: Doug Anderson <dianders@chromium.org>
>>
>> Added TPS65090 regulator related nodes to Snow board.
>>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> ---
>>  arch/arm/boot/dts/exynos5250-snow.dts |   90 +++++++++++++++++++++++++++++++++
>>  1 file changed, 90 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
>> index 469c85d..a38fd18 100644
>> --- a/arch/arm/boot/dts/exynos5250-snow.dts
>> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
>> @@ -44,6 +44,13 @@
>>                 sd3_bus4: sd3-bus-width4 {
>>                         samsung,pin-drv = <0>;
>>                 };
>> +
>> +               tps65090_irq: tps65090-irq {
>> +                       samsung,pins = "gpx2-6";
>> +                       samsung,pin-function = <0>;
>> +                       samsung,pin-pud = <0>;
>> +                       samsung,pin-drv = <0>;
>> +               };
>>         };
>>
>>         pinctrl@13400000 {
>> @@ -75,6 +82,12 @@
>>                 };
>>         };
>>
>> +       vbat: vbat-fixed-regulator {
>> +               compatible = "regulator-fixed";
>> +               regulator-name = "vbat-supply";
>> +               regulator-boot-on;
>> +       };
>> +
>>         i2c-arbitrator {
>>                 compatible = "i2c-arb-gpio-challenge";
>>                 #address-cells = <1>;
>> @@ -201,6 +214,83 @@
>>                                                         0x070c0069>;    /* LEFT */
>>                                 };
>>                         };
>> +
>> +                       power-regulator {
>> +                               compatible = "ti,tps65090";
>> +                               reg = <0x48>;
>> +
>> +                               /*
>> +                                * Config irq to disable internal pulls
>> +                                * even though we run in polling mode.
>
> This comment refers to a feature that hasn't yet been sent upstream.
> ...but it's a good reminder and I can send it upstream now.  You can
> see it at:
>
> * https://chromium-review.googlesource.com/62630
> * https://chromium-review.googlesource.com/175653
>
> Since this is only a comment I guess it's OK to refer to a feature
> that hasn't yet landed (?).
>
>
>> +                                */
>> +                               pinctrl-names = "default";
>> +                               pinctrl-0 = <&tps65090_irq>;
>> +
>> +                               vsys1-supply = <&vbat>;
>> +                               vsys2-supply = <&vbat>;
>> +                               vsys3-supply = <&vbat>;
>> +                               infet1-supply = <&vbat>;
>> +                               infet2-supply = <&vbat>;
>> +                               infet3-supply = <&vbat>;
>> +                               infet4-supply = <&vbat>;
>> +                               infet5-supply = <&vbat>;
>> +                               infet6-supply = <&vbat>;
>> +                               infet7-supply = <&vbat>;
>> +                               vsys-l1-supply = <&vbat>;
>> +                               vsys-l2-supply = <&vbat>;
>> +
>> +                               regulators {
>> +                                       dcdc1 {
>> +                                               ti,enable-ext-control;
>> +                                       };
>> +                                       dcdc2 {
>> +                                               ti,enable-ext-control;
>> +                                       };
>> +                                       dcdc3 {
>> +                                               ti,enable-ext-control;
>> +                                       };
>> +                                       fet1 {
>> +                                               regulator-name = "vcd_led";
>> +                                               ti,overcurrent-wait = <3>;
>
> This is also referring to something that hasn't been sent upstream yet.  See:
> * https://chromium-review.googlesource.com/184607
>
>> +                                       };
>> +                                       tps65090_fet2: fet2 {
>> +                                               regulator-name = "video_mid";
>> +                                               regulator-always-on;
>> +                                               ti,overcurrent-wait = <3>;
>> +                                       };
>> +                                       fet3 {
>> +                                               regulator-name = "wwan_r";
>> +                                               regulator-always-on;
>> +                                               ti,overcurrent-wait = <3>;
>> +                                       };
>> +                                       fet4 {
>> +                                               regulator-name = "sdcard";
>> +                                               ti,overcurrent-wait = <3>;
>> +                                       };
>> +                                       fet5 {
>> +                                               regulator-name = "camout";
>> +                                               regulator-always-on;
>> +                                               ti,overcurrent-wait = <3>;
>> +                                       };
>> +                                       fet6 {
>> +                                               regulator-name = "lcd_vdd";
>> +                                               ti,overcurrent-wait = <3>;
>> +                                       };
>> +                                       tps65090_fet7: fet7 {
>> +                                               regulator-name = "video_mid_1a";
>> +                                               regulator-always-on;
>> +                                               ti,overcurrent-wait = <3>;
>> +                                       };
>> +                                       ldo1 {
>> +                                       };
>> +                                       ldo2 {
>> +                                       };
>> +                               };
>> +
>> +                               charger {
>> +                                       compatible = "ti,tps65090-charger";
>> +                               };
>
> I'm nearly certain that the charger will be pretty unhappy without the
> polling mode change mentioned above.

Since the patches that you pointed out were in the internal tree, did
not realise they
had not yet hit mainline. This patch should certainly come after that.

-- 
With warm regards,
Sachin

Re: [PATCH 3/4] ARM: dts: exynos5250-snow: add tps65090 power regulator

[Doug Anderson]

Sachin,

On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> From: Doug Anderson <dianders@chromium.org>
>
> Added TPS65090 regulator related nodes to Snow board.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  arch/arm/boot/dts/exynos5250-snow.dts |   90 +++++++++++++++++++++++++++++++++
>  1 file changed, 90 insertions(+)

I've now sent up the relevant patches:
* https://patchwork.kernel.org/patch/3995361/
* https://patchwork.kernel.org/patch/3995401/
* https://patchwork.kernel.org/patch/3995411/

So assuming those land then I think we're all good with this one.
I've also done basic testing with it.

Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>

Re: [PATCH 4/4] ARM: dts: exynos5250-snow: add ptn3460 node

[Ajay kumar]

Hi,

On Tue, Apr 15, 2014 at 4:24 AM, Olof Johansson <olof@lixom.net> wrote:
> Hi Sachin,
>
> On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>> From: Doug Anderson <dianders@chromium.org>
>>
>> Added nodes for ptn3460 driver to Snow board.
>>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>
> I'm curious, how are you testing this code on the upstream kernel
> before you post the patches?
>
> Especially since Doug was referring to some of these containing parts
> that is needed for drivers that are not yet upstreamed.

Its better to ignore this patch as of now, since discussion is still going
on, at http://www.spinics.net/lists/dri-devel/msg58474.html
I guess we will have few more DT bindings. Once it is finalized,
I shall resend this again.

Regards,
Ajay Kumar

Re: [PATCH 4/4] ARM: dts: exynos5250-snow: add ptn3460 node

[Kukjin Kim]

On 04/30/14 22:03, Ajay kumar wrote:
> Hi,
>
Hi,

> On Tue, Apr 15, 2014 at 4:24 AM, Olof Johansson<olof@lixom.net>  wrote:
>> Hi Sachin,
>>
>> On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat<sachin.kamat@linaro.org>  wrote:
>>> From: Doug Anderson<dianders@chromium.org>
>>>
>>> Added nodes for ptn3460 driver to Snow board.
>>>
>>> Signed-off-by: Doug Anderson<dianders@chromium.org>
>>> Signed-off-by: Ajay Kumar<ajaykumar.rs@samsung.com>
>>> Signed-off-by: Sachin Kamat<sachin.kamat@linaro.org>
>>
>> I'm curious, how are you testing this code on the upstream kernel
>> before you post the patches?
>>
>> Especially since Doug was referring to some of these containing parts
>> that is needed for drivers that are not yet upstreamed.
>
> Its better to ignore this patch as of now, since discussion is still going
> on, at http://www.spinics.net/lists/dri-devel/msg58474.html
> I guess we will have few more DT bindings. Once it is finalized,
> I shall resend this again.
>
This means this whole series?

Can you please re-spin as soon as possible if you want to see this 
series in 3.16?

Thanks,
Kukjin

Re: [PATCH 4/4] ARM: dts: exynos5250-snow: add ptn3460 node

[Doug Anderson]

Kukjin,

On Thu, May 15, 2014 at 12:38 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> On 04/30/14 22:03, Ajay kumar wrote:
>>
>> Hi,
>>
> Hi,
>
>> On Tue, Apr 15, 2014 at 4:24 AM, Olof Johansson<olof@lixom.net>  wrote:
>>>
>>> Hi Sachin,
>>>
>>> On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat<sachin.kamat@linaro.org>
>>> wrote:
>>>>
>>>> From: Doug Anderson<dianders@chromium.org>
>>>>
>>>> Added nodes for ptn3460 driver to Snow board.
>>>>
>>>> Signed-off-by: Doug Anderson<dianders@chromium.org>
>>>> Signed-off-by: Ajay Kumar<ajaykumar.rs@samsung.com>
>>>> Signed-off-by: Sachin Kamat<sachin.kamat@linaro.org>
>>>
>>>
>>> I'm curious, how are you testing this code on the upstream kernel
>>> before you post the patches?
>>>
>>> Especially since Doug was referring to some of these containing parts
>>> that is needed for drivers that are not yet upstreamed.
>>
>>
>> Its better to ignore this patch as of now, since discussion is still going
>> on, at http://www.spinics.net/lists/dri-devel/msg58474.html
>> I guess we will have few more DT bindings. Once it is finalized,
>> I shall resend this again.
>>
> This means this whole series?

I think you can land everything except the last patch.

The tps65090 charger portion of the MFD will fail to probe (assuming
it is configured) since we're still waiting for a review on
<https://patchwork.kernel.org/patch/4042741/>.  ...but it doesn't hurt
to take the device tree part.

-Doug

Re: [PATCH 1/4] ARM: dts: exynos5250-snow: add pinctrl for i2c-arbitrator

[Kukjin Kim]

On 04/15/14 07:53, Doug Anderson wrote:

+ DT ML

> Tomasz,
>

> On Mon, Apr 14, 2014 at 3:38 PM, Tomasz Figa<tomasz.figa@gmail.com>  wrote:
>> Hi Doug,
>>
>>
>> On 15.04.2014 00:30, Doug Anderson wrote:
>>>
>>> Sachin,
>>>
>>> On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat<sachin.kamat@linaro.org>
>>> wrote:
>>>>
>>>> From: Doug Anderson<dianders@chromium.org>
>>>
>>>
>>> I probably wouldn't have bothered giving me authorship since this
>>> isn't exactly a clean patch from the chromium tree (you pulled the
>>> proper pieces yourself, did the commit message yourself, etc).  ...but
>>> I appreciate the thought and as far as I know setting the "author" in
>>> cases like this is a bit of a judgement call...
>>>
>>> The Signed-off-by is certainly correct.  ;)
>>>
>>>>
>>>> Added i2c-arbitrator pinctrl node to Snow board.
>>>>
>>>> Signed-off-by: Doug Anderson<dianders@chromium.org>
>>>> Signed-off-by: Sachin Kamat<sachin.kamat@linaro.org>
>>>> ---
>>>>    arch/arm/boot/dts/exynos5250-snow.dts |   19 +++++++++++++++++++
>>>>    1 file changed, 19 insertions(+)
>>>
>>>
>>> This matches what's in our tree and and is what people are using, so:
>>>
>>> Reviewed-by: Doug Anderson<dianders@chromium.org>
>>>
>>>
>>>> diff --git a/arch/arm/boot/dts/exynos5250-snow.dts
>>>> b/arch/arm/boot/dts/exynos5250-snow.dts
>>>> index 1ce1088..32715b3 100644
>>>> --- a/arch/arm/boot/dts/exynos5250-snow.dts
>>>> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
>>>> @@ -39,6 +39,22 @@
>>>>                   };
>>>>           };
>>>>
>>>> +       pinctrl@13400000 {
>>>> +               arb_their_claim: arb-their-claim {
>>>> +                       samsung,pins = "gpe0-4";
>>>> +                       samsung,pin-function =<0>;
>>>> +                       samsung,pin-pud =<3>;
>>>> +                       samsung,pin-drv =<0>;
>>>> +               };
>>>> +
>>>> +               arb_our_claim: arb-our-claim {
>>>> +                       samsung,pins = "gpf0-3";
>>>> +                       samsung,pin-function =<1>;
>>>> +                       samsung,pin-pud =<0>;
>>>> +                       samsung,pin-drv =<0>;
>>>> +               };
>>>
>>>
>>> It's odd to me that one of these has a pullup but not the other, but I
>>> think that's because the arbitration lines ended up using some other
>>> signals that were originally hooked up for other usage.  Certainly the
>>> pullups / pulldowns match what's in our tree and also match what we
>>> had in the original shipping 3.4 kernel.
>>
>>
>> Just a wild guess, but probably the input needs a pull-up, while obviously
>> the output doesn't. I don't have much idea about the arbitration thing
>> happening on snow, so I can't say much about this series. (Maybe description
>> of patch 1/4 should be saying a bit more about the meaning of this?)
>
> Right, of course.  I'm not sure quite what I was thinking.  I think I
> was getting confused since these go through level converters and my
> brain was in open drain mode.  ...but looking at this again this looks
> reasonable.
>
> I think the whole discussion of arbitration was from a long time ago.
> I think it's fairly well documented in the "i2c-arb-gpio-challenge"
> driver.
>
> Basically it looks like Sachin is getting pinctrl stuff matched up
> properly for the device tree that's upstream.
>
Sounds OK to me.

Tomasz, do you have any concerns still?

Thanks,
Kukjin

Re: [PATCH 1/4] ARM: dts: exynos5250-snow: add pinctrl for i2c-arbitrator

[Tomasz Figa]

On 15.05.2014 21:54, Kukjin Kim wrote:
> On 04/15/14 07:53, Doug Anderson wrote:
> 
> + DT ML
> 
>> Tomasz,
>>
> 
>> On Mon, Apr 14, 2014 at 3:38 PM, Tomasz Figa<tomasz.figa@gmail.com> 
>> wrote:
>>> Hi Doug,
>>>
>>>
>>> On 15.04.2014 00:30, Doug Anderson wrote:
>>>>
>>>> Sachin,
>>>>
>>>> On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat<sachin.kamat@linaro.org>
>>>> wrote:
>>>>>
>>>>> From: Doug Anderson<dianders@chromium.org>
>>>>
>>>>
>>>> I probably wouldn't have bothered giving me authorship since this
>>>> isn't exactly a clean patch from the chromium tree (you pulled the
>>>> proper pieces yourself, did the commit message yourself, etc).  ...but
>>>> I appreciate the thought and as far as I know setting the "author" in
>>>> cases like this is a bit of a judgement call...
>>>>
>>>> The Signed-off-by is certainly correct.  ;)
>>>>
>>>>>
>>>>> Added i2c-arbitrator pinctrl node to Snow board.
>>>>>
>>>>> Signed-off-by: Doug Anderson<dianders@chromium.org>
>>>>> Signed-off-by: Sachin Kamat<sachin.kamat@linaro.org>
>>>>> ---
>>>>>    arch/arm/boot/dts/exynos5250-snow.dts |   19 +++++++++++++++++++
>>>>>    1 file changed, 19 insertions(+)
>>>>
>>>>
>>>> This matches what's in our tree and and is what people are using, so:
>>>>
>>>> Reviewed-by: Doug Anderson<dianders@chromium.org>
>>>>
>>>>
>>>>> diff --git a/arch/arm/boot/dts/exynos5250-snow.dts
>>>>> b/arch/arm/boot/dts/exynos5250-snow.dts
>>>>> index 1ce1088..32715b3 100644
>>>>> --- a/arch/arm/boot/dts/exynos5250-snow.dts
>>>>> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
>>>>> @@ -39,6 +39,22 @@
>>>>>                   };
>>>>>           };
>>>>>
>>>>> +       pinctrl@13400000 {
>>>>> +               arb_their_claim: arb-their-claim {
>>>>> +                       samsung,pins = "gpe0-4";
>>>>> +                       samsung,pin-function =<0>;
>>>>> +                       samsung,pin-pud =<3>;
>>>>> +                       samsung,pin-drv =<0>;
>>>>> +               };
>>>>> +
>>>>> +               arb_our_claim: arb-our-claim {
>>>>> +                       samsung,pins = "gpf0-3";
>>>>> +                       samsung,pin-function =<1>;
>>>>> +                       samsung,pin-pud =<0>;
>>>>> +                       samsung,pin-drv =<0>;
>>>>> +               };
>>>>
>>>>
>>>> It's odd to me that one of these has a pullup but not the other, but I
>>>> think that's because the arbitration lines ended up using some other
>>>> signals that were originally hooked up for other usage.  Certainly the
>>>> pullups / pulldowns match what's in our tree and also match what we
>>>> had in the original shipping 3.4 kernel.
>>>
>>>
>>> Just a wild guess, but probably the input needs a pull-up, while
>>> obviously
>>> the output doesn't. I don't have much idea about the arbitration thing
>>> happening on snow, so I can't say much about this series. (Maybe
>>> description
>>> of patch 1/4 should be saying a bit more about the meaning of this?)
>>
>> Right, of course.  I'm not sure quite what I was thinking.  I think I
>> was getting confused since these go through level converters and my
>> brain was in open drain mode.  ...but looking at this again this looks
>> reasonable.
>>
>> I think the whole discussion of arbitration was from a long time ago.
>> I think it's fairly well documented in the "i2c-arb-gpio-challenge"
>> driver.
>>
>> Basically it looks like Sachin is getting pinctrl stuff matched up
>> properly for the device tree that's upstream.
>>
> Sounds OK to me.
> 
> Tomasz, do you have any concerns still?

Nope. This series looked quite fine for me from the beginning, just
wanted to make sure I understand things happening here correctly.

Feel free to add

Reviewed-by: Tomasz Figa <t.figa@samsung.com>

to all four patches if not too late yet.

Best regards,
Tomasz

Re: [PATCH 4/4] ARM: dts: exynos5250-snow: add ptn3460 node

[Kukjin Kim]

On 05/16/14 04:50, Doug Anderson wrote:
> Kukjin,
>
Hi Doug,

> On Thu, May 15, 2014 at 12:38 PM, Kukjin Kim<kgene.kim@samsung.com>  wrote:
>> On 04/30/14 22:03, Ajay kumar wrote:
>>>
>>> Hi,
>>>
>> Hi,
>>
>>> On Tue, Apr 15, 2014 at 4:24 AM, Olof Johansson<olof@lixom.net>   wrote:
>>>>
>>>> Hi Sachin,
>>>>
>>>> On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat<sachin.kamat@linaro.org>
>>>> wrote:
>>>>>
>>>>> From: Doug Anderson<dianders@chromium.org>
>>>>>
>>>>> Added nodes for ptn3460 driver to Snow board.
>>>>>
>>>>> Signed-off-by: Doug Anderson<dianders@chromium.org>
>>>>> Signed-off-by: Ajay Kumar<ajaykumar.rs@samsung.com>
>>>>> Signed-off-by: Sachin Kamat<sachin.kamat@linaro.org>
>>>>
>>>>
>>>> I'm curious, how are you testing this code on the upstream kernel
>>>> before you post the patches?
>>>>
>>>> Especially since Doug was referring to some of these containing parts
>>>> that is needed for drivers that are not yet upstreamed.
>>>
>>>
>>> Its better to ignore this patch as of now, since discussion is still going
>>> on, at http://www.spinics.net/lists/dri-devel/msg58474.html
>>> I guess we will have few more DT bindings. Once it is finalized,
>>> I shall resend this again.
>>>
>> This means this whole series?
>
> I think you can land everything except the last patch.
>
OK.

> The tps65090 charger portion of the MFD will fail to probe (assuming
> it is configured) since we're still waiting for a review on
> <https://patchwork.kernel.org/patch/4042741/>.  ...but it doesn't hurt
> to take the device tree part.
>
Good. Thanks for your confirmation.

Will apply 1 to 3 with Tomasz Figa's reviewed tag.

- Kukjin

Re: [PATCH 1/4] ARM: dts: exynos5250-snow: add pinctrl for i2c-arbitrator

[Kukjin Kim]

On 05/16/14 05:00, Tomasz Figa wrote:
> On 15.05.2014 21:54, Kukjin Kim wrote:
>> On 04/15/14 07:53, Doug Anderson wrote:
>>
>> + DT ML
>>
>>> Tomasz,
>>>
>>
>>> On Mon, Apr 14, 2014 at 3:38 PM, Tomasz Figa<tomasz.figa@gmail.com>
>>> wrote:
>>>> Hi Doug,
>>>>
>>>>
>>>> On 15.04.2014 00:30, Doug Anderson wrote:
>>>>>
>>>>> Sachin,
>>>>>
>>>>> On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat<sachin.kamat@linaro.org>
>>>>> wrote:
>>>>>>
>>>>>> From: Doug Anderson<dianders@chromium.org>
>>>>>
>>>>>
>>>>> I probably wouldn't have bothered giving me authorship since this
>>>>> isn't exactly a clean patch from the chromium tree (you pulled the
>>>>> proper pieces yourself, did the commit message yourself, etc).  ...but
>>>>> I appreciate the thought and as far as I know setting the "author" in
>>>>> cases like this is a bit of a judgement call...
>>>>>
>>>>> The Signed-off-by is certainly correct.  ;)
>>>>>
>>>>>>
>>>>>> Added i2c-arbitrator pinctrl node to Snow board.
>>>>>>
>>>>>> Signed-off-by: Doug Anderson<dianders@chromium.org>
>>>>>> Signed-off-by: Sachin Kamat<sachin.kamat@linaro.org>
>>>>>> ---
>>>>>>     arch/arm/boot/dts/exynos5250-snow.dts |   19 +++++++++++++++++++
>>>>>>     1 file changed, 19 insertions(+)
>>>>>
>>>>>
>>>>> This matches what's in our tree and and is what people are using, so:
>>>>>
>>>>> Reviewed-by: Doug Anderson<dianders@chromium.org>
>>>>>
>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/exynos5250-snow.dts
>>>>>> b/arch/arm/boot/dts/exynos5250-snow.dts
>>>>>> index 1ce1088..32715b3 100644
>>>>>> --- a/arch/arm/boot/dts/exynos5250-snow.dts
>>>>>> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
>>>>>> @@ -39,6 +39,22 @@
>>>>>>                    };
>>>>>>            };
>>>>>>
>>>>>> +       pinctrl@13400000 {
>>>>>> +               arb_their_claim: arb-their-claim {
>>>>>> +                       samsung,pins = "gpe0-4";
>>>>>> +                       samsung,pin-function =<0>;
>>>>>> +                       samsung,pin-pud =<3>;
>>>>>> +                       samsung,pin-drv =<0>;
>>>>>> +               };
>>>>>> +
>>>>>> +               arb_our_claim: arb-our-claim {
>>>>>> +                       samsung,pins = "gpf0-3";
>>>>>> +                       samsung,pin-function =<1>;
>>>>>> +                       samsung,pin-pud =<0>;
>>>>>> +                       samsung,pin-drv =<0>;
>>>>>> +               };
>>>>>
>>>>>
>>>>> It's odd to me that one of these has a pullup but not the other, but I
>>>>> think that's because the arbitration lines ended up using some other
>>>>> signals that were originally hooked up for other usage.  Certainly the
>>>>> pullups / pulldowns match what's in our tree and also match what we
>>>>> had in the original shipping 3.4 kernel.
>>>>
>>>>
>>>> Just a wild guess, but probably the input needs a pull-up, while
>>>> obviously
>>>> the output doesn't. I don't have much idea about the arbitration thing
>>>> happening on snow, so I can't say much about this series. (Maybe
>>>> description
>>>> of patch 1/4 should be saying a bit more about the meaning of this?)
>>>
>>> Right, of course.  I'm not sure quite what I was thinking.  I think I
>>> was getting confused since these go through level converters and my
>>> brain was in open drain mode.  ...but looking at this again this looks
>>> reasonable.
>>>
>>> I think the whole discussion of arbitration was from a long time ago.
>>> I think it's fairly well documented in the "i2c-arb-gpio-challenge"
>>> driver.
>>>
>>> Basically it looks like Sachin is getting pinctrl stuff matched up
>>> properly for the device tree that's upstream.
>>>
>> Sounds OK to me.
>>
>> Tomasz, do you have any concerns still?
>
> Nope. This series looked quite fine for me from the beginning, just
> wanted to make sure I understand things happening here correctly.
>
> Feel free to add
>
> Reviewed-by: Tomasz Figa<t.figa@samsung.com>
>
> to all four patches if not too late yet.
>
Tomasz,

Thanks for your review.

- Kukjin

Re: [PATCH 4/4] ARM: dts: exynos5250-snow: add ptn3460 node

[Ajay kumar]

On 5/16/14, Doug Anderson <dianders@chromium.org> wrote:
> Kukjin,
>
> On Thu, May 15, 2014 at 12:38 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
>> On 04/30/14 22:03, Ajay kumar wrote:
>>>
>>> Hi,
>>>
>> Hi,
>>
>>> On Tue, Apr 15, 2014 at 4:24 AM, Olof Johansson<olof@lixom.net>  wrote:
>>>>
>>>> Hi Sachin,
>>>>
>>>> On Mon, Apr 14, 2014 at 6:16 AM, Sachin Kamat<sachin.kamat@linaro.org>
>>>> wrote:
>>>>>
>>>>> From: Doug Anderson<dianders@chromium.org>
>>>>>
>>>>> Added nodes for ptn3460 driver to Snow board.
>>>>>
>>>>> Signed-off-by: Doug Anderson<dianders@chromium.org>
>>>>> Signed-off-by: Ajay Kumar<ajaykumar.rs@samsung.com>
>>>>> Signed-off-by: Sachin Kamat<sachin.kamat@linaro.org>
>>>>
>>>>
>>>> I'm curious, how are you testing this code on the upstream kernel
>>>> before you post the patches?
>>>>
>>>> Especially since Doug was referring to some of these containing parts
>>>> that is needed for drivers that are not yet upstreamed.
>>>
>>>
>>> Its better to ignore this patch as of now, since discussion is still
>>> going
>>> on, at http://www.spinics.net/lists/dri-devel/msg58474.html
>>> I guess we will have few more DT bindings. Once it is finalized,
>>> I shall resend this again.
>>>
>> This means this whole series?
>
> I think you can land everything except the last patch.
I think you can take this ptn3460 DT patch also.
Because, ptn3460 driver is already present in Inki's tree. Its true
that the driver may need some changes, the discussions are still going
on, and if we need to add a new DT binding, I will send that change as
a seperate patch sometime down the line.
As of now, this can go in (but I am sure we won't get the display
until we keep FET1 and FET6 "always on" for snow)

Ajay

> The tps65090 charger portion of the MFD will fail to probe (assuming
> it is configured) since we're still waiting for a review on
> <https://patchwork.kernel.org/patch/4042741/>.  ...but it doesn't hurt
> to take the device tree part.
>
> -Doug
>