Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
Date:   Tue, 10 Oct 2023 13:19:36 +0100
From:   Mark Brown <broonie@kernel.org>
To:     Oleksij Rempel <o.rempel@pengutronix.de>
Cc:     Liam Girdwood <lgirdwood@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>, kernel@pengutronix.de,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v1 3/3] regulator: fixed: forward under-voltage events
Message-ID: <5e51792a-cc93-4364-a51b-c2b116d89369@sirena.org.uk>
References: <20231010085906.3440452-1-o.rempel@pengutronix.de>
 <20231010085906.3440452-3-o.rempel@pengutronix.de>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="sc4/D+hultAuc0Xk"
Content-Disposition: inline
In-Reply-To: <20231010085906.3440452-3-o.rempel@pengutronix.de>
Precedence: bulk


--sc4/D+hultAuc0Xk
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Oct 10, 2023 at 10:59:06AM +0200, Oleksij Rempel wrote:
> Add handler to forward under-voltage events.
> On systems for more or less complicated regulator chains we need to
> forward under-voltage events to actual driver which need to react on
> them.

It isn't clear to me why this would be implemented in one specific
driver, nor why this would be done unconditionally.  Could you provide
some information on the problem you're trying to solve here?  This feels
like something that should be a core feature.

> +static int reg_fixed_regulator_notifier(struct notifier_block *nb,
> +					unsigned long event, void *data)
> +{
> +	struct fixed_voltage_data *priv =
> +		container_of(nb, struct fixed_voltage_data, nb);
> +	struct regulator_dev *rdev = priv->dev;
> +
> +	if (event != REGULATOR_EVENT_UNDER_VOLTAGE_WARN &&
> +	    event != REGULATOR_EVENT_UNDER_VOLTAGE)
> +		return NOTIFY_OK;
> +
> +	regulator_notifier_call_chain(rdev, event, NULL);

This would be better written as a switch statement for extensibility,
and it's not clear why the filtering?

--sc4/D+hultAuc0Xk
Content-Type: application/pgp-signature; name="signature.asc"

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

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmUlQVgACgkQJNaLcl1U
h9BzmQf8DlvDTySkrhtUhIgfciqU+pjEPtHsKQqoDt7De9vv7WBpPjMCs9c8j27a
Bh1J0WSWgde+2u39XUSddSwJyWnNwUdJSM0CeZKXd0bEYPfsVHpCrxVbq3RGes83
sr+3cD6QylEJmNwtmvqiMKfrzfRWZIZIAAJb6a/o1hYY+OgrY1ddK+KAJTIP195t
azfhhfMmHANcCAqqsdIJX3++16Wg+fI+XuntB4nb4LKQFfDtOdtuK29rh0Vd3ox6
hODpV3nf4yF2h/QugxX/neL4Cjg8Xi/Tkgq90E557WcMH2MRzYchNfgwFjztfHzK
JQWDqvRIw/+u3TdwWkTYuxLw2LlsQg==
=K/BW
-----END PGP SIGNATURE-----

--sc4/D+hultAuc0Xk--