diff mbox

[7/7] ARM: tegra: Add the EC i2c tunnel to tegra124-venice2

Message ID 1397757570-19750-8-git-send-email-dianders@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Doug Anderson April 17, 2014, 5:59 p.m. UTC
This adds the EC i2c tunnel (and devices under it) to the
tegra124-venice2 device tree.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 arch/arm/boot/dts/tegra124-venice2.dts | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Stephen Warren April 21, 2014, 6:18 p.m. UTC | #1
On 04/17/2014 11:59 AM, Doug Anderson wrote:
> This adds the EC i2c tunnel (and devices under it) to the
> tegra124-venice2 device tree.

The series,
Tested-by: Stephen Warren <swarren@nvidia.com>

I can apply this one patch once the other patches in the series are
acked or applied (in order to make sure the DT binding is acceptable to
others).

I guess I'll send separate patches for tegra_defconfig and
multi_v7_defconfig to add the required options once I've applied this,
unless you beat me to it.

> diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts

>  	aliases {
> +		i2c20 = "/spi@0,7000d400/cros-ec@0/i2c-tunnel";

Is that needed? I'd prefer not to add it unless there's a specific
reason. I don't think I2C buses need specific names, do they?
Doug Anderson April 21, 2014, 7:35 p.m. UTC | #2
Stephen,

On Mon, Apr 21, 2014 at 11:18 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/17/2014 11:59 AM, Doug Anderson wrote:
>> This adds the EC i2c tunnel (and devices under it) to the
>> tegra124-venice2 device tree.
>
> The series,
> Tested-by: Stephen Warren <swarren@nvidia.com>
>
> I can apply this one patch once the other patches in the series are
> acked or applied (in order to make sure the DT binding is acceptable to
> others).

Sounds good.  If I don't get any feedback (positive or negative) in
the next few days I'll resend with your Tested-by.


> I guess I'll send separate patches for tegra_defconfig and
> multi_v7_defconfig to add the required options once I've applied this,
> unless you beat me to it.
>
>> diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts
>
>>       aliases {
>> +             i2c20 = "/spi@0,7000d400/cros-ec@0/i2c-tunnel";
>
> Is that needed? I'd prefer not to add it unless there's a specific
> reason. I don't think I2C buses need specific names, do they?

It is not strictly needed, but from a usability standpoint it is
terribly helpful.  It serves to make it obvious to someone looking at
the device that it's _not_ an i2c bus associated with the main SoC.
If you don't include a number I believe that the i2c core will pick
the first available number.

It seems worth it to save a few people a few hours of head scratching.

...but this is your dts and if you think it's a terrible idea then
I'll remove it.  It looks to be less critical on tegra than it is on
exynos (which has ~9 i2c busses, they are numbered in the user manual,
and if you have one set to "disable" in the dts then the tunnel will
end up getting a very confusing number).

-Doug
Stephen Warren April 21, 2014, 8:03 p.m. UTC | #3
On 04/21/2014 01:35 PM, Doug Anderson wrote:
> Stephen,
> 
> On Mon, Apr 21, 2014 at 11:18 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 04/17/2014 11:59 AM, Doug Anderson wrote:
>>> This adds the EC i2c tunnel (and devices under it) to the
>>> tegra124-venice2 device tree.

>>> diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts
>>
>>>       aliases {
>>> +             i2c20 = "/spi@0,7000d400/cros-ec@0/i2c-tunnel";
>>
>> Is that needed? I'd prefer not to add it unless there's a specific
>> reason. I don't think I2C buses need specific names, do they?
> 
> It is not strictly needed, but from a usability standpoint it is
> terribly helpful.  It serves to make it obvious to someone looking at
> the device that it's _not_ an i2c bus associated with the main SoC.
> If you don't include a number I believe that the i2c core will pick
> the first available number.
> 
> It seems worth it to save a few people a few hours of head scratching.
> 
> ...but this is your dts and if you think it's a terrible idea then
> I'll remove it.  It looks to be less critical on tegra than it is on
> exynos (which has ~9 i2c busses, they are numbered in the user manual,
> and if you have one set to "disable" in the dts then the tunnel will
> end up getting a very confusing number).

My opinion is that the in-kernel I2C bus numbering is an entirely
unrelated numbering space to the HW controller numbering space precisely
because of issues like that. DT aliases are more useful for user-visible
port numbering (e.g. HDMI 0, 1 connectors on a case) than purely
internal details like this. So, I would leave it out.
Stephen Warren May 15, 2014, 6:42 p.m. UTC | #4
On 04/17/2014 11:59 AM, Doug Anderson wrote:
> This adds the EC i2c tunnel (and devices under it) to the
> tegra124-venice2 device tree.

Did the MFD patches at the start of this series get applied yet? I was
hoping to apply this one patch to the Tegra tree for 3.16, and that
needs to happen by tomorrow morning at the latest since -rc6 is looming
fast. However, I'm holding off until the rest of the series is applied
to the MFD tree so I can be sure the DT binding is accepted first...
Doug Anderson May 15, 2014, 6:55 p.m. UTC | #5
Stephen,

On Thu, May 15, 2014 at 11:42 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/17/2014 11:59 AM, Doug Anderson wrote:
>> This adds the EC i2c tunnel (and devices under it) to the
>> tegra124-venice2 device tree.
>
> Did the MFD patches at the start of this series get applied yet? I was
> hoping to apply this one patch to the Tegra tree for 3.16, and that
> needs to happen by tomorrow morning at the latest since -rc6 is looming
> fast. However, I'm holding off until the rest of the series is applied
> to the MFD tree so I can be sure the DT binding is accepted first...

As far as I know, they didn't.  Lee was waiting on Wolfram's ack to
the i2c tunnel before landing them.  ...and I'm still eagerly awaiting
Wolfram's next i2c tunnel review.

-Doug
Olof Johansson May 15, 2014, 8:12 p.m. UTC | #6
On Thu, May 15, 2014 at 11:55 AM, Doug Anderson <dianders@chromium.org> wrote:
> Stephen,
>
> On Thu, May 15, 2014 at 11:42 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 04/17/2014 11:59 AM, Doug Anderson wrote:
>>> This adds the EC i2c tunnel (and devices under it) to the
>>> tegra124-venice2 device tree.
>>
>> Did the MFD patches at the start of this series get applied yet? I was
>> hoping to apply this one patch to the Tegra tree for 3.16, and that
>> needs to happen by tomorrow morning at the latest since -rc6 is looming
>> fast. However, I'm holding off until the rest of the series is applied
>> to the MFD tree so I can be sure the DT binding is accepted first...
>
> As far as I know, they didn't.  Lee was waiting on Wolfram's ack to
> the i2c tunnel before landing them.  ...and I'm still eagerly awaiting
> Wolfram's next i2c tunnel review.

The i2c portion can be revisited if needed, the MFD piece should be
possible to land sooner than that... Lee?


-Olof
Lee Jones May 19, 2014, 9:18 a.m. UTC | #7
On Thu, 15 May 2014, Olof Johansson wrote:

> On Thu, May 15, 2014 at 11:55 AM, Doug Anderson <dianders@chromium.org> wrote:
> > Stephen,
> >
> > On Thu, May 15, 2014 at 11:42 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> >> On 04/17/2014 11:59 AM, Doug Anderson wrote:
> >>> This adds the EC i2c tunnel (and devices under it) to the
> >>> tegra124-venice2 device tree.
> >>
> >> Did the MFD patches at the start of this series get applied yet? I was
> >> hoping to apply this one patch to the Tegra tree for 3.16, and that
> >> needs to happen by tomorrow morning at the latest since -rc6 is looming
> >> fast. However, I'm holding off until the rest of the series is applied
> >> to the MFD tree so I can be sure the DT binding is accepted first...
> >
> > As far as I know, they didn't.  Lee was waiting on Wolfram's ack to
> > the i2c tunnel before landing them.  ...and I'm still eagerly awaiting
> > Wolfram's next i2c tunnel review.
> 
> The i2c portion can be revisited if needed, the MFD piece should be
> possible to land sooner than that... Lee?

The set 'will' make it in during this cycle.  The trouble is, once
it's applied, the I2C portion will have to wait until the next cycle.
By applying late, I'm giving this set every chance to go in as a
whole.
diff mbox

Patch

diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts
index c17283c..df97d15 100644
--- a/arch/arm/boot/dts/tegra124-venice2.dts
+++ b/arch/arm/boot/dts/tegra124-venice2.dts
@@ -8,6 +8,7 @@ 
 	compatible = "nvidia,venice2", "nvidia,tegra124";
 
 	aliases {
+		i2c20 = "/spi@0,7000d400/cros-ec@0/i2c-tunnel";
 		rtc0 = "/i2c@0,7000d000/pmic@40";
 		rtc1 = "/rtc@0,7000e000";
 	};
@@ -813,6 +814,32 @@ 
 
 			google,cros-ec-spi-msg-delay = <2000>;
 
+			i2c-tunnel {
+				compatible = "google,cros-ec-i2c-tunnel";
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				google,remote-bus = <0>;
+
+				charger: bq24735@9 {
+					compatible = "ti,bq24735";
+					reg = <0x9>;
+					interrupt-parent = <&gpio>;
+					interrupts = <TEGRA_GPIO(J, 0)
+							GPIO_ACTIVE_HIGH>;
+					ti,ac-detect-gpios = <&gpio
+							TEGRA_GPIO(J, 0)
+							GPIO_ACTIVE_HIGH>;
+				};
+
+				battery: sbs-battery@b {
+					compatible = "sbs,sbs-battery";
+					reg = <0xb>;
+					sbs,i2c-retry-count = <2>;
+					sbs,poll-retry-count = <1>;
+				};
+			};
+
 			cros-ec-keyb {
 				compatible = "google,cros-ec-keyb";
 				keypad,num-rows = <8>;