lkml.org 
[lkml]   [2021]   [Mar]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2] hwmon: corsair-psu: add support for critical values
    From
    Date
    On 3/16/21 7:21 AM, Wilken Gottwalt wrote:
    > On Mon, 15 Mar 2021 11:00:53 -0700
    > Guenter Roeck <linux@roeck-us.net> wrote:
    >
    >> On 3/15/21 9:55 AM, Wilken Gottwalt wrote:
    >>> On Mon, 15 Mar 2021 08:53:25 -0700
    >>> Guenter Roeck <linux@roeck-us.net> 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

    \
     
     \ /
      Last update: 2021-03-16 21:08    [W:4.292 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site