Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Date:   Thu, 23 Nov 2023 12:57:25 +0000
From:   Mark Brown <broonie@kernel.org>
To:     Naresh Solanki <naresh.solanki@9elements.com>
Cc:     Liam Girdwood <lgirdwood@gmail.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] regulator: event: Add regulator netlink event support
Message-ID: <ZV9MNdOCkIpLB+6Y@finisterre.sirena.org.uk>
References: <20231121101331.22837-1-naresh.solanki@9elements.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="hfmqmatwhTT9Ynoi"
Content-Disposition: inline
In-Reply-To: <20231121101331.22837-1-naresh.solanki@9elements.com>
Precedence: bulk


--hfmqmatwhTT9Ynoi
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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.

--hfmqmatwhTT9Ynoi
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmVfTDQACgkQJNaLcl1U
h9DE+AgAga4ENL0Saql9gBXIwkH6zvOVuaGmUXjNr3LkVsPIwx/Q+Y1ibLCLedxx
AV7Z3sEFpJehQspzg9X6lkbiX+yskuv6ri/PMpSi2MbKAt61H/uvkKG7rGQ7O4LI
Wgl49dQgmOsEGWLb5MQP+iw5YBxoHUzehMipRbVcRrm+04UAK6AFyo4sXu5TCoRQ
jHnN6UpiNTFn0SNmdJD/BVLuT3tpAFTVIRE42maydS0+Xg5twXR2YnAsoiGrwUoJ
5exsjlFYBhOg6GOcU4iWIm1UlnBQ1A5nnG2p8vPDvjNL5ZXHSYj1sUkpROXDkZXK
VdMJNENScxdGePD/EFHYeyV6uZkOgg==
=Xef/
-----END PGP SIGNATURE-----

--hfmqmatwhTT9Ynoi--