2013-07-04 09:10:38

by Wei Ni

[permalink] [raw]
Subject: [PATCH 0/2] Enable lm90 in Tegra30 and Tegra114.

Enable thermal sensor lm90 for Tegra30 Cardhu and Tegra114 Dalmore.

This series is v1, based on [RFC 1/9] submitted here:
http://thread.gmane.org/gmane.linux.power-management.general/31056

Changes from RFC:
1. Enable it for Tegra114 Dalmore.

Wei Ni (2):
ARM: dt: t30 cardhu: add dt entry for lm90
ARM: dt: t114 dalmore: add dt entry for lm90

arch/arm/boot/dts/tegra114-dalmore.dts | 7 +++++++
arch/arm/boot/dts/tegra30-cardhu.dtsi | 7 +++++++
2 files changed, 14 insertions(+)

--
1.7.9.5


2013-07-04 09:10:39

by Wei Ni

[permalink] [raw]
Subject: [PATCH 1/2] ARM: dt: t30 cardhu: add dt entry for lm90

Enable thermal sensor lm90 for t30 cardhu.

Signed-off-by: Wei Ni <[email protected]>
---
arch/arm/boot/dts/tegra30-cardhu.dtsi | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi
index f65b53d..8778c94 100644
--- a/arch/arm/boot/dts/tegra30-cardhu.dtsi
+++ b/arch/arm/boot/dts/tegra30-cardhu.dtsi
@@ -286,6 +286,13 @@
};
};
};
+
+ nct1008: nct1008 {
+ compatible = "lm90,nct1008";
+ reg = <0x4c>;
+ interrupt-parent = <&gpio>;
+ interrupts = <TEGRA_GPIO(CC, 2) IRQ_TYPE_LEVEL_LOW>;
+ };
};

spi@7000da00 {
--
1.7.9.5

2013-07-04 09:12:09

by Wei Ni

[permalink] [raw]
Subject: [PATCH 2/2] ARM: dt: t114 dalmore: add dt entry for lm90

Enable thermal sensor lm90 for t114 dalmore.

Signed-off-by: Wei Ni <[email protected]>
---
arch/arm/boot/dts/tegra114-dalmore.dts | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/tegra114-dalmore.dts b/arch/arm/boot/dts/tegra114-dalmore.dts
index cb640eb..8325797 100644
--- a/arch/arm/boot/dts/tegra114-dalmore.dts
+++ b/arch/arm/boot/dts/tegra114-dalmore.dts
@@ -738,6 +738,13 @@
realtek,ldo1-en-gpios =
<&gpio TEGRA_GPIO(V, 3) GPIO_ACTIVE_HIGH>;
};
+
+ nct1008: nct1008 {
+ compatible = "lm90,nct1008";
+ reg = <0x4c>;
+ interrupt-parent = <&gpio>;
+ interrupts = <TEGRA_GPIO(O, 4) IRQ_TYPE_LEVEL_LOW>;
+ };
};

i2c@7000d000 {
--
1.7.9.5

2013-07-05 17:38:15

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: dt: t30 cardhu: add dt entry for lm90

On 07/04/2013 03:09 AM, Wei Ni wrote:
> Enable thermal sensor lm90 for t30 cardhu.

> diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi

> + nct1008: nct1008 {
> + compatible = "lm90,nct1008";

"lm90" isn't a valid vendor prefix. I believe the value you want is
"onnn,nct1008". Same comment for patch 2/2.

2013-07-08 07:35:37

by Wei Ni

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: dt: t30 cardhu: add dt entry for lm90

On 07/06/2013 01:38 AM, Stephen Warren wrote:
> On 07/04/2013 03:09 AM, Wei Ni wrote:
>> Enable thermal sensor lm90 for t30 cardhu.
>
>> diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi
>
>> + nct1008: nct1008 {
>> + compatible = "lm90,nct1008";
>
> "lm90" isn't a valid vendor prefix. I believe the value you want is
> "onnn,nct1008". Same comment for patch 2/2.
>
The lm90 doesn't support device tree very well. In the DT, we need to
named as "lm90" so that the lm90 driver can be loaded, and we also need
to add "nct1008" to indicate this is the nct1008 device, so that the
lm90 driver can be loaded with the right i2c_device_id->driver_data.

I set the " compatible = "lm90,nct1008" ", this is the simplest way, and
we doesn't need to change the lm90.c.

Thanks.
Wei.

2013-07-08 07:50:51

by Jean Delvare

[permalink] [raw]
Subject: Re: [lm-sensors] [PATCH 1/2] ARM: dt: t30 cardhu: add dt entry for lm90

On Mon, 8 Jul 2013 15:35:48 +0800, Wei Ni wrote:
> On 07/06/2013 01:38 AM, Stephen Warren wrote:
> > On 07/04/2013 03:09 AM, Wei Ni wrote:
> >> Enable thermal sensor lm90 for t30 cardhu.
> >
> >> diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi
> >
> >> + nct1008: nct1008 {
> >> + compatible = "lm90,nct1008";
> >
> > "lm90" isn't a valid vendor prefix. I believe the value you want is
> > "onnn,nct1008". Same comment for patch 2/2.
>
> The lm90 doesn't support device tree very well.

I doubt it, but if this is the case, then please fix it, instead of
working the problem around in a different place.

> In the DT, we need to
> named as "lm90" so that the lm90 driver can be loaded,

Not that I am an expert with regards to DT, but this doesn't make any
sense to me. AFAIK DT is about devices, not which drivers handle them.

> and we also need
> to add "nct1008" to indicate this is the nct1008 device, so that the
> lm90 driver can be loaded with the right i2c_device_id->driver_data.
>
> I set the " compatible = "lm90,nct1008" ", this is the simplest way, and
> we doesn't need to change the lm90.c.

There's no problem with changing the lm90 driver, if this is the right
thing to do.

--
Jean Delvare

2013-07-08 09:36:13

by Wei Ni

[permalink] [raw]
Subject: Re: [lm-sensors] [PATCH 1/2] ARM: dt: t30 cardhu: add dt entry for lm90

On 07/08/2013 03:50 PM, Jean Delvare wrote:
> On Mon, 8 Jul 2013 15:35:48 +0800, Wei Ni wrote:
>> On 07/06/2013 01:38 AM, Stephen Warren wrote:
>>> On 07/04/2013 03:09 AM, Wei Ni wrote:
>>>> Enable thermal sensor lm90 for t30 cardhu.
>>>
>>>> diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi
>>>
>>>> + nct1008: nct1008 {
>>>> + compatible = "lm90,nct1008";
>>>
>>> "lm90" isn't a valid vendor prefix. I believe the value you want is
>>> "onnn,nct1008". Same comment for patch 2/2.
>>
>> The lm90 doesn't support device tree very well.
>
> I doubt it, but if this is the case, then please fix it, instead of
> working the problem around in a different place.
>
>> In the DT, we need to
>> named as "lm90" so that the lm90 driver can be loaded,
>
> Not that I am an expert with regards to DT, but this doesn't make any
> sense to me. AFAIK DT is about devices, not which drivers handle them.
>
>> and we also need
>> to add "nct1008" to indicate this is the nct1008 device, so that the
>> lm90 driver can be loaded with the right i2c_device_id->driver_data.
>>
>> I set the " compatible = "lm90,nct1008" ", this is the simplest way, and
>> we doesn't need to change the lm90.c.
>
> There's no problem with changing the lm90 driver, if this is the right
> thing to do.

Ok, I will add DT support in the lm90.c in my next version patch.

Thanks.
Wei.

>

2013-07-08 13:12:51

by Guenter Roeck

[permalink] [raw]
Subject: Re: [lm-sensors] [PATCH 1/2] ARM: dt: t30 cardhu: add dt entry for lm90

On Mon, Jul 08, 2013 at 03:35:48PM +0800, Wei Ni wrote:
> On 07/06/2013 01:38 AM, Stephen Warren wrote:
> > On 07/04/2013 03:09 AM, Wei Ni wrote:
> >> Enable thermal sensor lm90 for t30 cardhu.
> >
> >> diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi
> >
> >> + nct1008: nct1008 {
> >> + compatible = "lm90,nct1008";
> >
> > "lm90" isn't a valid vendor prefix. I believe the value you want is
> > "onnn,nct1008". Same comment for patch 2/2.
> >
> The lm90 doesn't support device tree very well. In the DT, we need to
> named as "lm90" so that the lm90 driver can be loaded, and we also need
> to add "nct1008" to indicate this is the nct1008 device, so that the
> lm90 driver can be loaded with the right i2c_device_id->driver_data.
>
> I set the " compatible = "lm90,nct1008" ", this is the simplest way, and
> we doesn't need to change the lm90.c.
>
Did you really test "onn,nct1008" or, for that matter, "anything,nct1008" ?

I have been using
maxim,max1137
maxim,max1139
maxim,max6697
dallas,ds1621
linear,ltc3880
linear,ltc2978
intersil,zl2006
ti,tmp421
ti,tmp431

without problems, and without having to add code to the respective drivers.
I would be quite surprised if "onn,nct1008" would not work.

Guenter

2013-07-08 13:14:26

by Guenter Roeck

[permalink] [raw]
Subject: Re: [lm-sensors] [PATCH 1/2] ARM: dt: t30 cardhu: add dt entry for lm90

On Mon, Jul 08, 2013 at 05:36:05PM +0800, Wei Ni wrote:
> On 07/08/2013 03:50 PM, Jean Delvare wrote:
> > On Mon, 8 Jul 2013 15:35:48 +0800, Wei Ni wrote:
> >> On 07/06/2013 01:38 AM, Stephen Warren wrote:
> >>> On 07/04/2013 03:09 AM, Wei Ni wrote:
> >>>> Enable thermal sensor lm90 for t30 cardhu.
> >>>
> >>>> diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi
> >>>
> >>>> + nct1008: nct1008 {
> >>>> + compatible = "lm90,nct1008";
> >>>
> >>> "lm90" isn't a valid vendor prefix. I believe the value you want is
> >>> "onnn,nct1008". Same comment for patch 2/2.
> >>
> >> The lm90 doesn't support device tree very well.
> >
> > I doubt it, but if this is the case, then please fix it, instead of
> > working the problem around in a different place.
> >
> >> In the DT, we need to
> >> named as "lm90" so that the lm90 driver can be loaded,
> >
> > Not that I am an expert with regards to DT, but this doesn't make any
> > sense to me. AFAIK DT is about devices, not which drivers handle them.
> >
> >> and we also need
> >> to add "nct1008" to indicate this is the nct1008 device, so that the
> >> lm90 driver can be loaded with the right i2c_device_id->driver_data.
> >>
> >> I set the " compatible = "lm90,nct1008" ", this is the simplest way, and
> >> we doesn't need to change the lm90.c.
> >
> > There's no problem with changing the lm90 driver, if this is the right
> > thing to do.
>
> Ok, I will add DT support in the lm90.c in my next version patch.
>
Only if you can show that it is necessary.

Guenter

2013-07-09 06:21:24

by Thierry Reding

[permalink] [raw]
Subject: Re: [lm-sensors] [PATCH 1/2] ARM: dt: t30 cardhu: add dt entry for lm90

On Mon, Jul 08, 2013 at 06:14:21AM -0700, Guenter Roeck wrote:
> On Mon, Jul 08, 2013 at 05:36:05PM +0800, Wei Ni wrote:
> > On 07/08/2013 03:50 PM, Jean Delvare wrote:
> > > On Mon, 8 Jul 2013 15:35:48 +0800, Wei Ni wrote:
> > >> On 07/06/2013 01:38 AM, Stephen Warren wrote:
> > >>> On 07/04/2013 03:09 AM, Wei Ni wrote:
> > >>>> Enable thermal sensor lm90 for t30 cardhu.
> > >>>
> > >>>> diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi
> > >>>
> > >>>> + nct1008: nct1008 {
> > >>>> + compatible = "lm90,nct1008";
> > >>>
> > >>> "lm90" isn't a valid vendor prefix. I believe the value you want is
> > >>> "onnn,nct1008". Same comment for patch 2/2.
> > >>
> > >> The lm90 doesn't support device tree very well.
> > >
> > > I doubt it, but if this is the case, then please fix it, instead of
> > > working the problem around in a different place.
> > >
> > >> In the DT, we need to
> > >> named as "lm90" so that the lm90 driver can be loaded,
> > >
> > > Not that I am an expert with regards to DT, but this doesn't make any
> > > sense to me. AFAIK DT is about devices, not which drivers handle them.
> > >
> > >> and we also need
> > >> to add "nct1008" to indicate this is the nct1008 device, so that the
> > >> lm90 driver can be loaded with the right i2c_device_id->driver_data.
> > >>
> > >> I set the " compatible = "lm90,nct1008" ", this is the simplest way, and
> > >> we doesn't need to change the lm90.c.
> > >
> > > There's no problem with changing the lm90 driver, if this is the right
> > > thing to do.
> >
> > Ok, I will add DT support in the lm90.c in my next version patch.
> >
> Only if you can show that it is necessary.

It should work out of the box. As a matter of fact the same chip is used
on Tamonten and the DTS files use "onnn,nct1008". That used to work. If
it no longer does then that's a regression.

Thierry


Attachments:
(No filename) (1.85 kB)
(No filename) (836.00 B)
Download all attachments

2013-07-09 07:49:30

by Wei Ni

[permalink] [raw]
Subject: Re: [lm-sensors] [PATCH 1/2] ARM: dt: t30 cardhu: add dt entry for lm90

On 07/09/2013 02:21 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Mon, Jul 08, 2013 at 06:14:21AM -0700, Guenter Roeck wrote:
>> On Mon, Jul 08, 2013 at 05:36:05PM +0800, Wei Ni wrote:
>>> On 07/08/2013 03:50 PM, Jean Delvare wrote:
>>>> On Mon, 8 Jul 2013 15:35:48 +0800, Wei Ni wrote:
>>>>> On 07/06/2013 01:38 AM, Stephen Warren wrote:
>>>>>> On 07/04/2013 03:09 AM, Wei Ni wrote:
>>>>>>> Enable thermal sensor lm90 for t30 cardhu.
>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi
>>>>>>
>>>>>>> + nct1008: nct1008 {
>>>>>>> + compatible = "lm90,nct1008";
>>>>>>
>>>>>> "lm90" isn't a valid vendor prefix. I believe the value you want is
>>>>>> "onnn,nct1008". Same comment for patch 2/2.
>>>>>
>>>>> The lm90 doesn't support device tree very well.
>>>>
>>>> I doubt it, but if this is the case, then please fix it, instead of
>>>> working the problem around in a different place.
>>>>
>>>>> In the DT, we need to
>>>>> named as "lm90" so that the lm90 driver can be loaded,
>>>>
>>>> Not that I am an expert with regards to DT, but this doesn't make any
>>>> sense to me. AFAIK DT is about devices, not which drivers handle them.
>>>>
>>>>> and we also need
>>>>> to add "nct1008" to indicate this is the nct1008 device, so that the
>>>>> lm90 driver can be loaded with the right i2c_device_id->driver_data.
>>>>>
>>>>> I set the " compatible = "lm90,nct1008" ", this is the simplest way, and
>>>>> we doesn't need to change the lm90.c.
>>>>
>>>> There's no problem with changing the lm90 driver, if this is the right
>>>> thing to do.
>>>
>>> Ok, I will add DT support in the lm90.c in my next version patch.
>>>
>> Only if you can show that it is necessary.
>
> It should work out of the box. As a matter of fact the same chip is used
> on Tamonten and the DTS files use "onnn,nct1008". That used to work. If
> it no longer does then that's a regression.

I synced the linux-next from:
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
and use the tag v3.10-rc7, but in the lm90.c it doesn't have DT support,
such as "onnn,nct1008".
I googled it and found there has patch:
[PATCH] hwmon: (lm90) Add device tree support , which is in:
http://lists.lm-sensors.org/pipermail/lm-sensors/2013-February/038099.html
, but it didn't be merged into the linux-next.

which git repository and branch should I use ?

Thanks.
Wei.

>
> Thierry
>
> * Unknown Key
> * 0x7F3EB3A1
>

2013-07-09 07:55:44

by Jean Delvare

[permalink] [raw]
Subject: Re: [lm-sensors] [PATCH 1/2] ARM: dt: t30 cardhu: add dt entry for lm90

On Tue, 9 Jul 2013 15:48:40 +0800, Wei Ni wrote:
> On 07/09/2013 02:21 PM, Thierry Reding wrote:
> > It should work out of the box. As a matter of fact the same chip is used
> > on Tamonten and the DTS files use "onnn,nct1008". That used to work. If
> > it no longer does then that's a regression.
>
> I synced the linux-next from:
> git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> and use the tag v3.10-rc7, but in the lm90.c it doesn't have DT support,
> such as "onnn,nct1008".
> I googled it and found there has patch:
> [PATCH] hwmon: (lm90) Add device tree support , which is in:
> http://lists.lm-sensors.org/pipermail/lm-sensors/2013-February/038099.html
> , but it didn't be merged into the linux-next.

It was not, because of the next reply in the same thread:
http://lists.lm-sensors.org/pipermail/lm-sensors/2013-February/038100.html

> which git repository and branch should I use ?

It is supposed to just work. What doesn't work for you exactly?

--
Jean Delvare

2013-07-09 08:59:45

by Wei Ni

[permalink] [raw]
Subject: Re: [lm-sensors] [PATCH 1/2] ARM: dt: t30 cardhu: add dt entry for lm90

On 07/09/2013 03:55 PM, Jean Delvare wrote:
> On Tue, 9 Jul 2013 15:48:40 +0800, Wei Ni wrote:
>> On 07/09/2013 02:21 PM, Thierry Reding wrote:
>>> It should work out of the box. As a matter of fact the same chip is used
>>> on Tamonten and the DTS files use "onnn,nct1008". That used to work. If
>>> it no longer does then that's a regression.
>>
>> I synced the linux-next from:
>> git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>> and use the tag v3.10-rc7, but in the lm90.c it doesn't have DT support,
>> such as "onnn,nct1008".
>> I googled it and found there has patch:
>> [PATCH] hwmon: (lm90) Add device tree support , which is in:
>> http://lists.lm-sensors.org/pipermail/lm-sensors/2013-February/038099.html
>> , but it didn't be merged into the linux-next.
>
> It was not, because of the next reply in the same thread:
> http://lists.lm-sensors.org/pipermail/lm-sensors/2013-February/038100.html
>
>> which git repository and branch should I use ?
>
> It is supposed to just work. What doesn't work for you exactly?
>

Oh, yes, the "onnn,nct1008" can work. I read the i2c driver and the
of_i2c.c carefully, I understand why the lm90 can be loaded by setting
"onnn,nct1008". Indeed, we can set the vendor string to anything, even
"xxx,nct1008", the lm90 also can be loaded correctly.

Anyway, I will use the "onnn,nct1008" in my next version.

Thanks for your comments.
Wei.