Message ID | 1518456406-21564-6-git-send-email-pchandru@nvidia.com |
---|---|
State | Deferred |
Headers | show |
Series | Refactor and add AHCI support for tegra210 | expand |
On Mon, Feb 12, 2018 at 10:56:44PM +0530, Preetham Chandru Ramchandra wrote: > From: Preetham Ramchandra <pchandru@nvidia.com> > > t124 does not support devslp and it should be > disabled. Please spell out t124 as Tegra124 in the subject and the commit message. > > Signed-off-by: Preetham Chandru R <pchandru@nvidia.com> > --- > drivers/ata/ahci_tegra.c | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c > index 6aaf8a4571c8..62f2afb54789 100644 > --- a/drivers/ata/ahci_tegra.c > +++ b/drivers/ata/ahci_tegra.c > @@ -145,6 +145,10 @@ > #define FUSE_SATA_CALIB 0x124 > #define FUSE_SATA_CALIB_MASK 0x3 > > +enum { > + NO_DEVSLP = (1 << 0), > +}; > + > struct sata_pad_calibration { > u8 gen1_tx_amp; > u8 gen1_tx_peak; > @@ -166,12 +170,14 @@ struct tegra_ahci_ops { > struct tegra_ahci_soc { > const char *const *supply_names; > u32 num_supplies; > + u32 quirks; > struct tegra_ahci_ops ops; > }; I'd prefer these to be simple booleans, which make the code easier to read, in my opinion. This could be: bool supports_devslp; > > struct tegra_ahci_priv { > struct platform_device *pdev; > void __iomem *sata_regs; > + void __iomem *sata_aux_regs; > struct reset_control *sata_rst; > struct reset_control *sata_oob_rst; > struct reset_control *sata_cold_rst; > @@ -181,6 +187,18 @@ struct tegra_ahci_priv { > struct tegra_ahci_soc *soc_data; > }; > > +static void tegra_ahci_handle_quirks(struct ahci_host_priv *hpriv) > +{ > + struct tegra_ahci_priv *tegra = hpriv->plat_data; > + u32 val; > + > + if (tegra->sata_aux_regs && (tegra->soc_data->quirks & NO_DEVSLP)) { And then this becomes: if (tegra->sata_aux_regs && !tegra->soc->supports_devslp) > + val = readl(tegra->sata_aux_regs + SATA_AUX_MISC_CNTL_1_0); > + val &= ~SATA_AUX_MISC_CNTL_1_0_SDS_SUPPORT; > + writel(val, tegra->sata_aux_regs + SATA_AUX_MISC_CNTL_1_0); > + } > +} > + > static int tegra124_ahci_init(struct ahci_host_priv *hpriv) > { > struct tegra_ahci_priv *tegra = hpriv->plat_data; > @@ -400,6 +418,7 @@ static int tegra_ahci_controller_init(struct ahci_host_priv *hpriv) > val &= ~SATA_CONFIGURATION_0_CLK_OVERRIDE; > writel(val, tegra->sata_regs + SATA_CONFIGURATION_0); > > + tegra_ahci_handle_quirks(hpriv); > > /* Unmask SATA interrupts */ > > @@ -441,6 +460,7 @@ static const char *const tegra124_supply_names[] = { > static const struct tegra_ahci_soc tegra124_ahci_soc_data = { > .supply_names = tegra124_supply_names, > .num_supplies = ARRAY_SIZE(tegra124_supply_names), > + .quirks = NO_DEVSLP, > .ops = { > .init = tegra124_ahci_init, > }, > @@ -485,6 +505,15 @@ static int tegra_ahci_probe(struct platform_device *pdev) > tegra->sata_regs = devm_ioremap_resource(&pdev->dev, res); > if (IS_ERR(tegra->sata_regs)) > return PTR_ERR(tegra->sata_regs); > + res = platform_get_resource(pdev, IORESOURCE_MEM, 2); > + /* > + * Aux register is optional. > + */ "Aux register" -> "AUX registers". Also, I think it's more idiomatic to place the comment above platform_get_resource() so that the assignment and the check below are not separated. Thierry
diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c index 6aaf8a4571c8..62f2afb54789 100644 --- a/drivers/ata/ahci_tegra.c +++ b/drivers/ata/ahci_tegra.c @@ -145,6 +145,10 @@ #define FUSE_SATA_CALIB 0x124 #define FUSE_SATA_CALIB_MASK 0x3 +enum { + NO_DEVSLP = (1 << 0), +}; + struct sata_pad_calibration { u8 gen1_tx_amp; u8 gen1_tx_peak; @@ -166,12 +170,14 @@ struct tegra_ahci_ops { struct tegra_ahci_soc { const char *const *supply_names; u32 num_supplies; + u32 quirks; struct tegra_ahci_ops ops; }; struct tegra_ahci_priv { struct platform_device *pdev; void __iomem *sata_regs; + void __iomem *sata_aux_regs; struct reset_control *sata_rst; struct reset_control *sata_oob_rst; struct reset_control *sata_cold_rst; @@ -181,6 +187,18 @@ struct tegra_ahci_priv { struct tegra_ahci_soc *soc_data; }; +static void tegra_ahci_handle_quirks(struct ahci_host_priv *hpriv) +{ + struct tegra_ahci_priv *tegra = hpriv->plat_data; + u32 val; + + if (tegra->sata_aux_regs && (tegra->soc_data->quirks & NO_DEVSLP)) { + val = readl(tegra->sata_aux_regs + SATA_AUX_MISC_CNTL_1_0); + val &= ~SATA_AUX_MISC_CNTL_1_0_SDS_SUPPORT; + writel(val, tegra->sata_aux_regs + SATA_AUX_MISC_CNTL_1_0); + } +} + static int tegra124_ahci_init(struct ahci_host_priv *hpriv) { struct tegra_ahci_priv *tegra = hpriv->plat_data; @@ -400,6 +418,7 @@ static int tegra_ahci_controller_init(struct ahci_host_priv *hpriv) val &= ~SATA_CONFIGURATION_0_CLK_OVERRIDE; writel(val, tegra->sata_regs + SATA_CONFIGURATION_0); + tegra_ahci_handle_quirks(hpriv); /* Unmask SATA interrupts */ @@ -441,6 +460,7 @@ static const char *const tegra124_supply_names[] = { static const struct tegra_ahci_soc tegra124_ahci_soc_data = { .supply_names = tegra124_supply_names, .num_supplies = ARRAY_SIZE(tegra124_supply_names), + .quirks = NO_DEVSLP, .ops = { .init = tegra124_ahci_init, }, @@ -485,6 +505,15 @@ static int tegra_ahci_probe(struct platform_device *pdev) tegra->sata_regs = devm_ioremap_resource(&pdev->dev, res); if (IS_ERR(tegra->sata_regs)) return PTR_ERR(tegra->sata_regs); + res = platform_get_resource(pdev, IORESOURCE_MEM, 2); + /* + * Aux register is optional. + */ + if (res) { + tegra->sata_aux_regs = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(tegra->sata_aux_regs)) + return PTR_ERR(tegra->sata_aux_regs); + } tegra->sata_rst = devm_reset_control_get(&pdev->dev, "sata"); if (IS_ERR(tegra->sata_rst)) {