* Tomi Valkeinen tomi.valkeinen@ti.com [190206 16:09]:
On 06/02/2019 18:00, Tony Lindgren wrote:
OK I'll give it a try. Based on a quick glance, we need to still check for enabled regulator to avoid unpaired calls.
static int dsi_dump_dsi_clocks(struct seq_file *s, void *p) @@ -4108,6 +4094,10 @@ static int dsi_display_init_dsi(struct dsi_data *dsi)
DSSDBG("PLL OK\n");
- // XXX enable the regulator for the lanes
- regulator_enable(dsi->vdds_dsi_reg);
- dsi->vdds_dsi_enabled = true;
So the above should only be done if !dsi->vdds_dsi_enabled?
Actually the conditional handling is only needed above. And the regulator_enable needs error handling added that causes some renumbering of the exit path.
r = dsi_cio_init(dsi); if (r) goto err2; @@ -4136,6 +4126,10 @@ static int dsi_display_init_dsi(struct dsi_data *dsi) err3: dsi_cio_uninit(dsi); err2:
- // XXX disable the regulator for the lanes
- regulator_disable(dsi->vdds_dsi_reg);
- dsi->vdds_dsi_enabled = false;
And here only if dsi->vdds_dsi_enabled?
On errors here we should just shut it down.
@@ -4158,7 +4152,12 @@ static void dsi_display_uninit_dsi(struct dsi_data *dsi, bool disconnect_lanes,
dss_select_dsi_clk_source(dsi->dss, dsi->module_id, DSS_CLK_SRC_FCK); dsi_cio_uninit(dsi);
- dsi_pll_uninit(dsi, disconnect_lanes);
- dss_pll_disable(&dsi->pll);
- if (disconnect_lanes) {
regulator_disable(dsi->vdds_dsi_reg);dsi->vdds_dsi_enabled = false;- }
}
Since they would be paired with the conditional handling here?
Hmm, yes, I think you're right. And there's already one in dsi_remove(), which handles the final disable at unload time.
OK. Looks good to me otherwise.
So I guess we should fix. Do you want me to post it all as a single patch after some testing?
It seems that we'll be breaking things one way or another trying to do it in two patches :)
Regards,
Tony