RE: [PATCH v7 2/4] mmc: sdhci-tegra: Add support to program MC stream ID
From: Prathamesh Shete
Date: Tue Oct 11 2022 - 07:46:01 EST
Hi Ulf
The initial patches were without the #ifdef. #ifdef is being added as per review comments and kernel robot errors.
Following error was detected by kernel robot
>>
All errors (new ones prefixed by >>):
drivers/mmc/host/sdhci-tegra.c: In function 'sdhci_tegra_probe':
>> drivers/mmc/host/sdhci-tegra.c:1794:54: error: 'struct iommu_fwspec' has no member named 'ids'
1794 | tegra_host->streamid = fwspec->ids[0] & 0xffff;
| ^~
vim +1794 drivers/mmc/host/sdhci-tegra.c
>>
Adrian also pointed out this issue so to address these issues #ifdef was added
Thanks
Prathamesh
> -----Original Message-----
> From: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> Sent: Tuesday, October 11, 2022 3:29 PM
> To: Prathamesh Shete <pshete@xxxxxxxxxx>
> Cc: adrian.hunter@xxxxxxxxx; thierry.reding@xxxxxxxxx; Jonathan Hunter
> <jonathanh@xxxxxxxxxx>; p.zabel@xxxxxxxxxxxxxx; linux-
> mmc@xxxxxxxxxxxxxxx; linux-tegra@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Aniruddha Tvs Rao <anrao@xxxxxxxxxx>; Suresh
> Mangipudi <smangipudi@xxxxxxxxxx>; Krishna Yarlagadda
> <kyarlagadda@xxxxxxxxxx>
> Subject: Re: [PATCH v7 2/4] mmc: sdhci-tegra: Add support to program MC
> stream ID
>
> External email: Use caution opening links or attachments
>
>
> On Thu, 6 Oct 2022 at 15:07, Prathamesh Shete <pshete@xxxxxxxxxx> 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@xxxxxxxxxx>
> > Signed-off-by: Prathamesh Shete <pshete@xxxxxxxxxx>
> > Acked-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> > Acked-by: Thierry Reding <treding@xxxxxxxxxx>
> > ---
> > drivers/mmc/host/sdhci-tegra.c | 53
> > ++++++++++++++++++++++++++++++++++
> > 1 file changed, 53 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-tegra.c
> > b/drivers/mmc/host/sdhci-tegra.c index a6c5bbae77b4..e88294a6912d
> > 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
>
> I would rather avoid these kinds of "#ifdef" micro optimizations.
> Please just add the streamid without the #ifdef.
>
> > + 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
>
> Again, please drop the #ifdefs here.
>
> We already have stub functions for dev_iommu_fwspec_get() in case
> CONFIG_IOMMU_API is unset.
>
> > + 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
>
> Ditto.
>
> > + 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,25 @@ static int sdhci_tegra_resume(struct device *dev)
> > if (ret)
> > return ret;
> >
> > + /* Re-program MC streamID for DMA transfers */
> > +#ifdef CONFIG_IOMMU_API
>
> Ditto.
>
> > + if (tegra_host->soc_data->nvquirks & NVQUIRK_PROGRAM_STREAMID) {
> > + struct iommu_fwspec *fwspec;
> > +
> > + fwspec = dev_iommu_fwspec_get(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
> > +
> > ret = sdhci_resume_host(host);
> > if (ret)
> > goto disable_clk;
> > --
> > 2.17.1
> >
>
> Kind regards
> Uffe