Skip to contentSkip to conversation

arm: lpc32xx: remove direct control of GPIOs from shared mach file

PATCH Superseded Vladimir Zapolskiy 3 participants 10 messages Dec 19, 2015 – Dec 21, 2015

Subsystems (4)
Device tree bindings Arm (32-bit) Arm sub-architectures Arm/LPC32XX SoC support

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Vladimir Zapolskiy

author maintainer
#0 lore raw

cover arm: lpc32xx: remove direct control of GPIOs from shared mach file

The change moves GPIO controlled voltage regulators from mach-lpc32xx/phy3250.c to Phytec PHY3250 DTS board file.

The change makes previous versions of PHY3250 DTB incompatible with the new kernel version, however the change is needed, because regardless of its name mach-lpc32xx/phy3250.c file is a common file for all NXP LPC32xx boards and PHY3250 specifics are not needed there.

Vladimir Zapolskiy (3):
arm: dts: phy3250: add lcd and backlight fixed regulators
arm: dts: phy3250: add SD fixed regulator
arm: lpc32xx: remove direct control of GPIOs from shared mach file

 arch/arm/boot/dts/phy3250.dts   | 32 +++++++++++++++++++++++++
 arch/arm/mach-lpc32xx/phy3250.c | 52 -----------------------------------------
 2 files changed, 32 insertions(+), 52 deletions(-)
Signature · 1 line
2.1.4
author maintainer
#1 lore raw

PATCH 1/3 arm: dts: phy3250: add lcd and backlight fixed regulators

Phytec PHY3250 board has GPIO controlled regulators for LCD and backlight, add their descriptions to board DTS file.

Signed-off-by Vladimir Zapolskiy <[email protected]>
---
 arch/arm/boot/dts/phy3250.dts | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
arch/arm/boot/dts/phy3250.dts +22 -0
index 7d253bb..34e15fd 100644
@@ -25,6 +25,28 @@
 		reg = <0x80000000 0x4000000>;
 	};
 
+	regulators {
+		backlight_reg: fixed-regulator at 0 {
+			compatible = "regulator-fixed";
+			regulator-name = "backlight_reg";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			gpio = <&gpio 5 4 0>;
+			enable-active-high;
+			regulator-boot-on;
+		};
+
+		lcd_reg: fixed-regulator at 1 {
+			compatible = "regulator-fixed";
+			regulator-name = "lcd_reg";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			gpio = <&gpio 5 0 0>;
+			enable-active-high;
+			regulator-boot-on;
+		};
+	};
+
 	ahb {
 		mac: ethernet at 31060000 {
 			phy-mode = "rmii";
Signature · 1 line
2.1.4
author maintainer
#2 lore raw

PATCH 2/3 arm: dts: phy3250: add SD fixed regulator

In reply to Vladimir Zapolskiy

The change adds fixed voltage regulator for SD controller, ARM MMCI controller driver uses it to control card power management.

Signed-off-by Vladimir Zapolskiy <[email protected]>
---
 arch/arm/boot/dts/phy3250.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)
arch/arm/boot/dts/phy3250.dts +10 -0
index 34e15fd..5465b9e 100644
@@ -45,6 +45,15 @@
 			enable-active-high;
 			regulator-boot-on;
 		};
+
+		sd_reg: fixed-regulator at 2 {
+			compatible = "regulator-fixed";
+			regulator-name = "sd_reg";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			gpio = <&gpio 5 5 0>;
+			enable-active-high;
+		};
 	};
 
 	ahb {

@@ -162,6 +171,7 @@
 				cd-gpios = <&gpio 3 1 0>;
 				cd-inverted;
 				bus-width = <4>;
+				vqmmc-supply = <&sd_reg>;
 				status = "okay";
 			};
 		};
Signature · 1 line
2.1.4
#3 lore raw

PATCH 2/3 arm: dts: phy3250: add SD fixed regulator

arch/arm/boot/dts/phy3250.dts line 174

Quoted 5 lines · Vladimir Zapolskiy
arch/arm/boot/dts/phy3250.dts +1 -0
@@ -162,6 +171,7 @@
 				cd-gpios = <&gpio 3 1 0>;
 				cd-inverted;
 				bus-width = <4>;
+				vqmmc-supply = <&sd_reg>;

This looks wrong. ios->power_mode controls the _card_ power (which is vmmc-supply), not the signalling power (which is vqmmc-supply):

- vmmc-supply : phandle to the regulator device tree node, mentioned
as the VCC/VDD supply in the eMMC/SD specs.

- vqmmc-supply : phandle to the regulator device tree node, mentioned
as the VCCQ/VDD_IO supply in the eMMC/SD specs.

Signature · 3 lines
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
author maintainer
#4 lore raw

PATCH 2/3 arm: dts: phy3250: add SD fixed regulator

Quoted 11 lines · Russell King - ARM Linux

arch/arm/boot/dts/phy3250.dts line 174

Quoted 5 lines · Vladimir Zapolskiy
arch/arm/boot/dts/phy3250.dts +1 -0
@@ -162,6 +171,7 @@
 				cd-gpios = <&gpio 3 1 0>;
 				cd-inverted;
 				bus-width = <4>;
+				vqmmc-supply = <&sd_reg>;

This looks wrong. ios->power_mode controls the _card_ power (which is vmmc-supply), not the signalling power (which is vqmmc-supply):

From drivers/mmc/host/mmci.c mmci_set_ios() manages both regulators,

mmc->supply.vmmc is changed on MMC_POWER_OFF and MMC_POWER_UP, mmc->supply.vqmmc is changed on MMC_POWER_OFF and MMC_POWER_ON.

But I agree that vmmc-supply is the right choice here, GPIO regulator controls an SD card power here, thanks for the finding.

Quoted 7 lines

- vmmc-supply : phandle to the regulator device tree node, mentioned
as the VCC/VDD supply in the eMMC/SD specs.

- vqmmc-supply : phandle to the regulator device tree node, mentioned
as the VCCQ/VDD_IO supply in the eMMC/SD specs.

--
With best wishes,
Vladimir

author maintainer
#5 lore raw

PATCH v2 2/3 arm: dts: phy3250: add SD fixed regulator

In reply to Vladimir Zapolskiy

The change adds fixed voltage regulator for SD controller, ARM MMCI controller driver uses it to control card power management.

Signed-off-by Vladimir Zapolskiy <[email protected]>

---
Changes from v1 to v2:
* specify SD card power supply instead of signal power, thanks to Russell

 arch/arm/boot/dts/phy3250.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)
arch/arm/boot/dts/phy3250.dts +10 -0
index 34e15fd..bcb87b3 100644
@@ -45,6 +45,15 @@
 			enable-active-high;
 			regulator-boot-on;
 		};
+
+		sd_reg: fixed-regulator at 2 {
+			compatible = "regulator-fixed";
+			regulator-name = "sd_reg";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			gpio = <&gpio 5 5 0>;
+			enable-active-high;
+		};
 	};
 
 	ahb {

@@ -162,6 +171,7 @@
 				cd-gpios = <&gpio 3 1 0>;
 				cd-inverted;
 				bus-width = <4>;
+				vmmc-supply = <&sd_reg>;
 				status = "okay";
 			};
 		};
Signature · 1 line
2.1.4
#6 lore raw

PATCH 2/3 arm: dts: phy3250: add SD fixed regulator

In reply to Vladimir Zapolskiy

Hello.

Quoted 13 lines · Vladimir Zapolskiy

The change adds fixed voltage regulator for SD controller, ARM MMCI controller driver uses it to control card power management.

Signed-off-by Vladimir Zapolskiy <[email protected]>
---
  arch/arm/boot/dts/phy3250.dts | 10 ++++++++++
  1 file changed, 10 insertions(+)
arch/arm/boot/dts/phy3250.dts +0 -0
index 34e15fd..5465b9e 100644

arch/arm/boot/dts/phy3250.dts line 49

Quoted 5 lines
arch/arm/boot/dts/phy3250.dts +2 -0
@@ -45,6 +45,15 @@
  			enable-active-high;
  			regulator-boot-on;
  		};
+
+		sd_reg: fixed-regulator at 2 {

Please just "regulator at 2", to comply with the ePAPR standard.

[...]

MBR, Sergei

author maintainer
#7 lore raw

PATCH 2/3 arm: dts: phy3250: add SD fixed regulator

Hell Sergei,

Quoted 12 lines · Sergei Shtylyov

Hello.

Quoted 8 lines · Vladimir Zapolskiy

The change adds fixed voltage regulator for SD controller, ARM MMCI controller driver uses it to control card power management.

Signed-off-by Vladimir Zapolskiy <[email protected]>
---
  arch/arm/boot/dts/phy3250.dts | 10 ++++++++++
  1 file changed, 10 insertions(+)
Quoted 12 lines

arch/arm/boot/dts/phy3250.dts line 49

Quoted 10 lines
arch/arm/boot/dts/phy3250.dts +2 -0
index 34e15fd..5465b9e 100644
@@ -45,6 +45,15 @@
  			enable-active-high;
  			regulator-boot-on;
  		};
+
+		sd_reg: fixed-regulator at 2 {

Please just "regulator at 2", to comply with the ePAPR standard.

thanks for review, just for my information, do I understand correctly that you reference to "Generic Names Recommendation" ?

I have no objection to generalize device tree node name as you suggested, will do it.

With best wishes,
Vladimir

#8 lore raw

PATCH 2/3 arm: dts: phy3250: add SD fixed regulator

Quoted 21 lines · Vladimir Zapolskiy
Quoted 8 lines
Quoted 8 lines

The change adds fixed voltage regulator for SD controller, ARM MMCI controller driver uses it to control card power management.

Signed-off-by Vladimir Zapolskiy <[email protected]>
---
   arch/arm/boot/dts/phy3250.dts | 10 ++++++++++
   1 file changed, 10 insertions(+)
Quoted 12 lines

arch/arm/boot/dts/phy3250.dts line 49

Quoted 10 lines
arch/arm/boot/dts/phy3250.dts +2 -0
index 34e15fd..5465b9e 100644
@@ -45,6 +45,15 @@
   			enable-active-high;
   			regulator-boot-on;
   		};
+
+		sd_reg: fixed-regulator at 2 {

Please just "regulator at 2", to comply with the ePAPR standard.

Quoted 2 lines

thanks for review, just for my information, do I understand correctly that you reference to "Generic Names Recommendation" ?

    Sure, AKA 2.2.2. :-)
Quoted 2 lines

I have no objection to generalize device tree node name as you suggested, will do it.

TIA.

Quoted 2 lines

With best wishes,
Vladimir

MBR, Sergei

author maintainer
#9 lore raw

PATCH 3/3 arm: lpc32xx: remove direct control of GPIOs from shared mach file

In reply to Vladimir Zapolskiy

The change removes GPIO configuration and control of LCD, backlight and SD voltage regulators from the shared among all LPC32xx boards mach file, Phytec PHY3250 board should have the description of these regulators in its DTS file.

Signed-off-by Vladimir Zapolskiy <[email protected]>
---
 arch/arm/mach-lpc32xx/phy3250.c | 52 -----------------------------------------
 1 file changed, 52 deletions(-)
arch/arm/mach-lpc32xx/phy3250.c +0 -52
index 77d6b1b..0bdfa08 100644
@@ -36,7 +36,6 @@
 #include <linux/clk.h>
 #include <linux/mtd/lpc32xx_slc.h>
 #include <linux/mtd/lpc32xx_mlc.h>
-#include <linux/platform_data/gpio-lpc32xx.h>
 
 #include <asm/setup.h>
 #include <asm/mach-types.h>

@@ -48,13 +47,6 @@
 #include "common.h"
 
 /*
- * Mapped GPIOLIB GPIOs
- */
-#define LCD_POWER_GPIO		LPC32XX_GPIO(LPC32XX_GPO_P3_GRP, 0)
-#define BKL_POWER_GPIO		LPC32XX_GPIO(LPC32XX_GPO_P3_GRP, 4)
-#define MMC_PWR_ENABLE_GPIO	LPC32XX_GPIO(LPC32XX_GPO_P3_GRP, 5)
-
-/*
  * AMBA LCD controller
  */
 static struct clcd_panel conn_lcd_panel = {

@@ -97,20 +89,6 @@ static int lpc32xx_clcd_setup(struct clcd_fb *fb)
 	fb->fb.fix.smem_len = PANEL_SIZE;
 	fb->panel = &conn_lcd_panel;
 
-	if (gpio_request(LCD_POWER_GPIO, "LCD power"))
-		printk(KERN_ERR "Error requesting gpio %u",
-			LCD_POWER_GPIO);
-	else if (gpio_direction_output(LCD_POWER_GPIO, 1))
-		printk(KERN_ERR "Error setting gpio %u to output",
-			LCD_POWER_GPIO);
-
-	if (gpio_request(BKL_POWER_GPIO, "LCD backlight power"))
-		printk(KERN_ERR "Error requesting gpio %u",
-			BKL_POWER_GPIO);
-	else if (gpio_direction_output(BKL_POWER_GPIO, 1))
-		printk(KERN_ERR "Error setting gpio %u to output",
-			BKL_POWER_GPIO);
-
 	return 0;
 }
 

@@ -127,29 +105,10 @@ static void lpc32xx_clcd_remove(struct clcd_fb *fb)
 		fb->fb.screen_base, fb->fb.fix.smem_start);
 }
 
-/*
- * On some early LCD modules (1307.0), the backlight logic is inverted.
- * For those board variants, swap the disable and enable states for
- * BKL_POWER_GPIO.
-*/
-static void clcd_disable(struct clcd_fb *fb)
-{
-	gpio_set_value(BKL_POWER_GPIO, 0);
-	gpio_set_value(LCD_POWER_GPIO, 0);
-}
-
-static void clcd_enable(struct clcd_fb *fb)
-{
-	gpio_set_value(BKL_POWER_GPIO, 1);
-	gpio_set_value(LCD_POWER_GPIO, 1);
-}
-
 static struct clcd_board lpc32xx_clcd_data = {
 	.name		= "Phytec LCD",
 	.check		= clcdfb_check,
 	.decode		= clcdfb_decode,
-	.disable	= clcd_disable,
-	.enable		= clcd_enable,
 	.setup		= lpc32xx_clcd_setup,
 	.mmap		= lpc32xx_clcd_mmap,
 	.remove		= lpc32xx_clcd_remove,

@@ -188,20 +147,9 @@ static struct pl08x_platform_data pl08x_pd = {
 	.mem_buses = PL08X_AHB1,
 };
 
-static int mmc_handle_ios(struct device *dev, struct mmc_ios *ios)
-{
-	/* Only on and off are supported */
-	if (ios->power_mode == MMC_POWER_OFF)
-		gpio_set_value(MMC_PWR_ENABLE_GPIO, 0);
-	else
-		gpio_set_value(MMC_PWR_ENABLE_GPIO, 1);
-	return 0;
-}
-
 static struct mmci_platform_data lpc32xx_mmci_data = {
 	.ocr_mask	= MMC_VDD_30_31 | MMC_VDD_31_32 |
 			  MMC_VDD_32_33 | MMC_VDD_33_34,
-	.ios_handler	= mmc_handle_ios,
 };
 
 static struct lpc32xx_slc_platform_data lpc32xx_slc_data = {
Signature · 1 line
2.1.4