linux-kernel - Re: [PATCH 1/4] hwmon (max6639): Use regmap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABqG17inSB8stBf=ObaYKj4FREpXKWSy=7XXO9O3RhEx_v74vg@mail.gmail.com>
Date: Mon, 29 Apr 2024 13:49:30 +0530
From: Naresh Solanki <naresh.solanki@...ements.com>
To: Guenter Roeck <linux@...ck-us.net>
Cc: krzysztof.kozlowski+dt@...aro.org, u.kleine-koenig@...gutronix.de, 
	Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] hwmon (max6639): Use regmap

Hi Guenter,


On Sun, 28 Apr 2024 at 22:48, Guenter Roeck <linux@...ck-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 ?

Regards,
Naresh
>
> Guenter
>