Re: Device name length restriction?

[Paul Goyette <paul%whooppee.com@localhost>]

On Mon, 8 Sep 2008, Michael Lorenz wrote:

> On Sep 8, 2008, at 4:41 PM, Paul Goyette wrote:
>
> > BTW, would it be reasonable to add a chip-verification routine to
> > the adt7467 driver?
>
> Yes.
>
> > Currently the driver's match routine blindly returns "1".
>
> That's because it's main use is on macppc where it attaches as
> adt7467c at ki2c, the attachment to iic has never been well tested.
> For ki2c we don't need to specify an address since we can get it from
> OpenFirmware's device tree.
> A verify function should be easy - the chip has three registers which
> return a device ID of some sort.

Well, I found two, the company_id and device_id registers;  this is 
analgous (no pun intended!) to what is done in Analog's adt7463.

> > The adt7463 driver has a "chip verify" routine to check the chip's
> > company_id and device_id registers.  The 7467 has similar registers
> > (documented in the data sheet) and hard-coded values.  Since both
> > chips (and possibly other dbCool chips) share the same i2c bus
> > address, it seems to me to be a worthwhile thing to check.
>
> Feel free to fix it.

Done!  See attached diffs.  While I was at it, I also added a URL 
reference to the datasheet.  Unless/until we get a NetBSD datasheet 
library to store all these in, I figured a pointer was the next best 
thing to do.

> Also, the chip supports more fans than the driver currently reports,
> mainly because in iBooks only one fan is wired up - you may want to
> fix that too.

Ah, OK, I'll check into that, too.  Of course, we'll probably end up 
with another situation where we can't tell if a fan is broken or just 
not wired - see yesterday's discussion regarding ipmi.  :)

> I have one in my iBook G4 but no NetBSD on it since its disk died and
> the only replacement I had at hand was a 6GB Toshiba, barely enough
> for MacOS X. I'd like to hear from someone with an adt7467 in
> something that's not a Mac and maybe uses more than fan input.

Yeah, I'd like to hear from them, too.  I'm not going to commit the 
attached diffs until someone actually tries them out and reports 
success.  I'll just keep them around in my home tree for now.


----------------------------------------------------------------------
|   Paul Goyette   | PGP DSS Key fingerprint: |  E-mail addresses:   |
| Customer Service | FA29 0E3B 35AF E8AE 6651 |  paul%whooppee.com@localhost   |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette%juniper.net@localhost |
----------------------------------------------------------------------

Index: adt7467.c
===================================================================
RCS file: /cvsroot/src/sys/dev/i2c/adt7467.c,v
retrieving revision 1.13
diff -u -p -r1.13 adt7467.c
--- adt7467.c   8 May 2008 02:03:22 -0000       1.13
+++ adt7467.c   8 Sep 2008 21:36:37 -0000
@@ -27,6 +27,9 @@
 /* 
  * a driver for the ADT7467 environmental controller found in the iBook G4 
  * and probably other Apple machines 
+ *
+ * Data sheet available at
+ * http://www.onsemi.com/pub_link/Collateral/ADT7467-D.PDF
  */
 
 /* 
@@ -51,6 +54,7 @@ __KERNEL_RCSID(0, "$NetBSD: adt7467.c,v 
 
 static void adt7467c_attach(device_t, device_t, void *);
 static int adt7467c_match(device_t, cfdata_t, void *);
+static int adt7463c_verify(struct adt7467c_softc *);
 
 static uint8_t adt7467c_readreg(struct adt7467c_softc *, uint8_t);
 static void adt7467c_writereg(struct adt7467c_softc *, uint8_t, uint8_t);
@@ -65,8 +69,16 @@ CFATTACH_DECL_NEW(adt7467c, sizeof(struc
 int
 adt7467c_match(device_t parent, cfdata_t cf, void *aux)
 {
-       /* no probing if we attach to iic */
-       return 1;
+       struct i2c_attach_args *ia = aux;
+       struct adt7467c_softc sc;
+       sc.sc_tag = ia->ia_tag;
+       sc.sc_address = ia->ia_addr;
+
+       /* no probing if we attach to iic, but verify chip id */
+       if (adt7467c_verify(&sc))
+               return 1;
+
+       return 0;
 }
 
 void
@@ -288,3 +300,19 @@ adt7467c_refresh(struct sysmon_envsys *s
        edata->state = ENVSYS_SVALID;
 }
 #endif /* NSYSMON_ENVSYS > 0 */
+
+static int
+adt7463c_verify(struct adt7467c_softc *sc)
+{
+       /* verify this is an adt7467 */
+       int c_id, d_id;
+
+       c_id = adt7463c_readreg(sc, ADT7467_COMPANYID_REG);
+       d_id = adt7463c_receive_1(sc, ADT7467_DEVICEID_REG);
+    
+       if ((c_id == ADT7467_COMPANYID) && (d_id == ADT7467_DEVICEID))
+               return 1;
+
+       return 0;
+}  
+
Index: adt7467var.h
===================================================================
RCS file: /cvsroot/src/sys/dev/i2c/adt7467var.h,v
retrieving revision 1.6
diff -u -p -r1.6 adt7467var.h
--- adt7467var.h        8 May 2008 02:03:22 -0000       1.6
+++ adt7467var.h        8 Sep 2008 21:36:37 -0000
@@ -41,6 +41,13 @@ __KERNEL_RCSID(0, "$NetBSD: adt7467var.h
 #include "sysmon_envsys.h"
 
 #define ADT7467_MAXSENSORS     5
+#define ADT7467_ADDRMASK       0x7f
+#define        ADT7467_ADDR            0x2e
+
+#define        ADT7467_DEVICE_ID_REG   0x3d
+#define        ADT7467_COMPANY_ID_REG  0x3e
+#define        ADT7467_DEVICE_ID       0x68
+#define        ADT7467_COMPANY_ID      0x41
 
 struct adt7467c_softc {
        struct device *parent;