Re: [PATCH v4 3/3] ata: ahci_tegra: Add AHCI support for Tegra186

[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.

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.