| Message ID | 20220928125648.19636-2-pshete@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v6,1/4] mmc: sdhci-tegra: Separate Tegra194 and Tegra234 SoC data | expand |
On 28/09/22 15:56, Prathamesh Shete wrote: > SMMU clients are supposed to program stream ID from > their respective address spaces instead of MC override. > Define NVQUIRK_PROGRAM_STREAMID and use it to program > SMMU stream ID from the SDMMC client address space. > > Signed-off-by: Aniruddha TVS Rao <anrao@nvidia.com> > Signed-off-by: Prathamesh Shete <pshete@nvidia.com> One observation below. Nevertheless: Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/host/sdhci-tegra.c | 44 ++++++++++++++++++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c > index a6c5bbae77b4..60ce3e80f248 100644 > --- a/drivers/mmc/host/sdhci-tegra.c > +++ b/drivers/mmc/host/sdhci-tegra.c > @@ -25,6 +25,10 @@ > #include <linux/mmc/slot-gpio.h> > #include <linux/gpio/consumer.h> > #include <linux/ktime.h> > +#ifdef CONFIG_IOMMU_API > +#include <linux/iommu.h> > +#include <linux/bitops.h> > +#endif > > #include <soc/tegra/common.h> > > @@ -94,6 +98,8 @@ > #define SDHCI_TEGRA_AUTO_CAL_STATUS 0x1ec > #define SDHCI_TEGRA_AUTO_CAL_ACTIVE BIT(31) > > +#define SDHCI_TEGRA_CIF2AXI_CTRL_0 0x1fc > + > #define NVQUIRK_FORCE_SDHCI_SPEC_200 BIT(0) > #define NVQUIRK_ENABLE_BLOCK_GAP_DET BIT(1) > #define NVQUIRK_ENABLE_SDHCI_SPEC_300 BIT(2) > @@ -121,6 +127,7 @@ > #define NVQUIRK_HAS_TMCLK BIT(10) > > #define NVQUIRK_HAS_ANDROID_GPT_SECTOR BIT(11) > +#define NVQUIRK_PROGRAM_STREAMID BIT(12) > > /* SDMMC CQE Base Address for Tegra Host Ver 4.1 and Higher */ > #define SDHCI_TEGRA_CQE_BASE_ADDR 0xF000 > @@ -177,6 +184,9 @@ struct sdhci_tegra { > bool enable_hwcq; > unsigned long curr_clk_rate; > u8 tuned_tap_delay; > +#ifdef CONFIG_IOMMU_API > + u32 streamid; > +#endif > }; > > static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg) > @@ -1564,6 +1574,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra234 = { > NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | > NVQUIRK_ENABLE_SDR50 | > NVQUIRK_ENABLE_SDR104 | > + NVQUIRK_PROGRAM_STREAMID | > NVQUIRK_HAS_TMCLK, > .min_tap_delay = 95, > .max_tap_delay = 111, > @@ -1775,6 +1786,25 @@ static int sdhci_tegra_probe(struct platform_device *pdev) > if (rc) > goto err_add_host; > > + /* Program MC streamID for DMA transfers */ > +#ifdef CONFIG_IOMMU_API > + if (soc_data->nvquirks & NVQUIRK_PROGRAM_STREAMID) { > + struct iommu_fwspec *fwspec; > + > + fwspec = dev_iommu_fwspec_get(&pdev->dev); > + if (fwspec == NULL) { > + dev_warn(mmc_dev(host->mmc), > + "iommu fwspec is NULL, continue without stream ID\n"); It will still program a zero streamid upon resume. > + } else { > + tegra_host->streamid = fwspec->ids[0] & 0xff; > + tegra_sdhci_writel(host, tegra_host->streamid | > + FIELD_PREP(GENMASK(15, 8), > + tegra_host->streamid), > + SDHCI_TEGRA_CIF2AXI_CTRL_0); > + } > + } > +#endif > + > return 0; > > err_add_host: > @@ -1861,6 +1891,10 @@ static int sdhci_tegra_suspend(struct device *dev) > static int sdhci_tegra_resume(struct device *dev) > { > struct sdhci_host *host = dev_get_drvdata(dev); > +#ifdef CONFIG_IOMMU_API > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host); > +#endif > int ret; > > ret = mmc_gpio_set_cd_wake(host->mmc, false); > @@ -1871,6 +1905,16 @@ static int sdhci_tegra_resume(struct device *dev) > if (ret) > return ret; > > + /* Re-program MC streamID for DMA transfers */ > +#ifdef CONFIG_IOMMU_API > + if (tegra_host->soc_data->nvquirks & NVQUIRK_PROGRAM_STREAMID) { > + tegra_sdhci_writel(host, tegra_host->streamid | > + FIELD_PREP(GENMASK(15, 8), > + tegra_host->streamid), > + SDHCI_TEGRA_CIF2AXI_CTRL_0); > + } > +#endif > + > ret = sdhci_resume_host(host); > if (ret) > goto disable_clk;
On Wed, Sep 28, 2022 at 05:18:19PM +0300, Adrian Hunter wrote: > On 28/09/22 15:56, Prathamesh Shete wrote: > > SMMU clients are supposed to program stream ID from > > their respective address spaces instead of MC override. > > Define NVQUIRK_PROGRAM_STREAMID and use it to program > > SMMU stream ID from the SDMMC client address space. > > > > Signed-off-by: Aniruddha TVS Rao <anrao@nvidia.com> > > Signed-off-by: Prathamesh Shete <pshete@nvidia.com> > > One observation below. > > Nevertheless: > > Acked-by: Adrian Hunter <adrian.hunter@intel.com> > > > --- > > drivers/mmc/host/sdhci-tegra.c | 44 ++++++++++++++++++++++++++++++++++ > > 1 file changed, 44 insertions(+) > > > > diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c > > index a6c5bbae77b4..60ce3e80f248 100644 > > --- a/drivers/mmc/host/sdhci-tegra.c > > +++ b/drivers/mmc/host/sdhci-tegra.c > > @@ -25,6 +25,10 @@ > > #include <linux/mmc/slot-gpio.h> > > #include <linux/gpio/consumer.h> > > #include <linux/ktime.h> > > +#ifdef CONFIG_IOMMU_API > > +#include <linux/iommu.h> > > +#include <linux/bitops.h> > > +#endif > > > > #include <soc/tegra/common.h> > > > > @@ -94,6 +98,8 @@ > > #define SDHCI_TEGRA_AUTO_CAL_STATUS 0x1ec > > #define SDHCI_TEGRA_AUTO_CAL_ACTIVE BIT(31) > > > > +#define SDHCI_TEGRA_CIF2AXI_CTRL_0 0x1fc > > + > > #define NVQUIRK_FORCE_SDHCI_SPEC_200 BIT(0) > > #define NVQUIRK_ENABLE_BLOCK_GAP_DET BIT(1) > > #define NVQUIRK_ENABLE_SDHCI_SPEC_300 BIT(2) > > @@ -121,6 +127,7 @@ > > #define NVQUIRK_HAS_TMCLK BIT(10) > > > > #define NVQUIRK_HAS_ANDROID_GPT_SECTOR BIT(11) > > +#define NVQUIRK_PROGRAM_STREAMID BIT(12) > > > > /* SDMMC CQE Base Address for Tegra Host Ver 4.1 and Higher */ > > #define SDHCI_TEGRA_CQE_BASE_ADDR 0xF000 > > @@ -177,6 +184,9 @@ struct sdhci_tegra { > > bool enable_hwcq; > > unsigned long curr_clk_rate; > > u8 tuned_tap_delay; > > +#ifdef CONFIG_IOMMU_API > > + u32 streamid; > > +#endif > > }; > > > > static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg) > > @@ -1564,6 +1574,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra234 = { > > NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | > > NVQUIRK_ENABLE_SDR50 | > > NVQUIRK_ENABLE_SDR104 | > > + NVQUIRK_PROGRAM_STREAMID | > > NVQUIRK_HAS_TMCLK, > > .min_tap_delay = 95, > > .max_tap_delay = 111, > > @@ -1775,6 +1786,25 @@ static int sdhci_tegra_probe(struct platform_device *pdev) > > if (rc) > > goto err_add_host; > > > > + /* Program MC streamID for DMA transfers */ > > +#ifdef CONFIG_IOMMU_API > > + if (soc_data->nvquirks & NVQUIRK_PROGRAM_STREAMID) { > > + struct iommu_fwspec *fwspec; > > + > > + fwspec = dev_iommu_fwspec_get(&pdev->dev); > > + if (fwspec == NULL) { > > + dev_warn(mmc_dev(host->mmc), > > + "iommu fwspec is NULL, continue without stream ID\n"); > > It will still program a zero streamid upon resume. This was confusing to me, but I think what you mean is that the check for fwspec == NULL should also be done in sdhci_tegra_resume(). > > > + } else { > > + tegra_host->streamid = fwspec->ids[0] & 0xff; > > + tegra_sdhci_writel(host, tegra_host->streamid | > > + FIELD_PREP(GENMASK(15, 8), > > + tegra_host->streamid), > > + SDHCI_TEGRA_CIF2AXI_CTRL_0); > > + } > > + } > > +#endif > > + > > return 0; > > > > err_add_host: > > @@ -1861,6 +1891,10 @@ static int sdhci_tegra_suspend(struct device *dev) > > static int sdhci_tegra_resume(struct device *dev) > > { > > struct sdhci_host *host = dev_get_drvdata(dev); > > +#ifdef CONFIG_IOMMU_API > > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > > + struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host); > > +#endif > > int ret; > > > > ret = mmc_gpio_set_cd_wake(host->mmc, false); > > @@ -1871,6 +1905,16 @@ static int sdhci_tegra_resume(struct device *dev) > > if (ret) > > return ret; > > > > + /* Re-program MC streamID for DMA transfers */ > > +#ifdef CONFIG_IOMMU_API > > + if (tegra_host->soc_data->nvquirks & NVQUIRK_PROGRAM_STREAMID) { > > + tegra_sdhci_writel(host, tegra_host->streamid | > > + FIELD_PREP(GENMASK(15, 8), > > + tegra_host->streamid), > > + SDHCI_TEGRA_CIF2AXI_CTRL_0); > > + } > > +#endif So this here should be exactly the same as in sdhci_tegra_probe(). Which would be an argument for putting this into a separate function. That said, I'm not sure if this matters at all. If fwspec == NULL, that implies that device tree didn't have an "iommus" property, which also implies that SMMU translations for this device will not be enabled at all, if I recall correctly, so whether this CIF2AXI_CTRL register is programmed or not in that case should be irrelevant. Still, might be good to keep this the same in both sdhci_tegra_probe() and sdhci_tegra_resume() for consistency. With that fixed, the series: Acked-by: Thierry Reding <treding@nvidia.com>
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c index a6c5bbae77b4..60ce3e80f248 100644 --- a/drivers/mmc/host/sdhci-tegra.c +++ b/drivers/mmc/host/sdhci-tegra.c @@ -25,6 +25,10 @@ #include <linux/mmc/slot-gpio.h> #include <linux/gpio/consumer.h> #include <linux/ktime.h> +#ifdef CONFIG_IOMMU_API +#include <linux/iommu.h> +#include <linux/bitops.h> +#endif #include <soc/tegra/common.h> @@ -94,6 +98,8 @@ #define SDHCI_TEGRA_AUTO_CAL_STATUS 0x1ec #define SDHCI_TEGRA_AUTO_CAL_ACTIVE BIT(31) +#define SDHCI_TEGRA_CIF2AXI_CTRL_0 0x1fc + #define NVQUIRK_FORCE_SDHCI_SPEC_200 BIT(0) #define NVQUIRK_ENABLE_BLOCK_GAP_DET BIT(1) #define NVQUIRK_ENABLE_SDHCI_SPEC_300 BIT(2) @@ -121,6 +127,7 @@ #define NVQUIRK_HAS_TMCLK BIT(10) #define NVQUIRK_HAS_ANDROID_GPT_SECTOR BIT(11) +#define NVQUIRK_PROGRAM_STREAMID BIT(12) /* SDMMC CQE Base Address for Tegra Host Ver 4.1 and Higher */ #define SDHCI_TEGRA_CQE_BASE_ADDR 0xF000 @@ -177,6 +184,9 @@ struct sdhci_tegra { bool enable_hwcq; unsigned long curr_clk_rate; u8 tuned_tap_delay; +#ifdef CONFIG_IOMMU_API + u32 streamid; +#endif }; static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg) @@ -1564,6 +1574,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra234 = { NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 | NVQUIRK_ENABLE_SDR104 | + NVQUIRK_PROGRAM_STREAMID | NVQUIRK_HAS_TMCLK, .min_tap_delay = 95, .max_tap_delay = 111, @@ -1775,6 +1786,25 @@ static int sdhci_tegra_probe(struct platform_device *pdev) if (rc) goto err_add_host; + /* Program MC streamID for DMA transfers */ +#ifdef CONFIG_IOMMU_API + if (soc_data->nvquirks & NVQUIRK_PROGRAM_STREAMID) { + struct iommu_fwspec *fwspec; + + fwspec = dev_iommu_fwspec_get(&pdev->dev); + if (fwspec == NULL) { + dev_warn(mmc_dev(host->mmc), + "iommu fwspec is NULL, continue without stream ID\n"); + } else { + tegra_host->streamid = fwspec->ids[0] & 0xff; + tegra_sdhci_writel(host, tegra_host->streamid | + FIELD_PREP(GENMASK(15, 8), + tegra_host->streamid), + SDHCI_TEGRA_CIF2AXI_CTRL_0); + } + } +#endif + return 0; err_add_host: @@ -1861,6 +1891,10 @@ static int sdhci_tegra_suspend(struct device *dev) static int sdhci_tegra_resume(struct device *dev) { struct sdhci_host *host = dev_get_drvdata(dev); +#ifdef CONFIG_IOMMU_API + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); + struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host); +#endif int ret; ret = mmc_gpio_set_cd_wake(host->mmc, false); @@ -1871,6 +1905,16 @@ static int sdhci_tegra_resume(struct device *dev) if (ret) return ret; + /* Re-program MC streamID for DMA transfers */ +#ifdef CONFIG_IOMMU_API + if (tegra_host->soc_data->nvquirks & NVQUIRK_PROGRAM_STREAMID) { + tegra_sdhci_writel(host, tegra_host->streamid | + FIELD_PREP(GENMASK(15, 8), + tegra_host->streamid), + SDHCI_TEGRA_CIF2AXI_CTRL_0); + } +#endif + ret = sdhci_resume_host(host); if (ret) goto disable_clk;