Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230630004533.26644-1-hayashi.kunihiko@socionext.com>
In-Reply-To: <20230630004533.26644-1-hayashi.kunihiko@socionext.com>
From:   Ulf Hansson <ulf.hansson@linaro.org>
Date:   Thu, 13 Jul 2023 13:06:10 +0200
Message-ID: <CAPDyKFqJU3VwXJJDQPWvNBDz1+gSjYxFJBST3s8mpJoh7-c9-Q@mail.gmail.com>
Subject: Re: [PATCH v2] mmc: sdhci-f-sdh30: Replace with sdhci_pltfm
To:     Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Cc:     Adrian Hunter <adrian.hunter@intel.com>, linux-mmc@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, 30 Jun 2023 at 02:45, Kunihiko Hayashi
<hayashi.kunihiko@socionext.com> wrote:
>
> Even if sdhci_pltfm_pmops is specified for PM, this driver doesn't apply
> sdhci_pltfm, so the structure is not correctly referenced in PM functions.
> This applies sdhci_pltfm to this driver to fix this issue.
>
> - Call sdhci_pltfm_init() instead of sdhci_alloc_host() and
>   other functions that covered by sdhci_pltfm.
> - Move ops and quirks to sdhci_pltfm_data
> - Replace sdhci_priv() with own private function sdhci_f_sdh30_priv().
>
> Fixes: 87a507459f49 ("mmc: sdhci: host: add new f_sdh30")
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Applied for next, thanks!

Or maybe this should go for fixes and have a stable tag too?

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci_f_sdh30.c | 60 ++++++++++++++------------------
>  1 file changed, 27 insertions(+), 33 deletions(-)
>
> Changes since v1:
> - Add Fixes and Acked-by tags
>
> diff --git a/drivers/mmc/host/sdhci_f_sdh30.c b/drivers/mmc/host/sdhci_f_sdh30.c
> index a202a69a4b08..b01ffb4d0973 100644
> --- a/drivers/mmc/host/sdhci_f_sdh30.c
> +++ b/drivers/mmc/host/sdhci_f_sdh30.c
> @@ -29,9 +29,16 @@ struct f_sdhost_priv {
>         bool enable_cmd_dat_delay;
>  };
>
> +static void *sdhci_f_sdhost_priv(struct sdhci_host *host)
> +{
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +
> +       return sdhci_pltfm_priv(pltfm_host);
> +}
> +
>  static void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
>  {
> -       struct f_sdhost_priv *priv = sdhci_priv(host);
> +       struct f_sdhost_priv *priv = sdhci_f_sdhost_priv(host);
>         u32 ctrl = 0;
>
>         usleep_range(2500, 3000);
> @@ -64,7 +71,7 @@ static unsigned int sdhci_f_sdh30_get_min_clock(struct sdhci_host *host)
>
>  static void sdhci_f_sdh30_reset(struct sdhci_host *host, u8 mask)
>  {
> -       struct f_sdhost_priv *priv = sdhci_priv(host);
> +       struct f_sdhost_priv *priv = sdhci_f_sdhost_priv(host);
>         u32 ctl;
>
>         if (sdhci_readw(host, SDHCI_CLOCK_CONTROL) == 0)
> @@ -95,30 +102,32 @@ static const struct sdhci_ops sdhci_f_sdh30_ops = {
>         .set_uhs_signaling = sdhci_set_uhs_signaling,
>  };
>
> +static const struct sdhci_pltfm_data sdhci_f_sdh30_pltfm_data = {
> +       .ops = &sdhci_f_sdh30_ops,
> +       .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC
> +               | SDHCI_QUIRK_INVERTED_WRITE_PROTECT,
> +       .quirks2 = SDHCI_QUIRK2_SUPPORT_SINGLE
> +               |  SDHCI_QUIRK2_TUNING_WORK_AROUND,
> +};
> +
>  static int sdhci_f_sdh30_probe(struct platform_device *pdev)
>  {
>         struct sdhci_host *host;
>         struct device *dev = &pdev->dev;
> -       int irq, ctrl = 0, ret = 0;
> +       int ctrl = 0, ret = 0;
>         struct f_sdhost_priv *priv;
> +       struct sdhci_pltfm_host *pltfm_host;
>         u32 reg = 0;
>
> -       irq = platform_get_irq(pdev, 0);
> -       if (irq < 0)
> -               return irq;
> -
> -       host = sdhci_alloc_host(dev, sizeof(struct f_sdhost_priv));
> +       host = sdhci_pltfm_init(pdev, &sdhci_f_sdh30_pltfm_data,
> +                               sizeof(struct f_sdhost_priv));
>         if (IS_ERR(host))
>                 return PTR_ERR(host);
>
> -       priv = sdhci_priv(host);
> +       pltfm_host = sdhci_priv(host);
> +       priv = sdhci_pltfm_priv(pltfm_host);
>         priv->dev = dev;
>
> -       host->quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
> -                      SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
> -       host->quirks2 = SDHCI_QUIRK2_SUPPORT_SINGLE |
> -                       SDHCI_QUIRK2_TUNING_WORK_AROUND;
> -
>         priv->enable_cmd_dat_delay = device_property_read_bool(dev,
>                                                 "fujitsu,cmd-dat-delay-select");
>
> @@ -126,18 +135,6 @@ static int sdhci_f_sdh30_probe(struct platform_device *pdev)
>         if (ret)
>                 goto err;
>
> -       platform_set_drvdata(pdev, host);
> -
> -       host->hw_name = "f_sdh30";
> -       host->ops = &sdhci_f_sdh30_ops;
> -       host->irq = irq;
> -
> -       host->ioaddr = devm_platform_ioremap_resource(pdev, 0);
> -       if (IS_ERR(host->ioaddr)) {
> -               ret = PTR_ERR(host->ioaddr);
> -               goto err;
> -       }
> -
>         if (dev_of_node(dev)) {
>                 sdhci_get_of_property(pdev);
>
> @@ -204,24 +201,21 @@ static int sdhci_f_sdh30_probe(struct platform_device *pdev)
>  err_clk:
>         clk_disable_unprepare(priv->clk_iface);
>  err:
> -       sdhci_free_host(host);
> +       sdhci_pltfm_free(pdev);
> +
>         return ret;
>  }
>
>  static int sdhci_f_sdh30_remove(struct platform_device *pdev)
>  {
>         struct sdhci_host *host = platform_get_drvdata(pdev);
> -       struct f_sdhost_priv *priv = sdhci_priv(host);
> -
> -       sdhci_remove_host(host, readl(host->ioaddr + SDHCI_INT_STATUS) ==
> -                         0xffffffff);
> +       struct f_sdhost_priv *priv = sdhci_f_sdhost_priv(host);
>
>         reset_control_assert(priv->rst);
>         clk_disable_unprepare(priv->clk);
>         clk_disable_unprepare(priv->clk_iface);
>
> -       sdhci_free_host(host);
> -       platform_set_drvdata(pdev, NULL);
> +       sdhci_pltfm_unregister(pdev);
>
>         return 0;
>  }
> --
> 2.25.1
>