Messages in this thread | | | From | "Vaittinen, Matti" <> | Subject | Re: [PATCH v4 1/4] gpu: drm: meson: Use devm_regulator_*get_enable*() | Date | Mon, 24 Oct 2022 04:40:19 +0000 |
| |
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@gmail.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! ~~
| |