[PATCH v2 4/6] mmc: atmel-sdhci: do not check clk_set_rate return value

[PATCH v2 4/6] mmc: atmel-sdhci: do not check clk_set_rate return value

[Eugen Hristev]

clk_set_rate will return rate in case of success and zero in case of
error, however it can also return -ev, but it's an ulong function.
To avoid any issues, disregard the return value of this call.
In case this call actually fails, nothing much we can do anyway, but we
can at least try with the previous values (or DT assigned-clocks)

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/mmc/atmel_sdhci.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
index f03c0457e1..54b660c34a 100644
--- a/drivers/mmc/atmel_sdhci.c
+++ b/drivers/mmc/atmel_sdhci.c
@@ -79,9 +79,7 @@ static int atmel_sdhci_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	ret = clk_set_rate(&clk, ATMEL_SDHC_GCK_RATE);
-	if (ret)
-		return ret;
+	clk_set_rate(&clk, ATMEL_SDHC_GCK_RATE);
 
 	max_clk = clk_get_rate(&clk);
 	if (!max_clk)
-- 
2.25.1

[PATCH v2 5/6] mmc: atmel-sdhci: enable the required generic clock

[Eugen Hristev]

The second clock of the IP block (the generic clock), must be explicitly
enabled.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/mmc/atmel_sdhci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
index 54b660c34a..c67b065061 100644
--- a/drivers/mmc/atmel_sdhci.c
+++ b/drivers/mmc/atmel_sdhci.c
@@ -85,6 +85,10 @@ static int atmel_sdhci_probe(struct udevice *dev)
 	if (!max_clk)
 		return -EINVAL;
 
+	ret = clk_enable(&clk);
+	if (ret)
+		return ret;
+
 	host->max_clk = max_clk;
 	host->mmc = &plat->mmc;
 	host->mmc->dev = dev;
-- 
2.25.1

[PATCH v2 5/6] mmc: atmel-sdhci: enable the required generic clock

[Peng Fan]

> Subject: [PATCH v2 5/6] mmc: atmel-sdhci: enable the required generic clock
> 
> The second clock of the IP block (the generic clock), must be explicitly
> enabled.
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---
>  drivers/mmc/atmel_sdhci.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c index
> 54b660c34a..c67b065061 100644
> --- a/drivers/mmc/atmel_sdhci.c
> +++ b/drivers/mmc/atmel_sdhci.c
> @@ -85,6 +85,10 @@ static int atmel_sdhci_probe(struct udevice *dev)
>  	if (!max_clk)
>  		return -EINVAL;
> 
> +	ret = clk_enable(&clk);
> +	if (ret)
> +		return ret;
> +
>  	host->max_clk = max_clk;
>  	host->mmc = &plat->mmc;
>  	host->mmc->dev = dev;

Reviewed-by: Peng Fan <pengfan@nxp.com>

> --
> 2.25.1

[PATCH v2 6/6] mmc: atmel-sdhci: use mmc_of_parse to get the DT properties

[Eugen Hristev]

Call mmc_of_parse at probe time to fetch all the host properties
from the DT.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/mmc/atmel_sdhci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
index c67b065061..f56ae63bc2 100644
--- a/drivers/mmc/atmel_sdhci.c
+++ b/drivers/mmc/atmel_sdhci.c
@@ -89,6 +89,10 @@ static int atmel_sdhci_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
+	ret = mmc_of_parse(dev, &plat->cfg);
+	if (ret)
+		return ret;
+
 	host->max_clk = max_clk;
 	host->mmc = &plat->mmc;
 	host->mmc->dev = dev;
-- 
2.25.1

[PATCH v2 6/6] mmc: atmel-sdhci: use mmc_of_parse to get the DT properties

[Peng Fan]

> Subject: [PATCH v2 6/6] mmc: atmel-sdhci: use mmc_of_parse to get the DT
> properties
> 
> Call mmc_of_parse at probe time to fetch all the host properties from the DT.
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---

Please add a changelog for future new version patches.

>  drivers/mmc/atmel_sdhci.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c index
> c67b065061..f56ae63bc2 100644
> --- a/drivers/mmc/atmel_sdhci.c
> +++ b/drivers/mmc/atmel_sdhci.c
> @@ -89,6 +89,10 @@ static int atmel_sdhci_probe(struct udevice *dev)
>  	if (ret)
>  		return ret;
> 
> +	ret = mmc_of_parse(dev, &plat->cfg);
> +	if (ret)
> +		return ret;
> +
>  	host->max_clk = max_clk;
>  	host->mmc = &plat->mmc;
>  	host->mmc->dev = dev;

Reviewed-by: Peng Fan <peng.fan@nxp.com>

Thanks,
Peng.

> --
> 2.25.1

[PATCH v2 4/6] mmc: atmel-sdhci: do not check clk_set_rate return value

[Eugen Hristev]

clk_set_rate will return rate in case of success and zero in case of
error, however it can also return -ev, but it's an ulong function.
To avoid any issues, disregard the return value of this call.
In case this call actually fails, nothing much we can do anyway, but we
can at least try with the previous values (or DT assigned-clocks)

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/mmc/atmel_sdhci.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
index f03c0457e1..54b660c34a 100644
--- a/drivers/mmc/atmel_sdhci.c
+++ b/drivers/mmc/atmel_sdhci.c
@@ -79,9 +79,7 @@ static int atmel_sdhci_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	ret = clk_set_rate(&clk, ATMEL_SDHC_GCK_RATE);
-	if (ret)
-		return ret;
+	clk_set_rate(&clk, ATMEL_SDHC_GCK_RATE);
 
 	max_clk = clk_get_rate(&clk);
 	if (!max_clk)
-- 
2.25.1

[PATCH v2 4/6] mmc: atmel-sdhci: do not check clk_set_rate return value

[Peng Fan]

> Subject: [PATCH v2 4/6] mmc: atmel-sdhci: do not check clk_set_rate return
> value
> 
> clk_set_rate will return rate in case of success and zero in case of error,
> however it can also return -ev, but it's an ulong function.
> To avoid any issues, disregard the return value of this call.
> In case this call actually fails, nothing much we can do anyway, but we can at
> least try with the previous values (or DT assigned-clocks)
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---
>  drivers/mmc/atmel_sdhci.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c index
> f03c0457e1..54b660c34a 100644
> --- a/drivers/mmc/atmel_sdhci.c
> +++ b/drivers/mmc/atmel_sdhci.c
> @@ -79,9 +79,7 @@ static int atmel_sdhci_probe(struct udevice *dev)
>  	if (ret)
>  		return ret;
> 
> -	ret = clk_set_rate(&clk, ATMEL_SDHC_GCK_RATE);
> -	if (ret)
> -		return ret;
> +	clk_set_rate(&clk, ATMEL_SDHC_GCK_RATE);
> 
>  	max_clk = clk_get_rate(&clk);
>  	if (!max_clk)

Since clk_set_rate will return the new rate or 0, is there
a need to use clk_get_rate following there?

Regards,
Peng.


> --
> 2.25.1

[PATCH v2 4/6] mmc: atmel-sdhci: do not check clk_set_rate return value

[Eugen.Hristev at microchip.com]

On 28.08.2020 08:37, Peng Fan wrote:
>> Subject: [PATCH v2 4/6] mmc: atmel-sdhci: do not check clk_set_rate return
>> value
>>
>> clk_set_rate will return rate in case of success and zero in case of error,
>> however it can also return -ev, but it's an ulong function.
>> To avoid any issues, disregard the return value of this call.
>> In case this call actually fails, nothing much we can do anyway, but we can at
>> least try with the previous values (or DT assigned-clocks)
>>
>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
>> ---
>>   drivers/mmc/atmel_sdhci.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c index
>> f03c0457e1..54b660c34a 100644
>> --- a/drivers/mmc/atmel_sdhci.c
>> +++ b/drivers/mmc/atmel_sdhci.c
>> @@ -79,9 +79,7 @@ static int atmel_sdhci_probe(struct udevice *dev)
>>        if (ret)
>>                return ret;
>>
>> -     ret = clk_set_rate(&clk, ATMEL_SDHC_GCK_RATE);
>> -     if (ret)
>> -             return ret;
>> +     clk_set_rate(&clk, ATMEL_SDHC_GCK_RATE);
>>
>>        max_clk = clk_get_rate(&clk);
>>        if (!max_clk)
> 
> Since clk_set_rate will return the new rate or 0, is there
> a need to use clk_get_rate following there?
> 
> Regards,
> Peng.

Hi Peng,

Yes. The clk_set_rate will return failure(0) if the clock was not 
changed, due to maybe an incorrect rate was set. However, this does not 
mean that we need to bail out.
The clock can be preconfigured from DT property assigned-clocks.
When calling clk_get_rate, we get the currently assigned rate.

Here is an example:

sama7g5 SoC supports clock at maximum 200 Mhz. Thus we assign 200 Mhz 
from assigned-clocks property.

The driver will call set_rate with 240 Mhz (predefined in driver). It 
will fail, return code 0.
However, the clock works at 200 Mhz and already configured.
When requesting the rate, we get the 200 Mhz from the clock subsystem.
We can move forward, and have host->max_clk = max_clk, as done below in 
the driver.

Does this look good to you ?

Thanks,
Eugen
> 
> 
>> --
>> 2.25.1
>