| Message ID | 3aacca7d9a0240d95dc3cf103d114b5c85392e71.1660934107.git.mazziesaccount@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Neil Armstrong |
| Headers | show |
| Series | Use devm helpers for regulator get and enable | expand |
Hi Matti, On Fri, 19 Aug 2022 at 21:18, Matti Vaittinen <mazziesaccount@gmail.com> wrote: > > Simplify drivers using managed "regulator get and enable". > > meson: > Use the devm_regulator_get_enable_optional(). Also drop the seemingly > unused struct member 'hdmi_supply'. > > sii902x: > Simplify using devm_regulator_bulk_get_enable() > > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> > > --- > v2 => v3: > No changes > > 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/bridge/sii902x.c | 22 +++------------------- > drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++-------------------- > 2 files changed, 6 insertions(+), 39 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c > index 7ab38d734ad6..162f9c87eeb2 100644 > --- a/drivers/gpu/drm/bridge/sii902x.c > +++ b/drivers/gpu/drm/bridge/sii902x.c > @@ -171,7 +171,6 @@ struct sii902x { > struct drm_connector connector; > struct gpio_desc *reset_gpio; > struct i2c_mux_core *i2cmux; > - struct regulator_bulk_data supplies[2]; > bool sink_is_hdmi; > /* > * Mutex protects audio and video functions from interfering > @@ -1072,6 +1071,7 @@ static int sii902x_probe(struct i2c_client *client, > struct device *dev = &client->dev; > struct device_node *endpoint; > struct sii902x *sii902x; > + static const char * const supplies[] = {"iovcc", "cvcc12"}; > int ret; > > ret = i2c_check_functionality(client->adapter, > @@ -1122,27 +1122,13 @@ static int sii902x_probe(struct i2c_client *client, > > mutex_init(&sii902x->mutex); > > - sii902x->supplies[0].supply = "iovcc"; > - sii902x->supplies[1].supply = "cvcc12"; > - ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies), > - sii902x->supplies); > - if (ret < 0) > - return ret; > - > - ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies), > - sii902x->supplies); > + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies), supplies); > if (ret < 0) { > dev_err_probe(dev, ret, "Failed to enable supplies"); > return ret; > } > > - ret = sii902x_init(sii902x); > - if (ret < 0) { > - regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies), > - sii902x->supplies); > - } > - > - return ret; > + return sii902x_init(sii902x); > } > > static int sii902x_remove(struct i2c_client *client) > @@ -1152,8 +1138,6 @@ static int sii902x_remove(struct i2c_client *client) > > i2c_mux_del_adapters(sii902x->i2cmux); > drm_bridge_remove(&sii902x->bridge); > - regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies), > - sii902x->supplies); > > return 0; > } Ideally this patch would be split into two parts here, due to maintainership boundaries. For the sii902x part, please add my r-b. Reviewed-by: Robert Foss <robert.foss@linaro.org> > 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; > > meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev, > "hdmitx_apb"); > -- > 2.37.1
On 8/29/22 17:25, Robert Foss wrote: Thanks for the review Robert. > Hi Matti, > > On Fri, 19 Aug 2022 at 21:18, Matti Vaittinen <mazziesaccount@gmail.com> wrote: >> >> Simplify drivers using managed "regulator get and enable". >> >> meson: >> Use the devm_regulator_get_enable_optional(). Also drop the seemingly >> unused struct member 'hdmi_supply'. >> >> sii902x: >> Simplify using devm_regulator_bulk_get_enable() >> >> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> >> >> --- >> v2 => v3: >> No changes >> >> 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/bridge/sii902x.c | 22 +++------------------- >> drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++-------------------- >> 2 files changed, 6 insertions(+), 39 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c >> index 7ab38d734ad6..162f9c87eeb2 100644 >> --- a/drivers/gpu/drm/bridge/sii902x.c >> +++ b/drivers/gpu/drm/bridge/sii902x.c >> @@ -171,7 +171,6 @@ struct sii902x { >> struct drm_connector connector; >> struct gpio_desc *reset_gpio; >> struct i2c_mux_core *i2cmux; >> - struct regulator_bulk_data supplies[2]; >> bool sink_is_hdmi; >> /* >> * Mutex protects audio and video functions from interfering >> @@ -1072,6 +1071,7 @@ static int sii902x_probe(struct i2c_client *client, >> struct device *dev = &client->dev; >> struct device_node *endpoint; >> struct sii902x *sii902x; >> + static const char * const supplies[] = {"iovcc", "cvcc12"}; >> int ret; >> >> ret = i2c_check_functionality(client->adapter, >> @@ -1122,27 +1122,13 @@ static int sii902x_probe(struct i2c_client *client, >> >> mutex_init(&sii902x->mutex); >> >> - sii902x->supplies[0].supply = "iovcc"; >> - sii902x->supplies[1].supply = "cvcc12"; >> - ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies), >> - sii902x->supplies); >> - if (ret < 0) >> - return ret; >> - >> - ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies), >> - sii902x->supplies); >> + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies), supplies); >> if (ret < 0) { >> dev_err_probe(dev, ret, "Failed to enable supplies"); >> return ret; >> } >> >> - ret = sii902x_init(sii902x); >> - if (ret < 0) { >> - regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies), >> - sii902x->supplies); >> - } >> - >> - return ret; >> + return sii902x_init(sii902x); >> } >> >> static int sii902x_remove(struct i2c_client *client) >> @@ -1152,8 +1138,6 @@ static int sii902x_remove(struct i2c_client *client) >> >> i2c_mux_del_adapters(sii902x->i2cmux); >> drm_bridge_remove(&sii902x->bridge); >> - regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies), >> - sii902x->supplies); >> >> return 0; >> } > > Ideally this patch would be split into two parts here, due to > maintainership boundaries. Ok. I will in any case respin this series when the dependency patches from Mark's tree have been merged to the -rc1. I can split this to patch/driver if it is preferred. I just though I'll decrease amount of mails by squashing these almost trivial changes. > > For the sii902x part, please add my r-b. > > Reviewed-by: Robert Foss <robert.foss@linaro.org> Thanks. Best Regards -- Matti
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 7ab38d734ad6..162f9c87eeb2 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -171,7 +171,6 @@ struct sii902x { struct drm_connector connector; struct gpio_desc *reset_gpio; struct i2c_mux_core *i2cmux; - struct regulator_bulk_data supplies[2]; bool sink_is_hdmi; /* * Mutex protects audio and video functions from interfering @@ -1072,6 +1071,7 @@ static int sii902x_probe(struct i2c_client *client, struct device *dev = &client->dev; struct device_node *endpoint; struct sii902x *sii902x; + static const char * const supplies[] = {"iovcc", "cvcc12"}; int ret; ret = i2c_check_functionality(client->adapter, @@ -1122,27 +1122,13 @@ static int sii902x_probe(struct i2c_client *client, mutex_init(&sii902x->mutex); - sii902x->supplies[0].supply = "iovcc"; - sii902x->supplies[1].supply = "cvcc12"; - ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies), - sii902x->supplies); - if (ret < 0) - return ret; - - ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies), - sii902x->supplies); + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies), supplies); if (ret < 0) { dev_err_probe(dev, ret, "Failed to enable supplies"); return ret; } - ret = sii902x_init(sii902x); - if (ret < 0) { - regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies), - sii902x->supplies); - } - - return ret; + return sii902x_init(sii902x); } static int sii902x_remove(struct i2c_client *client) @@ -1152,8 +1138,6 @@ static int sii902x_remove(struct i2c_client *client) i2c_mux_del_adapters(sii902x->i2cmux); drm_bridge_remove(&sii902x->bridge); - regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies), - sii902x->supplies); return 0; } 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; meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev, "hdmitx_apb");
Simplify drivers using managed "regulator get and enable". meson: Use the devm_regulator_get_enable_optional(). Also drop the seemingly unused struct member 'hdmi_supply'. sii902x: Simplify using devm_regulator_bulk_get_enable() Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> --- v2 => v3: No changes 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/bridge/sii902x.c | 22 +++------------------- drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++-------------------- 2 files changed, 6 insertions(+), 39 deletions(-)