Re: [PATCH v3 1/2] thermal/debugfs: Add thermal cooling device debugfs information

From: Daniel Lezcano
Date: Fri Dec 22 2023 - 10:04:35 EST


On 21/12/2023 18:19, Rafael J. Wysocki wrote:

[ ... ]

+struct cdev_value {

I'm not sure about the name here. I would rather call it cdev_record,
because it consists of two items, the id and the value.

+ struct list_head node;
+ int id;
+ u64 value;

This is kind of a union, but sort of in disguise.

Why not make it a union proper, that is

struct cdev_record {
struct list_head node;
int id;
union {
krime_t residency; /* for duration records */
u64 count; /* for occurrences records */
} data;
};

which then would result in a bit cleaner code in some places below, if
I'm not mistaken?

Can we stick to

struct cdev_record {
struct list_head node;
int id;
union {
u64 residency_ms; <----- ?
u64 count;
};
};

?

The usage of the ktime_t will have a more important impact in the code.



--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog