lkml.org 
[lkml]   [2024]   [Apr]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 1/4] hwmon (max6639): Use regmap
    From
    On 4/29/24 01:19, Naresh Solanki wrote:
    > Hi Guenter,
    >
    >
    > On Sun, 28 Apr 2024 at 22:48, Guenter Roeck <linux@roeck-us.net> wrote:
    >>
    >> On 4/25/24 02:50, Naresh Solanki wrote:
    >> ...
    >>> This driver has 27 regmap accesses. Except volatile registers, others are
    >>> cached by regmap.
    >>> Some function which only access volatile registers will not be able to take
    >>> advantage of caching. This is also the case in various other drivers for similar
    >>> devices.
    >>> Also regmap offers bit handling which makes the code much cleaner.
    >>>
    >>
    >> Maybe I need to make it explicit in documentation. I will not accept regmap
    >> conversions unless local caching is dropped. Yes, that means that volatile
    >> registers will not be cached. I consider that a positive.
    > I agree with you. Regmap conversion wouldn't make sense if local caching
    > is present.
    > Correct me if I'm wrong, but in this context, local caching points to the
    > various variables in max6639_data ?
    > i.e.,
    > bool valid; /* true if following fields are valid */
    > unsigned long last_updated; /* In jiffies */
    >
    > /* Register values sampled regularly */
    > u16 temp[2]; /* Temperature, in 1/8 C, 0..255 C */
    > bool temp_fault[2]; /* Detected temperature diode failure */
    > u8 fan[2]; /* Register value: TACH count for fans >=30 */
    > u8 status; /* Detected channel alarms and fan failures */
    >
    > /* Register values only written to */
    > u8 pwm[2]; /* Register value: Duty cycle 0..120 */
    > u8 temp_therm[2]; /* THERM Temperature, 0..255 C (->_max) */
    > u8 temp_alert[2]; /* ALERT Temperature, 0..255 C (->_crit) */
    > u8 temp_ot[2]; /* OT Temperature, 0..255 C (->_emergency) */
    >
    > /* Register values initialized only once */
    > u8 ppr; /* Pulses per rotation 0..3 for 1..4 ppr */
    > u8 rpm_range; /* Index in above rpm_ranges table */
    >
    > Are you asking for removal of all these variables & each read sysfs
    > attribute read should access regmap cache directly ?
    >

    Mostly yes. Note that "ppr" is only used in max6639_init_client(),
    and it is unnecessary to keep it in max6639_data to start with.
    rpm_range is ok to keep because it is a calculated initialization value.
    The fixed initialization of temp_therm, temp_alert, and temp_ot
    is questionable to start with because it overrides bios/rommon
    initialization values.

    Guenter


    \
     
     \ /
      Last update: 2024-05-27 18:08    [W:3.144 / U:0.220 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site