lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 11 Oct 2022 11:44:13 +0000
From:   Prathamesh Shete <pshete@...dia.com>
To:     Ulf Hansson <ulf.hansson@...aro.org>
CC:     "adrian.hunter@...el.com" <adrian.hunter@...el.com>,
        "thierry.reding@...il.com" <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Aniruddha Tvs Rao <anrao@...dia.com>,
        Suresh Mangipudi <smangipudi@...dia.com>,
        Krishna Yarlagadda <kyarlagadda@...dia.com>
Subject: RE: [PATCH v7 2/4] mmc: sdhci-tegra: Add support to program MC stream
 ID

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@...aro.org>
> Sent: Tuesday, October 11, 2022 3:29 PM
> To: Prathamesh Shete <pshete@...dia.com>
> Cc: adrian.hunter@...el.com; thierry.reding@...il.com; Jonathan Hunter
> <jonathanh@...dia.com>; p.zabel@...gutronix.de; linux-
> mmc@...r.kernel.org; linux-tegra@...r.kernel.org; linux-
> kernel@...r.kernel.org; Aniruddha Tvs Rao <anrao@...dia.com>; Suresh
> Mangipudi <smangipudi@...dia.com>; Krishna Yarlagadda
> <kyarlagadda@...dia.com>
> 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@...dia.com> 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@...dia.com>
> > Signed-off-by: Prathamesh Shete <pshete@...dia.com>
> > Acked-by: Adrian Hunter <adrian.hunter@...el.com>
> > Acked-by: Thierry Reding <treding@...dia.com>
> > ---
> >  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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ