Re: [PATCH RESEND v4 05/10] PM: EM: Add a skeleton code for netlink notification

[Lukasz Luba <lukasz.luba@arm.com>]

On 10/13/25 14:46, Changwoo Min wrote:
> 
> 
> On 10/7/25 00:44, Lukasz Luba wrote:
>>
>>
>> On 9/21/25 04:19, Changwoo Min wrote:
>>> Add a boilerplate code for netlink notification to register the new
>>> protocol family. Also, initialize and register the netlink during 
>>> booting.
>>> The initialization is called at the postcore level, which is late enough
>>> after the generic netlink is initialized.
>>>
>>> Finally, update MAINTAINERS to include new files.
>>>
>>> Signed-off-by: Changwoo Min <changwoo@igalia.com>
>>> ---
>>>   MAINTAINERS               |  2 +-
>>>   kernel/power/Makefile     |  5 ++++-
>>>   kernel/power/em_netlink.c | 35 +++++++++++++++++++++++++++++++++++
>>>   kernel/power/em_netlink.h | 16 ++++++++++++++++
>>>   4 files changed, 56 insertions(+), 2 deletions(-)
>>>   create mode 100644 kernel/power/em_netlink.c
>>>   create mode 100644 kernel/power/em_netlink.h
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 0992029d271d..ba528836eac1 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -9034,7 +9034,7 @@ F:    include/linux/energy_model.h
>>>   F:    Documentation/power/energy-model.rst
>>>   F:    Documentation/netlink/specs/em.yaml
>>>   F:    include/uapi/linux/energy_model.h
>>> -F:    kernel/power/em_netlink_autogen.*
>>> +F:    kernel/power/em_netlink*.*
>>>   EPAPR HYPERVISOR BYTE CHANNEL DEVICE DRIVER
>>>   M:    Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>> diff --git a/kernel/power/Makefile b/kernel/power/Makefile
>>> index 874ad834dc8d..284a760aade7 100644
>>> --- a/kernel/power/Makefile
>>> +++ b/kernel/power/Makefile
>>> @@ -21,4 +21,7 @@ obj-$(CONFIG_PM_WAKELOCKS)    += wakelock.o
>>>   obj-$(CONFIG_MAGIC_SYSRQ)    += poweroff.o
>>> -obj-$(CONFIG_ENERGY_MODEL)    += energy_model.o
>>> +obj-$(CONFIG_ENERGY_MODEL)    += em.o
>>> +em-y                := energy_model.o
>>> +em-$(CONFIG_NET)        += em_netlink_autogen.o em_netlink.o
>>> +
>>> diff --git a/kernel/power/em_netlink.c b/kernel/power/em_netlink.c
>>> new file mode 100644
>>> index 000000000000..f3fbfeff29a4
>>> --- /dev/null
>>> +++ b/kernel/power/em_netlink.c
>>> @@ -0,0 +1,35 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + *
>>> + * Generic netlink for energy model.
>>> + *
>>> + * Copyright (c) 2025 Valve Corporation.
>>> + * Author: Changwoo Min <changwoo@igalia.com>
>>> + */
>>> +
>>> +#define pr_fmt(fmt) "energy_model: " fmt
>>> +
>>> +#include <linux/energy_model.h>
>>> +#include <net/sock.h>
>>> +#include <net/genetlink.h>
>>> +#include <uapi/linux/energy_model.h>
>>> +
>>> +#include "em_netlink.h"
>>> +#include "em_netlink_autogen.h"
>>> +
>>> +int em_nl_get_pds_doit(struct sk_buff *skb, struct genl_info *info)
>>> +{
>>> +    return -EOPNOTSUPP;
>>> +}
>>> +
>>> +int em_nl_get_pd_table_doit(struct sk_buff *skb, struct genl_info 
>>> *info)
>>> +{
>>> +    return -EOPNOTSUPP;
>>> +}
>>> +
>>> +static int __init em_netlink_init(void)
>>> +{
>>> +    return genl_register_family(&em_nl_family);
>>> +}
>>> +postcore_initcall(em_netlink_init);
>>> +
>>> diff --git a/kernel/power/em_netlink.h b/kernel/power/em_netlink.h
>>> new file mode 100644
>>> index 000000000000..acd186c92d6b
>>> --- /dev/null
>>> +++ b/kernel/power/em_netlink.h
>>> @@ -0,0 +1,16 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + *
>>> + * Generic netlink for energy model.
>>> + *
>>> + * Copyright (c) 2025 Valve Corporation.
>>> + * Author: Changwoo Min <changwoo@igalia.com>
>>> + */
>>> +#ifndef _EM_NETLINK_H
>>> +#define _EM_NETLINK_H
>>> +
>>> +#if defined(CONFIG_ENERGY_MODEL) && defined(CONFIG_NET)
>>> +#else
>>> +#endif
>>> +
>>> +#endif /* _EM_NETLINK_H */
>>
>> Actually, those declarations of functions from patch 3/10 can
>> live in this header. We would avoid creating more local headers
>> in such case.
> 
>   That makes sense to me.
> 
>>
>> Then the patch 3/10 would have to go after this patch when
>> this header is introduced.
>>
>> Please ignore the comment in the patch 3/10 and try to
>> use this header. It is also logically linked to the
>> notifications, so belongs to such header IMHO.
>>
> 
> Sure, after moving 3/10 after this, I will move the changes made in 3/10
> to em_netlink.h. I will keep the implementation of
> for_each_em_perf_domain() and em_perf_domain_get_by_id() in
> energy_model.c since it it not ideal to expose em_pd_list_mutex, etc.
> outside of energy_mode.c. And, this requires to include "em_netlink.h"
> from energy_model.c.
> 

Sounds good. These are only minor changes because it has nice shape
already.

I was able to test it last weekend and it works on my setup now.
I can see the messages coming in the user-space, so it should
work as designed.

Thank you Changwoo! Looking forward for your v5.

Regards,
Lukasz