Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <cover.1636103151.git.hns@goldelico.com> <3ca9a3099d86d631235b6c03ae260bc581cc8d60.1636103151.git.hns@goldelico.com>
In-Reply-To: <3ca9a3099d86d631235b6c03ae260bc581cc8d60.1636103151.git.hns@goldelico.com>
From:   Ulf Hansson <ulf.hansson@linaro.org>
Date:   Mon, 8 Nov 2021 16:33:52 +0100
Message-ID: <CAPDyKFrH8f80cs5dbh=3ugjyEzoUYXhStpHQyhUSd6b9wD78vw@mail.gmail.com>
Subject: Re: [RFC v4 5/6] mmc: core: transplant ti,wl1251 quirks from to be
 retired omap_hsmmc
To:     "H. Nikolaus Schaller" <hns@goldelico.com>
Cc:     =?UTF-8?B?SsOpcsO0bWUgUG91aWxsZXI=?= <Jerome.Pouiller@silabs.com>,
        Avri Altman <avri.altman@wdc.com>,
        Shawn Lin <shawn.lin@rock-chips.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        Tony Lindgren <tony@atomide.com>,
        Bean Huo <beanhuo@micron.com>, notasas@gmail.com,
        linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org,
        letux-kernel@openphoenux.org, kernel@pyra-handheld.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, 5 Nov 2021 at 10:06, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
> The TiWi WL1251 WiFi chip needs special setup of the sdio
> interface before it can be probed.
>
> So far, this is done in omap_hsmmc_init_card() in omap_hsmmc.c
> which makes it useable only if connected to omap devices
> which use the omap_hsmmc. The OpenPandora is the most promient
> example.
>
> There are plans to switch to a newer sdhci-omap driver and
> retire omap_hsmmc. Hence this quirk must be reworked or moved
> somewhere else. Ideally to some location that is not dependent
> on the specific SoC mmc host driver.
>
> This is achieved by the new mmc_fixup_device() option introduced
> by ("mmc: allow to match the device tree to apply quirks") to match
> through device tree compatible string.
>
> This quirk will be called early right after where host->ops->init_card()
> and thus omap_hsmmc_init_card() was previously called.
>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  drivers/mmc/core/card.h   | 19 +++++++++++++++++++
>  drivers/mmc/core/quirks.h |  7 +++++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
> index 089ede71d3150..20c8dfd6831cf 100644
> --- a/drivers/mmc/core/card.h
> +++ b/drivers/mmc/core/card.h
> @@ -168,6 +168,25 @@ static inline void __maybe_unused add_limit_rate_quirk(struct mmc_card *card,
>         card->quirk_max_rate = data;
>  }
>
> +static inline void __maybe_unused wl1251_quirk(struct mmc_card *card,
> +                                              int data)
> +{
> +       /*
> +        * We have TI wl1251 attached to this mmc. Pass this
> +        * information to the SDIO core because it can't be
> +        * probed by normal methods.
> +        */
> +
> +       dev_info(card->host->parent, "found wl1251\n");
> +       card->quirks |= MMC_QUIRK_NONSTD_SDIO;
> +       card->cccr.wide_bus = 1;
> +       card->cis.vendor = 0x104c;
> +       card->cis.device = 0x9066;
> +       card->cis.blksize = 512;
> +       card->cis.max_dtr = 24000000;
> +       card->ocr = 0x80;

In the past, we discussed a bit around why card->ocr needs to be set here.

The reason could very well be that the DTS file is specifying the
vmmc-supply with 1.8V fixed regulator, which seems wrong to me.

I would be very interested to know if we would change
"regulator-min|max-microvolt" of the regulator in the DTS, into
somewhere in between 2700000-3600000 (2.7-3.6V) - and see if that
allows us to drop the assignment of "card->ocr =  0x80;" above. Would
you mind doing some tests for this?

If that works, we should add some comments about it above, I think.

> +}
> +
>  /*
>   * Quirk add/remove for MMC products.
>   */
> diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h
> index 41c418527199c..e9813f1f8b23c 100644
> --- a/drivers/mmc/core/quirks.h
> +++ b/drivers/mmc/core/quirks.h
> @@ -146,7 +146,14 @@ static const struct mmc_fixup __maybe_unused sdio_fixup_methods[] = {
>         END_FIXUP
>  };
>
> +static const char *__maybe_unused wl1251_compatible_list[] = {
> +       "ti,wl1251",
> +       NULL
> +};
> +
>  static const struct mmc_fixup __maybe_unused sdio_card_init_methods[] = {
> +       SDIO_FIXUP_COMPATIBLE(wl1251_compatible_list, wl1251_quirk, 0),
> +
>         END_FIXUP
>  };
>

Kind regards
Uffe