Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1605501 > unrolled thread
| Started by | Peter Hüwe <PeterHuewe@gmx.de> |
|---|---|
| First post | 2017-03-21 12:00 +0100 |
| Last post | 2017-03-23 02:20 +0100 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: Conversion of w83627ehf to hwmon_device_register_with_info ? Peter Hüwe <PeterHuewe@gmx.de> - 2017-03-21 12:00 +0100
Re: Conversion of w83627ehf to hwmon_device_register_with_info ? Guenter Roeck <linux@roeck-us.net> - 2017-03-21 14:40 +0100
Re: Conversion of w83627ehf to hwmon_device_register_with_info ? Peter Hüwe <PeterHuewe@gmx.de> - 2017-03-23 02:20 +0100
| From | Peter Hüwe <PeterHuewe@gmx.de> |
|---|---|
| Date | 2017-03-21 12:00 +0100 |
| Subject | Re: Conversion of w83627ehf to hwmon_device_register_with_info ? |
| Message-ID | <tnpGa-2bY-3@gated-at.bofh.it> |
Hi On Friday 03 March 2017 03:56:01 Guenter Roeck wrote: > Hi Peter, > > On 03/02/2017 04:33 PM, Peter Hüwe wrote: > > Hi, > > > > is anybody else working on the conversion of the w83627ehf to the new > > hwmon_device_register_with_info interface? > > I don't think so. > > > Otherwise I will probably update the driver to this interface within the > > next days - but since it's a lot of work I wanted to check for > > duplication first. > > Go ahead. So I'm close to have to conversion done, the current diff stat is about 647 insertions(+), 787 deletions(-) for the whole conversion. Should I try to break it down into smaller chunks for easier review? Although this would mean to convert stuff from A to B and then from B to C - otherwise the intermediate steps would be not fully functional. (since the sysfs nodes partially would exist under hwmon1/ and partially under hwmon1/device/ (as they currently are)). or just submit it? It saves about 20k in compiled size, so the savings from reduced boilerplate are huge. (and I think it's more readable) > I would suggest to drop nct6775/nct6776 support to simplify the > code when you do that. Maybe as separate commit, though. Hehe - I'm testing on a nct6776 :) I'll look into it once the first conversion has been accepted. Thanks, Peter
[toc] | [next] | [standalone]
| From | Guenter Roeck <linux@roeck-us.net> |
|---|---|
| Date | 2017-03-21 14:40 +0100 |
| Message-ID | <tnsaZ-3W4-5@gated-at.bofh.it> |
| In reply to | #1605501 |
On 03/21/2017 03:46 AM, Peter Hüwe wrote: > Hi > > On Friday 03 March 2017 03:56:01 Guenter Roeck wrote: >> Hi Peter, >> >> On 03/02/2017 04:33 PM, Peter Hüwe wrote: >>> Hi, >>> >>> is anybody else working on the conversion of the w83627ehf to the new >>> hwmon_device_register_with_info interface? >> >> I don't think so. >> >>> Otherwise I will probably update the driver to this interface within the >>> next days - but since it's a lot of work I wanted to check for >>> duplication first. >> >> Go ahead. > > So I'm close to have to conversion done, > the current diff stat is about > 647 insertions(+), 787 deletions(-) > for the whole conversion. > > Should I try to break it down into smaller chunks for easier review? > > Although this would mean to convert stuff from A to B and then from B to C - > otherwise the intermediate steps would be not fully functional. > (since the sysfs nodes partially would exist under hwmon1/ and partially under > hwmon1/device/ (as they currently are)). > > or just submit it? > Just submit it. I don't really think a conversion like this can be split up cleanly. > It saves about 20k in compiled size, so the savings from reduced boilerplate > are huge. (and I think it's more readable) > >> I would suggest to drop nct6775/nct6776 support to simplify the >> code when you do that. Maybe as separate commit, though. > Hehe - I'm testing on a nct6776 :) > I'll look into it once the first conversion has been accepted. > Wondering - why don't you use the nct6775 driver ? Thanks, Guenter
[toc] | [prev] | [next] | [standalone]
| From | Peter Hüwe <PeterHuewe@gmx.de> |
|---|---|
| Date | 2017-03-23 02:20 +0100 |
| Message-ID | <tnZzY-3c4-11@gated-at.bofh.it> |
| In reply to | #1605592 |
> > It saves about 20k in compiled size, so the savings from reduced
> > boilerplate are huge. (and I think it's more readable)
> >
> >> I would suggest to drop nct6775/nct6776 support to simplify the
> >> code when you do that. Maybe as separate commit, though.
> >
> > Hehe - I'm testing on a nct6776 :)
> > I'll look into it once the first conversion has been accepted.
>
> Wondering - why don't you use the nct6775 driver ?
Probably because it got matched by sensors-detect before the nct6775 driver
(?) or because I hadn't had it turned on in my kernel config.
Both drivers work, but interestingly, they show different values for
intrusion1 (and fan alarm).
nct6776.c:
nct6776-isa-0290
Adapter: ISA adapter
Vcore: +1.16 V (min = +0.00 V, max = +1.74 V)
in1: +1.85 V (min = +0.00 V, max = +0.00 V) ALARM
AVCC: +3.36 V (min = +2.98 V, max = +3.63 V)
+3.3V: +3.36 V (min = +2.98 V, max = +3.63 V)
in4: +0.26 V (min = +0.00 V, max = +0.00 V) ALARM
in5: +1.67 V (min = +0.00 V, max = +0.00 V) ALARM
in6: +0.79 V (min = +0.00 V, max = +0.00 V) ALARM
3VSB: +3.44 V (min = +2.98 V, max = +3.63 V)
Vbat: +3.26 V (min = +2.70 V, max = +3.63 V)
fan1: 0 RPM (min = 0 RPM)
fan2: 2106 RPM (min = 0 RPM)
fan3: 0 RPM (min = 0 RPM)
fan4: 0 RPM (min = 0 RPM)
fan5: 0 RPM (min = 0 RPM)
SYSTIN: +35.0°C (high = +0.0°C, hyst = +0.0°C) ALARM sensor =
thermistor
CPUTIN: +44.0°C (high = +80.0°C, hyst = +75.0°C) sensor = thermistor
AUXTIN: +43.5°C (high = +80.0°C, hyst = +75.0°C) sensor = thermistor
PECI Agent 0: +59.0°C (high = +80.0°C, hyst = +75.0°C)
(crit = +105.0°C)
PCH_CHIP_TEMP: +0.0°C
PCH_CPU_TEMP: +0.0°C
PCH_MCH_TEMP: +0.0°C
intrusion0: ALARM
intrusion1: OK
beep_enable: disabled
w83627ehf.c:
nct6776-isa-0290
Adapter: ISA adapter
Vcore: +1.14 V (min = +0.00 V, max = +1.74 V)
in1: +1.85 V (min = +0.00 V, max = +0.00 V) ALARM
AVCC: +3.36 V (min = +2.98 V, max = +3.63 V)
+3.3V: +3.36 V (min = +2.98 V, max = +3.63 V)
in4: +0.24 V (min = +0.00 V, max = +0.00 V) ALARM
in5: +1.68 V (min = +0.00 V, max = +0.00 V) ALARM
3VSB: +3.44 V (min = +2.98 V, max = +3.63 V)
Vbat: +3.26 V (min = +2.70 V, max = +3.63 V)
fan1: 0 RPM (min = 0 RPM) ALARM
fan2: 2142 RPM (min = 0 RPM) ALARM
fan3: 0 RPM (min = 0 RPM) ALARM
fan4: 0 RPM (min = 0 RPM) ALARM
fan5: 0 RPM (min = 0 RPM) ALARM
SYSTIN: +35.0°C (high = +0.0°C, hyst = +0.0°C) ALARM sensor =
thermistor
CPUTIN: +46.0°C (high = +80.0°C, hyst = +75.0°C) sensor = thermistor
AUXTIN: +44.0°C (high = +80.0°C, hyst = +75.0°C) sensor = thermistor
PECI Agent 0: +64.0°C
cpu0_vid: +0.000 V
intrusion0: ALARM
intrusion1: ALARM
Once the other patches have been accepted I'll work on that conversion/removal
of the nct6775 code from the w83627ehf.
Peter
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web