08.04.2021 02:00, Sowjanya Komatineni пишет: > > On 4/7/21 3:57 PM, Sowjanya Komatineni wrote: >> >> On 4/7/21 2:36 PM, Dmitry Osipenko wrote: >>> 07.04.2021 04:25, Sowjanya Komatineni пишет: >>>> + if (!tegra->pdev->dev.pm_domain) { >>>> + ret = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_SATA, >>>> + tegra->sata_clk, >>>> + tegra->sata_rst); >>>> + if (ret) >>>> + goto disable_regulators; >>>> + } >>> Hi, >>> >>> Why you haven't added condition for tegra_powergate_power_off()? I think >>> it should break GENPD and legacy PD API isn't not supported by T186 >>> at all. >>> >>> I'm also not sure whether the power up/down sequence is correct using >>> GENPD. >>> >>> Moreover the driver doesn't support runtime PM, so GENPD should be >>> always off? >> >> This driver already using legacy PD API's so thought its supported and >> added power domain device check during powergate_sequence_power_up and >> yes same should apply for powergate_power_off as well. But if legacy >> PD is not supported by T186 then not sure why original driver even >> using these API's. >> >> > Sorry just took a look and driver supports T210 and prior tegra as well. > T210 and prior supports legacy PD and this check is applicable for > those. So we should add power domain device check for power off as well. You could fix it with a follow up patch. Please try to test that power-off works properly, at least try to unload the driver module and re-load it. > But for T186, we should have GENPD working once we add runtime PM > support to driver. > > Preetham/Thierry, Can you confirm where SATA is un powergated prior to > kernel? > > >> But as RPM is not implemented yet for this driver, GENPD will be OFF >> but SATA is not in power-gate by the time kernel starts and >> functionally works. >> >> But with RPM implementation, I guess we can do proper power gate on/off. >> I now recalled that GENPD turns ON all domains by default and then turns them OFF only when driver entered into the RPM-suspended state. This means that AHCI GENPD should be always-ON for T186, which should be okay if this doesn't break power sequences.