On 3/16/21 7:21 AM, Wilken Gottwalt wrote: > On Mon, 15 Mar 2021 11:00:53 -0700 > Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > >> On 3/15/21 9:55 AM, Wilken Gottwalt wrote: >>> On Mon, 15 Mar 2021 08:53:25 -0700 >>> Guenter Roeck <linux@xxxxxxxxxxxx> wrote: >>> >>>> On 3/15/21 8:02 AM, Wilken Gottwalt wrote: >>>>> Adds support for reading the critical values of the temperature sensors >>>>> and the rail sensors (voltage and current) once and caches them. Updates >>>>> the naming of the constants following a more clear scheme. Also updates >>>>> the documentation and fixes a typo. >>>>> >>>>> The new sensors output of a Corsair HX850i will look like this: >>>>> corsairpsu-hid-3-1 >>>>> Adapter: HID adapter >>>>> v_in: 230.00 V >>>>> v_out +12v: 12.14 V (crit min = +8.41 V, crit max = +15.59 V) >>>>> v_out +5v: 5.03 V (crit min = +3.50 V, crit max = +6.50 V) >>>>> v_out +3.3v: 3.30 V (crit min = +2.31 V, crit max = +4.30 V) >>>>> psu fan: 0 RPM >>>>> vrm temp: +46.2°C (crit = +70.0°C) >>>>> case temp: +39.8°C (crit = +70.0°C) >>>>> power total: 152.00 W >>>>> power +12v: 108.00 W >>>>> power +5v: 41.00 W >>>>> power +3.3v: 5.00 W >>>>> curr in: N/A >>>> >>>> What does that mean ? If it isn't supported by the power supply, >>>> should we drop that entirely ? Maybe drop it via the is_visible >>>> function if it is available for some variants, but always displaying >>>> N/A doesn't add value. >>>> >>>> This is a bit odd, though, since I would assume it translates >>>> to the PSU_CMD_IN_AMPS command. Any chance to track down what is >>>> happening here ? >>> >>> I have one of the earliest PSUs of this series, it is just not supported on >>> mine. I'm not sure if it would be worth the trouble to catch that and turn >>> it off dynamically. >>> >> >> I think so, because otherwise we'll get complaints about it (people >> are really picky abut such things lately). Better not display it at all >> if it is not supported on a given PSU version. This should be relatively >> easy to catch in the is_visible function. > > So do you have any idea how to do it? The PSU does not tell you what is > supported or not, you only find out by running the commands. I mean the > only thing I think of is like I did it for the critical values, but only > keeping the *_support bits. But if I do it that way, I actually should do > it for all the commands. This is the point which I ment with "worth the > trouble." > It is not really necessary to do it for all commands; only for those known to not be supported on all power supplies. >> Nice PS, anyway. Too bad it is so expensive (and large). Do you know >> if the HX750i uses the same protocol ? > > All HX_num_i and RM_num_i PSUs support the same protocol. There are only > small differences in supported commands based on release version. What do > you mean by "large"? The size of the case? All HXi and RMi should have > the same size (standard ATX). Maybe you looked at one of the AXi series, They don't fit into my small mini-ATX chassis, and the "i" series all seem to be full-size ATX. [ ... ] >> >> Making the code less readable to meet a line limit isn't really that desirable. >> If you want to stick with one line, you could drop the "hwmon_" from function prefixes >> instead. Those don't really add any value. > > I know I know, it's a personal taste. I really dislike splitting functions > headers about serveral lines, especially if indenting is tab based. > Fortunately you can get there by dropping 'hwmon_' from the function names. Thanks, Guenter
