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