linux-kernel - Re: [PATCH 2/2] drivers: mmc: sdhci_am654: Add the quirk to set TESTCD bit

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]

Message-ID: <CAPDyKFreftOExcZ9hgxpDGtdHhniB847vGAGpoO-McTy5_ao1w@mail.gmail.com>
Date:   Thu, 21 Apr 2022 14:11:59 +0200
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Aswath Govindraju <a-govindraju@...com>
Cc:     Vignesh Raghavendra <vigneshr@...com>,
        Kishon Vijay Abraham I <kishon@...com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        linux-mmc@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] drivers: mmc: sdhci_am654: Add the quirk to set
 TESTCD bit

On Mon, 18 Apr 2022 at 12:21, Aswath Govindraju <a-govindraju@...com> wrote:
>
> From: Vignesh Raghavendra <vigneshr@...com>
>
> The ARASAN MMC controller on Keystone 3 class of devices need the SDCD
> line to be connected for proper functioning. Similar to the issue pointed
> out in sdhci-of-arasan.c driver, commit 3794c542641f ("mmc:
> sdhci-of-arasan: Set controller to test mode when no CD bit").
>
> In cases where this can't be connected, add a quirk to force the
> controller into test mode and set the TESTCD bit. Use the flag
> "ti,fails-without-test-cd", to implement this above quirk when required.
>
> Signed-off-by: Vignesh Raghavendra <vigneshr@...com>
> Signed-off-by: Aswath Govindraju <a-govindraju@...com>
> ---
>  drivers/mmc/host/sdhci_am654.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> index e54fe24d47e7..c36b969ed1b6 100644
> --- a/drivers/mmc/host/sdhci_am654.c
> +++ b/drivers/mmc/host/sdhci_am654.c
> @@ -147,6 +147,9 @@ struct sdhci_am654_data {
>         int drv_strength;
>         int strb_sel;
>         u32 flags;
> +       u32 quirks;
> +
> +#define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0)
>  };
>
>  struct sdhci_am654_driver_data {
> @@ -369,6 +372,21 @@ static void sdhci_am654_write_b(struct sdhci_host *host, u8 val, int reg)
>         }
>  }
>
> +void sdhci_am654_reset(struct sdhci_host *host, u8 mask)

This should be a static function.

> +{
> +       u8 ctrl;
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> +
> +       sdhci_reset(host, mask);
> +
> +       if (sdhci_am654->quirks & SDHCI_AM654_QUIRK_FORCE_CDTEST) {
> +               ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
> +               ctrl |= SDHCI_CTRL_CDTEST_INS | SDHCI_CTRL_CDTEST_EN;
> +               sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
> +       }
> +}
> +
>  static int sdhci_am654_execute_tuning(struct mmc_host *mmc, u32 opcode)
>  {
>         struct sdhci_host *host = mmc_priv(mmc);
> @@ -500,7 +518,7 @@ static struct sdhci_ops sdhci_j721e_4bit_ops = {
>         .set_clock = sdhci_j721e_4bit_set_clock,
>         .write_b = sdhci_am654_write_b,
>         .irq = sdhci_am654_cqhci_irq,
> -       .reset = sdhci_reset,
> +       .reset = sdhci_am654_reset,
>  };
>
>  static const struct sdhci_pltfm_data sdhci_j721e_4bit_pdata = {
> @@ -719,6 +737,9 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,
>         device_property_read_u32(dev, "ti,clkbuf-sel",
>                                  &sdhci_am654->clkbuf_sel);
>
> +       if (device_property_read_bool(dev, "ti,fails-without-test-cd"))
> +               sdhci_am654->quirks |= SDHCI_AM654_QUIRK_FORCE_CDTEST;
> +
>         sdhci_get_of_property(pdev);
>
>         return 0;
> --
> 2.17.1
>

Other than the minor thing above, this looks good to me.

Kind regards
Uffe