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:8.625 / U:0.292 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site