Skip to contentSkip to conversation

ADT7460 I2C Monitor Chip Support

U-Boot-Users New Ricardo Ribalda Delgado 4 participants 7 messages Jul 11, 2008 – Jul 14, 2008 Hardware monitoring

lore
Share & tools
lore
author
#1 lore raw

PATCH 1/1 I2C Monitor Chip ADT7460 support

Signed-off-by Ricardo Ribalda Delgado <[email protected]>
---
 drivers/hwmon/Makefile  |    1 +
 drivers/hwmon/adt7460.c |   81 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/hwmon/adt7460.h |   49 ++++++++++++++++++++++++++++
 include/dtt.h           |    3 +-
 4 files changed, 133 insertions(+), 1 deletions(-)
 create mode 100644 drivers/hwmon/adt7460.c
 create mode 100644 drivers/hwmon/adt7460.h
drivers/hwmon/Makefile +1 -0
index 065433a..e301eaa 100644
@@ -37,6 +37,7 @@ COBJS-y += ds1775.o
 COBJS-$(CONFIG_DTT_LM73) += lm73.o
 COBJS-y += lm75.o
 COBJS-y += lm81.o
+COBJS-$(CONFIG_DTT_ADT7460) += adt7460.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
drivers/hwmon/adt7460.c new file +81 -0

81 lines not on this page · raw message

drivers/hwmon/adt7460.h new file +49 -0

49 lines not on this page · raw message

include/dtt.h +2 -1
index 4e8aaad..db448ce 100644
@@ -32,7 +32,8 @@
     defined(CONFIG_DTT_DS1775) || \
     defined(CONFIG_DTT_LM81) || \
     defined(CONFIG_DTT_ADM1021) || \
-    defined(CONFIG_DTT_LM73)
+    defined(CONFIG_DTT_LM73)|| \
+    defined(CONFIG_DTT_ADT7460)
 
 #define CONFIG_DTT				/* We have a DTT */
Signature · 1 line
1.5.6.2
#2 lore raw

PATCH 1/1 I2C Monitor Chip ADT7460 support

Quoted 1 line · Ricardo Ribalda Delgado
Signed-off-by Ricardo Ribalda Delgado <[email protected]>

Please find some comments below (mostly nitpicking).

Quoted 46 lines
 drivers/hwmon/Makefile  |    1 +
 drivers/hwmon/adt7460.c |   81
+++++++++++++++++++++++++++++++++++++++++++++++ drivers/hwmon/adt7460.h |  
49 ++++++++++++++++++++++++++++
 include/dtt.h           |    3 +-
 4 files changed, 133 insertions(+), 1 deletions(-)
 create mode 100644 drivers/hwmon/adt7460.c
 create mode 100644 drivers/hwmon/adt7460.h
drivers/hwmon/Makefile +1 -0
index 065433a..e301eaa 100644
@@ -37,6 +37,7 @@ COBJS-y += ds1775.o
 COBJS-$(CONFIG_DTT_LM73) += lm73.o
 COBJS-y += lm75.o
 COBJS-y += lm81.o
+COBJS-$(CONFIG_DTT_ADT7460) += adt7460.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
drivers/hwmon/adt7460.c new file +19 -0
new file mode 100644
index 0000000..3fbd6fa
@@ -0,0 +1,81 @@
+/*
+    (C) Copyright 2008
+    Ricado Ribalda-Universidad Autonoma de Madrid, ricardo.ribalda at uam.es
+    This work has been supported by: Q-Technology  http://qtec.com/
+
+    This program is free software: you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation, either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+

drivers/hwmon/adt7460.c line 29

Quoted 10 lines
drivers/hwmon/adt7460.c new file +10 -0
+#include <common.h>
+
+#include <i2c.h>
+#include <dtt.h>
+#include "adt7460.h"
+
+#define ADT7460_ADDRESS 0x2c
+int dtt_read(int sensor, int reg)
+{
+	u8 dir=reg;

Codingstyle:

	u8 dir = reg;

drivers/hwmon/adt7460.c line 32

Quoted 3 lines
drivers/hwmon/adt7460.c new file +3 -0
+	u8 data;
+
+	if (i2c_read(ADT7460_ADDRESS,dir,1,&data,1)==-1)

Spaces are sometimes nice too:

	if (i2c_read(ADT7460_ADDRESS, dir, 1, &data, 1) == -1)

drivers/hwmon/adt7460.c line 35

Quoted 3 lines
drivers/hwmon/adt7460.c new file +3 -0
+		return -1;
+
+	if (data==ADT7460_INVALID)

Again spaces missing:

	if (data == ADT7460_INVALID)

I'll stop here with it. Please fix all this codingstyle issues in this file.

<snip>

Quoted 45 lines
drivers/hwmon/adt7460.h new file +39 -0
new file mode 100644
index 0000000..d915d1b
@@ -0,0 +1,49 @@
+/*
+    (C) Copyright 2008
+    Ricado Ribalda-Universidad Autonoma de Madrid, ricardo.ribalda at uam.es
+    This work has been supported by: Q-Technology  http://qtec.com/
+
+    This program is free software: you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation, either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+#ifndef ADT7460
+#define ADT7460
+
+
+#define ADT7460_INVALID 128
+
+#define ADT7460_2_5V		0x20
+#define ADT7460_VCCP		0x21
+#define ADT7460_VCC		0x22
+#define ADT7460_V5		0x23
+#define ADT7460_V12		0x24
+#define ADT7460_REM1_TEMP	0x25
+#define ADT7460_LOCAL_TEMP	0x26
+#define ADT7460_REM2_TEMP	0x27
+#define ADT7460_TACH1L		0x28
+#define ADT7460_TACH1H		0x29
+#define ADT7460_TACH2L		0x2a
+#define ADT7460_TACH2H		0x2b
+#define ADT7460_TACH3L		0x2c
+#define ADT7460_TACH3H		0x2d
+#define ADT7460_TACH4L		0x2e

drivers/hwmon/adt7460.h line 49

Quoted 10 lines
drivers/hwmon/adt7460.h new file +10 -0
+#define ADT7460_TACH4H		0x2f
+#define ADT7460_TACH5L		0xa9
+#define ADT7460_TACH5H		0xaa
+#define ADT7460_TACH6L		0xab
+#define ADT7460_TACH6H		0xac
+#define ADT7460_REVISION	0x3f
+#define ADT7460_CONFIG		0x40
+
+
+#endif

Do you really think this header is necessary? I would prefer to move those defines into the C file instead and drop the header completely.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

#3 lore raw

PATCH 1/1 I2C Monitor Chip ADT7460 support

In reply to Ricardo Ribalda Delgado

Still is there coding style issue.

and you have one extra header file which is uneeded. I recommend you move usefull values directly to driver.

WD: I looked at include/dtt.h there are many driver specific values. IMHO these value should be usefull move to driver c file. What do you think about?

Michal

Signed-off-by Ricardo Ribalda Delgado <[email protected]>
---
 drivers/hwmon/Makefile  |    1 +
 drivers/hwmon/adt7460.c |   81 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/hwmon/adt7460.h |   49 ++++++++++++++++++++++++++++
 include/dtt.h           |    3 +-
 4 files changed, 133 insertions(+), 1 deletions(-)
 create mode 100644 drivers/hwmon/adt7460.c
 create mode 100644 drivers/hwmon/adt7460.h
drivers/hwmon/Makefile +1 -0
index 065433a..e301eaa 100644
@@ -37,6 +37,7 @@ COBJS-y += ds1775.o
 COBJS-$(CONFIG_DTT_LM73) += lm73.o
 COBJS-y += lm75.o
 COBJS-y += lm81.o
+COBJS-$(CONFIG_DTT_ADT7460) += adt7460.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
drivers/hwmon/adt7460.c new file +53 -0

53 lines not on this page · raw message

Many blank lines.

+int dtt_init (void)
+{
+ printf("ADT7460 at I2C address 0x%2x\n",ADT7460_ADDRESS);
+ if (dtt_write(0,ADT7460_CONFIG,1)==-1){
+ printf("Error initialiting ADT7460\n");
+ return -1;
+ }
+ return 0;
+}
+
+int dtt_get_temp(int sensor)
+{
+ int aux;
+ u8 table[]={ADT7460_REM1_TEMP,ADT7460_LOCAL_TEMP,ADT7460_REM2_TEMP};
+ if (sensor>2){
+ printf("DTT sensor does not exist\n");
+ return -1;
+ }
+
+ aux=dtt_read(0,table[sensor]);
+
+ if (aux==-1){
+ printf("DTT temperature read failed\n");
+ return -1;
+
+ }
+ return aux;
+}

IMHO this header should be in c file and these values are not used anywhere.

+#define ADT7460_2_5V 0x20
+#define ADT7460_VCCP 0x21
+#define ADT7460_VCC 0x22
+#define ADT7460_V5 0x23
+#define ADT7460_V12 0x24
+#define ADT7460_TACH1L 0x28
+#define ADT7460_TACH1H 0x29
+#define ADT7460_TACH2L 0x2a
+#define ADT7460_TACH2H 0x2b
+#define ADT7460_TACH3L 0x2c
+#define ADT7460_TACH3H 0x2d
+#define ADT7460_TACH4L 0x2e
+#define ADT7460_TACH4H 0x2f
+#define ADT7460_TACH5L 0xa9
+#define ADT7460_TACH5H 0xaa
+#define ADT7460_TACH6L 0xab
+#define ADT7460_TACH6H 0xac
+#define ADT7460_REVISION 0x3f

drivers/hwmon/adt7460.h new file +49 -0

49 lines not on this page · raw message

include/dtt.h +2 -1
index 4e8aaad..db448ce 100644
@@ -32,7 +32,8 @@
     defined(CONFIG_DTT_DS1775) || \
     defined(CONFIG_DTT_LM81) || \
     defined(CONFIG_DTT_ADM1021) || \
-    defined(CONFIG_DTT_LM73)
+    defined(CONFIG_DTT_LM73)|| \
+    defined(CONFIG_DTT_ADT7460)
 
 #define CONFIG_DTT				/* We have a DTT */
Signature · 12 lines
1.5.6.2


-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
U-Boot-Users mailing list
U-Boot-Users at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users
#4 lore raw

PATCH 1/1 I2C Monitor Chip ADT7460 support

Quoted 8 lines · Michal Simek

Still is there coding style issue.

and you have one extra header file which is uneeded. I recommend you move usefull values directly to driver.

WD: I looked at include/dtt.h there are many driver specific values. IMHO these value should be usefull move to driver c file. What do you think about?

Full ACK. Patches welcome. :)

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

#5 lore raw

PATCH 1/1 I2C Monitor Chip ADT7460 support

In reply to Michal Simek
Quoted 7 lines · In message <[email protected]> you wrote:

Still is there coding style issue.

and you have one extra header file which is uneeded. I recommend you move usefull values directly to driver.

WD: I looked at include/dtt.h there are many driver specific values. IMHO these value should be usefull move to driver c file. What do you think about?

Please feel free to send a patch to clean this up.

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A good marriage would be between a blind wife and deaf husband.
                                               -- Michel de Montaigne
author
#6 lore raw

PATCH 1/1 I2C Monitor Chip ADT7460 support

From: Ricardo Ribalda Delgado <[email protected]>

Add support to the ADT7460 Monitor Chip by Analog
Devices.

Signed-off-by Ricardo Ribalda Delgado <[email protected]>
---
 drivers/hwmon/Makefile  |    1 +
 drivers/hwmon/adt7460.c |   86 +++++++++++++++++++++++++++++++++++++++++++++++
 include/dtt.h           |    3 +-
 3 files changed, 89 insertions(+), 1 deletions(-)
 create mode 100644 drivers/hwmon/adt7460.c
drivers/hwmon/Makefile +1 -0
index f09f145..7342b91 100644
@@ -37,6 +37,7 @@ COBJS-$(CONFIG_DTT_DS1775) += ds1775.o
 COBJS-$(CONFIG_DTT_LM73) += lm73.o
 COBJS-$(CONFIG_DTT_LM75) += lm75.o
 COBJS-$(CONFIG_DTT_LM81) += lm81.o
+COBJS-$(CONFIG_DTT_ADT7460) += adt7460.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
drivers/hwmon/adt7460.c new file +86 -0

86 lines not on this page · raw message

include/dtt.h +2 -1
index 34053d1..ce0fdfa 100644
@@ -32,7 +32,8 @@
     defined(CONFIG_DTT_DS1775) || \
     defined(CONFIG_DTT_LM81) || \
     defined(CONFIG_DTT_ADM1021) || \
-    defined(CONFIG_DTT_LM73)
+    defined(CONFIG_DTT_LM73) || \
+    defined(CONFIG_DTT_ADT7460)
 
 #define CONFIG_DTT				/* We have a DTT */
Signature · 1 line
1.5.6.2