[U-Boot] [PATCH v3 0/4] x86: gpio: Tidy up GPIO driver

[U-Boot] [PATCH v3 0/4] x86: gpio: Tidy up GPIO driver

[Simon Glass]

The ICH GPIO driver does not work correctly at present. This series tidies
things up and fixes a few bugs.

One issue remaining is that reading from output GPIOs does not work - it
always returns 0.

Changes in v3:
- Correct failure logic in pci_get_bus()
- Use pci_get_bus in pci_bus_to_hose()

Changes in v2:
- Adjust pci_get_bus() to probe bus 0 so that the required bus is found

Simon Glass (4):
  dm: pci: Allow a PCI bus to be found without an alias
  gpio: Report errors when GPIOs cannot be read
  WIP: x86: gpio: Handle the case where reading a GPIO fails
  WIP: x86: gpio: Make sure that output GPIOs can be read

 common/cmd_gpio.c              | 34 +++++++++++++++++++++++++++-------
 drivers/gpio/intel_ich6_gpio.c | 28 ++++++++++++++++++++++++++--
 drivers/pci/pci-uclass.c       | 28 ++++++++++++++++++++++++----
 3 files changed, 77 insertions(+), 13 deletions(-)

-- 
2.5.0.457.gab17608

[U-Boot] [PATCH v3 1/4] dm: pci: Allow a PCI bus to be found without an alias

[Simon Glass]

At present, until a PCI bus is probed, it cannot be found by its sequence
number unless it has an alias. This is the same with any device.

However with PCI this is more annoying than usual, since bus 0 is always the
same device.

Add a function that tries a little harder to locate PCI bus 0. This means
that PCI enumeration will happen automatically on the first access.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Correct failure logic in pci_get_bus()
- Use pci_get_bus in pci_bus_to_hose()

Changes in v2:
- Adjust pci_get_bus() to probe bus 0 so that the required bus is found

 drivers/pci/pci-uclass.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index b25298f..ea70853 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -20,16 +20,36 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+static int pci_get_bus(int busnum, struct udevice **busp)
+{
+	int ret;
+
+	ret = uclass_get_device_by_seq(UCLASS_PCI, busnum, busp);
+
+	/* Since buses may not be numbered yet try a little harder with bus 0 */
+	if (ret == -ENODEV) {
+		ret = uclass_first_device(UCLASS_PCI, busp);
+		if (ret)
+			return ret;
+		else if (!*busp)
+			return -ENODEV;
+		ret = uclass_get_device_by_seq(UCLASS_PCI, busnum, busp);
+	}
+
+	return ret;
+}
+
 struct pci_controller *pci_bus_to_hose(int busnum)
 {
 	struct udevice *bus;
 	int ret;
 
-	ret = uclass_get_device_by_seq(UCLASS_PCI, busnum, &bus);
+	ret = pci_get_bus(busnum, &bus);
 	if (ret) {
 		debug("%s: Cannot get bus %d: ret=%d\n", __func__, busnum, ret);
 		return NULL;
 	}
+
 	return dev_get_uclass_priv(bus);
 }
 
@@ -128,7 +148,7 @@ int pci_bus_find_bdf(pci_dev_t bdf, struct udevice **devp)
 	struct udevice *bus;
 	int ret;
 
-	ret = uclass_get_device_by_seq(UCLASS_PCI, PCI_BUS(bdf), &bus);
+	ret = pci_get_bus(PCI_BUS(bdf), &bus);
 	if (ret)
 		return ret;
 	return pci_bus_find_devfn(bus, PCI_MASK_BUS(bdf), devp);
@@ -206,7 +226,7 @@ int pci_write_config(pci_dev_t bdf, int offset, unsigned long value,
 	struct udevice *bus;
 	int ret;
 
-	ret = uclass_get_device_by_seq(UCLASS_PCI, PCI_BUS(bdf), &bus);
+	ret = pci_get_bus(PCI_BUS(bdf), &bus);
 	if (ret)
 		return ret;
 
@@ -271,7 +291,7 @@ int pci_read_config(pci_dev_t bdf, int offset, unsigned long *valuep,
 	struct udevice *bus;
 	int ret;
 
-	ret = uclass_get_device_by_seq(UCLASS_PCI, PCI_BUS(bdf), &bus);
+	ret = pci_get_bus(PCI_BUS(bdf), &bus);
 	if (ret)
 		return ret;
 
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH v3 2/4] gpio: Report errors when GPIOs cannot be read

[Simon Glass]

Some controllers do not allow the output value to be read. Detect this and
report the error in that case.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2: None

 common/cmd_gpio.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/common/cmd_gpio.c b/common/cmd_gpio.c
index 65d6df4..7cd1ba1 100644
--- a/common/cmd_gpio.c
+++ b/common/cmd_gpio.c
@@ -119,7 +119,7 @@ static int do_gpio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	unsigned int gpio;
 	enum gpio_cmd sub_cmd;
-	ulong value;
+	int value;
 	const char *str_cmd, *str_gpio = NULL;
 	int ret;
 #ifdef CONFIG_DM_GPIO
@@ -195,15 +195,35 @@ static int do_gpio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		value = gpio_get_value(gpio);
 	} else {
 		switch (sub_cmd) {
-			case GPIO_SET:    value = 1; break;
-			case GPIO_CLEAR:  value = 0; break;
-			case GPIO_TOGGLE: value = !gpio_get_value(gpio); break;
-			default:          goto show_usage;
+		case GPIO_SET:
+			value = 1;
+			break;
+		case GPIO_CLEAR:
+			value = 0;
+			break;
+		case GPIO_TOGGLE:
+			value = gpio_get_value(gpio);
+			if (!IS_ERR_VALUE(value))
+				value = !value;
+			break;
+		default:
+			goto show_usage;
 		}
 		gpio_direction_output(gpio, value);
 	}
-	printf("gpio: pin %s (gpio %i) value is %lu\n",
-		str_gpio, gpio, value);
+	printf("gpio: pin %s (gpio %i) value is ", str_gpio, gpio);
+	if (IS_ERR_VALUE(value))
+		printf("unknown (ret=%d)\n", value);
+	else
+		printf("%d\n", value);
+	if (sub_cmd != GPIO_INPUT && !IS_ERR_VALUE(value)) {
+		int nval = gpio_get_value(gpio);
+
+		if (IS_ERR_VALUE(nval))
+			printf("   Warning: no access to GPIO output value\n");
+		else if (nval != value)
+			printf("   Warning: value of pin is still %d\n", nval);
+	}
 
 	if (ret != -EBUSY)
 		gpio_free(gpio);
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH v3 3/4] WIP: x86: gpio: Handle the case where reading a GPIO fails

[Simon Glass]

It should be possible to read an output GPIO. For now it is not clear
how to do this. Perhaps the driver should be adjusted to use memory-mapped
access throughout. For now, return an error to avoid confusion.

Not to apply.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2: None

 drivers/gpio/intel_ich6_gpio.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpio/intel_ich6_gpio.c b/drivers/gpio/intel_ich6_gpio.c
index 67bf0a2..5cc86d7 100644
--- a/drivers/gpio/intel_ich6_gpio.c
+++ b/drivers/gpio/intel_ich6_gpio.c
@@ -195,6 +195,14 @@ static int _ich6_gpio_set_direction(uint16_t base, unsigned offset, int dir)
 	return 0;
 }
 
+static int _ich6_gpio_get_direction(uint16_t base, unsigned offset)
+{
+	u32 val;
+
+	val = inl(base);
+	return !(val & (1UL << offset));
+}
+
 static int _gpio_ich6_pinctrl_cfg_pin(s32 gpiobase, s32 iobase, int pin_node)
 {
 	u32 gpio_offset[2];
@@ -432,6 +440,13 @@ static int ich6_gpio_get_value(struct udevice *dev, unsigned offset)
 	u32 tmplong;
 	int r;
 
+	/*
+	 * TODO: Mirror this register since reading only works when it is an
+	 * input, Alternatively figure out how to make the hardware work with
+	 * this.
+	 */
+	if (_ich6_gpio_get_direction(bank->io_sel, offset))
+		return -EIO;
 	tmplong = inl(bank->lvl);
 	r = (tmplong & (1UL << offset)) ? 1 : 0;
 	return r;
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH v3 4/4] WIP: x86: gpio: Make sure that output GPIOs can be read

[Simon Glass]

The datasheet suggests that the state of an output pin can be read. In fact
this does not seem to work, perhaps because we are also accessing the GPIO
state through I/O instead of just memory-mapped.

Not to apply.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2: None

 drivers/gpio/intel_ich6_gpio.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/intel_ich6_gpio.c b/drivers/gpio/intel_ich6_gpio.c
index 5cc86d7..204a4b4 100644
--- a/drivers/gpio/intel_ich6_gpio.c
+++ b/drivers/gpio/intel_ich6_gpio.c
@@ -49,6 +49,8 @@ struct ich6_bank_priv {
 #define GPIO_USESEL_OFFSET(x)	(x)
 #define GPIO_IOSEL_OFFSET(x)	(x + 4)
 #define GPIO_LVL_OFFSET(x)	(x + 8)
+#define IOPAD_VAL_OFFSET(x)	(x + 8)
+#define IINENB_SHIFT		2
 
 #define IOPAD_MODE_MASK				0x7
 #define IOPAD_PULL_ASSIGN_SHIFT		7
@@ -210,6 +212,7 @@ static int _gpio_ich6_pinctrl_cfg_pin(s32 gpiobase, s32 iobase, int pin_node)
 	int val;
 	int ret;
 	const void *prop;
+	bool mode_gpio = false;
 
 	/*
 	 * GPIO node is not mandatory, so we only do the
@@ -221,11 +224,13 @@ static int _gpio_ich6_pinctrl_cfg_pin(s32 gpiobase, s32 iobase, int pin_node)
 		/* Do we want to force the GPIO mode? */
 		prop = fdt_getprop(gd->fdt_blob, pin_node, "mode-gpio",
 				      NULL);
-		if (prop)
+		if (prop) {
 			_ich6_gpio_set_function(GPIO_USESEL_OFFSET
 						(gpiobase) +
 						gpio_offset[0],
 						gpio_offset[1], 1);
+			mode_gpio = true;
+		}
 
 		val =
 		    fdtdec_get_int(gd->fdt_blob, pin_node, "direction", -1);
@@ -270,8 +275,12 @@ static int _gpio_ich6_pinctrl_cfg_pin(s32 gpiobase, s32 iobase, int pin_node)
 		 * be just ignored by the controller
 		 */
 		val = fdtdec_get_int(gd->fdt_blob, pin_node, "mode-func", -1);
-		if (val != -1)
+		if (val != -1) {
 			clrsetbits_le32(iobase_addr, IOPAD_MODE_MASK, val);
+		} else if (mode_gpio) {
+			clrbits_le32(IOPAD_VAL_OFFSET(iobase_addr),
+				     1 << IINENB_SHIFT);
+		}
 
 		/* Configure the pull-up/down if needed */
 		val = fdtdec_get_int(gd->fdt_blob, pin_node, "pull-assign", -1);
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH v3 1/4] dm: pci: Allow a PCI bus to be found without an alias

[Bin Meng]

On Tue, Sep 1, 2015 at 8:55 AM, Simon Glass <sjg@chromium.org> wrote:
> At present, until a PCI bus is probed, it cannot be found by its sequence
> number unless it has an alias. This is the same with any device.
>
> However with PCI this is more annoying than usual, since bus 0 is always the
> same device.
>
> Add a function that tries a little harder to locate PCI bus 0. This means
> that PCI enumeration will happen automatically on the first access.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v3:
> - Correct failure logic in pci_get_bus()
> - Use pci_get_bus in pci_bus_to_hose()
>
> Changes in v2:
> - Adjust pci_get_bus() to probe bus 0 so that the required bus is found
>
>  drivers/pci/pci-uclass.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index b25298f..ea70853 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -20,16 +20,36 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> +static int pci_get_bus(int busnum, struct udevice **busp)
> +{
> +       int ret;
> +
> +       ret = uclass_get_device_by_seq(UCLASS_PCI, busnum, busp);
> +
> +       /* Since buses may not be numbered yet try a little harder with bus 0 */
> +       if (ret == -ENODEV) {
> +               ret = uclass_first_device(UCLASS_PCI, busp);
> +               if (ret)
> +                       return ret;
> +               else if (!*busp)
> +                       return -ENODEV;
> +               ret = uclass_get_device_by_seq(UCLASS_PCI, busnum, busp);
> +       }
> +
> +       return ret;
> +}
> +
>  struct pci_controller *pci_bus_to_hose(int busnum)
>  {
>         struct udevice *bus;
>         int ret;
>
> -       ret = uclass_get_device_by_seq(UCLASS_PCI, busnum, &bus);
> +       ret = pci_get_bus(busnum, &bus);
>         if (ret) {
>                 debug("%s: Cannot get bus %d: ret=%d\n", __func__, busnum, ret);
>                 return NULL;
>         }
> +
>         return dev_get_uclass_priv(bus);
>  }
>
> @@ -128,7 +148,7 @@ int pci_bus_find_bdf(pci_dev_t bdf, struct udevice **devp)
>         struct udevice *bus;
>         int ret;
>
> -       ret = uclass_get_device_by_seq(UCLASS_PCI, PCI_BUS(bdf), &bus);
> +       ret = pci_get_bus(PCI_BUS(bdf), &bus);
>         if (ret)
>                 return ret;
>         return pci_bus_find_devfn(bus, PCI_MASK_BUS(bdf), devp);
> @@ -206,7 +226,7 @@ int pci_write_config(pci_dev_t bdf, int offset, unsigned long value,
>         struct udevice *bus;
>         int ret;
>
> -       ret = uclass_get_device_by_seq(UCLASS_PCI, PCI_BUS(bdf), &bus);
> +       ret = pci_get_bus(PCI_BUS(bdf), &bus);
>         if (ret)
>                 return ret;
>
> @@ -271,7 +291,7 @@ int pci_read_config(pci_dev_t bdf, int offset, unsigned long *valuep,
>         struct udevice *bus;
>         int ret;
>
> -       ret = uclass_get_device_by_seq(UCLASS_PCI, PCI_BUS(bdf), &bus);
> +       ret = pci_get_bus(PCI_BUS(bdf), &bus);
>         if (ret)
>                 return ret;
>
> --

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

[U-Boot] [PATCH v3 1/4] dm: pci: Allow a PCI bus to be found without an alias

[Simon Glass]

On 1 September 2015 at 00:25, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Tue, Sep 1, 2015 at 8:55 AM, Simon Glass <sjg@chromium.org> wrote:
>> At present, until a PCI bus is probed, it cannot be found by its sequence
>> number unless it has an alias. This is the same with any device.
>>
>> However with PCI this is more annoying than usual, since bus 0 is always the
>> same device.
>>
>> Add a function that tries a little harder to locate PCI bus 0. This means
>> that PCI enumeration will happen automatically on the first access.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> Changes in v3:
>> - Correct failure logic in pci_get_bus()
>> - Use pci_get_bus in pci_bus_to_hose()
>>
>> Changes in v2:
>> - Adjust pci_get_bus() to probe bus 0 so that the required bus is found
>>
>>  drivers/pci/pci-uclass.c | 28 ++++++++++++++++++++++++----
>>  1 file changed, 24 insertions(+), 4 deletions(-)
>>
[snip]

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

Applied to u-boot-x86.

[U-Boot] [PATCH v3 2/4] gpio: Report errors when GPIOs cannot be read

[Simon Glass]

On 31 August 2015 at 18:55, Simon Glass <sjg@chromium.org> wrote:
> Some controllers do not allow the output value to be read. Detect this and
> report the error in that case.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
>  common/cmd_gpio.c | 34 +++++++++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 7 deletions(-)

Applied to u-boot-dm/next.