Re: [PATCH 4/5] video: sunxi: dw-hdmi: Use DM for clock gates and resets

[Andre Przywara <andre.przywara@arm.com>]

On Mon, 28 Nov 2022 01:02:27 -0600
Samuel Holland <samuel@sholland.org> wrote:

Hi,

> This abstracts away the CCU register layout, which is necessary for
> supporting new SoCs like H6 with a reorganized CCU. One of the resets is
> referenced from the PHY node instead of the controller node, so it will
> have to wait until the PHY code is factored out to a separate driver.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
>  drivers/video/sunxi/sunxi_dw_hdmi.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/video/sunxi/sunxi_dw_hdmi.c b/drivers/video/sunxi/sunxi_dw_hdmi.c
> index 4f5d0989286..04588b570fd 100644
> --- a/drivers/video/sunxi/sunxi_dw_hdmi.c
> +++ b/drivers/video/sunxi/sunxi_dw_hdmi.c
> @@ -5,12 +5,14 @@
>   * (C) Copyright 2017 Jernej Skrabec <jernej.skrabec@siol.net>
>   */
>  
> +#include <clk.h>
>  #include <common.h>
>  #include <display.h>
>  #include <dm.h>
>  #include <dw_hdmi.h>
>  #include <edid.h>
>  #include <log.h>
> +#include <reset.h>
>  #include <time.h>
>  #include <asm/io.h>
>  #include <asm/arch/clock.h>
> @@ -327,6 +329,8 @@ static int sunxi_dw_hdmi_probe(struct udevice *dev)
>  	struct sunxi_dw_hdmi_priv *priv = dev_get_priv(dev);
>  	struct sunxi_ccm_reg * const ccm =
>  		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> +	struct reset_ctl_bulk resets;
> +	struct clk_bulk clocks;
>  	int ret;
>  
>  	/* Set pll3 to 297 MHz */
> @@ -336,14 +340,24 @@ static int sunxi_dw_hdmi_probe(struct udevice *dev)
>  	clrsetbits_le32(&ccm->hdmi_clk_cfg, CCM_HDMI_CTRL_PLL_MASK,
>  			CCM_HDMI_CTRL_PLL3);
>  
> -	/* Set ahb gating to pass */
> -	setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_HDMI);
> +	/* This reset is referenced from the PHY devicetree node. */
>  	setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_HDMI2);
> -	setbits_le32(&ccm->ahb_gate1, 1 << AHB_GATE_OFFSET_HDMI);
> -	setbits_le32(&ccm->hdmi_slow_clk_cfg, CCM_HDMI_SLOW_CTRL_DDC_GATE);
>  
> -	/* Clock on */
> -	setbits_le32(&ccm->hdmi_clk_cfg, CCM_HDMI_CTRL_GATE);
> +	ret = reset_get_bulk(dev, &resets);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_get_bulk(dev, &clocks);
> +	if (ret)
> +		return ret;

I understand that it complicates thing a bit, for little benefit, but
shouldn't those two get operations be done in of_to_plat()?

Cheers,
Andre

> +
> +	ret = reset_deassert_bulk(&resets);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_enable_bulk(&clocks);
> +	if (ret)
> +		return ret;
>  
>  	sunxi_dw_hdmi_phy_init(&priv->hdmi);
>