[PATCH 0/2] Versatile Express fixes

[PATCH 0/2] Versatile Express fixes

[Pawel Moll]

Hi Arnd, Olof,

These are two fixes for small problems with VE that has been brought
to my attention recently. It would be great to get them pulled into
one of the 3.9-rcX, but as they are not really critical they could
wait till 3.10 if you feel it's the best thing to do.

Thanks!

Pawel

The following changes since commit f6161aa153581da4a3867a2d1a7caf4be19b6ec9:

  Linux 3.9-rc2 (2013-03-10 16:54:19 -0700)

are available in the git repository at:

  git://git.linaro.org/people/pawelmoll/linux.git tags/vexpress/fixes-for-3.9-rc3

for you to fetch changes up to f5107e0e2e9d4f16fd7ebae8059b17faa6f8abd1:

  ARM: vexpress: Remove A9 compatible value for V2P-CA5s PMU node (2013-03-15 16:12:11 +0000)

----------------------------------------------------------------
Versatile Express related fixes for 3.9-rc3
* Fix PMU node in vexpress-v2p-ca5s.dts
* Enforce required regulator framework when MMCI driver is enabled

----------------------------------------------------------------
Pawel Moll (2):
      ARM: vexpress: Enable regulator framework when MMCI is in use
      ARM: vexpress: Remove A9 compatible value for V2P-CA5s PMU node

 arch/arm/boot/dts/vexpress-v2p-ca5s.dts |    2 +-
 arch/arm/mach-vexpress/Kconfig          |    1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

[PATCH 1/2] ARM: vexpress: Enable regulator framework when MMCI is in use

[Pawel Moll]

As the MMCI driver obtains some of the platform data from
a regulator (described by the device tree), the framework must
be present - otherwise device's initialisation will fail.

Signed-off-by: Pawel Moll <pawel.moll@arm.com>
---
 arch/arm/mach-vexpress/Kconfig |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
index 52d315b..46ab88d 100644
--- a/arch/arm/mach-vexpress/Kconfig
+++ b/arch/arm/mach-vexpress/Kconfig
@@ -17,6 +17,7 @@ config ARCH_VEXPRESS
 	select NO_IOPORT
 	select PLAT_VERSATILE
 	select PLAT_VERSATILE_CLCD
+	select REGULATOR if MMC_ARMMMCI
 	select REGULATOR_FIXED_VOLTAGE if REGULATOR
 	select VEXPRESS_CONFIG
 	help
-- 
1.7.10.4

[PATCH 1/2] ARM: vexpress: Enable regulator framework when MMCI is in use

[Russell King - ARM Linux]

On Fri, Mar 15, 2013 at 04:38:59PM +0000, Pawel Moll wrote:
> As the MMCI driver obtains some of the platform data from
> a regulator (described by the device tree), the framework must
> be present - otherwise device's initialisation will fail.

More description is required here.  How does it fail.  Please describe
the problem fully.

MMCI should work without regulator support.

[PATCH v2 1/2] ARM: vexpress: Enable regulator framework when MMCI is in use

[Pawel Moll]

The MMCI driver, when used with a Device Tree described device, relies
on the "vmmc" voltage regulator supply to set the OCR register voltage bits,
using MMC core's mmc_regulator_get_supply() function.

Without the regulator framework present there are no valid operating
voltages reported and the device initialisation fails:

mmci-pl18x 1c050000.mmci: mmc0: PL180 manf 41 rev0 at 0x1c050000 irq 41,42 (pio)
mmc0: host doesn't support card's voltages
mmc0: error -22 whilst initialising MMC card

Signed-off-by: Pawel Moll <pawel.moll@arm.com>
---
 arch/arm/mach-vexpress/Kconfig |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
index 52d315b..46ab88d 100644
--- a/arch/arm/mach-vexpress/Kconfig
+++ b/arch/arm/mach-vexpress/Kconfig
@@ -17,6 +17,7 @@ config ARCH_VEXPRESS
 	select NO_IOPORT
 	select PLAT_VERSATILE
 	select PLAT_VERSATILE_CLCD
+	select REGULATOR if MMC_ARMMMCI
 	select REGULATOR_FIXED_VOLTAGE if REGULATOR
 	select VEXPRESS_CONFIG
 	help
-- 
1.7.10.4

[PATCH v2 1/2] ARM: vexpress: Enable regulator framework when MMCI is in use

[Russell King - ARM Linux]

On Mon, Mar 18, 2013 at 06:00:55PM +0000, Pawel Moll wrote:
> The MMCI driver, when used with a Device Tree described device, relies
> on the "vmmc" voltage regulator supply to set the OCR register voltage bits,
> using MMC core's mmc_regulator_get_supply() function.
> 
> Without the regulator framework present there are no valid operating
> voltages reported and the device initialisation fails:
> 
> mmci-pl18x 1c050000.mmci: mmc0: PL180 manf 41 rev0 at 0x1c050000 irq 41,42 (pio)
> mmc0: host doesn't support card's voltages
> mmc0: error -22 whilst initialising MMC card

Okay, this isn't going to work, because this means it's broken on a whole
load of platforms.  What it means is that this commit:

commit 599c1d5c750ddf528c7c6d3cdc466708f0502e66
Author: Ulf Hansson <ulf.hansson@linaro.org>
Date:   Mon Jan 7 16:22:50 2013 +0100

    ARM: 7620/1: mmc: mmci: Convert to use mmc_regulator_get_supply
    
    By using the mmc_regulator_get_supply API we are able to do some
    cleanups of the regulator code. Additionally let the regulator
    API handle the error printing.
    
    Cc: Chris Ball <cjb@laptop.org>
    Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
    Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

is actually wrong and probably needs reverting.

[PATCH v2 1/2] ARM: vexpress: Enable regulator framework when MMCI is in use

[Pawel Moll]

On Mon, 2013-03-18 at 18:03 +0000, Russell King - ARM Linux wrote:
> > Without the regulator framework present there are no valid operating
> > voltages reported and the device initialisation fails:
> > 
> > mmci-pl18x 1c050000.mmci: mmc0: PL180 manf 41 rev0 at 0x1c050000 irq 41,42 (pio)
> > mmc0: host doesn't support card's voltages
> > mmc0: error -22 whilst initialising MMC card
> 
> Okay, this isn't going to work, because this means it's broken on a whole
> load of platforms.  What it means is that this commit:
> 
> commit 599c1d5c750ddf528c7c6d3cdc466708f0502e66
> Author: Ulf Hansson <ulf.hansson@linaro.org>
> Date:   Mon Jan 7 16:22:50 2013 +0100
> 
>     ARM: 7620/1: mmc: mmci: Convert to use mmc_regulator_get_supply
>     
>     By using the mmc_regulator_get_supply API we are able to do some
>     cleanups of the regulator code. Additionally let the regulator
>     API handle the error printing.
>     
>     Cc: Chris Ball <cjb@laptop.org>
>     Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>     Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> is actually wrong and probably needs reverting.

Reverting it will not change anything as the driver had the same
dependency before, just using :

@@ -1391,29 +1379,13 @@ static int mmci_probe(struct amba_device *dev,
 	} else
 		dev_warn(&dev->dev, "could not get default pinstate\n");
 
-#ifdef CONFIG_REGULATOR
-	/* If we're using the regulator framework, try to fetch a regulator */
-	host->vcc = regulator_get(&dev->dev, "vmmc");
-	if (IS_ERR(host->vcc))
-		host->vcc = NULL;
-	else {
-		int mask = mmc_regulator_get_ocrmask(host->vcc);
-
-		if (mask < 0)
-			dev_err(&dev->dev, "error getting OCR mask (%d)\n",
-				mask);
-		else {
-			host->mmc->ocr_avail = (u32) mask;
-			if (plat->ocr_mask)
-				dev_warn(&dev->dev,
-				 "Provided ocr_mask/setpower will not be used "
-				 "(using regulator instead)\n");
-		}
-	}
-#endif
-	/* Fall back to platform data if no regulator is found */
-	if (host->vcc == NULL)
+	/* Get regulators and the supported OCR mask */
+	mmc_regulator_get_supply(mmc);
+	if (!mmc->ocr_avail)
 		mmc->ocr_avail = plat->ocr_mask;
+	else if (plat->ocr_mask)
+		dev_warn(mmc_dev(mmc), "Platform OCR mask is ignored\n");
+
 	mmc->caps = plat->capabilities;
 	mmc->caps2 = plat->capabilities2;

It was using the other MMC/regulator core helper, introduced by:

commit 5c13941acc513669c7d07b28789c3f9ba66ddddf
Author: David Brownell <dbrownell@users.sourceforge.net>
Date:   Wed Mar 11 03:30:43 2009 -0800

    MMC: regulator utilities
    
    Glue between MMC and regulator stacks ... verified with
    some OMAP3 boards using adjustable and configured-as-fixed
    regulators on several MMC controllers.
    
    These calls are intended to be used by MMC host adapters
    using at least one regulator per host.  Examples include
    slots with regulators supporting multiple voltages and
    ones using multiple voltage rails (e.g. DAT4..DAT7 using a
    separate supply, or a split rail chip like certain SDIO
    WLAN or eMMC solutions).
    
    Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
    Acked-by: Pierre Ossman <drzeus@drzeus.cx>
    Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>

Pawel

[PATCH v2 1/2] ARM: vexpress: Enable regulator framework when MMCI is in use

[Russell King - ARM Linux]

On Mon, Mar 18, 2013 at 06:16:57PM +0000, Pawel Moll wrote:
> On Mon, 2013-03-18 at 18:03 +0000, Russell King - ARM Linux wrote:
> > > Without the regulator framework present there are no valid operating
> > > voltages reported and the device initialisation fails:
> > > 
> > > mmci-pl18x 1c050000.mmci: mmc0: PL180 manf 41 rev0 at 0x1c050000 irq 41,42 (pio)
> > > mmc0: host doesn't support card's voltages
> > > mmc0: error -22 whilst initialising MMC card
> > 
> > Okay, this isn't going to work, because this means it's broken on a whole
> > load of platforms.  What it means is that this commit:
> > 
> > commit 599c1d5c750ddf528c7c6d3cdc466708f0502e66
> > Author: Ulf Hansson <ulf.hansson@linaro.org>
> > Date:   Mon Jan 7 16:22:50 2013 +0100
> > 
> >     ARM: 7620/1: mmc: mmci: Convert to use mmc_regulator_get_supply
> >     
> >     By using the mmc_regulator_get_supply API we are able to do some
> >     cleanups of the regulator code. Additionally let the regulator
> >     API handle the error printing.
> >     
> >     Cc: Chris Ball <cjb@laptop.org>
> >     Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> >     Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > 
> > is actually wrong and probably needs reverting.
> 
> Reverting it will not change anything as the driver had the same
> dependency before, just using :

Umm.  If CONFIG_REGULATOR is not set with the patch reverted, the
regulator support is not used at all, and the ocrmask comes from
the platform data.

[PATCH v2 1/2] ARM: vexpress: Enable regulator framework when MMCI is in use

[Arnd Bergmann]

On Monday 18 March 2013, Russell King - ARM Linux wrote:
> On Mon, Mar 18, 2013 at 06:16:57PM +0000, Pawel Moll wrote:
> > 
> > Reverting it will not change anything as the driver had the same
> > dependency before, just using :
> 
> Umm.  If CONFIG_REGULATOR is not set with the patch reverted, the
> regulator support is not used at all, and the ocrmask comes from
> the platform data.

I think the point is that case of DT booting, the platform data has
a zero ocrmask, because Versatile Express has no auxdata to set
a platform_data pointer for the mmci device, and the default allocated
mmci device driver does not initialize the ocrmask unless the
regulator API is used.

This took me a while to figure out, the point being that this is not
actually a regression, it never worked as far as I can tell.

I don't think that selecting the regulator API is a good idea, for
the reasons that you mentioned, and I still wouldn't take the
patch. In theory we could do a workaround by adding a DT property
to the mmci node that describes the fixed voltage supplied to the
card if there are no regulators, but then again we already describe
the voltage using the regulator binding and I would not want to
duplicate that information.

What I think we should have though is a better diagnostic here:
If neither the platform data nor the DT contain a valid description
of the voltage supplied to the card, the probe should fail saying
that the information is missing, not that the card is incompatible.

	Arnd

[PATCH v2 1/2] ARM: vexpress: Enable regulator framework when MMCI is in use

[Ulf Hansson]

On 18 March 2013 22:40, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 18 March 2013, Russell King - ARM Linux wrote:
>> On Mon, Mar 18, 2013 at 06:16:57PM +0000, Pawel Moll wrote:
>> >
>> > Reverting it will not change anything as the driver had the same
>> > dependency before, just using :
>>
>> Umm.  If CONFIG_REGULATOR is not set with the patch reverted, the
>> regulator support is not used at all, and the ocrmask comes from
>> the platform data.
>
> I think the point is that case of DT booting, the platform data has
> a zero ocrmask, because Versatile Express has no auxdata to set
> a platform_data pointer for the mmci device, and the default allocated
> mmci device driver does not initialize the ocrmask unless the
> regulator API is used.
>
> This took me a while to figure out, the point being that this is not
> actually a regression, it never worked as far as I can tell.
>
> I don't think that selecting the regulator API is a good idea, for
> the reasons that you mentioned, and I still wouldn't take the
> patch. In theory we could do a workaround by adding a DT property
> to the mmci node that describes the fixed voltage supplied to the
> card if there are no regulators, but then again we already describe
> the voltage using the regulator binding and I would not want to
> duplicate that information.
>
> What I think we should have though is a better diagnostic here:
> If neither the platform data nor the DT contain a valid description
> of the voltage supplied to the card, the probe should fail saying
> that the information is missing, not that the card is incompatible.
>
>         Arnd

I agree Arnd.

If mmci during probe, don't get a regulator and don't have an OCR mask
from dt/platform data, probe should fail.

Kind regards
Ulf Hansson

[PATCH 2/2] ARM: vexpress: Remove A9 compatible value for V2P-CA5s PMU node

[Pawel Moll]

The Cortex-A5 PMU is not really compatible with Cortex-A9 one,
so remove the respective compatible value from the PMU node in
the V2P_CA5s' Device Tree.

Signed-off-by: Pawel Moll <pawel.moll@arm.com>
---
 arch/arm/boot/dts/vexpress-v2p-ca5s.dts |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/vexpress-v2p-ca5s.dts b/arch/arm/boot/dts/vexpress-v2p-ca5s.dts
index 6328cbc..c544a55 100644
--- a/arch/arm/boot/dts/vexpress-v2p-ca5s.dts
+++ b/arch/arm/boot/dts/vexpress-v2p-ca5s.dts
@@ -111,7 +111,7 @@
 	};
 
 	pmu {
-		compatible = "arm,cortex-a5-pmu", "arm,cortex-a9-pmu";
+		compatible = "arm,cortex-a5-pmu";
 		interrupts = <0 68 4>,
 			     <0 69 4>;
 	};
-- 
1.7.10.4