Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH RFC] regulator: core: fix handling negative voltages e.g. in EPD PMICs
Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\))
Content-Type: text/plain; charset=utf-8
From:   "H. Nikolaus Schaller" <hns@goldelico.com>
In-Reply-To: <20200224120512.GG6215@sirena.org.uk>
Date:   Mon, 24 Feb 2020 13:22:21 +0100
Cc:     Andreas Kemnade <andreas@kemnade.info>, j.neuschaefer@gmx.net,
        contact@paulk.fr, GNUtoo@cyberdimension.org, josua.mayer@jm0.eu,
        lgirdwood@gmail.com, linux-kernel@vger.kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <1548203B-9D64-4128-9BED-D3BC30F9DC49@goldelico.com>
References: <20200223153502.15306-1-andreas@kemnade.info> <20200224120512.GG6215@sirena.org.uk>
To:     Mark Brown <broonie@kernel.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> Am 24.02.2020 um 13:05 schrieb Mark Brown <broonie@kernel.org>:
>=20
> On Sun, Feb 23, 2020 at 04:35:01PM +0100, Andreas Kemnade wrote:
>=20
>> An alternative would be to handle voltages as absolute values.
>> There are probably no regulators with support both negative
>> and positive output.
>=20
> This is what'd be needed, your approach here is a bit of a hack and
> leaves some values unrepresentable if they overlap with errnos which
> obviously has issues if someone has a need for those values.

Negative ERRNOs have BIT(31) set.

Since voltage integers on a 32 bit architecture represent =C2=B5V this =
would
still allow voltages up to (2^31 - 1) =C2=B5V i.e. 2 kV with BIT(31) not =
set.

Therefore it seems very unlikely that anyone needs to represent voltages
above 2 kV within a Linux system through a regulator.

So I'd say any negative return value got regulator_get_voltage() can be
treated as an error.

And regulators for negative voltages could be represented by
their absolute value (and maybe a _neg component in the regulator
name).

But then it seems to be a little inconsistent that the voltage
parameters of regulator_set_voltage_unlocked() are signed integers
and not unsigned.

So shouldn't that be protected against attempting to set negative =
voltages?

Just my 2 cts.

BR,
Nikolaus=