linux-kernel - Re: [PATCH v3 5/5] hwmon: Add support for Amphenol ChipCap 2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6cce1b87-a9a9-4554-9dae-c24d1d276fb5@sirena.org.uk>
Date:   Thu, 7 Dec 2023 20:44:14 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Javier Carrasco <javier.carrasco.cruz@...il.com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Jean Delvare <jdelvare@...e.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Jonathan Corbet <corbet@....net>,
        Liam Girdwood <lgirdwood@...il.com>,
        Rob Herring <robh@...nel.org>, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-hwmon@...r.kernel.org,
        linux-doc@...r.kernel.org
Subject: Re: [PATCH v3 5/5] hwmon: Add support for Amphenol ChipCap 2

On Thu, Dec 07, 2023 at 09:34:58PM +0100, Javier Carrasco wrote:
> On 07.12.23 21:05, Mark Brown wrote:

> > This is very buggy.  A consumer should only disable a regulator if it
> > itself enabled that regulator (or it *requires* an exclusive regulator
> > which isn't a good fit here), and there's no guarantee that disabling a
> > regulator will actually result in a power off.  Either the board might
> > not physically or through constraints permit the state to change or
> > another user may have enabled the regulator.  The driver needs to keep
> > track of if it enabled the regulator and only disable it as many times
> > as it enabled it.

> The idea is actually that if alarms are required, an exclusive regulator
> will be necessary to trigger power cycles and enter the command mode.

There is a specific API for exclusive regulators which the driver is not
using, and it's unconditionally doing the disable/enable cycle here.

> In summary there would be two options: either a regulator is defined and
> can be controlled to trigger the command mode or no regulator was
> defined for this device and therefore no command mode is available i.e.
> interrupts cannot be configured. That would be the case for example when
> the supply is always on.

The driver needs to be explicitly configured for this and have separate
code paths for normal operation and operation where the supply can be
bounced like this.  In neither code path should the supply be optional.
Right now we don't have a mechanism for discovering optionally exclusive
and enable/disablable supplies which is what the device needs, we could
potentially add that since this does seem like a viable use case and we
already have enough information in the DT to say if the supply matches
the constraints.  Probably the two properties queryable separately.  If
that API were added then the driver would do a normal regulator_get()
then check if it has the capabilities it needs and either keep the
supply on all the time (or possibly just during measurements?) or enable
the alarm functionality.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)