Patchwork [1/2] ARM: dt: t30 cardhu: add dt entry for lm90

login
register
mail settings
Submitter lightning314
Date July 4, 2013, 9:09 a.m.
Message ID <1372928979-26938-2-git-send-email-wni@nvidia.com>
Download mbox | patch
Permalink /patch/256823/
State Superseded, archived
Headers show

Comments

lightning314 - July 4, 2013, 9:09 a.m.
Enable thermal sensor lm90 for t30 cardhu.

Signed-off-by: Wei Ni <wni@nvidia.com>
---
 arch/arm/boot/dts/tegra30-cardhu.dtsi |    7 +++++++
 1 file changed, 7 insertions(+)
Stephen Warren - July 5, 2013, 5:38 p.m.
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
lightning314 - July 8, 2013, 7:35 a.m.
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jean Delvare - July 8, 2013, 7:50 a.m.
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.
lightning314 - July 8, 2013, 9:36 a.m.
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.

> 

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Guenter Roeck - July 8, 2013, 1:12 p.m.
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
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Guenter Roeck - July 8, 2013, 1:14 p.m.
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
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding - July 9, 2013, 6:21 a.m.
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
lightning314 - July 9, 2013, 7:48 a.m.
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
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jean Delvare - July 9, 2013, 7:55 a.m.
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?
lightning314 - July 9, 2013, 8:58 a.m.
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.



--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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 {