lkml.org 
[lkml]   [2023]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] regulator: event: Add regulator netlink event support
    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.
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2023-11-23 13:57    [W:4.300 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site