On 16/11/2022 13:02, Vaittinen, Matti wrote:
Hi dee Ho Laurent & All,
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.
---
drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++--------------------
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 5cd2b2ebbbd3..7642f740272b 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -140,7 +140,6 @@ struct meson_dw_hdmi {
struct reset_control *hdmitx_apb;
struct reset_control *hdmitx_ctrl;
struct reset_control *hdmitx_phy;
- struct regulator *hdmi_supply;
u32 irq_stat;
struct dw_hdmi *hdmi;
struct drm_bridge *bridge;
@@ -665,11 +664,6 @@ static void meson_dw_hdmi_init(struct
meson_dw_hdmi *meson_dw_hdmi)
}
-static void meson_disable_regulator(void *data)
-{
- regulator_disable(data);
-}
-
static void meson_disable_clk(void *data)
{
clk_disable_unprepare(data);
@@ -723,20 +717,9 @@ static int meson_dw_hdmi_bind(struct device
*dev, struct device *master,
meson_dw_hdmi->data = match;
dw_plat_data = &meson_dw_hdmi->dw_plat_data;
- meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev,
"hdmi");
- if (IS_ERR(meson_dw_hdmi->hdmi_supply)) {
- if (PTR_ERR(meson_dw_hdmi->hdmi_supply) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
- meson_dw_hdmi->hdmi_supply = NULL;
- } else {
- ret = regulator_enable(meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- ret = devm_add_action_or_reset(dev, meson_disable_regulator,
- meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- }
+ ret = devm_regulator_get_enable_optional(dev, "hdmi");
+ if (ret != -ENODEV)
+ return ret;
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.
While we all agree that power savings gained by runtime PM would be
great - I don't think we should stop minor improvements while waiting
for that to magically happen ;)
I like the saying I first heard from Jonathan Cameron - "Don't let the
perfect be an enemy of good".
After that being said, should I re-spin this or just drop it? (I am
cleaning up my local git from old stuff, and these are about to vanish
from my books).
I'm ok with you, please re-spin it.
Neil
Yours,
-- Matti
