Re: Device name length restriction?

[Nicolas Joly <njoly%pasteur.fr@localhost>]

On Mon, Sep 08, 2008 at 02:42:46PM -0700, Paul Goyette wrote:
> 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.
[...]
> 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;
> +}  

s/adt7463c_verify/adt7467c_verify/ and likewise for others ...

IMHO this code should be merged into adt7467c_match(); there is no
gain with such a function here.

And c_id/d_id should be of type uint8_t.

> 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;


-- 
Nicolas Joly

Biological Software and Databanks.
Institut Pasteur, Paris.