2023-06-29 01:27:36

by Kunihiko Hayashi

Subject: [PATCH] mmc: sdhci-f-sdh30: Replace with sdhci_pltfm

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().

Signed-off-by: Kunihiko Hayashi <[email protected]>
---
drivers/mmc/host/sdhci_f_sdh30.c | 60 ++++++++++++++------------------
1 file changed, 27 insertions(+), 33 deletions(-)

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

2023-06-29 13:07:59

by Adrian Hunter

Subject: Re: [PATCH] mmc: sdhci-f-sdh30: Replace with sdhci_pltfm

On 29/06/23 03:49, Kunihiko Hayashi 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().
>

Does it need a Fixes tag?

> Signed-off-by: Kunihiko Hayashi <[email protected]>

Otherwise:

Acked-by: Adrian Hunter <[email protected]>

> ---
> drivers/mmc/host/sdhci_f_sdh30.c | 60 ++++++++++++++------------------
> 1 file changed, 27 insertions(+), 33 deletions(-)
>
> 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;
> }

2023-06-30 00:49:54

by Kunihiko Hayashi

Subject: Re: [PATCH] mmc: sdhci-f-sdh30: Replace with sdhci_pltfm

Hi Adrian,

On 2023/06/29 21:57, Adrian Hunter wrote:
> On 29/06/23 03:49, Kunihiko Hayashi 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().
>>
>
> Does it need a Fixes tag?

Ah, I see. I'll add it in v2.

>> Signed-off-by: Kunihiko Hayashi <[email protected]>
>
> Otherwise:
>
> Acked-by: Adrian Hunter <[email protected]>

Thank you,

---
Best Regards
Kunihiko Hayashi