diff mbox

[U-Boot,12/23] x86: ich6_gpio: Convert to use proper DM API

Message ID 1454319658-17431-13-git-send-email-bmeng.cn@gmail.com
State Accepted
Commit 3ddc1c7bd36addc0789c50edf71e45258a2b8901
Delegated to: Bin Meng
Headers show

Commit Message

Bin Meng Feb. 1, 2016, 9:40 a.m. UTC 
At present this GPIO driver still uses the legacy PCI API. Now that
we have proper PCH drivers we can use those to obtain the information
we need. While the device tree has nodes for the GPIO peripheral it is
not in the right place. It should be on the PCI bus as a sub-peripheral
of the PCH device.

Update the device tree files to show the GPIO controller within the PCH,
so that PCI access works as expected. This also adds #address-cells and

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 arch/x86/dts/bayleybay.dts         |  86 ++++++++++++-------------
 arch/x86/dts/chromebook_link.dts   |  42 ++++++-------
 arch/x86/dts/chromebox_panther.dts |  44 ++++++-------
 arch/x86/dts/crownbay.dts          |  30 ++++-----
 arch/x86/dts/galileo.dts           |  28 +++++----
 arch/x86/dts/minnowmax.dts         |  86 ++++++++++++-------------
 drivers/gpio/intel_ich6_gpio.c     | 125 +++++++------------------------------
 7 files changed, 186 insertions(+), 255 deletions(-)

Comments

Simon Glass Feb. 1, 2016, 4:20 p.m. UTC | #1 
On 1 February 2016 at 02:40, Bin Meng <bmeng.cn@gmail.com> wrote:
> At present this GPIO driver still uses the legacy PCI API. Now that
> we have proper PCH drivers we can use those to obtain the information
> we need. While the device tree has nodes for the GPIO peripheral it is
> not in the right place. It should be on the PCI bus as a sub-peripheral
> of the PCH device.
>
> Update the device tree files to show the GPIO controller within the PCH,
> so that PCI access works as expected. This also adds #address-cells and
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  arch/x86/dts/bayleybay.dts         |  86 ++++++++++++-------------
>  arch/x86/dts/chromebook_link.dts   |  42 ++++++-------
>  arch/x86/dts/chromebox_panther.dts |  44 ++++++-------
>  arch/x86/dts/crownbay.dts          |  30 ++++-----
>  arch/x86/dts/galileo.dts           |  28 +++++----
>  arch/x86/dts/minnowmax.dts         |  86 ++++++++++++-------------
>  drivers/gpio/intel_ich6_gpio.c     | 125 +++++++------------------------------
>  7 files changed, 186 insertions(+), 255 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Tested on Minnowmax:
Tested-by: Simon Glass <sjg@chromium.org>
Bin Meng Feb. 3, 2016, 4:33 a.m. UTC | #2 
On Tue, Feb 2, 2016 at 12:20 AM, Simon Glass <sjg@chromium.org> wrote:
> On 1 February 2016 at 02:40, Bin Meng <bmeng.cn@gmail.com> wrote:
>> At present this GPIO driver still uses the legacy PCI API. Now that
>> we have proper PCH drivers we can use those to obtain the information
>> we need. While the device tree has nodes for the GPIO peripheral it is
>> not in the right place. It should be on the PCI bus as a sub-peripheral
>> of the PCH device.
>>
>> Update the device tree files to show the GPIO controller within the PCH,
>> so that PCI access works as expected. This also adds #address-cells and
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>>
>>  arch/x86/dts/bayleybay.dts         |  86 ++++++++++++-------------
>>  arch/x86/dts/chromebook_link.dts   |  42 ++++++-------
>>  arch/x86/dts/chromebox_panther.dts |  44 ++++++-------
>>  arch/x86/dts/crownbay.dts          |  30 ++++-----
>>  arch/x86/dts/galileo.dts           |  28 +++++----
>>  arch/x86/dts/minnowmax.dts         |  86 ++++++++++++-------------
>>  drivers/gpio/intel_ich6_gpio.c     | 125 +++++++------------------------------
>>  7 files changed, 186 insertions(+), 255 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Tested on Minnowmax:
> Tested-by: Simon Glass <sjg@chromium.org>

applied to u-boot-x86/master, thanks!
diff mbox

Patch

diff --git a/arch/x86/dts/bayleybay.dts b/arch/x86/dts/bayleybay.dts
index cdd5121..4ea9262 100644
--- a/arch/x86/dts/bayleybay.dts
+++ b/arch/x86/dts/bayleybay.dts
@@ -65,48 +65,6 @@ 
 		};
 	};
 
-	gpioa {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0 0x20>;
-		bank-name = "A";
-	};
-
-	gpiob {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0x20 0x20>;
-		bank-name = "B";
-	};
-
-	gpioc {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0x40 0x20>;
-		bank-name = "C";
-	};
-
-	gpiod {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0x60 0x20>;
-		bank-name = "D";
-	};
-
-	gpioe {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0x80 0x20>;
-		bank-name = "E";
-	};
-
-	gpiof {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0xA0 0x20>;
-		bank-name = "F";
-	};
-
 	pci {
 		compatible = "pci-x86";
 		#address-cells = <3>;
@@ -119,6 +77,8 @@ 
 		pch@1f,0 {
 			reg = <0x0000f800 0 0 0 0>;
 			compatible = "intel,pch9";
+			#address-cells = <1>;
+			#size-cells = <1>;
 
 			irq-router {
 				compatible = "intel,irq-router";
@@ -201,6 +161,48 @@ 
 					};
 				};
 			};
+
+			gpioa {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0 0x20>;
+				bank-name = "A";
+			};
+
+			gpiob {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0x20 0x20>;
+				bank-name = "B";
+			};
+
+			gpioc {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0x40 0x20>;
+				bank-name = "C";
+			};
+
+			gpiod {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0x60 0x20>;
+				bank-name = "D";
+			};
+
+			gpioe {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0x80 0x20>;
+				bank-name = "E";
+			};
+
+			gpiof {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0xA0 0x20>;
+				bank-name = "F";
+			};
 		};
 	};
 
diff --git a/arch/x86/dts/chromebook_link.dts b/arch/x86/dts/chromebook_link.dts
index e5d77b6..f85e55c 100644
--- a/arch/x86/dts/chromebook_link.dts
+++ b/arch/x86/dts/chromebook_link.dts
@@ -54,27 +54,6 @@ 
 
 	};
 
-	gpioa {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0 0x10>;
-		bank-name = "A";
-	};
-
-	gpiob {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0x30 0x10>;
-		bank-name = "B";
-	};
-
-	gpioc {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0x40 0x10>;
-		bank-name = "C";
-	};
-
 	chosen {
 		stdout-path = "/serial";
 	};
@@ -270,6 +249,27 @@ 
 				};
 			};
 
+			gpioa {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0 0x10>;
+				bank-name = "A";
+			};
+
+			gpiob {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0x30 0x10>;
+				bank-name = "B";
+			};
+
+			gpioc {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0x40 0x10>;
+				bank-name = "C";
+			};
+
 			lpc {
 				compatible = "intel,bd82x6x-lpc";
 				#address-cells = <1>;
diff --git a/arch/x86/dts/chromebox_panther.dts b/arch/x86/dts/chromebox_panther.dts
index ce8825f..480b366 100644
--- a/arch/x86/dts/chromebox_panther.dts
+++ b/arch/x86/dts/chromebox_panther.dts
@@ -18,27 +18,6 @@ 
 		no-keyboard;
 	};
 
-	gpioa {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0 0x10>;
-		bank-name = "A";
-	};
-
-	gpiob {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0x30 0x10>;
-		bank-name = "B";
-	};
-
-	gpioc {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0x40 0x10>;
-		bank-name = "C";
-	};
-
 	chosen {
 		stdout-path = "/serial";
 	};
@@ -55,6 +34,8 @@ 
 		pch@1f,0 {
 			reg = <0x0000f800 0 0 0 0>;
 			compatible = "intel,pch9";
+			#address-cells = <1>;
+			#size-cells = <1>;
 
 			spi: spi {
 				#address-cells = <1>;
@@ -73,6 +54,27 @@ 
 					};
 				};
 			};
+
+			gpioa {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0 0x10>;
+				bank-name = "A";
+			};
+
+			gpiob {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0x30 0x10>;
+				bank-name = "B";
+			};
+
+			gpioc {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0x40 0x10>;
+				bank-name = "C";
+			};
 		};
 	};
 
diff --git a/arch/x86/dts/crownbay.dts b/arch/x86/dts/crownbay.dts
index ccf90fd..c6933cc 100644
--- a/arch/x86/dts/crownbay.dts
+++ b/arch/x86/dts/crownbay.dts
@@ -50,20 +50,6 @@ 
 
 	};
 
-	gpioa {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0 0x20>;
-		bank-name = "A";
-	};
-
-	gpiob {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0x20 0x20>;
-		bank-name = "B";
-	};
-
 	chosen {
 		/*
 		 * By default the legacy superio serial port is used as the
@@ -166,6 +152,8 @@ 
 		pch@1f,0 {
 			reg = <0x0000f800 0 0 0 0>;
 			compatible = "intel,pch7";
+			#address-cells = <1>;
+			#size-cells = <1>;
 
 			irq-router {
 				compatible = "intel,queensbay-irq-router";
@@ -242,6 +230,20 @@ 
 					memory-map = <0xffe00000 0x00200000>;
 				};
 			};
+
+			gpioa {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0 0x20>;
+				bank-name = "A";
+			};
+
+			gpiob {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0x20 0x20>;
+				bank-name = "B";
+			};
 		};
 	};
 
diff --git a/arch/x86/dts/galileo.dts b/arch/x86/dts/galileo.dts
index a9b2994..21c3641 100644
--- a/arch/x86/dts/galileo.dts
+++ b/arch/x86/dts/galileo.dts
@@ -82,6 +82,8 @@ 
 		pch@1f,0 {
 			reg = <0x0000f800 0 0 0 0>;
 			compatible = "intel,pch7";
+			#address-cells = <1>;
+			#size-cells = <1>;
 
 			irq-router {
 				compatible = "intel,quark-irq-router";
@@ -132,21 +134,21 @@ 
 					};
 				};
 			};
-		};
-	};
 
-	gpioa {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0 0x20>;
-		bank-name = "A";
-	};
+			gpioa {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0 0x20>;
+				bank-name = "A";
+			};
 
-	gpiob {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0x20 0x20>;
-		bank-name = "B";
+			gpiob {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0x20 0x20>;
+				bank-name = "B";
+			};
+		};
 	};
 
 };
diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts
index 5b4da6c..b7e3ba4 100644
--- a/arch/x86/dts/minnowmax.dts
+++ b/arch/x86/dts/minnowmax.dts
@@ -75,48 +75,6 @@ 
 		};
 	};
 
-	gpioa {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0 0x20>;
-		bank-name = "A";
-	};
-
-	gpiob {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0x20 0x20>;
-		bank-name = "B";
-	};
-
-	gpioc {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0x40 0x20>;
-		bank-name = "C";
-	};
-
-	gpiod {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0x60 0x20>;
-		bank-name = "D";
-	};
-
-	gpioe {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0x80 0x20>;
-		bank-name = "E";
-	};
-
-	gpiof {
-		compatible = "intel,ich6-gpio";
-		u-boot,dm-pre-reloc;
-		reg = <0xA0 0x20>;
-		bank-name = "F";
-	};
-
 	chosen {
 		stdout-path = "/serial";
 	};
@@ -153,6 +111,8 @@ 
 		pch@1f,0 {
 			reg = <0x0000f800 0 0 0 0>;
 			compatible = "pci8086,0f1c", "intel,pch9";
+			#address-cells = <1>;
+			#size-cells = <1>;
 
 			irq-router {
 				compatible = "intel,irq-router";
@@ -235,6 +195,48 @@ 
 					};
 				};
 			};
+
+			gpioa {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0 0x20>;
+				bank-name = "A";
+			};
+
+			gpiob {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0x20 0x20>;
+				bank-name = "B";
+			};
+
+			gpioc {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0x40 0x20>;
+				bank-name = "C";
+			};
+
+			gpiod {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0x60 0x20>;
+				bank-name = "D";
+			};
+
+			gpioe {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0x80 0x20>;
+				bank-name = "E";
+			};
+
+			gpiof {
+				compatible = "intel,ich6-gpio";
+				u-boot,dm-pre-reloc;
+				reg = <0xA0 0x20>;
+				bank-name = "F";
+			};
 		};
 	};
 
diff --git a/drivers/gpio/intel_ich6_gpio.c b/drivers/gpio/intel_ich6_gpio.c
index 67bf0a2..527ed6d 100644
--- a/drivers/gpio/intel_ich6_gpio.c
+++ b/drivers/gpio/intel_ich6_gpio.c
@@ -30,6 +30,7 @@ 
 #include <dm.h>
 #include <errno.h>
 #include <fdtdec.h>
+#include <pch.h>
 #include <pci.h>
 #include <asm/gpio.h>
 #include <asm/io.h>
@@ -62,91 +63,6 @@  void ich_gpio_set_gpio_map(const struct pch_gpio_map *map)
 	gd->arch.gpio_map = map;
 }
 
-static int gpio_ich6_get_base(unsigned long base)
-{
-	pci_dev_t pci_dev;			/* handle for 0:1f:0 */
-	u8 tmpbyte;
-	u16 tmpword;
-	u32 tmplong;
-
-	/* Where should it be? */
-	pci_dev = PCI_BDF(0, 0x1f, 0);
-
-	/* Is the device present? */
-	tmpword = x86_pci_read_config16(pci_dev, PCI_VENDOR_ID);
-	if (tmpword != PCI_VENDOR_ID_INTEL) {
-		debug("%s: wrong VendorID %x\n", __func__, tmpword);
-		return -ENODEV;
-	}
-
-	tmpword = x86_pci_read_config16(pci_dev, PCI_DEVICE_ID);
-	debug("Found %04x:%04x\n", PCI_VENDOR_ID_INTEL, tmpword);
-	/*
-	 * We'd like to validate the Device ID too, but pretty much any
-	 * value is either a) correct with slight differences, or b)
-	 * correct but undocumented. We'll have to check a bunch of other
-	 * things instead...
-	 */
-
-	/* I/O should already be enabled (it's a RO bit). */
-	tmpword = x86_pci_read_config16(pci_dev, PCI_COMMAND);
-	if (!(tmpword & PCI_COMMAND_IO)) {
-		debug("%s: device IO not enabled\n", __func__);
-		return -ENODEV;
-	}
-
-	/* Header Type must be normal (bits 6-0 only; see spec.) */
-	tmpbyte = x86_pci_read_config8(pci_dev, PCI_HEADER_TYPE);
-	if ((tmpbyte & 0x7f) != PCI_HEADER_TYPE_NORMAL) {
-		debug("%s: invalid Header type\n", __func__);
-		return -ENODEV;
-	}
-
-	/* Base Class must be a bridge device */
-	tmpbyte = x86_pci_read_config8(pci_dev, PCI_CLASS_CODE);
-	if (tmpbyte != PCI_CLASS_CODE_BRIDGE) {
-		debug("%s: invalid class\n", __func__);
-		return -ENODEV;
-	}
-	/* Sub Class must be ISA */
-	tmpbyte = x86_pci_read_config8(pci_dev, PCI_CLASS_SUB_CODE);
-	if (tmpbyte != PCI_CLASS_SUB_CODE_BRIDGE_ISA) {
-		debug("%s: invalid subclass\n", __func__);
-		return -ENODEV;
-	}
-
-	/* Programming Interface must be 0x00 (no others exist) */
-	tmpbyte = x86_pci_read_config8(pci_dev, PCI_CLASS_PROG);
-	if (tmpbyte != 0x00) {
-		debug("%s: invalid interface type\n", __func__);
-		return -ENODEV;
-	}
-
-	/*
-	 * GPIOBASE moved to its current offset with ICH6, but prior to
-	 * that it was unused (or undocumented). Check that it looks
-	 * okay: not all ones or zeros.
-	 *
-	 * Note we don't need check bit0 here, because the Tunnel Creek
-	 * GPIO base address register bit0 is reserved (read returns 0),
-	 * while on the Ivybridge the bit0 is used to indicate it is an
-	 * I/O space.
-	 */
-	tmplong = x86_pci_read_config32(pci_dev, base);
-	if (tmplong == 0x00000000 || tmplong == 0xffffffff) {
-		debug("%s: unexpected BASE value\n", __func__);
-		return -ENODEV;
-	}
-
-	/*
-	 * Okay, I guess we're looking at the right device. The actual
-	 * GPIO registers are in the PCI device's I/O space, starting
-	 * at the offset that we just read. Bit 0 indicates that it's
-	 * an I/O address, not a memory address, so mask that off.
-	 */
-	return tmplong & 1 ? tmplong & ~3 : tmplong & ~15;
-}
-
 static int _ich6_gpio_set_value(uint16_t base, unsigned offset, int value)
 {
 	u32 val;
@@ -288,20 +204,26 @@  static int _gpio_ich6_pinctrl_cfg_pin(s32 gpiobase, s32 iobase, int pin_node)
 
 int gpio_ich6_pinctrl_init(void)
 {
+	struct udevice *pch;
 	int pin_node;
 	int node;
 	int ret;
-	int gpiobase;
-	int iobase_offset;
-	int iobase = -1;
+	u32 gpiobase;
+	u32 iobase = -1;
+
+	ret = uclass_first_device(UCLASS_PCH, &pch);
+	if (ret)
+		return ret;
+	if (!pch)
+		return -ENODEV;
 
 	/*
 	 * Get the memory/io base address to configure every pins.
 	 * IOBASE is used to configure the mode/pads
 	 * GPIOBASE is used to configure the direction and default value
 	 */
-	gpiobase = gpio_ich6_get_base(PCI_CFG_GPIOBASE);
-	if (gpiobase < 0) {
+	ret = pch_get_gpio_base(pch, &gpiobase);
+	if (ret) {
 		debug("%s: invalid GPIOBASE address (%08x)\n", __func__,
 		      gpiobase);
 		return -EINVAL;
@@ -319,16 +241,11 @@  int gpio_ich6_pinctrl_init(void)
 	 * Get the IOBASE, this is not mandatory as this is not
 	 * supported by all the CPU
 	 */
-	iobase_offset = fdtdec_get_int(gd->fdt_blob, node, "io-base", -1);
-	if (iobase_offset == -1) {
-		debug("%s: io-base offset not present\n", __func__);
-	} else {
-		iobase = gpio_ich6_get_base(iobase_offset);
-		if (IS_ERR_VALUE(iobase)) {
-			debug("%s: invalid IOBASE address (%08x)\n", __func__,
-			      iobase);
-			return -EINVAL;
-		}
+	ret = pch_get_io_base(pch, &iobase);
+	if (ret && ret != -ENOSYS) {
+		debug("%s: invalid IOBASE address (%08x)\n", __func__,
+		      iobase);
+		return -EINVAL;
 	}
 
 	for (pin_node = fdt_first_subnode(gd->fdt_blob, node);
@@ -349,10 +266,14 @@  int gpio_ich6_pinctrl_init(void)
 static int gpio_ich6_ofdata_to_platdata(struct udevice *dev)
 {
 	struct ich6_bank_platdata *plat = dev_get_platdata(dev);
-	u16 gpiobase;
+	u32 gpiobase;
 	int offset;
+	int ret;
+
+	ret = pch_get_gpio_base(dev->parent, &gpiobase);
+	if (ret)
+		return ret;
 
-	gpiobase = gpio_ich6_get_base(PCI_CFG_GPIOBASE);
 	offset = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "reg", -1);
 	if (offset == -1) {
 		debug("%s: Invalid register offset %d\n", __func__, offset);