Hi Fabio,
First, sorry for the delay and thanks a lot for the review.
On 12/09/2024 at 09:40:32 -03, Fabio Estevam festevam@gmail.com wrote:
On Tue, Sep 10, 2024 at 7:21 AM Miquel Raynal miquel.raynal@bootlin.com wrote:
/* Make sure bus domain is awake */ret = power_domain_on(&priv->pd_bus);if (ret)return ret;/* Put devices into reset */clrbits_le32(priv->base + BLK_SFT_RSTN, reset);/* Enable upstream clocks */ret = clk_enable(&priv->clk_apb);if (ret)goto dis_bus_pd;ret = clk_enable(&priv->clk_axi);if (ret)goto dis_bus_pd;/* Enable blk-ctrl clock to allow reset to propagate */ret = clk_enable(clk);if (ret)goto dis_bus_pd;setbits_le32(priv->base + BLK_CLK_EN, reset);/* Power up upstream GPC domain */ret = power_domain_on(domain);if (ret)goto dis_bus_pd;The previously enabled clocks should be disabled on the error paths.
That's right, I will.
Could you use clk_enable_bulk()?
This not very convenient and not future-proof, because, while we just want to grab and enable the apb and axi clocks, the media_disp1_pix and media_disp2_pix will be used depending on the configuration. For not I've not included support for media_disp1_pix because I do not use it and cannot test it, but it is a legitimate addition that someone will soon or later want to bring. Hence I believe using the _bulk() helper is not really appropriate in this case.
Thanks! Miquèl