Mark,
sorry for not responding to your last review for so long. I hoped all the
time I might be able to add LM94 support as well. But unfortunately, the
LM94 datasheet is available under NDA only (hard to believe, but true).
National Semiconductor Germany said it were no problem to get an NDA that
still allows me to write an open source driver, but that National in USA
must decide that. Now I'm waiting, still haven't got that NDA, let alone
the datasheet.
Good news is that I will get an i2c-tiny-usb interface soon, together
with an LM94 test board. This means I'll have an LM94 always available
and can perform all sorts of tests with it. I can therefore offer to
become maintainer of this driver.
Find attached my current version of the LM93 patch. I addressed all
the issues you mentioned in your last review. There are a few things
I'd be glad if you could check if I got it right:
1.) I replaced pwmN_override with pwmN_enable.
2.) IMO, show_pwm() was actually buggy as you suspected. I had a look
at the datasheet and fixed it.
3.) IMO, this code is also not correct:
> +/* PWM: 0-255 per sensors documentation
> + ? REG: 0-13 as mapped below... right justified */
> +typedef enum { LM93_PWM_MAP_HI_FREQ, LM93_PWM_MAP_LO_FREQ } pwm_freq_t;
> +static int lm93_pwm_map[2][14] = {
> +?????{
> +?????????????0x00, /* ? 0.00% */ 0x40, /* ?25.00% */
> +?????????????0x50, /* ?31.25% */ 0x60, /* ?37.50% */
> +?????????????0x70, /* ?43.75% */ 0x80, /* ?50.00% */
> +?????????????0x90, /* ?56.25% */ 0xa0, /* ?62.50% */
> +?????????????0xb0, /* ?68.75% */ 0xc0, /* ?75.00% */
> +?????????????0xd0, /* ?81.25% */ 0xe0, /* ?87.50% */
> +?????????????0xf0, /* ?93.75% */ 0xff, /* 100.00% */
> +?????},
> +?????{
> +?????????????0x00, /* ? 0.00% */ 0x40, /* ?25.00% */
> +?????????????0x49, /* ?28.57% */ 0x52, /* ?32.14% */
> +?????????????0x5b, /* ?35.71% */ 0x64, /* ?39.29% */
> +?????????????0x6d, /* ?42.86% */ 0x76, /* ?46.43% */
> +?????????????0x80, /* ?50.00% */ 0x89, /* ?53.57% */
> +?????????????0x92, /* ?57.14% */ 0xb6, /* ?71.43% */
> +?????????????0xdb, /* ?85.71% */ 0xff, /* 100.00% */
> +?????},
> +};
> +
> +static int LM93_PWM_FROM_REG(u8 reg, pwm_freq_t freq)
> +{
> +?????return lm93_pwm_map[freq][reg & 0x0f];
> +}
OK, the values 14 and 15 for reg are reserved and should not occur.
But if for some reason you read 0xff from the chip, the above
code would access bytes beyond the end of the array. I added two
more 0xff bytes so that all 16 possible values are valid.
I didn't have the time to test that patch until now. In fact, I
cannot test everything with my current equipment. I'll test as
good as I can until Monday evening, perhaps you or somebody else
can help testing, too.
As it might take some more weeks until I can add LM94 support,
I think we should take LM93 as it is now and patch in LM94
later. I'd really like to see this going mainline in the next
merge window.
Please have a look at my patch and let me know if there are
still issues I need to fix.
Thanks,
Hans
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lm93-support.patch
Type: text/x-diff
Size: 105703 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20070701/cd30b216/attachment.bin
