Re: [PATCH v2] regulator: event: Add regulator netlink event support

From: Mark Brown
Date: Thu Nov 23 2023 - 07:57:31 EST


On Tue, Nov 21, 2023 at 10:13:30AM +0000, Naresh Solanki wrote:

> This commit introduces netlink event support to the regulator subsystem.

This looks a lot nicer, there's some feedback below but nothing
maissively substantial.

> +#ifdef CONFIG_NET

This is essentially the entire file - it's probably better to just put
the stub functions in the header and do the building with Kconfig.

> +static unsigned int reg_event_seqnum;
> +struct reg_genl_event {
> + char reg_name[15];
> + u64 event;
> +};

Given that this is going to get sent to userspace shouldn't it be in a
uapi header? Some of the other event types don't seem to do this
though... that might be an issue with those examples though. We'll
also make the event numbers into uapi so they should go in a uapi header
as well.

I'm also not clear on where the 15 byte limit comes from.

> +EXPORT_SYMBOL(reg_generate_netlink_event);

Everything else in regulator is EXPORT_SYMBOL_GPL() so this should be
too - though given that the only caller is _notifier_call_chain() which
is in the core does it need to be exported at all? I can't see a case
for anything calling it independently of that.

Attachment: signature.asc
Description: PGP signature