[lm-sensors] [PATCH RESEND] lm-sensors: add dme1737 support

[lm-sensors] [PATCH RESEND] lm-sensors: add dme1737 support

[Juerg Haefliger]

This patch fixes a bug in sensors-detect that prevents the SMSC
DME1737 from being discovered. It also adds full support for the chip
to libsensors and sensors.

Signed-off-by: Juerg Haefliger <juergh at gmail.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lm-sensors-add-dme1737-support.patch
Type: text/x-patch
Size: 14944 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20070326/436acab0/attachment-0001.bin 

Re: [lm-sensors] [PATCH RESEND] lm-sensors: add dme1737 support

[Jean Delvare]

Hi Juerg,

On Mon, 26 Mar 2007 17:46:49 -0700, Juerg Haefliger wrote:
> This patch fixes a bug in sensors-detect that prevents the SMSC
> DME1737 from being discovered. It also adds full support for the chip
> to libsensors and sensors.

> @@ -1791,9 +1793,17 @@ $chip_kern26_w83791d = {
>        {
>  	name => "SMSC DME1737 Super IO",
>  	# Hardware monitoring features are accessed on the SMBus
> -	driver => "not-a-sensor",
> +	driver => "via-smbus-only",
>  	devid => 0x78,
>        },
> +      {
> +	name => "SMSC DME1737 Super IO",
> +	# The DME1737 shows up twice in this list because it can return both

I think you mean "either", not "both". I doubt a given chip would
return two different values?

> +	# 0x78 or 0x77 as its device ID.
> +	# Hardware monitoring features are accessed on the SMBus
> +	driver => "via-smbus-only",
> +	devid => 0x77,
> +      },
>      ],
>    },
>    {

>  # Registers used:
>  #   0x3E: Manufacturer ID
>  #   0x3F: Version/Stepping
> -#   0x40: Configuration (2 reserved bits)
> -#   0x42: Interrupt Status 2 (1 reserved bit)
> -#   0x43: VID (2 reserved bits)
> +#   0x73: Read-only test register (4 test bits)
> +#   0x8A: Read-only test register (7 test bits)
>  sub dme1737_detect
>  {
>    my ($file, $addr) = @_;
> -  return unless i2c_smbus_read_byte_data($file, 0x3E) = 0x55
> +  return unless i2c_smbus_read_byte_data($file, 0x3E) = 0x5c
>             and (i2c_smbus_read_byte_data($file, 0x3F) & 0xF8) = 0x88
> -           and (i2c_smbus_read_byte_data($file, 0x40) & 0xC4) = 0x04
> -           and (i2c_smbus_read_byte_data($file, 0x42) & 0x02) = 0x00
> -           and (i2c_smbus_read_byte_data($file, 0x43) & 0xC0) = 0x00;
> +           and (i2c_smbus_read_byte_data($file, 0x73) & 0x0F) = 0x09
> +           and (i2c_smbus_read_byte_data($file, 0x8A) & 0x7F) = 0x4D;
>    return ($addr = 0x2e ? 6 : 5);
>  }

I'm fine with using register 0x8A, even though I don't think we ever
used a test register for identification purposes. But I am skeptical
about 0x73, it's essentially not documented in the datasheet I have,
but the only think they say is that it's read/write, that doesn't sound
right for identification purposes. Assuming that your datasheet is more
complete than mine, you'll know better, but please confirm it's really
that register you want to use. If not, I still think that register 0x40
is good for identification purposes.

Other than that, your patch looks good and I am willing to apply it.

Thanks,
-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH RESEND] lm-sensors: add dme1737 support

[Juerg Haefliger]

Hi Jean,


> > +     name => "SMSC DME1737 Super IO",
> > +     # The DME1737 shows up twice in this list because it can return both
>
> I think you mean "either", not "both". I doubt a given chip would
> return two different values?

Yes, sure.


> > +#   0x73: Read-only test register (4 test bits)
> > +#   0x8A: Read-only test register (7 test bits)
> >  sub dme1737_detect
> >  {
> >    my ($file, $addr) = @_;
> > -  return unless i2c_smbus_read_byte_data($file, 0x3E) = 0x55
> > +  return unless i2c_smbus_read_byte_data($file, 0x3E) = 0x5c
> >             and (i2c_smbus_read_byte_data($file, 0x3F) & 0xF8) = 0x88
> > -           and (i2c_smbus_read_byte_data($file, 0x40) & 0xC4) = 0x04
> > -           and (i2c_smbus_read_byte_data($file, 0x42) & 0x02) = 0x00
> > -           and (i2c_smbus_read_byte_data($file, 0x43) & 0xC0) = 0x00;
> > +           and (i2c_smbus_read_byte_data($file, 0x73) & 0x0F) = 0x09
> > +           and (i2c_smbus_read_byte_data($file, 0x8A) & 0x7F) = 0x4D;
> >    return ($addr = 0x2e ? 6 : 5);
> >  }
>
> I'm fine with using register 0x8A, even though I don't think we ever
> used a test register for identification purposes. But I am skeptical
> about 0x73, it's essentially not documented in the datasheet I have,
> but the only think they say is that it's read/write, that doesn't sound
> right for identification purposes. Assuming that your datasheet is more
> complete than mine, you'll know better, but please confirm it's really
> that register you want to use. If not, I still think that register 0x40
> is good for identification purposes.

Yes, 0x73 is the one I wanted. My datasheet is at rev 0.4
If you don't feel comfortable with the new registers I don't have a
problem going back to the original register set. I just thought
something a little bit more unique and meaningful makes sense.

Thanks
...juerg



> Other than that, your patch looks good and I am willing to apply it.
>
> Thanks,
> --
> Jean Delvare
>

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH RESEND] lm-sensors: add dme1737 support

[Jean Delvare]

On Tue, 27 Mar 2007 08:35:13 -0700, Juerg Haefliger wrote:
> Yes, 0x73 is the one I wanted. My datasheet is at rev 0.4

I have rev 0.2.

> If you don't feel comfortable with the new registers I don't have a
> problem going back to the original register set. I just thought
> something a little bit more unique and meaningful makes sense.

I don't care either way as long as you know what you're doing. We can
always change it in the future if problems arise. So I've applied your
patch.

About the Asus A8000, will it be detected as an SMSC DME1737 with the
current code in sensors-detect? Or do we need dedicated detection?

http://lists.lm-sensors.org/pipermail/lm-sensors/2005-June/012777.html

Thanks,
-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH RESEND] lm-sensors: add dme1737 support

[Juerg Haefliger]

Hi Jean,

On 3/27/07, Jean Delvare <khali@linux-fr.org> wrote:
> On Tue, 27 Mar 2007 08:35:13 -0700, Juerg Haefliger wrote:
> > Yes, 0x73 is the one I wanted. My datasheet is at rev 0.4
>
> I have rev 0.2.

Man, where the heck do you get all these datasheets from? :-)


> > If you don't feel comfortable with the new registers I don't have a
> > problem going back to the original register set. I just thought
> > something a little bit more unique and meaningful makes sense.
>
> I don't care either way as long as you know what you're doing. We can
> always change it in the future if problems arise. So I've applied your
> patch.
>
> About the Asus A8000, will it be detected as an SMSC DME1737 with the
> current code in sensors-detect? Or do we need dedicated detection?
>
> http://lists.lm-sensors.org/pipermail/lm-sensors/2005-June/012777.html
>

Yes, it will work just fine for the A8000.

...juerg


> Thanks,
> --
> Jean Delvare
>

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH RESEND] lm-sensors: add dme1737 support

[Jean Delvare]

Juerg,

On Tue, 27 Mar 2007 09:09:14 -0700, Juerg Haefliger wrote:
> On 3/27/07, Jean Delvare wrote:
> > On Tue, 27 Mar 2007 08:35:13 -0700, Juerg Haefliger wrote:
> > > Yes, 0x73 is the one I wanted. My datasheet is at rev 0.4
> >
> > I have rev 0.2.
> 
> Man, where the heck do you get all these datasheets from? :-)

I don't remember. All I know is that I tagged it "do not distribute",
which means that it's not public.

> > > If you don't feel comfortable with the new registers I don't have a
> > > problem going back to the original register set. I just thought
> > > something a little bit more unique and meaningful makes sense.
> >
> > I don't care either way as long as you know what you're doing. We can
> > always change it in the future if problems arise. So I've applied your
> > patch.
> >
> > About the Asus A8000, will it be detected as an SMSC DME1737 with the
> > current code in sensors-detect? Or do we need dedicated detection?
> >
> > http://lists.lm-sensors.org/pipermail/lm-sensors/2005-June/012777.html
> 
> Yes, it will work just fine for the A8000.

OK, great. I've updated our Devices wiki page accordingly.

I hope to find some time to take a first look at your driver by the end
of the week. Can't promise anything, though.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH RESEND] lm-sensors: add dme1737 support

[Hans de Goede]

Jean Delvare wrote:
> Juerg,
> 
> I hope to find some time to take a first look at your driver by the end
> of the week. Can't promise anything, though.
> 

Notice that I already went over it quite thoroughly (code wise, spelling and 
code style less so). So it should be fine for inclusion.

Regards,

Hans



_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH RESEND] lm-sensors: add dme1737 support

[Jean Delvare]

On Tue, 27 Mar 2007 21:16:09 +0200, Hans de Goede wrote:
> Jean Delvare wrote:
> > I hope to find some time to take a first look at your driver by the end
> > of the week. Can't promise anything, though.
> 
> Notice that I already went over it quite thoroughly (code wise, spelling and 
> code style less so). So it should be fine for inclusion.

There's an army of people out there, who stated the very same about
their own driver. Most of them were wrong ;) Partly because I see more
things, partly because I am harder to please. But I certainly hope that
the first review you made will make my own review faster :)

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors