* [PATCH v2 1/7] mmc: sdhci-pxav3: Fix SDR50 and DDR50 capabilities for the Armada 38x flavor
2015-01-23 10:56 ` Gregory CLEMENT
@ 2015-01-23 10:56 ` Gregory CLEMENT
-1 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-23 10:56 UTC (permalink / raw)
To: Chris Ball, Ulf Hansson, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory CLEMENT
Cc: Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
stable-u79uwXL29TY76Z2rM5mHXA, Marcin Wojtas
According to erratum 'FE-2946959' both SDR50 and DDR50 modes require
specific clock adjustments in SDIO3 Configuration register. However,
this register was not part of the device tree binding. Even if the
binding can (and will) be extended we still need handling the case
where this register was not available. In this case we use the
SDHCI_QUIRK_MISSING_CAPS quirk remove them from the capabilities.
This commit is based on the work done by Marcin Wojtas<mw-nYOzD4b6Jr9Wk0Htik3J/w@public.gmane.org>
Fixes: 5491ce3f79ee ("mmc: sdhci-pxav3: add support for the Armada 38x SDHCI controller")
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v3.15+
Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Marcin Wojtas <mw-nYOzD4b6Jr9Wk0Htik3J/w@public.gmane.org>
---
drivers/mmc/host/sdhci-pxav3.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
index ca3424e7ef71..7b07325b4fba 100644
--- a/drivers/mmc/host/sdhci-pxav3.c
+++ b/drivers/mmc/host/sdhci-pxav3.c
@@ -118,6 +118,20 @@ static int mv_conf_mbus_windows(struct platform_device *pdev,
return 0;
}
+static int armada_38x_quirks(struct sdhci_host *host)
+{
+ host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
+ /*
+ * According to erratum 'FE-2946959' both SDR50 and DDR50
+ * modes require specific clock adjustments in SDIO3
+ * Configuration register, if the adjustment is not done,
+ * remove them from the capabilities.
+ */
+ host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
+ host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
+ return 0;
+}
+
static void pxav3_reset(struct sdhci_host *host, u8 mask)
{
struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
@@ -319,6 +333,9 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
clk_prepare_enable(pxa->clk_core);
if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
+ ret = armada_38x_quirks(host);
+ if (ret < 0)
+ goto err_quirks;
ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
if (ret < 0)
goto err_mbus_win;
@@ -400,6 +417,7 @@ err_mbus_win:
if (!IS_ERR(pxa->clk_core))
clk_disable_unprepare(pxa->clk_core);
err_clk_get:
+err_quirks:
sdhci_pltfm_free(pdev);
return ret;
}
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 1/7] mmc: sdhci-pxav3: Fix SDR50 and DDR50 capabilities for the Armada 38x flavor
@ 2015-01-23 10:56 ` Gregory CLEMENT
0 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-23 10:56 UTC (permalink / raw)
To: linux-arm-kernel
According to erratum 'FE-2946959' both SDR50 and DDR50 modes require
specific clock adjustments in SDIO3 Configuration register. However,
this register was not part of the device tree binding. Even if the
binding can (and will) be extended we still need handling the case
where this register was not available. In this case we use the
SDHCI_QUIRK_MISSING_CAPS quirk remove them from the capabilities.
This commit is based on the work done by Marcin Wojtas<mw@semihalf.com>
Fixes: 5491ce3f79ee ("mmc: sdhci-pxav3: add support for the Armada 38x SDHCI controller")
Cc: <stable@vger.kernel.org> # v3.15+
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
drivers/mmc/host/sdhci-pxav3.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
index ca3424e7ef71..7b07325b4fba 100644
--- a/drivers/mmc/host/sdhci-pxav3.c
+++ b/drivers/mmc/host/sdhci-pxav3.c
@@ -118,6 +118,20 @@ static int mv_conf_mbus_windows(struct platform_device *pdev,
return 0;
}
+static int armada_38x_quirks(struct sdhci_host *host)
+{
+ host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
+ /*
+ * According to erratum 'FE-2946959' both SDR50 and DDR50
+ * modes require specific clock adjustments in SDIO3
+ * Configuration register, if the adjustment is not done,
+ * remove them from the capabilities.
+ */
+ host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
+ host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
+ return 0;
+}
+
static void pxav3_reset(struct sdhci_host *host, u8 mask)
{
struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
@@ -319,6 +333,9 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
clk_prepare_enable(pxa->clk_core);
if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
+ ret = armada_38x_quirks(host);
+ if (ret < 0)
+ goto err_quirks;
ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
if (ret < 0)
goto err_mbus_win;
@@ -400,6 +417,7 @@ err_mbus_win:
if (!IS_ERR(pxa->clk_core))
clk_disable_unprepare(pxa->clk_core);
err_clk_get:
+err_quirks:
sdhci_pltfm_free(pdev);
return ret;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH v2 1/7] mmc: sdhci-pxav3: Fix SDR50 and DDR50 capabilities for the Armada 38x flavor
2015-01-23 10:56 ` Gregory CLEMENT
@ 2015-01-28 20:36 ` Ulf Hansson
-1 siblings, 0 replies; 34+ messages in thread
From: Ulf Hansson @ 2015-01-28 20:36 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Thomas Petazzoni, Andrew Lunn, Jason Cooper, Tawfik Bayouk,
Boris BREZILLON, devicetree, Mark Rutland, linux-mmc, Chris Ball,
stable, Nadav Haklai, Lior Amsalem, Ezequiel Garcia,
Maxime Ripard, Marcin Wojtas, linux-arm-kernel,
Sebastian Hesselbarth
On 23 January 2015 at 11:56, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> According to erratum 'FE-2946959' both SDR50 and DDR50 modes require
> specific clock adjustments in SDIO3 Configuration register. However,
> this register was not part of the device tree binding. Even if the
> binding can (and will) be extended we still need handling the case
> where this register was not available. In this case we use the
> SDHCI_QUIRK_MISSING_CAPS quirk remove them from the capabilities.
>
> This commit is based on the work done by Marcin Wojtas<mw@semihalf.com>
>
> Fixes: 5491ce3f79ee ("mmc: sdhci-pxav3: add support for the Armada 38x SDHCI controller")
> Cc: <stable@vger.kernel.org> # v3.15+
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
> drivers/mmc/host/sdhci-pxav3.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
> index ca3424e7ef71..7b07325b4fba 100644
> --- a/drivers/mmc/host/sdhci-pxav3.c
> +++ b/drivers/mmc/host/sdhci-pxav3.c
> @@ -118,6 +118,20 @@ static int mv_conf_mbus_windows(struct platform_device *pdev,
> return 0;
> }
>
> +static int armada_38x_quirks(struct sdhci_host *host)
Seems like this function can be void instead of always returning 0.
> +{
> + host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
> + /*
> + * According to erratum 'FE-2946959' both SDR50 and DDR50
> + * modes require specific clock adjustments in SDIO3
> + * Configuration register, if the adjustment is not done,
> + * remove them from the capabilities.
> + */
> + host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
> + host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
> + return 0;
> +}
> +
> static void pxav3_reset(struct sdhci_host *host, u8 mask)
> {
> struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
> @@ -319,6 +333,9 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
> clk_prepare_enable(pxa->clk_core);
>
> if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
> + ret = armada_38x_quirks(host);
> + if (ret < 0)
> + goto err_quirks;
> ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
> if (ret < 0)
> goto err_mbus_win;
> @@ -400,6 +417,7 @@ err_mbus_win:
> if (!IS_ERR(pxa->clk_core))
> clk_disable_unprepare(pxa->clk_core);
> err_clk_get:
> +err_quirks:
> sdhci_pltfm_free(pdev);
> return ret;
> }
> --
> 2.1.0
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 1/7] mmc: sdhci-pxav3: Fix SDR50 and DDR50 capabilities for the Armada 38x flavor
@ 2015-01-28 20:36 ` Ulf Hansson
0 siblings, 0 replies; 34+ messages in thread
From: Ulf Hansson @ 2015-01-28 20:36 UTC (permalink / raw)
To: linux-arm-kernel
On 23 January 2015 at 11:56, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> According to erratum 'FE-2946959' both SDR50 and DDR50 modes require
> specific clock adjustments in SDIO3 Configuration register. However,
> this register was not part of the device tree binding. Even if the
> binding can (and will) be extended we still need handling the case
> where this register was not available. In this case we use the
> SDHCI_QUIRK_MISSING_CAPS quirk remove them from the capabilities.
>
> This commit is based on the work done by Marcin Wojtas<mw@semihalf.com>
>
> Fixes: 5491ce3f79ee ("mmc: sdhci-pxav3: add support for the Armada 38x SDHCI controller")
> Cc: <stable@vger.kernel.org> # v3.15+
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
> drivers/mmc/host/sdhci-pxav3.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
> index ca3424e7ef71..7b07325b4fba 100644
> --- a/drivers/mmc/host/sdhci-pxav3.c
> +++ b/drivers/mmc/host/sdhci-pxav3.c
> @@ -118,6 +118,20 @@ static int mv_conf_mbus_windows(struct platform_device *pdev,
> return 0;
> }
>
> +static int armada_38x_quirks(struct sdhci_host *host)
Seems like this function can be void instead of always returning 0.
> +{
> + host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
> + /*
> + * According to erratum 'FE-2946959' both SDR50 and DDR50
> + * modes require specific clock adjustments in SDIO3
> + * Configuration register, if the adjustment is not done,
> + * remove them from the capabilities.
> + */
> + host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
> + host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
> + return 0;
> +}
> +
> static void pxav3_reset(struct sdhci_host *host, u8 mask)
> {
> struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
> @@ -319,6 +333,9 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
> clk_prepare_enable(pxa->clk_core);
>
> if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
> + ret = armada_38x_quirks(host);
> + if (ret < 0)
> + goto err_quirks;
> ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
> if (ret < 0)
> goto err_mbus_win;
> @@ -400,6 +417,7 @@ err_mbus_win:
> if (!IS_ERR(pxa->clk_core))
> clk_disable_unprepare(pxa->clk_core);
> err_clk_get:
> +err_quirks:
> sdhci_pltfm_free(pdev);
> return ret;
> }
> --
> 2.1.0
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 1/7] mmc: sdhci-pxav3: Fix SDR50 and DDR50 capabilities for the Armada 38x flavor
2015-01-28 20:36 ` Ulf Hansson
@ 2015-01-29 8:31 ` Gregory CLEMENT
-1 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-29 8:31 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, linux-mmc, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai, Mark Rutland, devicetree, stable,
Marcin Wojtas
Hi Ulf,
On 28/01/2015 21:36, Ulf Hansson wrote:
> On 23 January 2015 at 11:56, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
>> According to erratum 'FE-2946959' both SDR50 and DDR50 modes require
>> specific clock adjustments in SDIO3 Configuration register. However,
>> this register was not part of the device tree binding. Even if the
>> binding can (and will) be extended we still need handling the case
>> where this register was not available. In this case we use the
>> SDHCI_QUIRK_MISSING_CAPS quirk remove them from the capabilities.
>>
>> This commit is based on the work done by Marcin Wojtas<mw@semihalf.com>
>>
>> Fixes: 5491ce3f79ee ("mmc: sdhci-pxav3: add support for the Armada 38x SDHCI controller")
>> Cc: <stable@vger.kernel.org> # v3.15+
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> ---
>> drivers/mmc/host/sdhci-pxav3.c | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
>> index ca3424e7ef71..7b07325b4fba 100644
>> --- a/drivers/mmc/host/sdhci-pxav3.c
>> +++ b/drivers/mmc/host/sdhci-pxav3.c
>> @@ -118,6 +118,20 @@ static int mv_conf_mbus_windows(struct platform_device *pdev,
>> return 0;
>> }
>>
>> +static int armada_38x_quirks(struct sdhci_host *host)
>
> Seems like this function can be void instead of always returning 0.
In patch 4 "mmc: sdhci-pxav3: Modify clock settings for the SDR50 and
DDR50 modes", this function can return other values than 0.
I could change the prototype in patch 4, but it would also imply
removing the test of the return value in this patch and adding in back
patch 4. By returning a value in this patch, it reduced the amount of
change over the patches.
But if you still prefer than I this function return void in this
patch, I can do it.
Thanks,
Gregory
>
>> +{
>> + host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
>> + /*
>> + * According to erratum 'FE-2946959' both SDR50 and DDR50
>> + * modes require specific clock adjustments in SDIO3
>> + * Configuration register, if the adjustment is not done,
>> + * remove them from the capabilities.
>> + */
>> + host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
>> + host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
>> + return 0;
>> +}
>> +
>> static void pxav3_reset(struct sdhci_host *host, u8 mask)
>> {
>> struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
>> @@ -319,6 +333,9 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>> clk_prepare_enable(pxa->clk_core);
>>
>> if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
>> + ret = armada_38x_quirks(host);
>> + if (ret < 0)
>> + goto err_quirks;
>> ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
>> if (ret < 0)
>> goto err_mbus_win;
>> @@ -400,6 +417,7 @@ err_mbus_win:
>> if (!IS_ERR(pxa->clk_core))
>> clk_disable_unprepare(pxa->clk_core);
>> err_clk_get:
>> +err_quirks:
>> sdhci_pltfm_free(pdev);
>> return ret;
>> }
>> --
>> 2.1.0
>>
>
> Kind regards
> Uffe
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 1/7] mmc: sdhci-pxav3: Fix SDR50 and DDR50 capabilities for the Armada 38x flavor
@ 2015-01-29 8:31 ` Gregory CLEMENT
0 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-29 8:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ulf,
On 28/01/2015 21:36, Ulf Hansson wrote:
> On 23 January 2015 at 11:56, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
>> According to erratum 'FE-2946959' both SDR50 and DDR50 modes require
>> specific clock adjustments in SDIO3 Configuration register. However,
>> this register was not part of the device tree binding. Even if the
>> binding can (and will) be extended we still need handling the case
>> where this register was not available. In this case we use the
>> SDHCI_QUIRK_MISSING_CAPS quirk remove them from the capabilities.
>>
>> This commit is based on the work done by Marcin Wojtas<mw@semihalf.com>
>>
>> Fixes: 5491ce3f79ee ("mmc: sdhci-pxav3: add support for the Armada 38x SDHCI controller")
>> Cc: <stable@vger.kernel.org> # v3.15+
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> ---
>> drivers/mmc/host/sdhci-pxav3.c | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
>> index ca3424e7ef71..7b07325b4fba 100644
>> --- a/drivers/mmc/host/sdhci-pxav3.c
>> +++ b/drivers/mmc/host/sdhci-pxav3.c
>> @@ -118,6 +118,20 @@ static int mv_conf_mbus_windows(struct platform_device *pdev,
>> return 0;
>> }
>>
>> +static int armada_38x_quirks(struct sdhci_host *host)
>
> Seems like this function can be void instead of always returning 0.
In patch 4 "mmc: sdhci-pxav3: Modify clock settings for the SDR50 and
DDR50 modes", this function can return other values than 0.
I could change the prototype in patch 4, but it would also imply
removing the test of the return value in this patch and adding in back
patch 4. By returning a value in this patch, it reduced the amount of
change over the patches.
But if you still prefer than I this function return void in this
patch, I can do it.
Thanks,
Gregory
>
>> +{
>> + host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
>> + /*
>> + * According to erratum 'FE-2946959' both SDR50 and DDR50
>> + * modes require specific clock adjustments in SDIO3
>> + * Configuration register, if the adjustment is not done,
>> + * remove them from the capabilities.
>> + */
>> + host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
>> + host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
>> + return 0;
>> +}
>> +
>> static void pxav3_reset(struct sdhci_host *host, u8 mask)
>> {
>> struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
>> @@ -319,6 +333,9 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>> clk_prepare_enable(pxa->clk_core);
>>
>> if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
>> + ret = armada_38x_quirks(host);
>> + if (ret < 0)
>> + goto err_quirks;
>> ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
>> if (ret < 0)
>> goto err_mbus_win;
>> @@ -400,6 +417,7 @@ err_mbus_win:
>> if (!IS_ERR(pxa->clk_core))
>> clk_disable_unprepare(pxa->clk_core);
>> err_clk_get:
>> +err_quirks:
>> sdhci_pltfm_free(pdev);
>> return ret;
>> }
>> --
>> 2.1.0
>>
>
> Kind regards
> Uffe
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 1/7] mmc: sdhci-pxav3: Fix SDR50 and DDR50 capabilities for the Armada 38x flavor
2015-01-29 8:31 ` Gregory CLEMENT
@ 2015-01-29 9:31 ` Ulf Hansson
-1 siblings, 0 replies; 34+ messages in thread
From: Ulf Hansson @ 2015-01-29 9:31 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Chris Ball, linux-mmc, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai, Mark Rutland, devicetree, stable,
Marcin Wojtas
[...]
>> Seems like this function can be void instead of always returning 0.
>
> In patch 4 "mmc: sdhci-pxav3: Modify clock settings for the SDR50 and
> DDR50 modes", this function can return other values than 0.
>
> I could change the prototype in patch 4, but it would also imply
> removing the test of the return value in this patch and adding in back
> patch 4. By returning a value in this patch, it reduced the amount of
> change over the patches.
>
> But if you still prefer than I this function return void in this
> patch, I can do it.
No worries, let's keep it as an int. But then I have a few other
comments, see below.
>
>
> Thanks,
>
> Gregory
>
>
>>
>>> +{
>>> + host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
>>> + /*
>>> + * According to erratum 'FE-2946959' both SDR50 and DDR50
>>> + * modes require specific clock adjustments in SDIO3
>>> + * Configuration register, if the adjustment is not done,
>>> + * remove them from the capabilities.
>>> + */
>>> + host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
>>> + host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
>>> + return 0;
>>> +}
>>> +
>>> static void pxav3_reset(struct sdhci_host *host, u8 mask)
>>> {
>>> struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
>>> @@ -319,6 +333,9 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>>> clk_prepare_enable(pxa->clk_core);
>>>
>>> if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
>>> + ret = armada_38x_quirks(host);
>>> + if (ret < 0)
Since in patch 4 you return a proper error code, let's also adopt to
that here by changing to:
"if (IS_ERR(ret))
>>> + goto err_quirks;
>>> ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
>>> if (ret < 0)
>>> goto err_mbus_win;
>>> @@ -400,6 +417,7 @@ err_mbus_win:
>>> if (!IS_ERR(pxa->clk_core))
>>> clk_disable_unprepare(pxa->clk_core);
>>> err_clk_get:
>>> +err_quirks:
You don't need the new label, you could use "err_clk_get".
>>> sdhci_pltfm_free(pdev);
>>> return ret;
>>> }
>>> --
>>> 2.1.0
>>>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 1/7] mmc: sdhci-pxav3: Fix SDR50 and DDR50 capabilities for the Armada 38x flavor
@ 2015-01-29 9:31 ` Ulf Hansson
0 siblings, 0 replies; 34+ messages in thread
From: Ulf Hansson @ 2015-01-29 9:31 UTC (permalink / raw)
To: linux-arm-kernel
[...]
>> Seems like this function can be void instead of always returning 0.
>
> In patch 4 "mmc: sdhci-pxav3: Modify clock settings for the SDR50 and
> DDR50 modes", this function can return other values than 0.
>
> I could change the prototype in patch 4, but it would also imply
> removing the test of the return value in this patch and adding in back
> patch 4. By returning a value in this patch, it reduced the amount of
> change over the patches.
>
> But if you still prefer than I this function return void in this
> patch, I can do it.
No worries, let's keep it as an int. But then I have a few other
comments, see below.
>
>
> Thanks,
>
> Gregory
>
>
>>
>>> +{
>>> + host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
>>> + /*
>>> + * According to erratum 'FE-2946959' both SDR50 and DDR50
>>> + * modes require specific clock adjustments in SDIO3
>>> + * Configuration register, if the adjustment is not done,
>>> + * remove them from the capabilities.
>>> + */
>>> + host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
>>> + host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
>>> + return 0;
>>> +}
>>> +
>>> static void pxav3_reset(struct sdhci_host *host, u8 mask)
>>> {
>>> struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
>>> @@ -319,6 +333,9 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>>> clk_prepare_enable(pxa->clk_core);
>>>
>>> if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
>>> + ret = armada_38x_quirks(host);
>>> + if (ret < 0)
Since in patch 4 you return a proper error code, let's also adopt to
that here by changing to:
"if (IS_ERR(ret))
>>> + goto err_quirks;
>>> ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
>>> if (ret < 0)
>>> goto err_mbus_win;
>>> @@ -400,6 +417,7 @@ err_mbus_win:
>>> if (!IS_ERR(pxa->clk_core))
>>> clk_disable_unprepare(pxa->clk_core);
>>> err_clk_get:
>>> +err_quirks:
You don't need the new label, you could use "err_clk_get".
>>> sdhci_pltfm_free(pdev);
>>> return ret;
>>> }
>>> --
>>> 2.1.0
>>>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 1/7] mmc: sdhci-pxav3: Fix SDR50 and DDR50 capabilities for the Armada 38x flavor
2015-01-29 9:31 ` Ulf Hansson
@ 2015-01-29 9:42 ` Gregory CLEMENT
-1 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-29 9:42 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, linux-mmc, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai, Mark Rutland, devicetree, stable,
Marcin Wojtas
Hi Ulf,
On 29/01/2015 10:31, Ulf Hansson wrote:
> [...]
>
>>> Seems like this function can be void instead of always returning 0.
>>
>> In patch 4 "mmc: sdhci-pxav3: Modify clock settings for the SDR50 and
>> DDR50 modes", this function can return other values than 0.
>>
>> I could change the prototype in patch 4, but it would also imply
>> removing the test of the return value in this patch and adding in back
>> patch 4. By returning a value in this patch, it reduced the amount of
>> change over the patches.
>>
>> But if you still prefer than I this function return void in this
>> patch, I can do it.
>
> No worries, let's keep it as an int. But then I have a few other
> comments, see below.
OK
>
>>
>>
>> Thanks,
>>
>> Gregory
>>
>>
>>>
>>>> +{
>>>> + host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
>>>> + /*
>>>> + * According to erratum 'FE-2946959' both SDR50 and DDR50
>>>> + * modes require specific clock adjustments in SDIO3
>>>> + * Configuration register, if the adjustment is not done,
>>>> + * remove them from the capabilities.
>>>> + */
>>>> + host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
>>>> + host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
>>>> + return 0;
>>>> +}
>>>> +
>>>> static void pxav3_reset(struct sdhci_host *host, u8 mask)
>>>> {
>>>> struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
>>>> @@ -319,6 +333,9 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>>>> clk_prepare_enable(pxa->clk_core);
>>>>
>>>> if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
>>>> + ret = armada_38x_quirks(host);
>>>> + if (ret < 0)
>
> Since in patch 4 you return a proper error code, let's also adopt to
> that here by changing to:
>
> "if (IS_ERR(ret))
The function returns an int and IS_ERR expects a pointer. I am not sure
this macro would be appropriate here.
>
>>>> + goto err_quirks;
>>>> ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
>>>> if (ret < 0)
>>>> goto err_mbus_win;
>>>> @@ -400,6 +417,7 @@ err_mbus_win:
>>>> if (!IS_ERR(pxa->clk_core))
>>>> clk_disable_unprepare(pxa->clk_core);
>>>> err_clk_get:
>>>> +err_quirks:
>
> You don't need the new label, you could use "err_clk_get".
Right.
Thanks,
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 1/7] mmc: sdhci-pxav3: Fix SDR50 and DDR50 capabilities for the Armada 38x flavor
@ 2015-01-29 9:42 ` Gregory CLEMENT
0 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-29 9:42 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ulf,
On 29/01/2015 10:31, Ulf Hansson wrote:
> [...]
>
>>> Seems like this function can be void instead of always returning 0.
>>
>> In patch 4 "mmc: sdhci-pxav3: Modify clock settings for the SDR50 and
>> DDR50 modes", this function can return other values than 0.
>>
>> I could change the prototype in patch 4, but it would also imply
>> removing the test of the return value in this patch and adding in back
>> patch 4. By returning a value in this patch, it reduced the amount of
>> change over the patches.
>>
>> But if you still prefer than I this function return void in this
>> patch, I can do it.
>
> No worries, let's keep it as an int. But then I have a few other
> comments, see below.
OK
>
>>
>>
>> Thanks,
>>
>> Gregory
>>
>>
>>>
>>>> +{
>>>> + host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
>>>> + /*
>>>> + * According to erratum 'FE-2946959' both SDR50 and DDR50
>>>> + * modes require specific clock adjustments in SDIO3
>>>> + * Configuration register, if the adjustment is not done,
>>>> + * remove them from the capabilities.
>>>> + */
>>>> + host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
>>>> + host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
>>>> + return 0;
>>>> +}
>>>> +
>>>> static void pxav3_reset(struct sdhci_host *host, u8 mask)
>>>> {
>>>> struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
>>>> @@ -319,6 +333,9 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>>>> clk_prepare_enable(pxa->clk_core);
>>>>
>>>> if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
>>>> + ret = armada_38x_quirks(host);
>>>> + if (ret < 0)
>
> Since in patch 4 you return a proper error code, let's also adopt to
> that here by changing to:
>
> "if (IS_ERR(ret))
The function returns an int and IS_ERR expects a pointer. I am not sure
this macro would be appropriate here.
>
>>>> + goto err_quirks;
>>>> ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
>>>> if (ret < 0)
>>>> goto err_mbus_win;
>>>> @@ -400,6 +417,7 @@ err_mbus_win:
>>>> if (!IS_ERR(pxa->clk_core))
>>>> clk_disable_unprepare(pxa->clk_core);
>>>> err_clk_get:
>>>> +err_quirks:
>
> You don't need the new label, you could use "err_clk_get".
Right.
Thanks,
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <54CA006F.8060108-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]
* Re: [PATCH v2 1/7] mmc: sdhci-pxav3: Fix SDR50 and DDR50 capabilities for the Armada 38x flavor
2015-01-29 9:42 ` Gregory CLEMENT
@ 2015-01-29 10:01 ` Ulf Hansson
-1 siblings, 0 replies; 34+ messages in thread
From: Ulf Hansson @ 2015-01-29 10:01 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Chris Ball, linux-mmc, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
stable-u79uwXL29TY76Z2rM5mHXA, Marcin Wojtas
On 29 January 2015 at 10:42, Gregory CLEMENT
<gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Hi Ulf,
>
> On 29/01/2015 10:31, Ulf Hansson wrote:
>> [...]
>>
>>>> Seems like this function can be void instead of always returning 0.
>>>
>>> In patch 4 "mmc: sdhci-pxav3: Modify clock settings for the SDR50 and
>>> DDR50 modes", this function can return other values than 0.
>>>
>>> I could change the prototype in patch 4, but it would also imply
>>> removing the test of the return value in this patch and adding in back
>>> patch 4. By returning a value in this patch, it reduced the amount of
>>> change over the patches.
>>>
>>> But if you still prefer than I this function return void in this
>>> patch, I can do it.
>>
>> No worries, let's keep it as an int. But then I have a few other
>> comments, see below.
>
> OK
>
>>
>>>
>>>
>>> Thanks,
>>>
>>> Gregory
>>>
>>>
>>>>
>>>>> +{
>>>>> + host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
>>>>> + /*
>>>>> + * According to erratum 'FE-2946959' both SDR50 and DDR50
>>>>> + * modes require specific clock adjustments in SDIO3
>>>>> + * Configuration register, if the adjustment is not done,
>>>>> + * remove them from the capabilities.
>>>>> + */
>>>>> + host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
>>>>> + host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> static void pxav3_reset(struct sdhci_host *host, u8 mask)
>>>>> {
>>>>> struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
>>>>> @@ -319,6 +333,9 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>>>>> clk_prepare_enable(pxa->clk_core);
>>>>>
>>>>> if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
>>>>> + ret = armada_38x_quirks(host);
>>>>> + if (ret < 0)
>>
>> Since in patch 4 you return a proper error code, let's also adopt to
>> that here by changing to:
>>
>> "if (IS_ERR(ret))
>
> The function returns an int and IS_ERR expects a pointer. I am not sure
> this macro would be appropriate here.
You are right. Don't know what I was thinking. :-)
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 1/7] mmc: sdhci-pxav3: Fix SDR50 and DDR50 capabilities for the Armada 38x flavor
@ 2015-01-29 10:01 ` Ulf Hansson
0 siblings, 0 replies; 34+ messages in thread
From: Ulf Hansson @ 2015-01-29 10:01 UTC (permalink / raw)
To: linux-arm-kernel
On 29 January 2015 at 10:42, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> Hi Ulf,
>
> On 29/01/2015 10:31, Ulf Hansson wrote:
>> [...]
>>
>>>> Seems like this function can be void instead of always returning 0.
>>>
>>> In patch 4 "mmc: sdhci-pxav3: Modify clock settings for the SDR50 and
>>> DDR50 modes", this function can return other values than 0.
>>>
>>> I could change the prototype in patch 4, but it would also imply
>>> removing the test of the return value in this patch and adding in back
>>> patch 4. By returning a value in this patch, it reduced the amount of
>>> change over the patches.
>>>
>>> But if you still prefer than I this function return void in this
>>> patch, I can do it.
>>
>> No worries, let's keep it as an int. But then I have a few other
>> comments, see below.
>
> OK
>
>>
>>>
>>>
>>> Thanks,
>>>
>>> Gregory
>>>
>>>
>>>>
>>>>> +{
>>>>> + host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
>>>>> + /*
>>>>> + * According to erratum 'FE-2946959' both SDR50 and DDR50
>>>>> + * modes require specific clock adjustments in SDIO3
>>>>> + * Configuration register, if the adjustment is not done,
>>>>> + * remove them from the capabilities.
>>>>> + */
>>>>> + host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
>>>>> + host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> static void pxav3_reset(struct sdhci_host *host, u8 mask)
>>>>> {
>>>>> struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
>>>>> @@ -319,6 +333,9 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>>>>> clk_prepare_enable(pxa->clk_core);
>>>>>
>>>>> if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
>>>>> + ret = armada_38x_quirks(host);
>>>>> + if (ret < 0)
>>
>> Since in patch 4 you return a proper error code, let's also adopt to
>> that here by changing to:
>>
>> "if (IS_ERR(ret))
>
> The function returns an int and IS_ERR expects a pointer. I am not sure
> this macro would be appropriate here.
You are right. Don't know what I was thinking. :-)
Kind regards
Uffe
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 2/7] mmc: sdhci-pxav3: Fix Armada 38x controller's caps according to erratum ERR-7878951
2015-01-23 10:56 ` Gregory CLEMENT
@ 2015-01-23 10:56 ` Gregory CLEMENT
-1 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-23 10:56 UTC (permalink / raw)
To: Chris Ball, Ulf Hansson, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory CLEMENT
Cc: Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Marcin Wojtas,
stable-u79uwXL29TY76Z2rM5mHXA
From: Marcin Wojtas <mw-nYOzD4b6Jr9Wk0Htik3J/w@public.gmane.org>
According to erratum 'ERR-7878951' Armada 38x SDHCI controller has
different capabilities than the ones shown in its registers:
- it doesn't support the voltage switching: it can work either with
3.3V or 1.8V supply
- it doesn't support the SDR104 mode
- SDR50 mode doesn't need tuning
The SDHCI_QUIRK_MISSING_CAPS quirk is used for updating the
capabilities accordingly.
[gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org: port from 3.10]
Fixes: 5491ce3f79ee ("mmc: sdhci-pxav3: add support for the Armada 38x SDHCI controller")
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v3.15+
Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
drivers/mmc/host/sdhci-pxav3.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
index 7b07325b4fba..a53968010991 100644
--- a/drivers/mmc/host/sdhci-pxav3.c
+++ b/drivers/mmc/host/sdhci-pxav3.c
@@ -118,8 +118,11 @@ static int mv_conf_mbus_windows(struct platform_device *pdev,
return 0;
}
-static int armada_38x_quirks(struct sdhci_host *host)
+static int armada_38x_quirks(struct platform_device *pdev,
+ struct sdhci_host *host)
{
+ struct device_node *np = pdev->dev.of_node;
+
host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
/*
* According to erratum 'FE-2946959' both SDR50 and DDR50
@@ -129,6 +132,20 @@ static int armada_38x_quirks(struct sdhci_host *host)
*/
host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
+
+ /*
+ * According to erratum 'ERR-7878951' Armada 38x SDHCI
+ * controller has different capabilities than the ones shown
+ * in its registers
+ */
+ host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
+ if (of_get_property(np, "no-1-8-v", NULL)) {
+ host->caps &= ~SDHCI_CAN_VDD_180;
+ host->mmc->caps &= ~MMC_CAP_1_8V_DDR;
+ } else
+ host->caps &= ~SDHCI_CAN_VDD_330;
+ host->caps1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_USE_SDR50_TUNING);
+
return 0;
}
@@ -333,7 +350,7 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
clk_prepare_enable(pxa->clk_core);
if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
- ret = armada_38x_quirks(host);
+ ret = armada_38x_quirks(pdev, host);
if (ret < 0)
goto err_quirks;
ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
@@ -341,9 +358,6 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
goto err_mbus_win;
}
- /* enable 1/8V DDR capable */
- host->mmc->caps |= MMC_CAP_1_8V_DDR;
-
match = of_match_device(of_match_ptr(sdhci_pxav3_of_match), &pdev->dev);
if (match) {
ret = mmc_of_parse(host->mmc);
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 2/7] mmc: sdhci-pxav3: Fix Armada 38x controller's caps according to erratum ERR-7878951
@ 2015-01-23 10:56 ` Gregory CLEMENT
0 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-23 10:56 UTC (permalink / raw)
To: linux-arm-kernel
From: Marcin Wojtas <mw@semihalf.com>
According to erratum 'ERR-7878951' Armada 38x SDHCI controller has
different capabilities than the ones shown in its registers:
- it doesn't support the voltage switching: it can work either with
3.3V or 1.8V supply
- it doesn't support the SDR104 mode
- SDR50 mode doesn't need tuning
The SDHCI_QUIRK_MISSING_CAPS quirk is used for updating the
capabilities accordingly.
[gregory.clement at free-electrons.com: port from 3.10]
Fixes: 5491ce3f79ee ("mmc: sdhci-pxav3: add support for the Armada 38x SDHCI controller")
Cc: <stable@vger.kernel.org> # v3.15+
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
drivers/mmc/host/sdhci-pxav3.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
index 7b07325b4fba..a53968010991 100644
--- a/drivers/mmc/host/sdhci-pxav3.c
+++ b/drivers/mmc/host/sdhci-pxav3.c
@@ -118,8 +118,11 @@ static int mv_conf_mbus_windows(struct platform_device *pdev,
return 0;
}
-static int armada_38x_quirks(struct sdhci_host *host)
+static int armada_38x_quirks(struct platform_device *pdev,
+ struct sdhci_host *host)
{
+ struct device_node *np = pdev->dev.of_node;
+
host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
/*
* According to erratum 'FE-2946959' both SDR50 and DDR50
@@ -129,6 +132,20 @@ static int armada_38x_quirks(struct sdhci_host *host)
*/
host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
+
+ /*
+ * According to erratum 'ERR-7878951' Armada 38x SDHCI
+ * controller has different capabilities than the ones shown
+ * in its registers
+ */
+ host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
+ if (of_get_property(np, "no-1-8-v", NULL)) {
+ host->caps &= ~SDHCI_CAN_VDD_180;
+ host->mmc->caps &= ~MMC_CAP_1_8V_DDR;
+ } else
+ host->caps &= ~SDHCI_CAN_VDD_330;
+ host->caps1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_USE_SDR50_TUNING);
+
return 0;
}
@@ -333,7 +350,7 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
clk_prepare_enable(pxa->clk_core);
if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
- ret = armada_38x_quirks(host);
+ ret = armada_38x_quirks(pdev, host);
if (ret < 0)
goto err_quirks;
ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
@@ -341,9 +358,6 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
goto err_mbus_win;
}
- /* enable 1/8V DDR capable */
- host->mmc->caps |= MMC_CAP_1_8V_DDR;
-
match = of_match_device(of_match_ptr(sdhci_pxav3_of_match), &pdev->dev);
if (match) {
ret = mmc_of_parse(host->mmc);
--
2.1.0
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] mmc: sdhci-pxav3: Fix Armada 38x controller's caps according to erratum ERR-7878951
2015-01-23 10:56 ` Gregory CLEMENT
@ 2015-01-23 11:03 ` Mark Rutland
-1 siblings, 0 replies; 34+ messages in thread
From: Mark Rutland @ 2015-01-23 11:03 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Chris Ball, Ulf Hansson, linux-mmc, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai, devicetree, Marcin Wojtas, stable
[...]
> + /*
> + * According to erratum 'ERR-7878951' Armada 38x SDHCI
> + * controller has different capabilities than the ones shown
> + * in its registers
> + */
> + host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
> + if (of_get_property(np, "no-1-8-v", NULL)) {
Please use of_property_read_bool(np, "no-1-8-v")
> + host->caps &= ~SDHCI_CAN_VDD_180;
> + host->mmc->caps &= ~MMC_CAP_1_8V_DDR;
Is SDHCI_CAN_VDD_330 always set elsewhere in this case?
> + } else
> + host->caps &= ~SDHCI_CAN_VDD_330;
If one branch in an if-else pair is braced, both sides should be (as
Documentation/CodingStyle says). Please brace the else case.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 2/7] mmc: sdhci-pxav3: Fix Armada 38x controller's caps according to erratum ERR-7878951
@ 2015-01-23 11:03 ` Mark Rutland
0 siblings, 0 replies; 34+ messages in thread
From: Mark Rutland @ 2015-01-23 11:03 UTC (permalink / raw)
To: linux-arm-kernel
[...]
> + /*
> + * According to erratum 'ERR-7878951' Armada 38x SDHCI
> + * controller has different capabilities than the ones shown
> + * in its registers
> + */
> + host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
> + if (of_get_property(np, "no-1-8-v", NULL)) {
Please use of_property_read_bool(np, "no-1-8-v")
> + host->caps &= ~SDHCI_CAN_VDD_180;
> + host->mmc->caps &= ~MMC_CAP_1_8V_DDR;
Is SDHCI_CAN_VDD_330 always set elsewhere in this case?
> + } else
> + host->caps &= ~SDHCI_CAN_VDD_330;
If one branch in an if-else pair is braced, both sides should be (as
Documentation/CodingStyle says). Please brace the else case.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] mmc: sdhci-pxav3: Fix Armada 38x controller's caps according to erratum ERR-7878951
2015-01-23 11:03 ` Mark Rutland
@ 2015-01-23 11:12 ` Marcin Wojtas
-1 siblings, 0 replies; 34+ messages in thread
From: Marcin Wojtas @ 2015-01-23 11:12 UTC (permalink / raw)
To: Mark Rutland
Cc: Gregory CLEMENT, Chris Ball, Ulf Hansson, linux-mmc,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Thomas Petazzoni, Ezequiel Garcia, linux-arm-kernel,
Maxime Ripard, Boris BREZILLON, Lior Amsalem, Tawfik Bayouk,
Nadav Haklai, devicetree, stable
Hi Mark
2015-01-23 12:03 GMT+01:00 Mark Rutland <mark.rutland@arm.com>:
> [...]
>
>> + /*
>> + * According to erratum 'ERR-7878951' Armada 38x SDHCI
>> + * controller has different capabilities than the ones shown
>> + * in its registers
>> + */
>> + host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
>> + if (of_get_property(np, "no-1-8-v", NULL)) {
>
> Please use of_property_read_bool(np, "no-1-8-v")
>
>> + host->caps &= ~SDHCI_CAN_VDD_180;
>> + host->mmc->caps &= ~MMC_CAP_1_8V_DDR;
>
> Is SDHCI_CAN_VDD_330 always set elsewhere in this case?
Yes, it's always set in SDHCI_CAPABILITIES register of Armada 38x
(assigned to host->caps few lines above), as well as
SDHCI_CAN_VDD_180. The point is, that in reality they can't be used
simultaneously for this device, i.e. voltage switching is impossible.
Thanks,
Marcin
>
>> + } else
>> + host->caps &= ~SDHCI_CAN_VDD_330;
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 2/7] mmc: sdhci-pxav3: Fix Armada 38x controller's caps according to erratum ERR-7878951
@ 2015-01-23 11:12 ` Marcin Wojtas
0 siblings, 0 replies; 34+ messages in thread
From: Marcin Wojtas @ 2015-01-23 11:12 UTC (permalink / raw)
To: linux-arm-kernel
Hi Mark
2015-01-23 12:03 GMT+01:00 Mark Rutland <mark.rutland@arm.com>:
> [...]
>
>> + /*
>> + * According to erratum 'ERR-7878951' Armada 38x SDHCI
>> + * controller has different capabilities than the ones shown
>> + * in its registers
>> + */
>> + host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
>> + if (of_get_property(np, "no-1-8-v", NULL)) {
>
> Please use of_property_read_bool(np, "no-1-8-v")
>
>> + host->caps &= ~SDHCI_CAN_VDD_180;
>> + host->mmc->caps &= ~MMC_CAP_1_8V_DDR;
>
> Is SDHCI_CAN_VDD_330 always set elsewhere in this case?
Yes, it's always set in SDHCI_CAPABILITIES register of Armada 38x
(assigned to host->caps few lines above), as well as
SDHCI_CAN_VDD_180. The point is, that in reality they can't be used
simultaneously for this device, i.e. voltage switching is impossible.
Thanks,
Marcin
>
>> + } else
>> + host->caps &= ~SDHCI_CAN_VDD_330;
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] mmc: sdhci-pxav3: Fix Armada 38x controller's caps according to erratum ERR-7878951
2015-01-23 10:56 ` Gregory CLEMENT
@ 2015-01-23 11:33 ` Marcin Wojtas
-1 siblings, 0 replies; 34+ messages in thread
From: Marcin Wojtas @ 2015-01-23 11:33 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Chris Ball, Ulf Hansson, linux-mmc, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai, Mark Rutland, devicetree, stable
Dear Gregory,
> @@ -118,8 +118,11 @@ static int mv_conf_mbus_windows(struct platform_device *pdev,
> return 0;
> }
>
> -static int armada_38x_quirks(struct sdhci_host *host)
> +static int armada_38x_quirks(struct platform_device *pdev,
> + struct sdhci_host *host)
> {
> + struct device_node *np = pdev->dev.of_node;
> +
> host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
> /*
> * According to erratum 'FE-2946959' both SDR50 and DDR50
> @@ -129,6 +132,20 @@ static int armada_38x_quirks(struct sdhci_host *host)
> */
> host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
> host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
> +
> + /*
> + * According to erratum 'ERR-7878951' Armada 38x SDHCI
> + * controller has different capabilities than the ones shown
> + * in its registers
> + */
> + host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
> + if (of_get_property(np, "no-1-8-v", NULL)) {
> + host->caps &= ~SDHCI_CAN_VDD_180;
> + host->mmc->caps &= ~MMC_CAP_1_8V_DDR;
> + } else
> + host->caps &= ~SDHCI_CAN_VDD_330;
In this "else" there is one thing missing
host->mmc->caps |= MMC_CAP_1_8V_DDR;
because it's not set by default, however it's not a must - please see
my remark below.
> + host->caps1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_USE_SDR50_TUNING);
> +
> return 0;
> }
>
> @@ -333,7 +350,7 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
> clk_prepare_enable(pxa->clk_core);
>
> if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
> - ret = armada_38x_quirks(host);
> + ret = armada_38x_quirks(pdev, host);
> if (ret < 0)
> goto err_quirks;
> ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
> @@ -341,9 +358,6 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
> goto err_mbus_win;
> }
>
> - /* enable 1/8V DDR capable */
> - host->mmc->caps |= MMC_CAP_1_8V_DDR;
> -
Now, with this change MMC_CAP_1_8V_DDR is disabled for devices other
than Armada 38x, that use sdhci-pxav3. There are two options:
1. Move those lines above the place where armada_38x_quirks is called
(IMO the easiest option - 'else' in this function would not need to be
supplemented, as I pointed above)
2. Leave it "as is" here, but a condition below is needed (+
supplementation of 'else' in armada_38x_quirks)
if (!of_device_is_compatible(np, "marvell,armada-380-sdhci"))
Best regards,
Marcin
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 2/7] mmc: sdhci-pxav3: Fix Armada 38x controller's caps according to erratum ERR-7878951
@ 2015-01-23 11:33 ` Marcin Wojtas
0 siblings, 0 replies; 34+ messages in thread
From: Marcin Wojtas @ 2015-01-23 11:33 UTC (permalink / raw)
To: linux-arm-kernel
Dear Gregory,
> @@ -118,8 +118,11 @@ static int mv_conf_mbus_windows(struct platform_device *pdev,
> return 0;
> }
>
> -static int armada_38x_quirks(struct sdhci_host *host)
> +static int armada_38x_quirks(struct platform_device *pdev,
> + struct sdhci_host *host)
> {
> + struct device_node *np = pdev->dev.of_node;
> +
> host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
> /*
> * According to erratum 'FE-2946959' both SDR50 and DDR50
> @@ -129,6 +132,20 @@ static int armada_38x_quirks(struct sdhci_host *host)
> */
> host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
> host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
> +
> + /*
> + * According to erratum 'ERR-7878951' Armada 38x SDHCI
> + * controller has different capabilities than the ones shown
> + * in its registers
> + */
> + host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
> + if (of_get_property(np, "no-1-8-v", NULL)) {
> + host->caps &= ~SDHCI_CAN_VDD_180;
> + host->mmc->caps &= ~MMC_CAP_1_8V_DDR;
> + } else
> + host->caps &= ~SDHCI_CAN_VDD_330;
In this "else" there is one thing missing
host->mmc->caps |= MMC_CAP_1_8V_DDR;
because it's not set by default, however it's not a must - please see
my remark below.
> + host->caps1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_USE_SDR50_TUNING);
> +
> return 0;
> }
>
> @@ -333,7 +350,7 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
> clk_prepare_enable(pxa->clk_core);
>
> if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
> - ret = armada_38x_quirks(host);
> + ret = armada_38x_quirks(pdev, host);
> if (ret < 0)
> goto err_quirks;
> ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
> @@ -341,9 +358,6 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
> goto err_mbus_win;
> }
>
> - /* enable 1/8V DDR capable */
> - host->mmc->caps |= MMC_CAP_1_8V_DDR;
> -
Now, with this change MMC_CAP_1_8V_DDR is disabled for devices other
than Armada 38x, that use sdhci-pxav3. There are two options:
1. Move those lines above the place where armada_38x_quirks is called
(IMO the easiest option - 'else' in this function would not need to be
supplemented, as I pointed above)
2. Leave it "as is" here, but a condition below is needed (+
supplementation of 'else' in armada_38x_quirks)
if (!of_device_is_compatible(np, "marvell,armada-380-sdhci"))
Best regards,
Marcin
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] mmc: sdhci-pxav3: Fix Armada 38x controller's caps according to erratum ERR-7878951
2015-01-23 11:33 ` Marcin Wojtas
@ 2015-01-23 14:18 ` Gregory CLEMENT
-1 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-23 14:18 UTC (permalink / raw)
To: Marcin Wojtas
Cc: Chris Ball, Ulf Hansson, linux-mmc, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai, Mark Rutland, devicetree, stable
Hi Marcin,
On 23/01/2015 12:33, Marcin Wojtas wrote:
> Dear Gregory,
>
>
>> @@ -118,8 +118,11 @@ static int mv_conf_mbus_windows(struct platform_device *pdev,
>> return 0;
>> }
>>
>> -static int armada_38x_quirks(struct sdhci_host *host)
>> +static int armada_38x_quirks(struct platform_device *pdev,
>> + struct sdhci_host *host)
>> {
>> + struct device_node *np = pdev->dev.of_node;
>> +
>> host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
>> /*
>> * According to erratum 'FE-2946959' both SDR50 and DDR50
>> @@ -129,6 +132,20 @@ static int armada_38x_quirks(struct sdhci_host *host)
>> */
>> host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
>> host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
>> +
>> + /*
>> + * According to erratum 'ERR-7878951' Armada 38x SDHCI
>> + * controller has different capabilities than the ones shown
>> + * in its registers
>> + */
>> + host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
>> + if (of_get_property(np, "no-1-8-v", NULL)) {
>> + host->caps &= ~SDHCI_CAN_VDD_180;
>> + host->mmc->caps &= ~MMC_CAP_1_8V_DDR;
>> + } else
>> + host->caps &= ~SDHCI_CAN_VDD_330;
>
> In this "else" there is one thing missing
> host->mmc->caps |= MMC_CAP_1_8V_DDR;
> because it's not set by default, however it's not a must - please see
> my remark below.
>
>> + host->caps1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_USE_SDR50_TUNING);
>> +
>> return 0;
>> }
>>
>> @@ -333,7 +350,7 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>> clk_prepare_enable(pxa->clk_core);
>>
>> if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
>> - ret = armada_38x_quirks(host);
>> + ret = armada_38x_quirks(pdev, host);
>> if (ret < 0)
>> goto err_quirks;
>> ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
>> @@ -341,9 +358,6 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>> goto err_mbus_win;
>> }
>>
>> - /* enable 1/8V DDR capable */
>> - host->mmc->caps |= MMC_CAP_1_8V_DDR;
>> -
>
> Now, with this change MMC_CAP_1_8V_DDR is disabled for devices other
> than Armada 38x, that use sdhci-pxav3. There are two options:
> 1. Move those lines above the place where armada_38x_quirks is called
> (IMO the easiest option - 'else' in this function would not need to be
> supplemented, as I pointed above)
> 2. Leave it "as is" here, but a condition below is needed (+
> supplementation of 'else' in armada_38x_quirks)
> if (!of_device_is_compatible(np, "marvell,armada-380-sdhci"))
>
I will take the first option.
A third version is coming soon addressing also Mark's comments.
Thanks,
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 2/7] mmc: sdhci-pxav3: Fix Armada 38x controller's caps according to erratum ERR-7878951
@ 2015-01-23 14:18 ` Gregory CLEMENT
0 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-23 14:18 UTC (permalink / raw)
To: linux-arm-kernel
Hi Marcin,
On 23/01/2015 12:33, Marcin Wojtas wrote:
> Dear Gregory,
>
>
>> @@ -118,8 +118,11 @@ static int mv_conf_mbus_windows(struct platform_device *pdev,
>> return 0;
>> }
>>
>> -static int armada_38x_quirks(struct sdhci_host *host)
>> +static int armada_38x_quirks(struct platform_device *pdev,
>> + struct sdhci_host *host)
>> {
>> + struct device_node *np = pdev->dev.of_node;
>> +
>> host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
>> /*
>> * According to erratum 'FE-2946959' both SDR50 and DDR50
>> @@ -129,6 +132,20 @@ static int armada_38x_quirks(struct sdhci_host *host)
>> */
>> host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
>> host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
>> +
>> + /*
>> + * According to erratum 'ERR-7878951' Armada 38x SDHCI
>> + * controller has different capabilities than the ones shown
>> + * in its registers
>> + */
>> + host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
>> + if (of_get_property(np, "no-1-8-v", NULL)) {
>> + host->caps &= ~SDHCI_CAN_VDD_180;
>> + host->mmc->caps &= ~MMC_CAP_1_8V_DDR;
>> + } else
>> + host->caps &= ~SDHCI_CAN_VDD_330;
>
> In this "else" there is one thing missing
> host->mmc->caps |= MMC_CAP_1_8V_DDR;
> because it's not set by default, however it's not a must - please see
> my remark below.
>
>> + host->caps1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_USE_SDR50_TUNING);
>> +
>> return 0;
>> }
>>
>> @@ -333,7 +350,7 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>> clk_prepare_enable(pxa->clk_core);
>>
>> if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
>> - ret = armada_38x_quirks(host);
>> + ret = armada_38x_quirks(pdev, host);
>> if (ret < 0)
>> goto err_quirks;
>> ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
>> @@ -341,9 +358,6 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>> goto err_mbus_win;
>> }
>>
>> - /* enable 1/8V DDR capable */
>> - host->mmc->caps |= MMC_CAP_1_8V_DDR;
>> -
>
> Now, with this change MMC_CAP_1_8V_DDR is disabled for devices other
> than Armada 38x, that use sdhci-pxav3. There are two options:
> 1. Move those lines above the place where armada_38x_quirks is called
> (IMO the easiest option - 'else' in this function would not need to be
> supplemented, as I pointed above)
> 2. Leave it "as is" here, but a condition below is needed (+
> supplementation of 'else' in armada_38x_quirks)
> if (!of_device_is_compatible(np, "marvell,armada-380-sdhci"))
>
I will take the first option.
A third version is coming soon addressing also Mark's comments.
Thanks,
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 4/7] mmc: sdhci-pxav3: Modify clock settings for the SDR50 and DDR50 modes
2015-01-23 10:56 ` Gregory CLEMENT
@ 2015-01-23 10:56 ` Gregory CLEMENT
-1 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-23 10:56 UTC (permalink / raw)
To: Chris Ball, Ulf Hansson, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory CLEMENT
Cc: Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Marcin Wojtas
From: Marcin Wojtas <mw-nYOzD4b6Jr9Wk0Htik3J/w@public.gmane.org>
According to erratum 'FE-2946959' both SDR50 and DDR50 modes require
specific clock adjustments in SDIO3 Configuration register.
This commit add the support of this register and for SDR50 or DDR50
mode use it as suggested by the erratum:
- Set the SDIO3 Clock Inv field in SDIO3 Configuration register to not
inverted.
- Set the Sample FeedBack Clock field to 0x1
[gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org: port from 3.10]
Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
drivers/mmc/host/sdhci-pxav3.c | 60 ++++++++++++++++++++++++++++++++++++------
1 file changed, 52 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
index a53968010991..2855e56dd2db 100644
--- a/drivers/mmc/host/sdhci-pxav3.c
+++ b/drivers/mmc/host/sdhci-pxav3.c
@@ -62,6 +62,7 @@ struct sdhci_pxa {
struct clk *clk_core;
struct clk *clk_io;
u8 power_mode;
+ void __iomem *sdio3_conf_reg;
};
/*
@@ -72,6 +73,14 @@ struct sdhci_pxa {
#define SDHCI_WINDOW_BASE(i) (0x84 + ((i) << 3))
#define SDHCI_MAX_WIN_NUM 8
+/*
+ * Fields below belong to SDIO3 Configuration Register (third register
+ * region for the Armada 38x flavor)
+ */
+
+#define SDIO3_CONF_CLK_INV BIT(0)
+#define SDIO3_CONF_SD_FB_CLK BIT(2)
+
static int mv_conf_mbus_windows(struct platform_device *pdev,
const struct mbus_dram_target_info *dram)
{
@@ -122,16 +131,31 @@ static int armada_38x_quirks(struct platform_device *pdev,
struct sdhci_host *host)
{
struct device_node *np = pdev->dev.of_node;
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_pxa *pxa = pltfm_host->priv;
+ struct resource *res;
host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
- /*
- * According to erratum 'FE-2946959' both SDR50 and DDR50
- * modes require specific clock adjustments in SDIO3
- * Configuration register, if the adjustment is not done,
- * remove them from the capabilities.
- */
- host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
- host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+ "conf-sdio3");
+ if (res) {
+ pxa->sdio3_conf_reg = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(pxa->sdio3_conf_reg))
+ return PTR_ERR(pxa->sdio3_conf_reg);
+ } else {
+ /*
+ * According to erratum 'FE-2946959' both SDR50 and DDR50
+ * modes require specific clock adjustments in SDIO3
+ * Configuration register, if the adjustment is not done,
+ * remove them from the capabilities.
+ */
+ host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
+ host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
+
+ dev_warn(&pdev->dev, "conf-sdio3 register not found\n");
+ dev_warn(&pdev->dev, "disabling SDR50 and DDR50 modes\n");
+ dev_warn(&pdev->dev, "consider updating your dtb\n");
+ }
/*
* According to erratum 'ERR-7878951' Armada 38x SDHCI
@@ -225,6 +249,8 @@ static void pxav3_gen_init_74_clocks(struct sdhci_host *host, u8 power_mode)
static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_pxa *pxa = pltfm_host->priv;
u16 ctrl_2;
/*
@@ -254,6 +280,24 @@ static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
break;
}
+ /*
+ * Update SDIO3 Configuration register according to erratum
+ * FE-2946959
+ */
+ if (pxa->sdio3_conf_reg) {
+ u8 reg_val = readb(pxa->sdio3_conf_reg);
+
+ if (uhs == MMC_TIMING_UHS_SDR50 ||
+ uhs == MMC_TIMING_UHS_DDR50) {
+ reg_val &= ~SDIO3_CONF_CLK_INV;
+ reg_val |= SDIO3_CONF_SD_FB_CLK;
+ } else {
+ reg_val |= SDIO3_CONF_CLK_INV;
+ reg_val &= ~SDIO3_CONF_SD_FB_CLK;
+ }
+ writeb(reg_val, pxa->sdio3_conf_reg);
+ }
+
sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
dev_dbg(mmc_dev(host->mmc),
"%s uhs = %d, ctrl_2 = %04X\n",
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 4/7] mmc: sdhci-pxav3: Modify clock settings for the SDR50 and DDR50 modes
@ 2015-01-23 10:56 ` Gregory CLEMENT
0 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-23 10:56 UTC (permalink / raw)
To: linux-arm-kernel
From: Marcin Wojtas <mw@semihalf.com>
According to erratum 'FE-2946959' both SDR50 and DDR50 modes require
specific clock adjustments in SDIO3 Configuration register.
This commit add the support of this register and for SDR50 or DDR50
mode use it as suggested by the erratum:
- Set the SDIO3 Clock Inv field in SDIO3 Configuration register to not
inverted.
- Set the Sample FeedBack Clock field to 0x1
[gregory.clement at free-electrons.com: port from 3.10]
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
drivers/mmc/host/sdhci-pxav3.c | 60 ++++++++++++++++++++++++++++++++++++------
1 file changed, 52 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
index a53968010991..2855e56dd2db 100644
--- a/drivers/mmc/host/sdhci-pxav3.c
+++ b/drivers/mmc/host/sdhci-pxav3.c
@@ -62,6 +62,7 @@ struct sdhci_pxa {
struct clk *clk_core;
struct clk *clk_io;
u8 power_mode;
+ void __iomem *sdio3_conf_reg;
};
/*
@@ -72,6 +73,14 @@ struct sdhci_pxa {
#define SDHCI_WINDOW_BASE(i) (0x84 + ((i) << 3))
#define SDHCI_MAX_WIN_NUM 8
+/*
+ * Fields below belong to SDIO3 Configuration Register (third register
+ * region for the Armada 38x flavor)
+ */
+
+#define SDIO3_CONF_CLK_INV BIT(0)
+#define SDIO3_CONF_SD_FB_CLK BIT(2)
+
static int mv_conf_mbus_windows(struct platform_device *pdev,
const struct mbus_dram_target_info *dram)
{
@@ -122,16 +131,31 @@ static int armada_38x_quirks(struct platform_device *pdev,
struct sdhci_host *host)
{
struct device_node *np = pdev->dev.of_node;
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_pxa *pxa = pltfm_host->priv;
+ struct resource *res;
host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
- /*
- * According to erratum 'FE-2946959' both SDR50 and DDR50
- * modes require specific clock adjustments in SDIO3
- * Configuration register, if the adjustment is not done,
- * remove them from the capabilities.
- */
- host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
- host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+ "conf-sdio3");
+ if (res) {
+ pxa->sdio3_conf_reg = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(pxa->sdio3_conf_reg))
+ return PTR_ERR(pxa->sdio3_conf_reg);
+ } else {
+ /*
+ * According to erratum 'FE-2946959' both SDR50 and DDR50
+ * modes require specific clock adjustments in SDIO3
+ * Configuration register, if the adjustment is not done,
+ * remove them from the capabilities.
+ */
+ host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
+ host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
+
+ dev_warn(&pdev->dev, "conf-sdio3 register not found\n");
+ dev_warn(&pdev->dev, "disabling SDR50 and DDR50 modes\n");
+ dev_warn(&pdev->dev, "consider updating your dtb\n");
+ }
/*
* According to erratum 'ERR-7878951' Armada 38x SDHCI
@@ -225,6 +249,8 @@ static void pxav3_gen_init_74_clocks(struct sdhci_host *host, u8 power_mode)
static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_pxa *pxa = pltfm_host->priv;
u16 ctrl_2;
/*
@@ -254,6 +280,24 @@ static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
break;
}
+ /*
+ * Update SDIO3 Configuration register according to erratum
+ * FE-2946959
+ */
+ if (pxa->sdio3_conf_reg) {
+ u8 reg_val = readb(pxa->sdio3_conf_reg);
+
+ if (uhs == MMC_TIMING_UHS_SDR50 ||
+ uhs == MMC_TIMING_UHS_DDR50) {
+ reg_val &= ~SDIO3_CONF_CLK_INV;
+ reg_val |= SDIO3_CONF_SD_FB_CLK;
+ } else {
+ reg_val |= SDIO3_CONF_CLK_INV;
+ reg_val &= ~SDIO3_CONF_SD_FB_CLK;
+ }
+ writeb(reg_val, pxa->sdio3_conf_reg);
+ }
+
sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
dev_dbg(mmc_dev(host->mmc),
"%s uhs = %d, ctrl_2 = %04X\n",
--
2.1.0
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 5/7] ARM: mvebu: Use macros for interrupt flags on Armada 38x sdhci node
2015-01-23 10:56 ` Gregory CLEMENT
@ 2015-01-23 10:56 ` Gregory CLEMENT
-1 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-23 10:56 UTC (permalink / raw)
To: Chris Ball, Ulf Hansson, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory CLEMENT
Cc: Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA
Instead of hardcoding the values of the interrupt flags, use the
macros provided by <include/dt-bindings/interrupt-controller/irq.h>
and <include/dt-bindings/interrupt-controller/arm-gic.h> for the
Armada 38x SDHCI node.
Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
arch/arm/boot/dts/armada-38x.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index 79c4acf186e2..71d4eca5b497 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -516,7 +516,7 @@
sdhci@d8000 {
compatible = "marvell,armada-380-sdhci";
reg = <0xd8000 0x1000>, <0xdc000 0x100>;
- interrupts = <0 25 0x4>;
+ interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&gateclk 17>;
mrvl,clk-delay-cycles = <0x1F>;
status = "disabled";
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 5/7] ARM: mvebu: Use macros for interrupt flags on Armada 38x sdhci node
@ 2015-01-23 10:56 ` Gregory CLEMENT
0 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-23 10:56 UTC (permalink / raw)
To: linux-arm-kernel
Instead of hardcoding the values of the interrupt flags, use the
macros provided by <include/dt-bindings/interrupt-controller/irq.h>
and <include/dt-bindings/interrupt-controller/arm-gic.h> for the
Armada 38x SDHCI node.
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
arch/arm/boot/dts/armada-38x.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index 79c4acf186e2..71d4eca5b497 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -516,7 +516,7 @@
sdhci at d8000 {
compatible = "marvell,armada-380-sdhci";
reg = <0xd8000 0x1000>, <0xdc000 0x100>;
- interrupts = <0 25 0x4>;
+ interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&gateclk 17>;
mrvl,clk-delay-cycles = <0x1F>;
status = "disabled";
--
2.1.0
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 7/7] ARM: mvebu: Add Device Tree description of SDHCI for Armada 388 RD
2015-01-23 10:56 ` Gregory CLEMENT
@ 2015-01-23 10:56 ` Gregory CLEMENT
-1 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-23 10:56 UTC (permalink / raw)
To: Chris Ball, Ulf Hansson, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory CLEMENT
Cc: Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA
The Device Tree description of SDHCI on Armada 388 RD board was
missing. This commit adds the node for it.
Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
arch/arm/boot/dts/armada-388-rd.dts | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/armada-388-rd.dts b/arch/arm/boot/dts/armada-388-rd.dts
index c98a8f8d01a9..7dfede145ea3 100644
--- a/arch/arm/boot/dts/armada-388-rd.dts
+++ b/arch/arm/boot/dts/armada-388-rd.dts
@@ -51,6 +51,16 @@
clock-frequency = <100000>;
};
+ sdhci@d8000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&sdhci_pins>;
+ broken-cd;
+ no-1-8-v;
+ wp-inverted;
+ bus-width = <8>;
+ status = "okay";
+ };
+
serial@12000 {
status = "okay";
};
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 7/7] ARM: mvebu: Add Device Tree description of SDHCI for Armada 388 RD
@ 2015-01-23 10:56 ` Gregory CLEMENT
0 siblings, 0 replies; 34+ messages in thread
From: Gregory CLEMENT @ 2015-01-23 10:56 UTC (permalink / raw)
To: linux-arm-kernel
The Device Tree description of SDHCI on Armada 388 RD board was
missing. This commit adds the node for it.
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
arch/arm/boot/dts/armada-388-rd.dts | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/armada-388-rd.dts b/arch/arm/boot/dts/armada-388-rd.dts
index c98a8f8d01a9..7dfede145ea3 100644
--- a/arch/arm/boot/dts/armada-388-rd.dts
+++ b/arch/arm/boot/dts/armada-388-rd.dts
@@ -51,6 +51,16 @@
clock-frequency = <100000>;
};
+ sdhci at d8000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&sdhci_pins>;
+ broken-cd;
+ no-1-8-v;
+ wp-inverted;
+ bus-width = <8>;
+ status = "okay";
+ };
+
serial at 12000 {
status = "okay";
};
--
2.1.0
^ permalink raw reply related [flat|nested] 34+ messages in thread