Messages in this thread | | | Subject | Re: [PATCH v3 3/4] thermal: core: genetlink support for events/cmd/sampling | From | Daniel Lezcano <> | Date | Mon, 6 Jul 2020 09:04:07 +0200 |
| |
On 05/07/2020 08:03, Zhang Rui wrote: > On Fri, 2020-07-03 at 10:53 +0200, Daniel Lezcano wrote: >> Initially the thermal framework had a very simple notification >> mechanism to send generic netlink messages to the userspace. >> >> The notification function was never called from anywhere and the >> corresponding dead code was removed. It was probably a first attempt >> to introduce the netlink notification. >> >> At LPC2018, the presentation "Linux thermal: User kernel interface", >> proposed to create the notifications to the userspace via a kfifo. >> >> The advantage of the kfifo is the performance. It is usually used >> from >> a 1:1 communication channel where a driver captures data and sends it >> as fast as possible to a userspace process. >> >> The drawback is that only one process uses the notification channel >> exclusively, thus no other process is allowed to use the channel to >> get temperature or notifications. >> >> This patch defines a generic netlink API to discover the current >> thermal setup and adds event notifications as well as temperature >> sampling. As any genetlink protocol, it can evolve and the versioning >> allows to keep the backward compatibility. >> >> In order to prevent the user from getting flooded with data on a >> single channel, there are two multicast channels, one for the >> temperature sampling when the thermal zone is updated and another one >> for the events, so the user can get the events only without the >> thermal zone temperature sampling. >> >> Also, a list of commands to discover the thermal setup is added and >> can be extended when needed. >> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> ---
[ ... ]
>> +static int thermal_genl_event_cdev_update(struct param *p) >> +{ >> + if (nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_ID, >> + p->cdev_id) || >> + nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_CUR_STATE, >> + p->cdev_state) || >> + nla_put_u32(p->msg, THERMAL_GENL_ATTR_CDEV_MAX_STATE, >> + p->cdev_max_state)) >> + return -EMSGSIZE; >> + >> + return 0; >> +} > >> +int thermal_notify_cdev_update(int cdev_id, int cdev_state) >>> +{ >>> + struct param p = { .cdev_id = cdev_id, .cdev_state = cdev_state >>> }; >> > > .cdev_max_state is not set here. > I think we need to add a second parameter for cdev_max_state when > invoking thermal_nofify_cdev_update().
I've been trying to figure when the max state is changed and the notification for the max state is less straightforward than the current state.
Actually merging both changes is not a good idea.
It would be preferable to add later the max state change notification and keep set current state as before, except changing the function and attributes to cdev_state_update.
Sounds good ?
-- <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
| |