These functions use devicetree and cannot wprl with of-platdata, which has no runtime devicetree.

If they are used, the current linker error is confusing, since it talks about missing functions in the bowels of driver model.

Avoid compiling these functions at all with of-platdata, so that a straightforward link error points to the problem.

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

drivers/gpio/gpio-uclass.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index bad6b71e0c3..e84b68db772 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -1023,6 +1023,7 @@ err: return ret; }

+#if !CONFIG_IS_ENABLED(OF_PLATDATA) static int _gpio_request_by_name_nodev(ofnode node, const char *list_name, int index, struct gpio_desc *desc, int flags, bool add_index) @@ -1109,6 +1110,7 @@ int gpio_get_list_count(struct udevice *dev, const char *list_name)

return ret; } +#endif /* OF_PLATDATA */

int dm_gpio_free(struct udevice *dev, struct gpio_desc *desc) {

It is more useful to be able to read all the flags, not just the direction ones. In fact this is what the STM32 driver does. Update the method name to reflect this.

Tweak the docs a little and use 'flagsp' as the return argument, as is common in driver model, to indicate it returns a value.

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

drivers/gpio/gpio-uclass.c | 30 +++++++++++++++--------------- drivers/gpio/sandbox.c | 8 ++++---- drivers/gpio/stm32_gpio.c | 8 ++++---- drivers/pinctrl/pinctrl-stmfx.c | 8 ++++---- include/asm-generic/gpio.h | 13 +++++++------ 5 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 0862a28bf86..83d3cf0a6b3 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -682,39 +682,39 @@ int dm_gpio_set_dir(struct gpio_desc *desc) return _dm_gpio_update_flags(desc, desc->flags); }

-int dm_gpio_get_dir_flags(struct gpio_desc *desc, ulong *flags) +int dm_gpio_get_dir_flags(struct gpio_desc *desc, ulong *flagsp) { struct udevice *dev = desc->dev; int ret, value; struct dm_gpio_ops *ops = gpio_get_ops(dev); - ulong dir_flags; + ulong flags;

- ret = check_reserved(desc, "get_dir_flags"); + ret = check_reserved(desc, "get_flags"); if (ret) return ret;

/* GPIOD_ are directly provided by driver except GPIOD_ACTIVE_LOW */ - if (ops->get_dir_flags) { - ret = ops->get_dir_flags(dev, desc->offset, &dir_flags); + if (ops->get_flags) { + ret = ops->get_flags(dev, desc->offset, &flags); if (ret) return ret;

/* GPIOD_ACTIVE_LOW is saved in desc->flags */ - value = dir_flags & GPIOD_IS_OUT_ACTIVE ? 1 : 0; + value = flags & GPIOD_IS_OUT_ACTIVE ? 1 : 0; if (desc->flags & GPIOD_ACTIVE_LOW) value = !value; - dir_flags &= ~(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE); - dir_flags |= (desc->flags & GPIOD_ACTIVE_LOW); + flags &= ~(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE); + flags |= (desc->flags & GPIOD_ACTIVE_LOW); if (value) - dir_flags |= GPIOD_IS_OUT_ACTIVE; + flags |= GPIOD_IS_OUT_ACTIVE; } else { - dir_flags = desc->flags; + flags = desc->flags; /* only GPIOD_IS_OUT_ACTIVE is provided by uclass */ - dir_flags &= ~GPIOD_IS_OUT_ACTIVE; + flags &= ~GPIOD_IS_OUT_ACTIVE; if ((desc->flags & GPIOD_IS_OUT) && _gpio_get_value(desc)) - dir_flags |= GPIOD_IS_OUT_ACTIVE; + flags |= GPIOD_IS_OUT_ACTIVE; } - *flags = dir_flags; + *flagsp = flags;

return 0; } @@ -1309,8 +1309,8 @@ static int gpio_post_bind(struct udevice *dev) ops->xlate += gd->reloc_off; if (ops->update_flags) ops->update_flags += gd->reloc_off; - if (ops->get_dir_flags) - ops->get_dir_flags += gd->reloc_off; + if (ops->get_flags) + ops->get_flags += gd->reloc_off;

reloc_done++; } diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index 029908dc9f9..fd14d4e8b5f 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -199,11 +199,11 @@ static int sb_gpio_update_flags(struct udevice *dev, unsigned int offset, return 0; }

-static int sb_gpio_get_dir_flags(struct udevice *dev, unsigned int offset, - ulong *flags) +static int sb_gpio_get_flags(struct udevice *dev, unsigned int offset, + ulong *flagsp) { debug("%s: offset:%u\n", __func__, offset); - *flags = *get_gpio_dir_flags(dev, offset); + *flagsp = *get_gpio_dir_flags(dev, offset);

return 0; } @@ -273,7 +273,7 @@ static const struct dm_gpio_ops gpio_sandbox_ops = { .get_function = sb_gpio_get_function, .xlate = sb_gpio_xlate, .update_flags = sb_gpio_update_flags, - .get_dir_flags = sb_gpio_get_dir_flags, + .get_flags = sb_gpio_get_flags, #if CONFIG_IS_ENABLED(ACPIGEN) .get_acpi = sb_gpio_get_acpi, #endif diff --git a/drivers/gpio/stm32_gpio.c b/drivers/gpio/stm32_gpio.c index daae6ddb93f..06a1f28cb77 100644 --- a/drivers/gpio/stm32_gpio.c +++ b/drivers/gpio/stm32_gpio.c @@ -221,8 +221,8 @@ static int stm32_gpio_update_flags(struct udevice *dev, unsigned int offset, return 0; }

-static int stm32_gpio_get_dir_flags(struct udevice *dev, unsigned int offset, - ulong *flags) +static int stm32_gpio_get_flags(struct udevice *dev, unsigned int offset, + ulong *flagsp) { struct stm32_gpio_priv *priv = dev_get_priv(dev); struct stm32_gpio_regs *regs = priv->regs; @@ -257,7 +257,7 @@ static int stm32_gpio_get_dir_flags(struct udevice *dev, unsigned int offset, default: break; } - *flags = dir_flags; + *flagsp = dir_flags;

return 0; } @@ -269,7 +269,7 @@ static const struct dm_gpio_ops gpio_stm32_ops = { .set_value = stm32_gpio_set_value, .get_function = stm32_gpio_get_function, .update_flags = stm32_gpio_update_flags, - .get_dir_flags = stm32_gpio_get_dir_flags, + .get_flags = stm32_gpio_get_flags, };

static int gpio_stm32_probe(struct udevice *dev) diff --git a/drivers/pinctrl/pinctrl-stmfx.c b/drivers/pinctrl/pinctrl-stmfx.c index 084d5cef7aa..6477febbaa1 100644 --- a/drivers/pinctrl/pinctrl-stmfx.c +++ b/drivers/pinctrl/pinctrl-stmfx.c @@ -199,8 +199,8 @@ static int stmfx_gpio_update_flags(struct udevice *dev, unsigned int offset, return ret; }

-static int stmfx_gpio_get_dir_flags(struct udevice *dev, unsigned int offset, - ulong *flags) +static int stmfx_gpio_get_flags(struct udevice *dev, unsigned int offset, + ulong *flagsp) { ulong dir_flags = 0; int ret; @@ -233,7 +233,7 @@ static int stmfx_gpio_get_dir_flags(struct udevice *dev, unsigned int offset, dir_flags |= GPIOD_PULL_DOWN; } } - *flags = dir_flags; + *flagsp = dir_flags;

return 0; } @@ -267,7 +267,7 @@ static const struct dm_gpio_ops stmfx_gpio_ops = { .direction_input = stmfx_gpio_direction_input, .direction_output = stmfx_gpio_direction_output, .update_flags = stmfx_gpio_update_flags, - .get_dir_flags = stmfx_gpio_get_dir_flags, + .get_flags = stmfx_gpio_get_flags, };

U_BOOT_DRIVER(stmfx_gpio) = { diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 4626e7d92ae..48e042dc44b 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -309,7 +309,7 @@ struct dm_gpio_ops { * If any flags cannot be set (e.g. the driver or hardware does not * support them or this particular GPIO does not have the requested * feature), the driver should perform what changes it can. The uclass - * can read the current flags back with a call to get_dir_flags() if + * can read the current flags back with a call to get_flags() if * desired. * * The uclass checks that flags do not obviously conflict (e.g. input @@ -335,19 +335,20 @@ struct dm_gpio_ops { ulong flags);

/** - * get_dir_flags() - Get GPIO dir flags + * get_flags() - Get GPIO flags * - * This function return the GPIO direction flags used. + * This function return the GPIO flags used. It should read this from + * the hardware directly. * * This method is optional. * * @dev: GPIO device * @offset: GPIO offset within that device - * @flags: place to put the used direction flags by GPIO + * @flagsp: place to put the current flags value * @return 0 if OK, -ve on error */ - int (*get_dir_flags)(struct udevice *dev, unsigned int offset, - ulong *flags); + int (*get_flags)(struct udevice *dev, unsigned int offset, + ulong *flagsp);

#if CONFIG_IS_ENABLED(ACPIGEN) /**

This function is not used. Drop it.

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

drivers/gpio/gpio-uclass.c | 11 ----------- include/asm-generic/gpio.h | 11 ----------- 2 files changed, 22 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 864a9003245..aa0e3852122 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -671,17 +671,6 @@ int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) return ret; }

-int dm_gpio_set_dir(struct gpio_desc *desc) -{ - int ret; - - ret = check_reserved(desc, "set_dir"); - if (ret) - return ret; - - return _dm_gpio_update_flags(desc, desc->flags); -} - int dm_gpio_get_flags(struct gpio_desc *desc, ulong *flagsp) { struct udevice *dev = desc->dev; diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 5ecb73c138d..30ff5feb160 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -659,17 +659,6 @@ int dm_gpio_get_value(const struct gpio_desc *desc);

int dm_gpio_set_value(const struct gpio_desc *desc, int value);

-/** - * dm_gpio_set_dir() - Set the direction for a GPIO - * - * This sets up the direction according to the GPIO flags: desc->flags. - * - * @desc: GPIO description containing device, offset and flags, - * previously returned by gpio_request_by_name() - * @return 0 if OK, -ve on error - */ -int dm_gpio_set_dir(struct gpio_desc *desc); - /** * dm_gpio_set_dir_flags() - Set direction using description and added flags *

Adjust the terminology in this driver to reflect that fact that all flags are handled, not just direction flags.

Create a new access function to get the full GPIO state, not just the direction flags. Drop the static invalid_dir_flags since we can rely on a segfault if something is wrong.

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

arch/sandbox/include/asm/gpio.h | 8 ++-- drivers/gpio/sandbox.c | 65 ++++++++++++++++++--------------- test/dm/gpio.c | 18 ++++----- 3 files changed, 49 insertions(+), 42 deletions(-)

diff --git a/arch/sandbox/include/asm/gpio.h b/arch/sandbox/include/asm/gpio.h index df4ba4fb5f3..20d78296551 100644 --- a/arch/sandbox/include/asm/gpio.h +++ b/arch/sandbox/include/asm/gpio.h @@ -69,17 +69,17 @@ int sandbox_gpio_set_direction(struct udevice *dev, unsigned int offset, * @param offset GPIO offset within bank * @return dir_flags: bitfield accesses by GPIOD_ defines */ -ulong sandbox_gpio_get_dir_flags(struct udevice *dev, unsigned int offset); +ulong sandbox_gpio_get_flags(struct udevice *dev, unsigned int offset);

/** * Set the simulated flags of a GPIO (used only in sandbox test code) * * @param dev device to use * @param offset GPIO offset within bank - * @param flags dir_flags: bitfield accesses by GPIOD_ defines + * @param flags bitfield accesses by GPIOD_ defines * @return -1 on error, 0 if ok */ -int sandbox_gpio_set_dir_flags(struct udevice *dev, unsigned int offset, - ulong flags); +int sandbox_gpio_set_flags(struct udevice *dev, unsigned int offset, + ulong flags);

#endif diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index fd14d4e8b5f..9ce5d823505 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -22,34 +22,44 @@

struct gpio_state { const char *label; /* label given by requester */ - ulong dir_flags; /* dir_flags (GPIOD_...) */ + ulong flags; /* flags (GPIOD_...) */ };

-/* Access routines for GPIO dir flags */ -static ulong *get_gpio_dir_flags(struct udevice *dev, unsigned int offset) +/* Access routines for GPIO info */ +static struct gpio_state *get_gpio_state(struct udevice *dev, uint offset) { struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); struct gpio_state *state = dev_get_priv(dev);

if (offset >= uc_priv->gpio_count) { - static ulong invalid_dir_flags; printf("sandbox_gpio: error: invalid gpio %u\n", offset); - return &invalid_dir_flags; + return NULL; }

- return &state[offset].dir_flags; + return &state[offset]; +} + +/* Access routines for GPIO dir flags */ +static ulong *get_gpio_flags(struct udevice *dev, unsigned int offset) +{ + struct gpio_state *state = get_gpio_state(dev, offset); + + if (!state) + return NULL; + + return &state->flags;

}

static int get_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag) { - return (*get_gpio_dir_flags(dev, offset) & flag) != 0; + return (*get_gpio_flags(dev, offset) & flag) != 0; }

static int set_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag, int value) { - ulong *gpio = get_gpio_dir_flags(dev, offset); + ulong *gpio = get_gpio_flags(dev, offset);

if (value) *gpio |= flag; @@ -88,15 +98,14 @@ int sandbox_gpio_set_direction(struct udevice *dev, unsigned offset, int output) return 0; }

-ulong sandbox_gpio_get_dir_flags(struct udevice *dev, unsigned int offset) +ulong sandbox_gpio_get_flags(struct udevice *dev, uint offset) { - return *get_gpio_dir_flags(dev, offset); + return *get_gpio_flags(dev, offset); }

-int sandbox_gpio_set_dir_flags(struct udevice *dev, unsigned int offset, - ulong flags) +int sandbox_gpio_set_flags(struct udevice *dev, uint offset, ulong flags) { - *get_gpio_dir_flags(dev, offset) = flags; + *get_gpio_flags(dev, offset) = flags;

return 0; } @@ -177,33 +186,31 @@ static int sb_gpio_xlate(struct udevice *dev, struct gpio_desc *desc, return 0; }

-static int sb_gpio_update_flags(struct udevice *dev, unsigned int offset, - ulong flags) +static int sb_gpio_update_flags(struct udevice *dev, uint offset, ulong newf) { - ulong *dir_flags; + ulong *flags;

- debug("%s: offset:%u, dir_flags = %lx\n", __func__, offset, flags); + debug("%s: offset:%u, flags = %lx\n", __func__, offset, newf);

- dir_flags = get_gpio_dir_flags(dev, offset); + flags = get_gpio_flags(dev, offset);

/* * For testing purposes keep the output value when switching to input. * This allows us to manipulate the input value via the gpio command. */ - if (flags & GPIOD_IS_IN) - *dir_flags = (flags & ~GPIOD_IS_OUT_ACTIVE) | - (*dir_flags & GPIOD_IS_OUT_ACTIVE); + if (newf & GPIOD_IS_IN) + *flags = (newf & ~GPIOD_IS_OUT_ACTIVE) | + (*flags & GPIOD_IS_OUT_ACTIVE); else - *dir_flags = flags; + *flags = newf;

return 0; }

-static int sb_gpio_get_flags(struct udevice *dev, unsigned int offset, - ulong *flagsp) +static int sb_gpio_get_flags(struct udevice *dev, uint offset, ulong *flagsp) { debug("%s: offset:%u\n", __func__, offset); - *flagsp = *get_gpio_dir_flags(dev, offset); + *flagsp = *get_gpio_flags(dev, offset);

return 0; } @@ -456,7 +463,7 @@ static const char *sb_pinctrl_get_pin_name(struct udevice *dev, return pin_name; }

-static char *get_dir_flags_string(ulong flags) +static char *get_flags_string(ulong flags) { if (flags & GPIOD_OPEN_DRAIN) return "drive-open-drain"; @@ -475,7 +482,7 @@ static int sb_pinctrl_get_pin_muxing(struct udevice *dev, { struct udevice *gpio_dev; unsigned int gpio_idx; - ulong dir_flags; + ulong flags; int function;

/* look up for the bank which owns the requested pin */ @@ -484,11 +491,11 @@ static int sb_pinctrl_get_pin_muxing(struct udevice *dev, snprintf(buf, size, "Error"); } else { function = sb_gpio_get_function(gpio_dev, gpio_idx); - dir_flags = *get_gpio_dir_flags(gpio_dev, gpio_idx); + flags = *get_gpio_flags(gpio_dev, gpio_idx);

snprintf(buf, size, "gpio %s %s", function == GPIOF_OUTPUT ? "output" : "input", - get_dir_flags_string(dir_flags)); + get_flags_string(flags)); }

return 0; diff --git a/test/dm/gpio.c b/test/dm/gpio.c index 6b9ec88ca2b..43e868cd4ea 100644 --- a/test/dm/gpio.c +++ b/test/dm/gpio.c @@ -80,15 +80,15 @@ static int dm_test_gpio(struct unit_test_state *uts)

/* Make it an open drain output, and reset it */ ut_asserteq(GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE, - sandbox_gpio_get_dir_flags(dev, offset)); + sandbox_gpio_get_flags(dev, offset)); ut_assertok(ops->update_flags(dev, offset, GPIOD_IS_OUT | GPIOD_OPEN_DRAIN)); ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_DRAIN, - sandbox_gpio_get_dir_flags(dev, offset)); + sandbox_gpio_get_flags(dev, offset)); ut_assertok(ops->update_flags(dev, offset, GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE)); ut_asserteq(GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE, - sandbox_gpio_get_dir_flags(dev, offset)); + sandbox_gpio_get_flags(dev, offset));

/* Make it an input */ ut_assertok(ops->direction_input(dev, offset)); @@ -176,7 +176,7 @@ static int dm_test_gpio_opendrain_opensource(struct unit_test_state *uts)

/* GPIO 0 is (GPIO_OUT|GPIO_OPEN_DRAIN) */ ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_DRAIN, - sandbox_gpio_get_dir_flags(gpio_c, 0)); + sandbox_gpio_get_flags(gpio_c, 0));

/* Set it as output high, should become an input */ ut_assertok(dm_gpio_set_value(&desc_list[0], 1)); @@ -190,7 +190,7 @@ static int dm_test_gpio_opendrain_opensource(struct unit_test_state *uts)

/* GPIO 1 is (GPIO_OUT|GPIO_OPEN_SOURCE) */ ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_SOURCE, - sandbox_gpio_get_dir_flags(gpio_c, 1)); + sandbox_gpio_get_flags(gpio_c, 1));

/* Set it as output high, should become output high */ ut_assertok(dm_gpio_set_value(&desc_list[1], 1)); @@ -204,7 +204,7 @@ static int dm_test_gpio_opendrain_opensource(struct unit_test_state *uts)

/* GPIO 6 is (GPIO_ACTIVE_LOW|GPIO_OUT|GPIO_OPEN_DRAIN) */ ut_asserteq(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT | GPIOD_OPEN_DRAIN, - sandbox_gpio_get_dir_flags(gpio_c, 6)); + sandbox_gpio_get_flags(gpio_c, 6));

/* Set it as output high, should become output low */ ut_assertok(dm_gpio_set_value(&desc_list[6], 1)); @@ -218,7 +218,7 @@ static int dm_test_gpio_opendrain_opensource(struct unit_test_state *uts)

/* GPIO 7 is (GPIO_ACTIVE_LOW|GPIO_OUT|GPIO_OPEN_SOURCE) */ ut_asserteq(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT | GPIOD_OPEN_SOURCE, - sandbox_gpio_get_dir_flags(gpio_c, 7)); + sandbox_gpio_get_flags(gpio_c, 7));

/* Set it as output high, should become an input */ ut_assertok(dm_gpio_set_value(&desc_list[7], 1)); @@ -363,12 +363,12 @@ static int dm_test_gpio_phandles(struct unit_test_state *uts) ut_assertok(gpio_free_list(dev, desc_list, 3));

ut_asserteq(GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE, - sandbox_gpio_get_dir_flags(gpio_a, 1)); + sandbox_gpio_get_flags(gpio_a, 1)); ut_asserteq(6, gpio_request_list_by_name(dev, "test2-gpios", desc_list, ARRAY_SIZE(desc_list), 0));

/* This was set to output previously but flags resetted to 0 = INPUT */ - ut_asserteq(0, sandbox_gpio_get_dir_flags(gpio_a, 1)); + ut_asserteq(0, sandbox_gpio_get_flags(gpio_a, 1)); ut_asserteq(GPIOF_INPUT, gpio_get_function(gpio_a, 1, NULL));

/* Active low should invert the input value */

At present with the sandbox GPIO driver it is not possible to change the value of GPIOD_IS_OUT_ACTIVE unless the GPIO is an output. This makes it hard to test changing the flags since we need to be aware of the internal workings of the driver.

The feature is designed to aid testing.

Split this feature out into a separate sandbox-specific flag, so that the flags can change unimpeded. This will make it easier to allow updating the flags in a future patch.

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

arch/sandbox/include/asm/gpio.h | 5 ++++ drivers/gpio/sandbox.c | 43 +++++++++++++++------------------ 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/arch/sandbox/include/asm/gpio.h b/arch/sandbox/include/asm/gpio.h index 20d78296551..3f267797644 100644 --- a/arch/sandbox/include/asm/gpio.h +++ b/arch/sandbox/include/asm/gpio.h @@ -23,6 +23,11 @@ */ #include <asm-generic/gpio.h>

+/* Our own private GPIO flags, which musn't conflict with GPIOD_... */ +#define GPIOD_EXT_HIGH BIT(20) /* external source is high (else low) */ + +#define GPIOD_SANDBOX_MASK BIT(20) + /** * Return the simulated value of a GPIO (used only in sandbox test code) * diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index 9ce5d823505..52e73e2300a 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -59,12 +59,12 @@ static int get_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag) static int set_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag, int value) { - ulong *gpio = get_gpio_flags(dev, offset); + struct gpio_state *state = get_gpio_state(dev, offset);

if (value) - *gpio |= flag; + state->flags |= flag; else - *gpio &= ~flag; + state->flags &= ~flag;

return 0; } @@ -75,14 +75,19 @@ static int set_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag,

int sandbox_gpio_get_value(struct udevice *dev, unsigned offset) { + struct gpio_state *state = get_gpio_state(dev, offset); + if (get_gpio_flag(dev, offset, GPIOD_IS_OUT)) debug("sandbox_gpio: get_value on output gpio %u\n", offset); - return get_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE); + + return state->flags & GPIOD_EXT_HIGH ? true : false; }

int sandbox_gpio_set_value(struct udevice *dev, unsigned offset, int value) { - return set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE, value); + set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE | GPIOD_EXT_HIGH, value); + + return 0; }

int sandbox_gpio_get_direction(struct udevice *dev, unsigned offset) @@ -93,19 +98,25 @@ int sandbox_gpio_get_direction(struct udevice *dev, unsigned offset) int sandbox_gpio_set_direction(struct udevice *dev, unsigned offset, int output) { set_gpio_flag(dev, offset, GPIOD_IS_OUT, output); - set_gpio_flag(dev, offset, GPIOD_IS_IN, !(output)); + set_gpio_flag(dev, offset, GPIOD_IS_IN, !output);

return 0; }

ulong sandbox_gpio_get_flags(struct udevice *dev, uint offset) { - return *get_gpio_flags(dev, offset); + ulong flags = *get_gpio_flags(dev, offset); + + return flags & ~GPIOD_SANDBOX_MASK; }

int sandbox_gpio_set_flags(struct udevice *dev, uint offset, ulong flags) { - *get_gpio_flags(dev, offset) = flags; + struct gpio_state *state = get_gpio_state(dev, offset); + + if (flags & GPIOD_IS_OUT_ACTIVE) + flags |= GPIOD_EXT_HIGH; + state->flags = (state->flags & GPIOD_SANDBOX_MASK) | flags;

return 0; } @@ -188,23 +199,9 @@ static int sb_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,

static int sb_gpio_update_flags(struct udevice *dev, uint offset, ulong newf) { - ulong *flags; - debug("%s: offset:%u, flags = %lx\n", __func__, offset, newf);

- flags = get_gpio_flags(dev, offset); - - /* - * For testing purposes keep the output value when switching to input. - * This allows us to manipulate the input value via the gpio command. - */ - if (newf & GPIOD_IS_IN) - *flags = (newf & ~GPIOD_IS_OUT_ACTIVE) | - (*flags & GPIOD_IS_OUT_ACTIVE); - else - *flags = newf; - - return 0; + return sandbox_gpio_set_flags(dev, offset, newf); }

static int sb_gpio_get_flags(struct udevice *dev, uint offset, ulong *flagsp)

At present we have the concept of a pin's external value. This is what is used when getting the value of a pin. But we still set the GPIOD_IS_OUT_ACTIVE flag when changing the value. This is not actually correct, since if the pin changes from output to input, the external value need not change. Adjust the logic for this difference.

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

drivers/gpio/sandbox.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index 52e73e2300a..ebc160cb849 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -85,7 +85,7 @@ int sandbox_gpio_get_value(struct udevice *dev, unsigned offset)

int sandbox_gpio_set_value(struct udevice *dev, unsigned offset, int value) { - set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE | GPIOD_EXT_HIGH, value); + set_gpio_flag(dev, offset, GPIOD_EXT_HIGH, value);

return 0; } @@ -137,10 +137,19 @@ static int sb_gpio_direction_input(struct udevice *dev, unsigned offset) static int sb_gpio_direction_output(struct udevice *dev, unsigned offset, int value) { + int ret; + debug("%s: offset:%u, value = %d\n", __func__, offset, value);

- return sandbox_gpio_set_direction(dev, offset, 1) | - sandbox_gpio_set_value(dev, offset, value); + ret = sandbox_gpio_set_direction(dev, offset, 1); + if (ret) + return ret; + ret = set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE | GPIOD_EXT_HIGH, + value); + if (ret) + return ret; + + return 0; }

/* read GPIO IN value of port 'offset' */ @@ -154,6 +163,8 @@ static int sb_gpio_get_value(struct udevice *dev, unsigned offset) /* write GPIO OUT value to port 'offset' */ static int sb_gpio_set_value(struct udevice *dev, unsigned offset, int value) { + int ret; + debug("%s: offset:%u, value = %d\n", __func__, offset, value);

if (!sandbox_gpio_get_direction(dev, offset)) { @@ -162,7 +173,12 @@ static int sb_gpio_set_value(struct udevice *dev, unsigned offset, int value) return -1; }

- return sandbox_gpio_set_value(dev, offset, value); + ret = set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE | GPIOD_EXT_HIGH, + value); + if (ret) + return ret; + + return 0; }

static int sb_gpio_get_function(struct udevice *dev, unsigned offset)

Allow this function to see all flags, including the internal sandbox ones. This allows the tests to fully control the behaviour of the driver.

To make this work, move the setting of GPIOD_EXT_HIGH -to where the flags are updated via driver model, rather than the sandbox 'back door'.

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

drivers/gpio/sandbox.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index ebc160cb849..11d4757abbb 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -114,9 +114,7 @@ int sandbox_gpio_set_flags(struct udevice *dev, uint offset, ulong flags) { struct gpio_state *state = get_gpio_state(dev, offset);

- if (flags & GPIOD_IS_OUT_ACTIVE) - flags |= GPIOD_EXT_HIGH; - state->flags = (state->flags & GPIOD_SANDBOX_MASK) | flags; + state->flags = flags;

return 0; } @@ -213,17 +211,26 @@ static int sb_gpio_xlate(struct udevice *dev, struct gpio_desc *desc, return 0; }

-static int sb_gpio_update_flags(struct udevice *dev, uint offset, ulong newf) +static int sb_gpio_update_flags(struct udevice *dev, uint offset, ulong flags) { - debug("%s: offset:%u, flags = %lx\n", __func__, offset, newf); + debug("%s: offset:%u, flags = %lx\n", __func__, offset, flags); + struct gpio_state *state = get_gpio_state(dev, offset); + + if (flags & GPIOD_IS_OUT) { + if (flags & GPIOD_IS_OUT_ACTIVE) + flags |= GPIOD_EXT_HIGH; + else + flags &= ~GPIOD_EXT_HIGH; + } + state->flags = flags;

- return sandbox_gpio_set_flags(dev, offset, newf); + return 0; }

static int sb_gpio_get_flags(struct udevice *dev, uint offset, ulong *flagsp) { debug("%s: offset:%u\n", __func__, offset); - *flagsp = *get_gpio_flags(dev, offset); + *flagsp = *get_gpio_flags(dev, offset) & ~GPIOD_SANDBOX_MASK;

return 0; }

It is convenient to be able to adjust some of the flags for a GPIO while leaving others alone. Add a function for this.

Update dm_gpio_set_dir_flags() to make use of this.

Also update dm_gpio_set_value() to use this also, since this allows the open-drain / open-source features to be implemented directly in the driver, rather than using the uclass workaround.

Update the sandbox tests accordingly. This involves a lot of changes to dm_test_gpio_opendrain_opensource() since we no-longer have the direciion being reported differently depending on the open drain/open source flags.

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

drivers/gpio/gpio-uclass.c | 65 ++++++++++++++----- include/asm-generic/gpio.h | 25 ++++++++ test/dm/gpio.c | 125 ++++++++++++++++++++++++++++++++----- 3 files changed, 184 insertions(+), 31 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index aa0e3852122..05627ecdc30 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -568,6 +568,7 @@ int dm_gpio_get_value(const struct gpio_desc *desc)

int dm_gpio_set_value(const struct gpio_desc *desc, int value) { + const struct dm_gpio_ops *ops; int ret;

ret = check_reserved(desc, "set_value"); @@ -577,21 +578,33 @@ int dm_gpio_set_value(const struct gpio_desc *desc, int value) if (desc->flags & GPIOD_ACTIVE_LOW) value = !value;

+ /* GPIOD_ are directly managed by driver in update_flags */ + ops = gpio_get_ops(desc->dev); + if (ops->update_flags) { + ulong flags = desc->flags; + + if (value) + flags |= GPIOD_IS_OUT_ACTIVE; + else + flags &= ~GPIOD_IS_OUT_ACTIVE; + return ops->update_flags(desc->dev, desc->offset, flags); + } + /* * Emulate open drain by not actively driving the line high or * Emulate open source by not actively driving the line low */ if ((desc->flags & GPIOD_OPEN_DRAIN && value) || (desc->flags & GPIOD_OPEN_SOURCE && !value)) - return gpio_get_ops(desc->dev)->direction_input(desc->dev, - desc->offset); + return ops->direction_input(desc->dev, desc->offset); else if (desc->flags & GPIOD_OPEN_DRAIN || desc->flags & GPIOD_OPEN_SOURCE) - return gpio_get_ops(desc->dev)->direction_output(desc->dev, - desc->offset, - value); + return ops->direction_output(desc->dev, desc->offset, value); + + ret = ops->set_value(desc->dev, desc->offset, value); + if (ret) + return ret;

- gpio_get_ops(desc->dev)->set_value(desc->dev, desc->offset, value); return 0; }

@@ -619,6 +632,17 @@ static int check_dir_flags(ulong flags) return 0; }

+/** + * _dm_gpio_update_flags() - Send flags to the driver + * + * This uses the best available method to send the given flags to the driver. + * Note that if flags & GPIOD_ACTIVE_LOW, the driver sees the opposite value + * of GPIOD_IS_OUT_ACTIVE. + * + * @desc: GPIO description + * @flags: flags value to set + * @return 0 if OK, -ve on error + */ static int _dm_gpio_update_flags(struct gpio_desc *desc, ulong flags) { struct udevice *dev = desc->dev; @@ -637,6 +661,11 @@ static int _dm_gpio_update_flags(struct gpio_desc *desc, ulong flags) return ret; }

+ /* If active low, invert the output state */ + if ((flags & (GPIOD_IS_OUT | GPIOD_ACTIVE_LOW)) == + (GPIOD_IS_OUT | GPIOD_ACTIVE_LOW)) + flags ^= GPIOD_IS_OUT_ACTIVE; + /* GPIOD_ are directly managed by driver in update_flags */ if (ops->update_flags) { ret = ops->update_flags(dev, desc->offset, flags); @@ -649,26 +678,34 @@ static int _dm_gpio_update_flags(struct gpio_desc *desc, ulong flags) } }

- /* save the flags also in descriptor */ - if (!ret) - desc->flags = flags; - return ret; }

-int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) +int dm_gpio_clrset_flags(struct gpio_desc *desc, ulong clr, ulong set) { + ulong flags; int ret;

ret = check_reserved(desc, "set_dir_flags"); if (ret) return ret;

- /* combine the requested flags (for IN/OUT) and the descriptor flags */ - flags |= desc->flags; + flags = (desc->flags & ~clr) | set; + ret = _dm_gpio_update_flags(desc, flags); + if (ret) + return ret;

- return ret; + /* save the flags also in descriptor */ + desc->flags = flags; + + return 0; +} + +int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) +{ + /* combine the requested flags (for IN/OUT) and the descriptor flags */ + return dm_gpio_clrset_flags(desc, 0, flags); }

int dm_gpio_get_flags(struct gpio_desc *desc, ulong *flagsp) diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 30ff5feb160..4a657b1bd2b 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -128,6 +128,12 @@ struct gpio_desc { #define GPIOD_PULL_UP BIT(7) /* GPIO has pull-up enabled */ #define GPIOD_PULL_DOWN BIT(8) /* GPIO has pull-down enabled */

+/* Flags for updating the above */ +#define GPIOD_MASK_DIR (GPIOD_IS_OUT | GPIOD_IS_IN | \ + GPIOD_IS_OUT_ACTIVE) +#define GPIOD_MASK_DSTYPE (GPIOD_OPEN_DRAIN | GPIOD_OPEN_SOURCE) +#define GPIOD_MASK_PULL (GPIOD_PULL_UP | GPIOD_PULL_DOWN) + uint offset; /* GPIO offset within the device */ /* * We could consider adding the GPIO label in here. Possibly we could @@ -659,6 +665,25 @@ int dm_gpio_get_value(const struct gpio_desc *desc);

int dm_gpio_set_value(const struct gpio_desc *desc, int value);

+/** + * dm_gpio_clrset_flags() - Update flags + * + * This updates the flags as directled. Note that desc->flags is updated by this + * function on success. If any changes cannot be made, best efforts are made. + * + * By use of @clr and @set any of flags can be individually updated, or left + * alone + * + * @desc: GPIO description containing device, offset and flags, + * previously returned by gpio_request_by_name() + * @clr: Flags to clear (GPIOD_...) + * @set: Flags to set (GPIOD_...) + * @return 0 if OK, -EINVAL if the flags had obvious conflicts, + * -ERECALLCONFLICT if there was a non-obvious hardware conflict when attempting + * to set the flags + */ +int dm_gpio_clrset_flags(struct gpio_desc *desc, ulong clr, ulong set); + /** * dm_gpio_set_dir_flags() - Set direction using description and added flags * diff --git a/test/dm/gpio.c b/test/dm/gpio.c index 43e868cd4ea..eaf78e9aed8 100644 --- a/test/dm/gpio.c +++ b/test/dm/gpio.c @@ -178,15 +178,15 @@ static int dm_test_gpio_opendrain_opensource(struct unit_test_state *uts) ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_DRAIN, sandbox_gpio_get_flags(gpio_c, 0));

- /* Set it as output high, should become an input */ + /* Set it as output high */ ut_assertok(dm_gpio_set_value(&desc_list[0], 1)); - ut_assertok(gpio_get_status(gpio_c, 0, buf, sizeof(buf))); - ut_asserteq_str("c0: input: 0 [x] a-test.test3-gpios0", buf); + ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_DRAIN | GPIOD_IS_OUT_ACTIVE, + sandbox_gpio_get_flags(gpio_c, 0));

- /* Set it as output low, should become output low */ + /* Set it as output low */ ut_assertok(dm_gpio_set_value(&desc_list[0], 0)); - ut_assertok(gpio_get_status(gpio_c, 0, buf, sizeof(buf))); - ut_asserteq_str("c0: output: 0 [x] a-test.test3-gpios0", buf); + ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_DRAIN, + sandbox_gpio_get_flags(gpio_c, 0));

/* GPIO 1 is (GPIO_OUT|GPIO_OPEN_SOURCE) */ ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_SOURCE, @@ -197,13 +197,21 @@ static int dm_test_gpio_opendrain_opensource(struct unit_test_state *uts) ut_assertok(gpio_get_status(gpio_c, 1, buf, sizeof(buf))); ut_asserteq_str("c1: output: 1 [x] a-test.test3-gpios1", buf);

- /* Set it as output low, should become an input */ + /* Set it as output low */ ut_assertok(dm_gpio_set_value(&desc_list[1], 0)); + ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_SOURCE, + sandbox_gpio_get_flags(gpio_c, 1)); + ut_assertok(gpio_get_status(gpio_c, 1, buf, sizeof(buf))); - ut_asserteq_str("c1: input: 1 [x] a-test.test3-gpios1", buf); + ut_asserteq_str("c1: output: 0 [x] a-test.test3-gpios1", buf);

- /* GPIO 6 is (GPIO_ACTIVE_LOW|GPIO_OUT|GPIO_OPEN_DRAIN) */ - ut_asserteq(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT | GPIOD_OPEN_DRAIN, + /* + * GPIO 6 is (GPIO_ACTIVE_LOW|GPIO_OUT|GPIO_OPEN_DRAIN). Looking at it + * directlt from the driver, we get GPIOD_IS_OUT_ACTIVE also, since it + * is active low + */ + ut_asserteq(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT | GPIOD_OPEN_DRAIN | + GPIOD_IS_OUT_ACTIVE, sandbox_gpio_get_flags(gpio_c, 6));

/* Set it as output high, should become output low */ @@ -211,19 +219,21 @@ static int dm_test_gpio_opendrain_opensource(struct unit_test_state *uts) ut_assertok(gpio_get_status(gpio_c, 6, buf, sizeof(buf))); ut_asserteq_str("c6: output: 0 [x] a-test.test3-gpios6", buf);

- /* Set it as output low, should become an input */ + /* Set it as output low */ ut_assertok(dm_gpio_set_value(&desc_list[6], 0)); - ut_assertok(gpio_get_status(gpio_c, 6, buf, sizeof(buf))); - ut_asserteq_str("c6: input: 0 [x] a-test.test3-gpios6", buf); + ut_asserteq(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT | GPIOD_OPEN_DRAIN | + GPIOD_IS_OUT_ACTIVE, + sandbox_gpio_get_flags(gpio_c, 6));

/* GPIO 7 is (GPIO_ACTIVE_LOW|GPIO_OUT|GPIO_OPEN_SOURCE) */ - ut_asserteq(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT | GPIOD_OPEN_SOURCE, + ut_asserteq(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT | GPIOD_OPEN_SOURCE | + GPIOD_IS_OUT_ACTIVE, sandbox_gpio_get_flags(gpio_c, 7));

- /* Set it as output high, should become an input */ + /* Set it as output high */ ut_assertok(dm_gpio_set_value(&desc_list[7], 1)); - ut_assertok(gpio_get_status(gpio_c, 7, buf, sizeof(buf))); - ut_asserteq_str("c7: input: 0 [x] a-test.test3-gpios7", buf); + ut_asserteq(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT | GPIOD_OPEN_SOURCE, + sandbox_gpio_get_flags(gpio_c, 7));

/* Set it as output low, should become output high */ ut_assertok(dm_gpio_set_value(&desc_list[7], 0)); @@ -582,3 +592,84 @@ static int dm_test_gpio_devm(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_gpio_devm, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); + +static int dm_test_clrset_flags(struct unit_test_state *uts) +{ + struct gpio_desc desc; + struct udevice *dev; + ulong flags; + + ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev)); + ut_asserteq_str("a-test", dev->name); + ut_assertok(gpio_request_by_name(dev, "test-gpios", 1, &desc, 0)); + + ut_assertok(dm_gpio_clrset_flags(&desc, GPIOD_MASK_DIR, GPIOD_IS_OUT)); + ut_assertok(dm_gpio_get_flags(&desc, &flags)); + ut_asserteq(GPIOD_IS_OUT, flags); + ut_asserteq(0, sandbox_gpio_get_value(desc.dev, desc.offset)); + + ut_assertok(dm_gpio_clrset_flags(&desc, 0, GPIOD_IS_OUT_ACTIVE)); + ut_assertok(dm_gpio_get_flags(&desc, &flags)); + ut_asserteq(GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE, flags); + ut_asserteq(1, sandbox_gpio_get_value(desc.dev, desc.offset)); + ut_asserteq(1, dm_gpio_get_value(&desc)); + + ut_assertok(dm_gpio_clrset_flags(&desc, GPIOD_MASK_DIR, GPIOD_IS_IN)); + ut_assertok(dm_gpio_get_flags(&desc, &flags)); + ut_asserteq(GPIOD_IS_IN, flags & GPIOD_MASK_DIR); + + ut_assertok(dm_gpio_clrset_flags(&desc, GPIOD_MASK_PULL, + GPIOD_PULL_UP)); + ut_assertok(dm_gpio_get_flags(&desc, &flags)); + ut_asserteq(GPIOD_IS_IN | GPIOD_PULL_UP, flags); + + /* Check we cannot set both PULL_UP and PULL_DOWN */ + ut_asserteq(-EINVAL, dm_gpio_clrset_flags(&desc, 0, GPIOD_PULL_DOWN)); + + return 0; +} +DM_TEST(dm_test_clrset_flags, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); + +/* Check that an active-low GPIO works as expected */ +static int dm_test_clrset_flags_invert(struct unit_test_state *uts) +{ + struct gpio_desc desc; + struct udevice *dev; + ulong flags; + + ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev)); + ut_asserteq_str("a-test", dev->name); + ut_assertok(gpio_request_by_name(dev, "test-gpios", 1, &desc, + GPIOD_IS_OUT | GPIOD_ACTIVE_LOW)); + + /* + * From this size we see it as 0 (active low), but the sandbox driver + * sees the pin value high + */ + ut_asserteq(0, dm_gpio_get_value(&desc)); + ut_asserteq(1, sandbox_gpio_get_value(desc.dev, desc.offset)); + + ut_assertok(dm_gpio_set_value(&desc, 1)); + ut_asserteq(1, dm_gpio_get_value(&desc)); + ut_asserteq(0, sandbox_gpio_get_value(desc.dev, desc.offset)); + + /* Do the same with dm_gpio_clrset_flags() */ + ut_assertok(dm_gpio_clrset_flags(&desc, GPIOD_IS_OUT_ACTIVE, 0)); + ut_asserteq(0, dm_gpio_get_value(&desc)); + ut_asserteq(1, sandbox_gpio_get_value(desc.dev, desc.offset)); + + ut_assertok(dm_gpio_clrset_flags(&desc, 0, GPIOD_IS_OUT_ACTIVE)); + ut_asserteq(1, dm_gpio_get_value(&desc)); + ut_asserteq(0, sandbox_gpio_get_value(desc.dev, desc.offset)); + + ut_assertok(dm_gpio_get_flags(&desc, &flags)); + ut_asserteq(GPIOD_IS_OUT | GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE, + flags); + + ut_assertok(dm_gpio_clrset_flags(&desc, GPIOD_IS_OUT_ACTIVE, 0)); + ut_assertok(dm_gpio_get_flags(&desc, &flags)); + ut_asserteq(GPIOD_IS_OUT | GPIOD_ACTIVE_LOW, flags); + + return 0; +} +DM_TEST(dm_test_clrset_flags_invert, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);

The new update_flags() method is more flexible since it allows the driver to see the full flags all at once. Use that in preference to these two functions. Add comments to that effect.

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

drivers/gpio/gpio-uclass.c | 15 ++++++--------- include/asm-generic/gpio.h | 26 +++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 05627ecdc30..68ed65d0899 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -512,13 +512,10 @@ int gpio_direction_input(unsigned gpio) int ret;

ret = gpio_to_device(gpio, &desc); - if (ret) - return ret; - ret = check_reserved(&desc, "dir_input"); if (ret) return ret;

- return gpio_get_ops(desc.dev)->direction_input(desc.dev, desc.offset); + return dm_gpio_clrset_flags(&desc, GPIOD_MASK_DIR, GPIOD_IS_IN); }

/** @@ -533,17 +530,17 @@ int gpio_direction_input(unsigned gpio) int gpio_direction_output(unsigned gpio, int value) { struct gpio_desc desc; + ulong flags; int ret;

ret = gpio_to_device(gpio, &desc); - if (ret) - return ret; - ret = check_reserved(&desc, "dir_output"); if (ret) return ret;

- return gpio_get_ops(desc.dev)->direction_output(desc.dev, - desc.offset, value); + flags = GPIOD_IS_OUT; + if (value) + flags |= GPIOD_IS_OUT_ACTIVE; + return dm_gpio_clrset_flags(&desc, GPIOD_MASK_DIR, flags); }

static int _gpio_get_value(const struct gpio_desc *desc) diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 4a657b1bd2b..4628d937259 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -266,10 +266,32 @@ int gpio_xlate_offs_flags(struct udevice *dev, struct gpio_desc *desc, struct dm_gpio_ops { int (*request)(struct udevice *dev, unsigned offset, const char *label); int (*rfree)(struct udevice *dev, unsigned int offset); + + /** + * direction_input() - deprecated + * + * Equivalent to update_flags(...GPIOD_IS_IN) + */ int (*direction_input)(struct udevice *dev, unsigned offset); + + /** + * direction_output() - deprecated + * + * Equivalent to update_flags(...GPIOD_IS_OUT) with GPIOD_IS_OUT_ACTIVE + * also set if @value + */ int (*direction_output)(struct udevice *dev, unsigned offset, int value); + int (*get_value)(struct udevice *dev, unsigned offset); + + /** + * set_value() - Sets the GPIO value of an output + * + * If the driver provides an @update_flags() method then that is used + * in preference to this, with GPIOD_IS_OUT_ACTIVE set according to + * @value. + */ int (*set_value)(struct udevice *dev, unsigned offset, int value); /** * get_function() Get the GPIO function @@ -328,7 +350,9 @@ struct dm_gpio_ops { * uclass, so the driver always sees the value that should be set at the * pin (1=high, 0=low). * - * This method is optional. + * This method is required and should be implemented by new drivers. At + * some point, it will supersede direction_input() and + * direction_output(), which wil be removed. * * @dev: GPIO device * @offset: GPIO offset within that device

Update this driver to use the common method of putting the driver operations in an 'ops' variable install of calling gpio_get_ops() repeatedly. Make it const since operations do not change.

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

drivers/gpio/gpio-uclass.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 68ed65d0899..77b40263bbd 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -219,7 +219,7 @@ int gpio_xlate_offs_flags(struct udevice *dev, struct gpio_desc *desc, static int gpio_find_and_xlate(struct gpio_desc *desc, struct ofnode_phandle_args *args) { - struct dm_gpio_ops *ops = gpio_get_ops(desc->dev); + const struct dm_gpio_ops *ops = gpio_get_ops(desc->dev);

if (ops->xlate) return ops->xlate(desc->dev, desc, args); @@ -352,6 +352,7 @@ int gpio_hog_lookup_name(const char *name, struct gpio_desc **desc)

int dm_gpio_request(struct gpio_desc *desc, const char *label) { + const struct dm_gpio_ops *ops = gpio_get_ops(desc->dev); struct udevice *dev = desc->dev; struct gpio_dev_priv *uc_priv; char *str; @@ -363,8 +364,8 @@ int dm_gpio_request(struct gpio_desc *desc, const char *label) str = strdup(label); if (!str) return -ENOMEM; - if (gpio_get_ops(dev)->request) { - ret = gpio_get_ops(dev)->request(dev, desc->offset, label); + if (ops->request) { + ret = ops->request(dev, desc->offset, label); if (ret) { free(str); return ret; @@ -441,14 +442,15 @@ int gpio_requestf(unsigned gpio, const char *fmt, ...)

int _dm_gpio_free(struct udevice *dev, uint offset) { + const struct dm_gpio_ops *ops = gpio_get_ops(dev); struct gpio_dev_priv *uc_priv; int ret;

uc_priv = dev_get_uclass_priv(dev); if (!uc_priv->name[offset]) return -ENXIO; - if (gpio_get_ops(dev)->rfree) { - ret = gpio_get_ops(dev)->rfree(dev, offset); + if (ops->rfree) { + ret = ops->rfree(dev, offset); if (ret) return ret; } @@ -545,9 +547,10 @@ int gpio_direction_output(unsigned gpio, int value)

static int _gpio_get_value(const struct gpio_desc *desc) { + const struct dm_gpio_ops *ops = gpio_get_ops(desc->dev); int value;

- value = gpio_get_ops(desc->dev)->get_value(desc->dev, desc->offset); + value = ops->get_value(desc->dev, desc->offset);

return desc->flags & GPIOD_ACTIVE_LOW ? !value : value; } @@ -643,7 +646,7 @@ static int check_dir_flags(ulong flags) static int _dm_gpio_update_flags(struct gpio_desc *desc, ulong flags) { struct udevice *dev = desc->dev; - struct dm_gpio_ops *ops = gpio_get_ops(dev); + const struct dm_gpio_ops *ops = gpio_get_ops(dev); struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); int ret = 0;

@@ -709,7 +712,7 @@ int dm_gpio_get_flags(struct gpio_desc *desc, ulong *flagsp) { struct udevice *dev = desc->dev; int ret, value; - struct dm_gpio_ops *ops = gpio_get_ops(dev); + const struct dm_gpio_ops *ops = gpio_get_ops(dev); ulong flags;

ret = check_reserved(desc, "get_flags"); @@ -807,7 +810,7 @@ static int get_function(struct udevice *dev, int offset, bool skip_unused, const char **namep) { struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); - struct dm_gpio_ops *ops = gpio_get_ops(dev); + const struct dm_gpio_ops *ops = gpio_get_ops(dev);

BUILD_BUG_ON(GPIOF_COUNT != ARRAY_SIZE(gpio_function)); if (!device_active(dev)) @@ -844,7 +847,7 @@ int gpio_get_raw_function(struct udevice *dev, int offset, const char **namep)

int gpio_get_status(struct udevice *dev, int offset, char *buf, int buffsize) { - struct dm_gpio_ops *ops = gpio_get_ops(dev); + const struct dm_gpio_ops *ops = gpio_get_ops(dev); struct gpio_dev_priv *priv; char *str = buf; int func; @@ -884,7 +887,7 @@ int gpio_get_status(struct udevice *dev, int offset, char *buf, int buffsize) #if CONFIG_IS_ENABLED(ACPIGEN) int gpio_get_acpi(const struct gpio_desc *desc, struct acpi_gpio *gpio) { - struct dm_gpio_ops *ops; + const struct dm_gpio_ops *ops;

memset(gpio, '\0', sizeof(*gpio)); if (!dm_gpio_is_valid(desc)) {

We don't need to implement direction_input() and direction_output() anymore. Drop them and use update_flags() instead.

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

arch/x86/include/asm/intel_pinctrl_defs.h | 5 ++ drivers/gpio/intel_gpio.c | 72 ++++++++++++----------- 2 files changed, 43 insertions(+), 34 deletions(-)

diff --git a/arch/x86/include/asm/intel_pinctrl_defs.h b/arch/x86/include/asm/intel_pinctrl_defs.h index 1ea141f082f..5d83d24bae2 100644 --- a/arch/x86/include/asm/intel_pinctrl_defs.h +++ b/arch/x86/include/asm/intel_pinctrl_defs.h @@ -11,6 +11,11 @@

/* This file is included by device trees, so avoid BIT() macros */

+#define GPIO_DW_SIZE(x) (sizeof(u32) * (x)) +#define PAD_CFG_OFFSET(x, dw_num) ((x) + GPIO_DW_SIZE(dw_num)) +#define PAD_CFG0_OFFSET(x) PAD_CFG_OFFSET(x, 0) +#define PAD_CFG1_OFFSET(x) PAD_CFG_OFFSET(x, 1) + #define PAD_CFG0_TX_STATE_BIT 0 #define PAD_CFG0_TX_STATE (1 << PAD_CFG0_TX_STATE_BIT) #define PAD_CFG0_RX_STATE_BIT 1 diff --git a/drivers/gpio/intel_gpio.c b/drivers/gpio/intel_gpio.c index eda95485c93..d596bed2465 100644 --- a/drivers/gpio/intel_gpio.c +++ b/drivers/gpio/intel_gpio.c @@ -3,6 +3,8 @@ * Copyright 2019 Google LLC */

+#define LOG_CATEGORY UCLASS_GPIO + #include <common.h> #include <dm.h> #include <errno.h> @@ -23,38 +25,6 @@ #include <dm/acpi.h> #include <dt-bindings/gpio/x86-gpio.h>

-static int intel_gpio_direction_input(struct udevice *dev, uint offset) -{ - struct udevice *pinctrl = dev_get_parent(dev); - uint config_offset; - - config_offset = intel_pinctrl_get_config_reg_offset(pinctrl, offset); - - pcr_clrsetbits32(pinctrl, config_offset, - PAD_CFG0_MODE_MASK | PAD_CFG0_TX_STATE | - PAD_CFG0_RX_DISABLE, - PAD_CFG0_MODE_GPIO | PAD_CFG0_TX_DISABLE); - - return 0; -} - -static int intel_gpio_direction_output(struct udevice *dev, uint offset, - int value) -{ - struct udevice *pinctrl = dev_get_parent(dev); - uint config_offset; - - config_offset = intel_pinctrl_get_config_reg_offset(pinctrl, offset); - - pcr_clrsetbits32(pinctrl, config_offset, - PAD_CFG0_MODE_MASK | PAD_CFG0_RX_STATE | - PAD_CFG0_TX_DISABLE | PAD_CFG0_TX_STATE, - PAD_CFG0_MODE_GPIO | PAD_CFG0_RX_DISABLE | - (value ? PAD_CFG0_TX_STATE : 0)); - - return 0; -} - static int intel_gpio_get_value(struct udevice *dev, uint offset) { struct udevice *pinctrl = dev_get_parent(dev); @@ -130,6 +100,41 @@ static int intel_gpio_xlate(struct udevice *orig_dev, struct gpio_desc *desc, return 0; }

+static int intel_gpio_update_flags(struct udevice *dev, unsigned int offset, + ulong flags) +{ + struct udevice *pinctrl = dev_get_parent(dev); + u32 bic0 = 0, bic1 = 0; + u32 or0, or1; + uint config_offset; + + config_offset = intel_pinctrl_get_config_reg_offset(pinctrl, offset); + + if (flags & GPIOD_IS_OUT) { + bic0 |= PAD_CFG0_MODE_MASK | PAD_CFG0_RX_STATE | + PAD_CFG0_TX_DISABLE; + or0 |= PAD_CFG0_MODE_GPIO | PAD_CFG0_RX_DISABLE; + } else if (flags & GPIOD_IS_IN) { + bic0 |= PAD_CFG0_MODE_MASK | PAD_CFG0_TX_STATE | + PAD_CFG0_RX_DISABLE; + or0 |= PAD_CFG0_MODE_GPIO | PAD_CFG0_TX_DISABLE; + } + if (flags & GPIOD_PULL_UP) { + bic1 |= PAD_CFG1_PULL_MASK; + or1 |= PAD_CFG1_PULL_UP_20K; + } else if (flags & GPIOD_PULL_DOWN) { + bic1 |= PAD_CFG1_PULL_MASK; + or1 |= PAD_CFG1_PULL_DN_20K; + } + + pcr_clrsetbits32(pinctrl, PAD_CFG0_OFFSET(config_offset), bic0, or0); + pcr_clrsetbits32(pinctrl, PAD_CFG1_OFFSET(config_offset), bic1, or1); + log_debug("%s: flags=%lx, offset=%x, config_offset=%x, %x/%x %x/%x\n", + dev->name, flags, offset, config_offset, bic0, or0, bic1, or1); + + return 0; +} + #if CONFIG_IS_ENABLED(ACPIGEN) static int intel_gpio_get_acpi(const struct gpio_desc *desc, struct acpi_gpio *gpio) @@ -177,12 +182,11 @@ static int intel_gpio_of_to_plat(struct udevice *dev) }

static const struct dm_gpio_ops gpio_intel_ops = { - .direction_input = intel_gpio_direction_input, - .direction_output = intel_gpio_direction_output, .get_value = intel_gpio_get_value, .set_value = intel_gpio_set_value, .get_function = intel_gpio_get_function, .xlate = intel_gpio_xlate, + .update_flags = intel_gpio_update_flags, #if CONFIG_IS_ENABLED(ACPIGEN) .get_acpi = intel_gpio_get_acpi, #endif