Use devm helpers for regulator get and enable
Subsystems (5)
v4 cover Use devm helpers for regulator get and enable
Simplify couple of drivers by using the new devm_regulator_*get_enable*()
These patches were previously part of the series:
https://lore.kernel.org/lkml/[email protected]/
"Devm helpers for regulator get and enable". I did keep the patch series versioning even though I changed the series name (subject of this mail) to "Use devm helpers for regulator get and enable". Name was changed because the devm helpers are already in 6.1-rc1.
Also, most of the patches in the series are already merged to subsystem trees so this series now contains only the patches that have not yet been merged. I hope they can be now directly taken sirectly into respective subsystem trees as the dependencies should be in v6.1-rc1.
Please note that these changes are only compile-tested as I don't have the HW to do proper testing. Thus, reviewing / testing is highly appreciated.
Revision history:
v4: RESEND2:
- resend code unchanged. Commit titles and and commit messages improved.
v4: RESEND:
- resend only the drm patches - others have been applied
v3 => v4:
- Drop applied patches
- rewrite cover-letter
- rebase on v6.1-rc1
- split meson and sii902x into own patches as requested.
- slightly modify dev_err_probe() return in sii902x.
---
Matti Vaittinen (2):
drm/bridge: sii902x: Use devm_regulator_bulk_get_enable() drm/meson: dw-hdmi: Use devm_regulator_*get_enable*()
drivers/gpu/drm/bridge/sii902x.c | 26 ++++---------------------- drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++-------------------- 2 files changed, 7 insertions(+), 42 deletions(-)
base-commit: 094226ad94f471a9f19e8f8e7140a09c2625abaa
Signature · 14 lines
2.38.1 -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ Simon says - in Latin please. ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~ Thanks to Simon Glass for the translation =]
PATCH v4 1/2 drm/bridge: sii902x: Use devm_regulator_bulk_get_enable()
Simplify using devm_regulator_bulk_get_enable()
---
v4 resend 2:
Resending unchanged code with commit title prefix adjusted as suggested by Robert
I am doing a clean-up for my local git and encountered this one. Respinning as it seems this one fell through the cracks.
--- drivers/gpu/drm/bridge/sii902x.c | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-)
drivers/gpu/drm/bridge/sii902x.c +4 -22
@@ -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,11 @@ 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); + ret =devm_regulator_bulk_get_enable (dev,ARRAY_SIZE (supplies), supplies);if (ret <0 ) -return ret; - - ret =regulator_bulk_enable (ARRAY_SIZE (sii902x->supplies), - sii902x->supplies); -if (ret <0 ) { -dev_err_probe (dev, ret,"Failed to enable supplies" ); -return ret; - } +return dev_err_probe (dev, ret,"Failed to enable supplies" ); - ret =sii902x_init (sii902x); -if (ret <0 ) { -regulator_bulk_disable (ARRAY_SIZE (sii902x->supplies), - sii902x->supplies); - } - -return ret; +return sii902x_init (sii902x); }static void sii902x_remove (struct i2c_client *client)
@@ -1152,8 +1136,6 @@ static void 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); }static const struct of_device_id sii902x_dt_ids[] = {
Signature · 14 lines
2.38.1 -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ Simon says - in Latin please. ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~ Thanks to Simon Glass for the translation =]
PATCH v4 2/2 drm/meson: dw-hdmi: Use devm_regulator_*get_enable*()
In reply to Matti VaittinenSimplify using the devm_regulator_get_enable_optional(). Also drop the now unused struct member 'hdmi_supply'.
Martin Blumenstingl <[email protected]>
---
v4 resend 2:
Respinning unchanged code with the commit title changed as wa suggested by Robert Foss and commit message changed as was suggested by Martin.
I am doing a clean-up for my local git and encountered this one. Respinning as it seems this one fell through the cracks.
--- drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-)
drivers/gpu/drm/meson/meson_dw_hdmi.c +3 -20
@@ -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" );
Signature · 14 lines
2.38.1 -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ Simon says - in Latin please. ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~ Thanks to Simon Glass for the translation =]
Neil Armstrong
maintainerRe: [PATCH RESEND2 v4 2/2] drm/meson: dw-hdmi: Use devm_regulator_*get_enable*()
Quoted 5 lines · Matti Vaittinen
Simplify using the
devm_regulator_get_enable_optional(). Also drop the now unused struct member 'hdmi_supply'.Signed-off-by Matti Vaittinen <[email protected]>Martin Blumenstingl <[email protected]>
Missing Acked-by, I'll add it while applying.
Neil
Quoted 60 lines
---
v4 resend 2:
Respinning unchanged code with the commit title changed as wa suggested by Robert Foss and commit message changed as was suggested by Martin.I am doing a clean-up for my local git and encountered this one. Respinning as it seems this one fell through the cracks.
--- drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-)drivers/gpu/drm/meson/meson_dw_hdmi.c +3
-20@@ -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" );
Re: [PATCH RESEND2 v4 2/2] drm/meson: dw-hdmi: Use devm_regulator_*get_enable*()
Quoted 8 lines · Neil Armstrong
Quoted 5 lines · Matti Vaittinen
Simplify using the
devm_regulator_get_enable_optional(). Also drop the now unused struct member 'hdmi_supply'.Signed-off-by Matti Vaittinen <[email protected]>Martin Blumenstingl <[email protected]>
Missing Acked-by, I'll add it while applying.
Oh, well spotted. I should've been more careful.. Sorry and thanks for sorting it out!
--Matti
Signature · 5 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
Re: [RESEND2,v4,2/2] drm/meson: dw-hdmi: Use devm_regulator_*get_enable*()
In reply to Matti VaittinenHi Matti,
Quoted 50 lines · Matti Vaittinen
Simplify using the
devm_regulator_get_enable_optional(). Also drop the now unused struct member 'hdmi_supply'.Signed-off-by Matti Vaittinen <[email protected]>Martin Blumenstingl <[email protected]>
---
v4 resend 2:
Respinning unchanged code with the commit title changed as wa suggested by Robert Foss and commit message changed as was suggested by Martin.I am doing a clean-up for my local git and encountered this one. Respinning as it seems this one fell through the cracks.
--- drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-)drivers/gpu/drm/meson/meson_dw_hdmi.c +0
-12@@ -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 {
drivers/gpu/drm/meson/meson_dw_hdmi.c static int meson_dw_hdmi_bind(struct device *dev, struct device *master, · line 721
Quoted 10 lines
drivers/gpu/drm/meson/meson_dw_hdmi.c +2
-8- 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 )
The above line should be "if (ret < 0)", otherwise it breaks hdmi support.
I've noticed this once this change has been merged to linux-next and all my Amlogic Meson based boards failed to initialize HDMI. Is it possible to fix this in drm tree or do I need to send the incremental fixup?
drivers/gpu/drm/meson/meson_dw_hdmi.c static int meson_dw_hdmi_bind(struct device *dev, struct device *master, · line 725
Quoted 4 lines
drivers/gpu/drm/meson/meson_dw_hdmi.c +1
-0+return ret; meson_dw_hdmi->hdmitx_apb =devm_reset_control_get_exclusive (dev,"hdmitx_apb" );
Best regards
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland
Neil Armstrong
maintainerRe: [RESEND2,v4,2/2] drm/meson: dw-hdmi: Use devm_regulator_*get_enable*()
Quoted 66 lines · Marek Szyprowski
Hi Matti,
Quoted 50 lines · Matti Vaittinen
Simplify using the
devm_regulator_get_enable_optional(). Also drop the now unused struct member 'hdmi_supply'.Signed-off-by Matti Vaittinen <[email protected]>Martin Blumenstingl <[email protected]>
---
v4 resend 2:
Respinning unchanged code with the commit title changed as wa suggested by Robert Foss and commit message changed as was suggested by Martin.I am doing a clean-up for my local git and encountered this one. Respinning as it seems this one fell through the cracks.
--- drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-)drivers/gpu/drm/meson/meson_dw_hdmi.c +0
-12@@ -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 {drivers/gpu/drm/meson/meson_dw_hdmi.c static int meson_dw_hdmi_bind(struct device *dev, struct device *master, · line 721
Quoted 10 lines
drivers/gpu/drm/meson/meson_dw_hdmi.c +2
-8- 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 )The above line should be "if (ret < 0)", otherwise it breaks hdmi support.
Quoted 3 lines
I've noticed this once this change has been merged to linux-next and all my Amlogic Meson based boards failed to initialize HDMI. Is it possible to fix this in drm tree or do I need to send the incremental fixup?
Nop, please send an incremental fix and I'll apply it asap.
Thanks,
Neil
Quoted 7 lines
drivers/gpu/drm/meson/meson_dw_hdmi.c static int meson_dw_hdmi_bind(struct device *dev, struct device *master, · line 725
Quoted 5 lines
drivers/gpu/drm/meson/meson_dw_hdmi.c +1
-0+return ret; meson_dw_hdmi->hdmitx_apb =devm_reset_control_get_exclusive (dev,"hdmitx_apb" );Best regards
Re: [RESEND2,v4,2/2] drm/meson: dw-hdmi: Use devm_regulator_*get_enable*()
Hi Marek & All,
ma 9. tammik. 2023 klo 17.23 Neil Armstrong
([email protected]) kirjoitti:
Quoted 60 lines · Marek Szyprowski
Quoted 58 lines
Hi Matti,
Quoted 55 lines · Matti Vaittinen
Simplify using the
devm_regulator_get_enable_optional(). Also drop the now unused struct member 'hdmi_supply'.Signed-off-by Matti Vaittinen <[email protected]>Martin Blumenstingl <[email protected]>
---
v4 resend 2:
Respinning unchanged code with the commit title changed as wa suggested by Robert Foss and commit message changed as was suggested by Martin.I am doing a clean-up for my local git and encountered this one. Respinning as it seems this one fell through the cracks.
--- drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-)drivers/gpu/drm/meson/meson_dw_hdmi.c +0
-17@@ -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);
Quoted 8 lines
Quoted 8 lines
drivers/gpu/drm/meson/meson_dw_hdmi.c static int meson_dw_hdmi_bind(struct device *dev, struct device *master, · line 721
Quoted 5 lines
drivers/gpu/drm/meson/meson_dw_hdmi.c +2
-3-if (ret) -return ret; - } + ret =devm_regulator_get_enable_optional (dev,"hdmi" ); +if (ret != -ENODEV )The above line should be "if (ret < 0)", otherwise it breaks hdmi support.
Sorry for the trouble and thanks for pointing this out! I believe the condition I was looking for was
if (ret && ret != -ENODEV).
At least in my eyes the use of regulator_get_optional() sounds like the -ENODEV should not be treated as an error. The obvious mistake is regarding return value 0 as such. Thanks for spotting this!
Quoted 5 lines
Quoted 3 lines
I've noticed this once this change has been merged to linux-next and all my Amlogic Meson based boards failed to initialize HDMI. Is it possible to fix this in drm tree or do I need to send the incremental fixup?
Nop, please send an incremental fix and I'll apply it asap.
Thanks for cleaning-up the mess I caused Marek! Please, let me know if you want me to cook the fix.
Yours,
- -Matti
Signature · 8 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~ Discuss - Estimate - Plan - Report and finally accomplish this: void do_work(int time) __attribute__ ((const));
Hi,
Quoted 10 lines · Matti Vaittinen
Simplify couple of drivers by using the new devm_regulator_*get_enable*()
These patches were previously part of the series:
https://lore.kernel.org/lkml/[email protected]/
"Devm helpers for regulator get and enable". I did keep the patch series versioning even though I changed the series name (subject of this mail) to "Use devm helpers for regulator get and enable". Name was changed because the devm helpers are already in 6.1-rc1.[...]
Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next)
[1/2] drm/bridge: sii902x: Use devm_regulator_bulk_get_enable()
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ff1eae1201a46f997126297d2d3440baa2d1b9a9 [2/2] drm/meson: dw-hdmi: Use devm_regulator_*get_enable*() https://cgit.freedesktop.org/drm/drm-misc/commit/?id=429e8706366166494314a46c82a9a9929aedbb8c
Signature · 1 line
Neil