diff mbox series

[v2,5/8] mmc: sdhci-pxav2: add optional core clock

Message ID 20221202031330.94130-6-doug@schmorgal.com (mailing list archive)
State New, archived
Headers show
Series mmc: sdhci-pxav2: Add support for PXA168 | expand

Commit Message

Doug Brown Dec. 2, 2022, 3:13 a.m. UTC 
Add ability to have an optional core clock just like the pxav3 driver.
The PXA168 needs this because its SDHC controllers have separate core
and io clocks that both need to be enabled. This also correctly matches
the documented devicetree bindings for this driver.

Signed-off-by: Doug Brown <doug@schmorgal.com>
---
 drivers/mmc/host/sdhci-pxav2.c | 40 ++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

Comments

Adrian Hunter Dec. 22, 2022, 5:33 p.m. UTC | #1 
On 2/12/22 05:13, Doug Brown wrote:
> Add ability to have an optional core clock just like the pxav3 driver.
> The PXA168 needs this because its SDHC controllers have separate core
> and io clocks that both need to be enabled. This also correctly matches
> the documented devicetree bindings for this driver.
> 
> Signed-off-by: Doug Brown <doug@schmorgal.com>
> ---
>  drivers/mmc/host/sdhci-pxav2.c | 40 ++++++++++++++++++++++++++++++----
>  1 file changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c
> index 509ba5dd4a4a..1f0c3028987a 100644
> --- a/drivers/mmc/host/sdhci-pxav2.c
> +++ b/drivers/mmc/host/sdhci-pxav2.c
> @@ -41,6 +41,10 @@
>  #define MMC_CARD		0x1000
>  #define MMC_WIDTH		0x0100
>  
> +struct sdhci_pxav2_host {
> +	struct clk *clk_core;
> +};
> +
>  static void pxav2_reset(struct sdhci_host *host, u8 mask)
>  {
>  	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
> @@ -176,6 +180,7 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
>  {
>  	struct sdhci_pltfm_host *pltfm_host;
>  	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
> +	struct sdhci_pxav2_host *pxav2_host;
>  	struct device *dev = &pdev->dev;
>  	struct sdhci_host *host = NULL;
>  	const struct of_device_id *match;
> @@ -183,11 +188,12 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
>  	int ret;
>  	struct clk *clk;
>  
> -	host = sdhci_pltfm_init(pdev, NULL, 0);
> +	host = sdhci_pltfm_init(pdev, NULL, sizeof(*pxav2_host));
>  	if (IS_ERR(host))
>  		return PTR_ERR(host);
>  
>  	pltfm_host = sdhci_priv(host);
> +	pxav2_host = sdhci_pltfm_priv(pltfm_host);
>  
>  	clk = devm_clk_get(dev, "io");
>  	if (IS_ERR(clk))
> @@ -204,6 +210,15 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
>  		goto free;
>  	}
>  
> +	pxav2_host->clk_core = devm_clk_get(dev, "core");

This can use devm_clk_get_optional_enabled()

> +	if (!IS_ERR(pxav2_host->clk_core)) {
> +		ret = clk_prepare_enable(pxav2_host->clk_core);
> +		if (ret) {
> +			dev_err(&pdev->dev, "failed to enable core clock\n");

	if (IS_ERR(pxav2_host->clk_core)) {
		dev_err_probe(&pdev->dev, PTR_ERR(pxav2_host->clk_core), "failed to enable core clock\n");

> +			goto disable_io_clk;
> +		}
> +	}
> +
>  	host->quirks = SDHCI_QUIRK_BROKEN_ADMA
>  		| SDHCI_QUIRK_BROKEN_TIMEOUT_VAL
>  		| SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN;
> @@ -240,17 +255,34 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
>  
>  	ret = sdhci_add_host(host);
>  	if (ret)
> -		goto disable_clk;
> +		goto disable_core_clk;
>  
>  	return 0;
>  
> -disable_clk:
> +disable_core_clk:
> +	if (!IS_ERR(pxav2_host->clk_core))

With changes above this would need to be: IS_ERR_OR_NULL


> +		clk_disable_unprepare(pxav2_host->clk_core);
> +disable_io_clk:
>  	clk_disable_unprepare(clk);
>  free:
>  	sdhci_pltfm_free(pdev);
>  	return ret;
>  }
>  
> +static int sdhci_pxav2_remove(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_pxav2_host *pxav2_host = sdhci_pltfm_priv(pltfm_host);
> +
> +	int ret = sdhci_pltfm_unregister(pdev);
> +
> +	if (!IS_ERR(pxav2_host->clk_core))

With changes above this would need to be: IS_ERR_OR_NULL

> +		clk_disable_unprepare(pxav2_host->clk_core);
> +
> +	return ret;
> +}
> +
>  static struct platform_driver sdhci_pxav2_driver = {
>  	.driver		= {
>  		.name	= "sdhci-pxav2",
> @@ -259,7 +291,7 @@ static struct platform_driver sdhci_pxav2_driver = {
>  		.pm	= &sdhci_pltfm_pmops,
>  	},
>  	.probe		= sdhci_pxav2_probe,
> -	.remove		= sdhci_pltfm_unregister,
> +	.remove		= sdhci_pxav2_remove,
>  };
>  
>  module_platform_driver(sdhci_pxav2_driver);
Adrian Hunter Dec. 22, 2022, 5:42 p.m. UTC | #2 
On 22/12/22 19:33, Adrian Hunter wrote:
> On 2/12/22 05:13, Doug Brown wrote:
>> Add ability to have an optional core clock just like the pxav3 driver.
>> The PXA168 needs this because its SDHC controllers have separate core
>> and io clocks that both need to be enabled. This also correctly matches
>> the documented devicetree bindings for this driver.
>>
>> Signed-off-by: Doug Brown <doug@schmorgal.com>
>> ---
>>  drivers/mmc/host/sdhci-pxav2.c | 40 ++++++++++++++++++++++++++++++----
>>  1 file changed, 36 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c
>> index 509ba5dd4a4a..1f0c3028987a 100644
>> --- a/drivers/mmc/host/sdhci-pxav2.c
>> +++ b/drivers/mmc/host/sdhci-pxav2.c
>> @@ -41,6 +41,10 @@
>>  #define MMC_CARD		0x1000
>>  #define MMC_WIDTH		0x0100
>>  
>> +struct sdhci_pxav2_host {
>> +	struct clk *clk_core;
>> +};
>> +
>>  static void pxav2_reset(struct sdhci_host *host, u8 mask)
>>  {
>>  	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
>> @@ -176,6 +180,7 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
>>  {
>>  	struct sdhci_pltfm_host *pltfm_host;
>>  	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
>> +	struct sdhci_pxav2_host *pxav2_host;
>>  	struct device *dev = &pdev->dev;
>>  	struct sdhci_host *host = NULL;
>>  	const struct of_device_id *match;
>> @@ -183,11 +188,12 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
>>  	int ret;
>>  	struct clk *clk;
>>  
>> -	host = sdhci_pltfm_init(pdev, NULL, 0);
>> +	host = sdhci_pltfm_init(pdev, NULL, sizeof(*pxav2_host));
>>  	if (IS_ERR(host))
>>  		return PTR_ERR(host);
>>  
>>  	pltfm_host = sdhci_priv(host);
>> +	pxav2_host = sdhci_pltfm_priv(pltfm_host);
>>  
>>  	clk = devm_clk_get(dev, "io");
>>  	if (IS_ERR(clk))
>> @@ -204,6 +210,15 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
>>  		goto free;
>>  	}
>>  
>> +	pxav2_host->clk_core = devm_clk_get(dev, "core");
> 
> This can use devm_clk_get_optional_enabled()
> 
>> +	if (!IS_ERR(pxav2_host->clk_core)) {
>> +		ret = clk_prepare_enable(pxav2_host->clk_core);
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "failed to enable core clock\n");
> 
> 	if (IS_ERR(pxav2_host->clk_core)) {
> 		dev_err_probe(&pdev->dev, PTR_ERR(pxav2_host->clk_core), "failed to enable core clock\n");
> 
>> +			goto disable_io_clk;
>> +		}
>> +	}
>> +
>>  	host->quirks = SDHCI_QUIRK_BROKEN_ADMA
>>  		| SDHCI_QUIRK_BROKEN_TIMEOUT_VAL
>>  		| SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN;
>> @@ -240,17 +255,34 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
>>  
>>  	ret = sdhci_add_host(host);
>>  	if (ret)
>> -		goto disable_clk;
>> +		goto disable_core_clk;
>>  
>>  	return 0;
>>  
>> -disable_clk:
>> +disable_core_clk:
>> +	if (!IS_ERR(pxav2_host->clk_core))
> 
> With changes above this would need to be: IS_ERR_OR_NULL

Actually with changes above, it is taken care of by devm so
clk_disable_unprepare is not needed.

> 
> 
>> +		clk_disable_unprepare(pxav2_host->clk_core);
>> +disable_io_clk:
>>  	clk_disable_unprepare(clk);
>>  free:
>>  	sdhci_pltfm_free(pdev);
>>  	return ret;
>>  }
>>  
>> +static int sdhci_pxav2_remove(struct platform_device *pdev)
>> +{
>> +	struct sdhci_host *host = platform_get_drvdata(pdev);
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_pxav2_host *pxav2_host = sdhci_pltfm_priv(pltfm_host);
>> +
>> +	int ret = sdhci_pltfm_unregister(pdev);
>> +
>> +	if (!IS_ERR(pxav2_host->clk_core))
> 
> With changes above this would need to be: IS_ERR_OR_NULL

Ditto

> 
>> +		clk_disable_unprepare(pxav2_host->clk_core);
>> +
>> +	return ret;
>> +}
>> +
>>  static struct platform_driver sdhci_pxav2_driver = {
>>  	.driver		= {
>>  		.name	= "sdhci-pxav2",
>> @@ -259,7 +291,7 @@ static struct platform_driver sdhci_pxav2_driver = {
>>  		.pm	= &sdhci_pltfm_pmops,
>>  	},
>>  	.probe		= sdhci_pxav2_probe,
>> -	.remove		= sdhci_pltfm_unregister,
>> +	.remove		= sdhci_pxav2_remove,
>>  };
>>  
>>  module_platform_driver(sdhci_pxav2_driver);
>
diff mbox series

Patch

diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c
index 509ba5dd4a4a..1f0c3028987a 100644
--- a/drivers/mmc/host/sdhci-pxav2.c
+++ b/drivers/mmc/host/sdhci-pxav2.c
@@ -41,6 +41,10 @@ 
 #define MMC_CARD		0x1000
 #define MMC_WIDTH		0x0100
 
+struct sdhci_pxav2_host {
+	struct clk *clk_core;
+};
+
 static void pxav2_reset(struct sdhci_host *host, u8 mask)
 {
 	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
@@ -176,6 +180,7 @@  static int sdhci_pxav2_probe(struct platform_device *pdev)
 {
 	struct sdhci_pltfm_host *pltfm_host;
 	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
+	struct sdhci_pxav2_host *pxav2_host;
 	struct device *dev = &pdev->dev;
 	struct sdhci_host *host = NULL;
 	const struct of_device_id *match;
@@ -183,11 +188,12 @@  static int sdhci_pxav2_probe(struct platform_device *pdev)
 	int ret;
 	struct clk *clk;
 
-	host = sdhci_pltfm_init(pdev, NULL, 0);
+	host = sdhci_pltfm_init(pdev, NULL, sizeof(*pxav2_host));
 	if (IS_ERR(host))
 		return PTR_ERR(host);
 
 	pltfm_host = sdhci_priv(host);
+	pxav2_host = sdhci_pltfm_priv(pltfm_host);
 
 	clk = devm_clk_get(dev, "io");
 	if (IS_ERR(clk))
@@ -204,6 +210,15 @@  static int sdhci_pxav2_probe(struct platform_device *pdev)
 		goto free;
 	}
 
+	pxav2_host->clk_core = devm_clk_get(dev, "core");
+	if (!IS_ERR(pxav2_host->clk_core)) {
+		ret = clk_prepare_enable(pxav2_host->clk_core);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to enable core clock\n");
+			goto disable_io_clk;
+		}
+	}
+
 	host->quirks = SDHCI_QUIRK_BROKEN_ADMA
 		| SDHCI_QUIRK_BROKEN_TIMEOUT_VAL
 		| SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN;
@@ -240,17 +255,34 @@  static int sdhci_pxav2_probe(struct platform_device *pdev)
 
 	ret = sdhci_add_host(host);
 	if (ret)
-		goto disable_clk;
+		goto disable_core_clk;
 
 	return 0;
 
-disable_clk:
+disable_core_clk:
+	if (!IS_ERR(pxav2_host->clk_core))
+		clk_disable_unprepare(pxav2_host->clk_core);
+disable_io_clk:
 	clk_disable_unprepare(clk);
 free:
 	sdhci_pltfm_free(pdev);
 	return ret;
 }
 
+static int sdhci_pxav2_remove(struct platform_device *pdev)
+{
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_pxav2_host *pxav2_host = sdhci_pltfm_priv(pltfm_host);
+
+	int ret = sdhci_pltfm_unregister(pdev);
+
+	if (!IS_ERR(pxav2_host->clk_core))
+		clk_disable_unprepare(pxav2_host->clk_core);
+
+	return ret;
+}
+
 static struct platform_driver sdhci_pxav2_driver = {
 	.driver		= {
 		.name	= "sdhci-pxav2",
@@ -259,7 +291,7 @@  static struct platform_driver sdhci_pxav2_driver = {
 		.pm	= &sdhci_pltfm_pmops,
 	},
 	.probe		= sdhci_pxav2_probe,
-	.remove		= sdhci_pltfm_unregister,
+	.remove		= sdhci_pxav2_remove,
 };
 
 module_platform_driver(sdhci_pxav2_driver);