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? > >> 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? > >> @@ -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. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel