[U-Boot] [PATCH v3 0/4] x86: gpio: Tidy up GPIO driver
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(-)
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;
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
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.
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);
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.
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;
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);
participants (2)
-
Bin Meng -
Simon Glass