| Message ID | 20240730125023.710237-4-jbrunet@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Neil Armstrong |
| Headers | show |
| Series | drm/meson: dw-hdmi: clean-up | expand |
On Tue, Jul 30, 2024 at 2:50 PM Jerome Brunet <jbrunet@baylibre.com> wrote: > > The Amlogic HDMI phy driver is not doing anything with the clocks > besides enabling on probe. CCF provides generic helpers to do that. > > Use the generic clock helpers rather than using a custom one to get and > enable clocks. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> note to self: even if we need to manage one of the clocks specifically we can do it with clk_bulk_data
On Tue 06 Aug 2024 at 22:28, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > On Tue, Jul 30, 2024 at 2:50 PM Jerome Brunet <jbrunet@baylibre.com> wrote: >> >> The Amlogic HDMI phy driver is not doing anything with the clocks >> besides enabling on probe. CCF provides generic helpers to do that. >> >> Use the generic clock helpers rather than using a custom one to get and >> enable clocks. >> >> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > > note to self: even if we need to manage one of the clocks specifically > we can do it with clk_bulk_data Honestly I've gone for bulk variant only because calling devm_clk_get_enabled() 3 times in row and do nothing with the clocks looks odd. In I had to do something specific with a clock later on, personnaly I would back to plain 'struct clk' and use devm_clk_get_enabled()
On 30/07/2024 14:50, Jerome Brunet wrote: > The Amlogic HDMI phy driver is not doing anything with the clocks > besides enabling on probe. CCF provides generic helpers to do that. > > Use the generic clock helpers rather than using a custom one to get and > enable clocks. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > drivers/gpu/drm/meson/meson_dw_hdmi.c | 36 +++------------------------ > 1 file changed, 3 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c > index bcf4f83582f2..2890796f9d49 100644 > --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c > +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c > @@ -619,29 +619,6 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi) > > } > > -static void meson_disable_clk(void *data) > -{ > - clk_disable_unprepare(data); > -} > - > -static int meson_enable_clk(struct device *dev, char *name) > -{ > - struct clk *clk; > - int ret; > - > - clk = devm_clk_get(dev, name); > - if (IS_ERR(clk)) { > - dev_err(dev, "Unable to get %s pclk\n", name); > - return PTR_ERR(clk); > - } > - > - ret = clk_prepare_enable(clk); > - if (!ret) > - ret = devm_add_action_or_reset(dev, meson_disable_clk, clk); > - > - return ret; > -} > - > static int meson_dw_hdmi_bind(struct device *dev, struct device *master, > void *data) > { > @@ -651,6 +628,7 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master, > struct drm_device *drm = data; > struct meson_drm *priv = drm->dev_private; > struct dw_hdmi_plat_data *dw_plat_data; > + struct clk_bulk_data *clks; > int irq; > int ret; > > @@ -701,17 +679,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master, > if (IS_ERR(meson_dw_hdmi->hdmitx)) > return PTR_ERR(meson_dw_hdmi->hdmitx); > > - ret = meson_enable_clk(dev, "isfr"); > - if (ret) > - return ret; > - > - ret = meson_enable_clk(dev, "iahb"); > + ret = devm_clk_bulk_get_all_enable(dev, &clks); > if (ret) > - return ret; > - > - ret = meson_enable_clk(dev, "venci"); > - if (ret) > - return ret; > + return dev_err_probe(dev, ret, "Failed to enable all clocks\n"); > > dw_plat_data->regm = devm_regmap_init(dev, NULL, meson_dw_hdmi, > &meson_dw_hdmi_regmap_config); Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c index bcf4f83582f2..2890796f9d49 100644 --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c @@ -619,29 +619,6 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi) } -static void meson_disable_clk(void *data) -{ - clk_disable_unprepare(data); -} - -static int meson_enable_clk(struct device *dev, char *name) -{ - struct clk *clk; - int ret; - - clk = devm_clk_get(dev, name); - if (IS_ERR(clk)) { - dev_err(dev, "Unable to get %s pclk\n", name); - return PTR_ERR(clk); - } - - ret = clk_prepare_enable(clk); - if (!ret) - ret = devm_add_action_or_reset(dev, meson_disable_clk, clk); - - return ret; -} - static int meson_dw_hdmi_bind(struct device *dev, struct device *master, void *data) { @@ -651,6 +628,7 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master, struct drm_device *drm = data; struct meson_drm *priv = drm->dev_private; struct dw_hdmi_plat_data *dw_plat_data; + struct clk_bulk_data *clks; int irq; int ret; @@ -701,17 +679,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master, if (IS_ERR(meson_dw_hdmi->hdmitx)) return PTR_ERR(meson_dw_hdmi->hdmitx); - ret = meson_enable_clk(dev, "isfr"); - if (ret) - return ret; - - ret = meson_enable_clk(dev, "iahb"); + ret = devm_clk_bulk_get_all_enable(dev, &clks); if (ret) - return ret; - - ret = meson_enable_clk(dev, "venci"); - if (ret) - return ret; + return dev_err_probe(dev, ret, "Failed to enable all clocks\n"); dw_plat_data->regm = devm_regmap_init(dev, NULL, meson_dw_hdmi, &meson_dw_hdmi_regmap_config);
The Amlogic HDMI phy driver is not doing anything with the clocks besides enabling on probe. CCF provides generic helpers to do that. Use the generic clock helpers rather than using a custom one to get and enable clocks. Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/gpu/drm/meson/meson_dw_hdmi.c | 36 +++------------------------ 1 file changed, 3 insertions(+), 33 deletions(-)