lkml.org 
[lkml]   [2014]   [Jul]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 7/8] ata: Add support for the Tegra124 SATA controller
Hi,

Thanks for making the changes to use more libahci_platform
functionality. Overall this looks good, I've a few small remarks
inline.

On 07/16/2014 10:54 AM, Mikko Perttunen wrote:
> This adds support for the integrated AHCI-compliant Serial ATA
> controller present on the NVIDIA Tegra124 system-on-chip.
>
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> changes since v2:
> - remove fuse reading: now there is no dependency on the fuse series,
> so merging should be easier. always using the zeroth calibration
> values works for me anyway. the fuse code should be added back once
> fuses have landed, in 3.17-rc1 or 3.18.
> - put calibration value variables on their own lines.
> - back to using ahci_platform to manage some resources
> - code style fixes
>
> drivers/ata/Kconfig | 9 ++
> drivers/ata/Makefile | 1 +
> drivers/ata/ahci_tegra.c | 406 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 416 insertions(+)
> create mode 100644 drivers/ata/ahci_tegra.c
>
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index b0d5b5a..e1b9278 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -142,6 +142,15 @@ config AHCI_SUNXI
>
> If unsure, say N.
>
> +config AHCI_TEGRA
> + tristate "NVIDIA Tegra124 AHCI SATA support"
> + depends on ARCH_TEGRA
> + help
> + This option enables support for the NVIDIA Tegra124 SoC's
> + onboard AHCI SATA.
> +
> + If unsure, say N.
> +
> config AHCI_XGENE
> tristate "APM X-Gene 6.0Gbps AHCI SATA host controller support"
> depends on PHY_XGENE
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index 5a02aee..ae41107 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_AHCI_IMX) += ahci_imx.o libahci.o libahci_platform.o
> obj-$(CONFIG_AHCI_MVEBU) += ahci_mvebu.o libahci.o libahci_platform.o
> obj-$(CONFIG_AHCI_SUNXI) += ahci_sunxi.o libahci.o libahci_platform.o
> obj-$(CONFIG_AHCI_ST) += ahci_st.o libahci.o libahci_platform.o
> +obj-$(CONFIG_AHCI_TEGRA) += ahci_tegra.o libahci.o libahci_platform.o
> obj-$(CONFIG_AHCI_XGENE) += ahci_xgene.o libahci.o libahci_platform.o
>
> # SFF w/ custom DMA
> diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c
> new file mode 100644
> index 0000000..d90d08c
> --- /dev/null
> +++ b/drivers/ata/ahci_tegra.c
> @@ -0,0 +1,406 @@
> +/*
> + * drivers/ata/ahci_tegra.c
> + *
> + * Copyright (c) 2014, NVIDIA CORPORATION. All rights reserved.
> + *
> + * Author:
> + * Mikko Perttunen <mperttunen@nvidia.com>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/ahci_platform.h>
> +#include <linux/reset.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/tegra-powergate.h>
> +#include <linux/regulator/consumer.h>
> +#include "ahci.h"
> +
> +#define SATA_CONFIGURATION_0 0x180
> +#define SATA_CONFIGURATION_EN_FPCI BIT(0)
> +
> +#define SCFG_OFFSET 0x1000
> +
> +#define T_SATA0_CFG_1 0x04
> +#define T_SATA0_CFG_1_IO_SPACE BIT(0)
> +#define T_SATA0_CFG_1_MEMORY_SPACE BIT(1)
> +#define T_SATA0_CFG_1_BUS_MASTER BIT(2)
> +#define T_SATA0_CFG_1_SERR BIT(8)
> +
> +#define T_SATA0_CFG_9 0x24
> +#define T_SATA0_CFG_9_BASE_ADDRESS_SHIFT 13
> +
> +#define SATA_FPCI_BAR5 0x94
> +#define SATA_FPCI_BAR5_START_SHIFT 4
> +
> +#define SATA_INTR_MASK 0x188
> +#define SATA_INTR_MASK_IP_INT_MASK BIT(16)
> +
> +#define T_SATA0_AHCI_HBA_CAP_BKDR 0x300
> +
> +#define T_SATA0_BKDOOR_CC 0x4a4
> +
> +#define T_SATA0_CFG_SATA 0x54c
> +#define T_SATA0_CFG_SATA_BACKDOOR_PROG_IF_EN BIT(12)
> +
> +#define T_SATA0_CFG_MISC 0x550
> +
> +#define T_SATA0_INDEX 0x680
> +
> +#define T_SATA0_CHX_PHY_CTRL1_GEN1 0x690
> +#define T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_MASK 0xff
> +#define T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT 0
> +#define T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_MASK (0xff << 8)
> +#define T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT 8
> +
> +#define T_SATA0_CHX_PHY_CTRL1_GEN2 0x694
> +#define T_SATA0_CHX_PHY_CTRL1_GEN2_TX_AMP_MASK 0xff
> +#define T_SATA0_CHX_PHY_CTRL1_GEN2_TX_AMP_SHIFT 0
> +#define T_SATA0_CHX_PHY_CTRL1_GEN2_TX_PEAK_MASK (0xff << 12)
> +#define T_SATA0_CHX_PHY_CTRL1_GEN2_TX_PEAK_SHIFT 12
> +
> +#define T_SATA0_CHX_PHY_CTRL2 0x69c
> +#define T_SATA0_CHX_PHY_CTRL2_CDR_CNTL_GEN1 0x23
> +
> +#define T_SATA0_CHX_PHY_CTRL11 0x6d0
> +#define T_SATA0_CHX_PHY_CTRL11_GEN2_RX_EQ (0x2800 << 16)
> +
> +#define FUSE_SATA_CALIB 0x124
> +#define FUSE_SATA_CALIB_MASK 0x3
> +
> +enum sata_clks {
> + AHCI_CLK_SATA = 0,
> + AHCI_CLK_SATA_OOB = 1,
> + AHCI_CLK_CML1 = 2,
> + AHCI_CLK_PLL_E = 3
> +};

You should mention that having the clocks in this order is mandatory
in Documentation/devicetree/bindings/ata/tegra-sata.txt / in
"[PATCH v3 1/8] of: Add NVIDIA Tegra SATA controller binding"


> +
> +struct sata_pad_calibration {
> + u8 gen1_tx_amp;
> + u8 gen1_tx_peak;
> + u8 gen2_tx_amp;
> + u8 gen2_tx_peak;
> +};
> +
> +static const struct sata_pad_calibration tegra124_pad_calibration[] = {
> + {0x18, 0x04, 0x18, 0x0a},
> + {0x0e, 0x04, 0x14, 0x0a},
> + {0x0e, 0x07, 0x1a, 0x0e},
> + {0x14, 0x0e, 0x1a, 0x0e},
> +};
> +
> +struct tegra_ahci_priv {
> + struct platform_device *pdev;
> + void __iomem *sata_regs;
> + struct reset_control *sata_rst;
> + struct reset_control *sata_oob_rst;
> + struct reset_control *sata_cold_rst;
> + struct regulator_bulk_data supplies[5];
> +};
> +
> +static int tegra_ahci_power_on(struct ahci_host_priv *hpriv)
> +{
> + struct tegra_ahci_priv *tegra = hpriv->plat_data;
> + int ret;
> +
> + ret = regulator_bulk_enable(ARRAY_SIZE(tegra->supplies),
> + tegra->supplies);
> + if (ret)
> + return ret;
> +
> + ret = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_SATA,
> + hpriv->clks[AHCI_CLK_SATA],
> + tegra->sata_rst);
> + if (ret)
> + goto disable_regulators;
> +
> + reset_control_assert(tegra->sata_oob_rst);
> + reset_control_assert(tegra->sata_cold_rst);
> +
> + ret = clk_prepare_enable(hpriv->clks[AHCI_CLK_SATA_OOB]);
> + if (ret)
> + goto disable_power;
> +
> + ret = clk_prepare_enable(hpriv->clks[AHCI_CLK_CML1]);
> + if (ret)
> + goto disable_oob;
> +
> + ret = clk_prepare_enable(hpriv->clks[AHCI_CLK_PLL_E]);
> + if (ret)
> + goto disable_cml1;
> +
> + reset_control_deassert(tegra->sata_cold_rst);
> + reset_control_deassert(tegra->sata_oob_rst);
> +
> + ret = phy_power_on(hpriv->phy);
> + if (ret)
> + goto disable_plle;
> +
> + return 0;
> +
> +disable_plle:
> + clk_disable_unprepare(hpriv->clks[AHCI_CLK_PLL_E]);
> +
> +disable_cml1:
> + clk_disable_unprepare(hpriv->clks[AHCI_CLK_CML1]);
> +
> +disable_oob:
> + clk_disable_unprepare(hpriv->clks[AHCI_CLK_SATA_OOB]);
> +
> +disable_power:
> + clk_disable_unprepare(hpriv->clks[AHCI_CLK_SATA]);
> +
> + tegra_powergate_power_off(TEGRA_POWERGATE_SATA);
> +
> +disable_regulators:
> + regulator_bulk_disable(ARRAY_SIZE(tegra->supplies), tegra->supplies);
> +
> + return ret;
> +}
> +
> +static void tegra_ahci_power_off(struct ahci_host_priv *hpriv)
> +{
> + struct tegra_ahci_priv *tegra = hpriv->plat_data;
> +
> + ahci_platform_disable_resources(hpriv);

Disable resources does a phy_exit, unless this is a nop with your
phy driver, you must re-do the phy_init in tegra_ahci_power_on()
arguably the phy_init / phy_exit in libahci_platform should be
moved to hci_platform_get_resources and ahci_platform_put_resources,
if you decide to go for that please make sure you coordinate with
other libahci_platform users which use a phy (see the dts files).

> +
> + reset_control_assert(tegra->sata_rst);
> + reset_control_assert(tegra->sata_oob_rst);
> + reset_control_assert(tegra->sata_cold_rst);
> +
> + tegra_powergate_power_off(TEGRA_POWERGATE_SATA);
> +
> + regulator_bulk_disable(ARRAY_SIZE(tegra->supplies), tegra->supplies);
> +}
> +
> +static int tegra_ahci_controller_init(struct ahci_host_priv *hpriv)
> +{
> + struct tegra_ahci_priv *tegra = hpriv->plat_data;
> + int ret;
> + unsigned int val;
> + struct sata_pad_calibration calib;
> +
> + ret = tegra_ahci_power_on(hpriv);
> + if (ret) {
> + dev_err(&tegra->pdev->dev,
> + "failed to power on AHCI controller: %d\n", ret);
> + return ret;
> + }
> +
> + val = readl(tegra->sata_regs + SATA_CONFIGURATION_0);
> + val |= SATA_CONFIGURATION_EN_FPCI;
> + writel(val, tegra->sata_regs + SATA_CONFIGURATION_0);
> +
> + /* Pad calibration */
> +
> + /* FIXME Always use calibration 0. Change this to read the calibration
> + * fuse once the fuse driver has landed. */
> + val = 0;
> +
> + calib = tegra124_pad_calibration[val & FUSE_SATA_CALIB_MASK];
> +
> + writel(BIT(0), tegra->sata_regs + SCFG_OFFSET + T_SATA0_INDEX);
> +
> + val = readl(tegra->sata_regs +
> + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL1_GEN1);
> + val &= ~T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_MASK;
> + val &= ~T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_MASK;
> + val |= calib.gen1_tx_amp <<
> + T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT;
> + val |= calib.gen1_tx_peak <<
> + T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT;
> + writel(val, tegra->sata_regs + SCFG_OFFSET +
> + T_SATA0_CHX_PHY_CTRL1_GEN1);
> +
> + val = readl(tegra->sata_regs +
> + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL1_GEN2);
> + val &= ~T_SATA0_CHX_PHY_CTRL1_GEN2_TX_AMP_MASK;
> + val &= ~T_SATA0_CHX_PHY_CTRL1_GEN2_TX_PEAK_MASK;
> + val |= calib.gen2_tx_amp <<
> + T_SATA0_CHX_PHY_CTRL1_GEN1_TX_AMP_SHIFT;
> + val |= calib.gen2_tx_peak <<
> + T_SATA0_CHX_PHY_CTRL1_GEN1_TX_PEAK_SHIFT;
> + writel(val, tegra->sata_regs + SCFG_OFFSET +
> + T_SATA0_CHX_PHY_CTRL1_GEN2);
> +
> + writel(T_SATA0_CHX_PHY_CTRL11_GEN2_RX_EQ,
> + tegra->sata_regs + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL11);
> + writel(T_SATA0_CHX_PHY_CTRL2_CDR_CNTL_GEN1,
> + tegra->sata_regs + SCFG_OFFSET + T_SATA0_CHX_PHY_CTRL2);
> +
> + writel(0, tegra->sata_regs + SCFG_OFFSET + T_SATA0_INDEX);
> +
> + /* Program controller device ID */
> +
> + val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
> + val |= T_SATA0_CFG_SATA_BACKDOOR_PROG_IF_EN;
> + writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
> +
> + writel(0x01060100, tegra->sata_regs + SCFG_OFFSET + T_SATA0_BKDOOR_CC);
> +
> + val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
> + val &= ~T_SATA0_CFG_SATA_BACKDOOR_PROG_IF_EN;
> + writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_SATA);
> +
> + /* Enable IO & memory access, bus master mode */
> +
> + val = readl(tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_1);
> + val |= T_SATA0_CFG_1_IO_SPACE | T_SATA0_CFG_1_MEMORY_SPACE |
> + T_SATA0_CFG_1_BUS_MASTER | T_SATA0_CFG_1_SERR;
> + writel(val, tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_1);
> +
> + /* Program SATA MMIO */
> +
> + writel(0x10000 << SATA_FPCI_BAR5_START_SHIFT,
> + tegra->sata_regs + SATA_FPCI_BAR5);
> +
> + writel(0x08000 << T_SATA0_CFG_9_BASE_ADDRESS_SHIFT,
> + tegra->sata_regs + SCFG_OFFSET + T_SATA0_CFG_9);
> +
> + /* Unmask SATA interrupts */
> +
> + val = readl(tegra->sata_regs + SATA_INTR_MASK);
> + val |= SATA_INTR_MASK_IP_INT_MASK;
> + writel(val, tegra->sata_regs + SATA_INTR_MASK);
> +
> + return 0;
> +}
> +
> +static void tegra_ahci_controller_deinit(struct ahci_host_priv *hpriv)
> +{
> + tegra_ahci_power_off(hpriv);
> +}
> +
> +static void tegra_ahci_host_stop(struct ata_host *host)
> +{
> + struct ahci_host_priv *hpriv = host->private_data;
> +
> + tegra_ahci_controller_deinit(hpriv);
> +}
> +
> +static struct ata_port_operations ahci_tegra_port_ops = {
> + .inherits = &ahci_ops,
> + .host_stop = tegra_ahci_host_stop,
> +};
> +
> +static const struct ata_port_info ahci_tegra_port_info = {
> + .flags = AHCI_FLAG_COMMON,
> + .pio_mask = ATA_PIO4,
> + .udma_mask = ATA_UDMA6,
> + .port_ops = &ahci_tegra_port_ops,
> +};
> +
> +static const struct of_device_id tegra_ahci_of_match[] = {
> + { .compatible = "nvidia,tegra124-ahci" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, tegra_ahci_of_match);
> +
> +static int tegra_ahci_probe(struct platform_device *pdev)
> +{
> + struct ahci_host_priv *hpriv;
> + struct tegra_ahci_priv *tegra;
> + struct resource *res;
> + int ret;
> +
> + hpriv = ahci_platform_get_resources(pdev);
> + if (IS_ERR(hpriv))
> + return PTR_ERR(hpriv);
> +
> + tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
> + if (!tegra)
> + return -ENOMEM;
> +
> + hpriv->plat_data = tegra;
> +
> + tegra->pdev = pdev;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + tegra->sata_regs = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(tegra->sata_regs))
> + return PTR_ERR(tegra->sata_regs);
> +
> + tegra->sata_rst = devm_reset_control_get(&pdev->dev, "sata");
> + if (IS_ERR(tegra->sata_rst)) {
> + dev_err(&pdev->dev, "Failed to get sata reset\n");
> + return PTR_ERR(tegra->sata_rst);
> + }
> +
> + tegra->sata_oob_rst = devm_reset_control_get(&pdev->dev, "sata-oob");
> + if (IS_ERR(tegra->sata_oob_rst)) {
> + dev_err(&pdev->dev, "Failed to get sata-oob reset\n");
> + return PTR_ERR(tegra->sata_oob_rst);
> + }
> +
> + tegra->sata_cold_rst = devm_reset_control_get(&pdev->dev, "sata-cold");
> + if (IS_ERR(tegra->sata_cold_rst)) {
> + dev_err(&pdev->dev, "Failed to get sata-cold reset\n");
> + return PTR_ERR(tegra->sata_cold_rst);
> + }
> +
> + if (!hpriv->clks[AHCI_CLK_PLL_E])
> + return -EPROBE_DEFER;
> +
> + tegra->supplies[0].supply = "avdd";
> + tegra->supplies[1].supply = "hvdd";
> + tegra->supplies[2].supply = "vddio";
> + tegra->supplies[3].supply = "target-5v";
> + tegra->supplies[4].supply = "target-12v";
> +
> + ret = devm_regulator_bulk_get(&pdev->dev, ARRAY_SIZE(tegra->supplies),
> + tegra->supplies);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to get regulators\n");
> + return ret;
> + }
> +
> + ret = phy_init(hpriv->phy);
> + if (ret)
> + return ret;
> +
> + ret = tegra_ahci_controller_init(hpriv);
> + if (ret)
> + goto exit_phy;
> +
> + ret = ahci_platform_init_host(pdev, hpriv, &ahci_tegra_port_info,
> + 0, 0, 0);
> + if (ret)
> + goto deinit_controller;
> +
> + return 0;
> +
> +deinit_controller:
> + tegra_ahci_controller_deinit(hpriv);
> +
> +exit_phy:
> + phy_exit(hpriv->phy);
> +
> + return ret;
> +};
> +
> +static struct platform_driver tegra_ahci_driver = {
> + .probe = tegra_ahci_probe,
> + .remove = ata_platform_remove_one,
> + .driver = {
> + .name = "tegra-ahci",
> + .of_match_table = tegra_ahci_of_match,
> + },
> + /* LP0 suspend support not implemented */
> +};
> +module_platform_driver(tegra_ahci_driver);
> +
> +MODULE_AUTHOR("Mikko Perttunen <mperttunen@nvidia.com>");
> +MODULE_DESCRIPTION("Tegra124 AHCI SATA driver");
> +MODULE_LICENSE("GPL v2");
>

Thanks & Regards,

Hans


\
 
 \ /
  Last update: 2014-07-16 14:01    [W:0.267 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site