linux-kernel - Re: [PATCH 5/8] mmc: sdhci-cadence: Add Pensando Elba SoC support

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:   Thu, 4 Mar 2021 10:41:13 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Brad Larson <brad@...sando.io>
Cc:     Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Mark Brown <broonie@...nel.org>,
        Serge Semin <fancer.lancer@...il.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Olof Johansson <olof@...om.net>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        linux-spi <linux-spi@...r.kernel.org>,
        linux-mmc <linux-mmc@...r.kernel.org>,
        DTML <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 5/8] mmc: sdhci-cadence: Add Pensando Elba SoC support

On Thu, Mar 4, 2021 at 4:41 AM Brad Larson <brad@...sando.io> wrote:

> +
> +static void elba_write_l(struct sdhci_host *host, u32 val, int reg)
> +{
> +       struct sdhci_cdns_priv *priv = sdhci_cdns_priv(host);
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&priv->wrlock, flags);
> +       writel(0x78, priv->ctl_addr);
> +       writel(val, host->ioaddr + reg);
> +       spin_unlock_irqrestore(&priv->wrlock, flags);
> +}

Please be aware that the spinlock does not actually guarantee serializing
a series of mmio writes unless the MMIO mapping is non-posted, or
you follow it with a readl() from the same device before the spin_unlock().

> @@ -453,8 +441,14 @@ static const struct dev_pm_ops sdhci_cdns_pm_ops = {
>  static const struct of_device_id sdhci_cdns_match[] = {
>         {
>                 .compatible = "socionext,uniphier-sd4hc",
> -               .data = &sdhci_cdns_uniphier_pltfm_data,
> +               .data = &sdhci_cdns_uniphier_drv_data,
>         },
> +#ifdef CONFIG_MMC_SDHCI_CADENCE_ELBA
> +       {
> +               .compatible = "pensando,elba-emmc",
> +               .data = &sdhci_elba_drv_data
> +       },
> +#endif
>         { .compatible = "cdns,sd4hc" },
>         { /* sentinel */ }
>  };

This introduces a reverse dependency between the modules, which will cause
problems at link time depending on how you configure it. There are two ways
to avoid this:

a) the simple method is to always link every backend into the driver module,
usually leaving them all enabled at compile time.

b) once this gets out of hand because there are too many variants, or the
differences between them are too big, you refactor the common code into
a library module that just exports a functions but has no module_init()
itself, plus a front-end driver for each variant, which now calls into
the common
code rather than being called by the common code.

      Arnd