Re: [PATCH v4 1/4] gpu: drm: meson: Use devm_regulator_*get_enable*()

[Vaittinen, Matti]

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@xxxxxxxxx>
>>>
>>> ---
>>> 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! ~~