Re: [RESEND2,v4,2/2] drm/meson: dw-hdmi: Use devm_regulator_*get_enable*()

2023-01-09 Thread Neil Armstrong 

On 09/01/2023 14:46, Marek Szyprowski wrote:

Hi Matti,

On 30.11.2022 10:23, Matti Vaittinen wrote:

Simplify using the devm_regulator_get_enable_optional(). Also drop the
now unused struct member 'hdmi_supply'.

Signed-off-by: Matti Vaittinen 
Martin Blumenstingl 
---
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(-)

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)


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?


Nop, please send an incremental fix and I'll apply it asap.

Thanks,
Neil





+   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*()

2023-01-09 Thread Marek Szyprowski 
Hi Matti,

On 30.11.2022 10:23, Matti Vaittinen wrote:
> Simplify using the devm_regulator_get_enable_optional(). Also drop the
> now unused struct member 'hdmi_supply'.
>
> Signed-off-by: Matti Vaittinen 
> Martin Blumenstingl 
> ---
> 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(-)
>
> 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)

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?

> + return ret;
>   
>   meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev,
>   "hdmitx_apb");

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland