linux-kernel - Re: [PATCH v4 1/4] gpu: drm: meson: Use devm_regulator_*get

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]

Date:   Mon, 24 Oct 2022 04:40:19 +0000
From:   "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>
To:     "neil.armstrong@...aro.org" <neil.armstrong@...aro.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Matti Vaittinen <mazziesaccount@...il.com>
CC:     Andrzej Hajda <andrzej.hajda@...el.com>,
        Robert Foss <robert.foss@...aro.org>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Kevin Hilman <khilman@...libre.com>,
        Jerome Brunet <jbrunet@...libre.com>,
        Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        Jean Delvare <jdelvare@...e.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-amlogic@...ts.infradead.org" 
        <linux-amlogic@...ts.infradead.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>
Subject: Re: [PATCH v4 1/4] gpu: drm: meson: Use devm_regulator_*get_enable*()

On 10/21/22 18:29, Neil Armstrong wrote:
> Hi,
> 
> On 21/10/2022 17:02, Laurent Pinchart wrote:
>> Hi Matti,
>>
>> On Fri, Oct 21, 2022 at 04:18:01PM +0300, Matti Vaittinen wrote:
>>> Simplify using the devm_regulator_get_enable_optional(). Also drop the
>>> seemingly unused struct member 'hdmi_supply'.
>>>
>>> Signed-off-by: Matti Vaittinen <mazziesaccount@...il.com>
>>>
>>> ---
>>> v3 => v4:
>>> - split meson part to own patch
>>>
>>> RFCv1 => v2:
>>> - Change also sii902x to use devm_regulator_bulk_get_enable()
>>>
>>> Please note - this is only compile-tested due to the lack of HW. Careful
>>> review and testing is _highly_ appreciated.
>>> ---

//Snip

>>
>> As noted in the review of the series that introduced
>> devm_regulator_get_enable_optional(), the right thing to do is to
>> implement runtime PM in this driver to avoid wasting power.
> 
> While I agree, it's not really the same level of effort as this patch
> should be functionally equivalent.
> 

Exactly. As I wrote, I do not have the HW to test this change. This 
intends to bring no functional changes. It is just a minor 
simplification while also making it harder to create a bug with the 
regulator control. (Registering the devm_action and leaving the handle 
to the regulator is more fragile than using this new API which does not 
give user the handle).

I am in no way against someone further improving the functionality here 
(by adding runtime PM) but this someone is not me. Yet, I don't see how 
this patch prevents runtime PM being implemented? The first thing that 
needs to be done is removing the devm-action assuming someone did 
implement power-saving by disabling the regulator(s) at runtime. After 
this patch is applied, the action removal is automatically a necessity 
in order to get the handle for regulator control.

I know this helper is not preferred by all but it is still safer than 
the current code which registers the action while allowing the regulator 
control.

Yours
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~