[U-Boot] [PATCH v4 0/4] dm: gpio: Add driver for MPC85xx GPIO controller

The functions for accessing GPIOs on MPC85xx are hardcoded in arch/powerpc/include/asm/mpc85xx_gpio.h This leads to problems if another GPIO controller supporting the driver model is to be used simultaneously.
Therefore, this patch moves the "static" functions into a DM-compatible driver, and also introduces a set of functions into the GPIO uclass that expose the controller's capability to switch individual GPIOs into open-drain-mode.
v3/v4 also implement shadowing of the GPDAT register to work around a known issue in some MPC85xx GPIO controllers (as pointed out by Joakim Tjernlund).
Furthermore, v4 adds tests for the open drain setting feature and extends the sandbox GPIO driver accordingly.
Mario Six (4): dm: gpio: Add driver for MPC85XX GPIO controller dm: gpio: Add methods for open drain setting dm: gpio: Implement open drain for MPC85XX GPIO dm: test: Add GPIO open drain tests
arch/powerpc/include/asm/arch-mpc85xx/gpio.h | 2 + arch/powerpc/include/asm/immap_85xx.h | 2 + drivers/gpio/Kconfig | 25 +++ drivers/gpio/Makefile | 1 + drivers/gpio/gpio-uclass.c | 32 ++++ drivers/gpio/mpc85xx_gpio.c | 228 +++++++++++++++++++++++++++ drivers/gpio/sandbox.c | 35 ++++ include/asm-generic/gpio.h | 34 ++++ test/dm/gpio.c | 7 + 9 files changed, 366 insertions(+) create mode 100644 drivers/gpio/mpc85xx_gpio.c
-- 2.7.0.GIT

From: "mario.six@gdsys.cc" mario.six@gdsys.cc
This patch adds a driver for the built-in GPIO controller of the MPC85XX SoC (probably supporting other PowerQUICC III SoCs as well).
Each GPIO bank is identified by its own entry in the device tree, i.e.
gpio-controller@fc00 { #gpio-cells = <2>; compatible = "fsl,pq3-gpio"; reg = <0xfc00 0x100> }
By default, each bank is assumed to have 32 GPIOs, but the ngpios setting is honored, so the number of GPIOs for each bank in configurable to match the actual GPIO count of the SoC (e.g. the 32/32/23 banks of the P1022 SoC).
The usual functions of GPIO drivers (setting input/output mode and output value setting) are supported.
The driver has been tested on MPC85XX, but it is likely that other PowerQUICC III devices will work as well.
Signed-off-by: Mario Six mario.six@gdsys.cc ---
v4: None
v3: - Added shadow for the GPDAT register, as suggested by Joakim Tjernlund - Switched to u32 for bit masks - Added some comments
v2: - Added missing commit message - Improved the Kconfig description - Fixed and documented the mpc85xx_gpio_data members - Introduced GPIO_MASK macro to simplify the code - Fixed white space issues in function headers - Removed unnecessary empty line - Use fdtdec_get_addr_size_auto_noparent to read the register base data
--- arch/powerpc/include/asm/arch-mpc85xx/gpio.h | 2 + arch/powerpc/include/asm/immap_85xx.h | 2 + drivers/gpio/Kconfig | 25 ++++ drivers/gpio/Makefile | 1 + drivers/gpio/mpc85xx_gpio.c | 187 +++++++++++++++++++++++++++ 5 files changed, 217 insertions(+) create mode 100644 drivers/gpio/mpc85xx_gpio.c
diff --git a/arch/powerpc/include/asm/arch-mpc85xx/gpio.h b/arch/powerpc/include/asm/arch-mpc85xx/gpio.h index da7352a..41b6677 100644 --- a/arch/powerpc/include/asm/arch-mpc85xx/gpio.h +++ b/arch/powerpc/include/asm/arch-mpc85xx/gpio.h @@ -14,6 +14,8 @@ #ifndef __ASM_ARCH_MX85XX_GPIO_H #define __ASM_ARCH_MX85XX_GPIO_H
+#ifndef CONFIG_MPC85XX_GPIO #include <asm/mpc85xx_gpio.h> +#endif
#endif diff --git a/arch/powerpc/include/asm/immap_85xx.h b/arch/powerpc/include/asm/immap_85xx.h index 53ca6d9..dcc50b2 100644 --- a/arch/powerpc/include/asm/immap_85xx.h +++ b/arch/powerpc/include/asm/immap_85xx.h @@ -265,6 +265,7 @@ typedef struct ccsr_pcix { #define PIWAR_WRITE_SNOOP 0x00005000 #define PIWAR_MEM_2G 0x0000001e
+#ifndef CONFIG_MPC85XX_GPIO typedef struct ccsr_gpio { u32 gpdir; u32 gpodr; @@ -273,6 +274,7 @@ typedef struct ccsr_gpio { u32 gpimr; u32 gpicr; } ccsr_gpio_t; +#endif
/* L2 Cache Registers */ typedef struct ccsr_l2cache { diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 2b4624d..068ee63 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -143,4 +143,29 @@ config ZYNQ_GPIO help Supports GPIO access on Zynq SoC.
+config MPC85XX_GPIO + bool "Freescale MPC85XX GPIO driver" + depends on DM_GPIO + help + This driver supports the built-in GPIO controller of MPC85XX CPUs. + Each GPIO bank is identified by its own entry in the device tree, + i.e. + + gpio-controller@fc00 { + #gpio-cells = <2>; + compatible = "fsl,pq3-gpio"; + reg = <0xfc00 0x100> + } + + By default, each bank is assumed to have 32 GPIOs, but the ngpios + setting is honored, so the number of GPIOs for each bank is + configurable to match the actual GPIO count of the SoC (e.g. the + 32/32/23 banks of the P1022 SoC). + + The standard functions of input/output mode, and output value setting + are supported; the open-drain capability of the controller is not + supported yet. + + The driver has been tested on MPC85XX, but it is likely that other + PowerQUICC III devices will work as well. endmenu diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 4f071c4..1e4f16b 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -32,6 +32,7 @@ obj-$(CONFIG_DA8XX_GPIO) += da8xx_gpio.o obj-$(CONFIG_DM644X_GPIO) += da8xx_gpio.o obj-$(CONFIG_ALTERA_PIO) += altera_pio.o obj-$(CONFIG_MPC83XX_GPIO) += mpc83xx_gpio.o +obj-$(CONFIG_MPC85XX_GPIO) += mpc85xx_gpio.o obj-$(CONFIG_SH_GPIO_PFC) += sh_pfc.o obj-$(CONFIG_OMAP_GPIO) += omap_gpio.o obj-$(CONFIG_DB8500_GPIO) += db8500_gpio.o diff --git a/drivers/gpio/mpc85xx_gpio.c b/drivers/gpio/mpc85xx_gpio.c new file mode 100644 index 0000000..17755df --- /dev/null +++ b/drivers/gpio/mpc85xx_gpio.c @@ -0,0 +1,187 @@ +/* + * (C) Copyright 2016 + * Mario Six, Guntermann & Drunck GmbH, six@gdsys.de + * + * based on arch/powerpc/include/asm/mpc85xx_gpio.h, which is + * + * Copyright 2010 eXMeritus, A Boeing Company + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <asm/gpio.h> +#include <mapmem.h> + +DECLARE_GLOBAL_DATA_PTR; + +struct ccsr_gpio { + u32 gpdir; + u32 gpodr; + u32 gpdat; + u32 gpier; + u32 gpimr; + u32 gpicr; +}; + +struct mpc85xx_gpio_data { + /* The bank's register base in memory */ + struct ccsr_gpio __iomem *base; + /* The address of the registers; used to identify the bank */ + ulong addr; + /* The GPIO count of the bank */ + uint gpio_count; + /* The GPDAT register cannot be used to determine the value of output + * pins on MPC8572/MPC8536, so we shadow it and use the shadowed value + * for output pins */ + u32 dat_shadow; +}; + +inline u32 gpio_mask(unsigned gpio) { + return (1U << (31 - (gpio))); +} + +static inline u32 mpc85xx_gpio_get_val(struct ccsr_gpio *base, u32 mask) +{ + return in_be32(&base->gpdat) & mask; +} + +static inline u32 mpc85xx_gpio_get_dir(struct ccsr_gpio *base, u32 mask) +{ + return in_be32(&base->gpdir) & mask; +} + +static inline void mpc85xx_gpio_set_in(struct ccsr_gpio *base, u32 gpios) +{ + clrbits_be32(&base->gpdat, gpios); + /* GPDIR register 0 -> input */ + clrbits_be32(&base->gpdir, gpios); +} + +static inline void mpc85xx_gpio_set_low(struct ccsr_gpio *base, u32 gpios) +{ + clrbits_be32(&base->gpdat, gpios); + /* GPDIR register 1 -> output */ + setbits_be32(&base->gpdir, gpios); +} + +static inline void mpc85xx_gpio_set_high(struct ccsr_gpio *base, u32 gpios) +{ + setbits_be32(&base->gpdat, gpios); + /* GPDIR register 1 -> output */ + setbits_be32(&base->gpdir, gpios); +} + +static int mpc85xx_gpio_direction_input(struct udevice *dev, unsigned gpio) +{ + struct mpc85xx_gpio_data *data = dev_get_priv(dev); + + mpc85xx_gpio_set_in(data->base, gpio_mask(gpio)); + return 0; +} + +static int mpc85xx_gpio_set_value(struct udevice *dev, unsigned gpio, + int value) +{ + struct mpc85xx_gpio_data *data = dev_get_priv(dev); + + if (value) { + data->dat_shadow |= gpio_mask(gpio); + mpc85xx_gpio_set_high(data->base, gpio_mask(gpio)); + } else { + data->dat_shadow &= ~gpio_mask(gpio); + mpc85xx_gpio_set_low(data->base, gpio_mask(gpio)); + } + return 0; +} + +static int mpc85xx_gpio_direction_output(struct udevice *dev, unsigned gpio, + int value) +{ + return mpc85xx_gpio_set_value(dev, gpio, value); +} + +static int mpc85xx_gpio_get_value(struct udevice *dev, unsigned gpio) +{ + struct mpc85xx_gpio_data *data = dev_get_priv(dev); + + if (!!mpc85xx_gpio_get_dir(data->base, gpio_mask(gpio))) { + /* Output -> use shadowed value */ + return !!(data->dat_shadow & gpio_mask(gpio)); + } else { + /* Input -> read value from GPDAT register */ + return !!mpc85xx_gpio_get_val(data->base, gpio_mask(gpio)); + } +} + +static int mpc85xx_gpio_get_function(struct udevice *dev, unsigned gpio) +{ + struct mpc85xx_gpio_data *data = dev_get_priv(dev); + int dir; + + dir = !!mpc85xx_gpio_get_dir(data->base, gpio_mask(gpio)); + return dir ? GPIOF_OUTPUT : GPIOF_INPUT; +} + +static int mpc85xx_gpio_ofdata_to_platdata(struct udevice *dev) { + struct mpc85xx_gpio_data *data = dev_get_priv(dev); + fdt_addr_t addr; + fdt_size_t size; + + addr = fdtdec_get_addr_size_auto_noparent(gd->fdt_blob, dev->of_offset, + "reg", 0, &size); + + data->addr = addr; + data->base = map_sysmem(CONFIG_SYS_IMMR + addr, size); + + if (!data->base) + return -ENOMEM; + + data->gpio_count = fdtdec_get_int(gd->fdt_blob, dev->of_offset, + "ngpios", 32); + data->dat_shadow = 0; + + return 0; +} + +static int mpc85xx_gpio_probe(struct udevice *dev) +{ + struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); + struct mpc85xx_gpio_data *data = dev_get_priv(dev); + char name[32], *str; + + snprintf(name, sizeof(name), "MPC@%lx_", data->addr); + str = strdup(name); + + if (!str) + return -ENOMEM; + + uc_priv->bank_name = str; + uc_priv->gpio_count = data->gpio_count; + + return 0; +} + +static const struct dm_gpio_ops gpio_mpc85xx_ops = { + .direction_input = mpc85xx_gpio_direction_input, + .direction_output = mpc85xx_gpio_direction_output, + .get_value = mpc85xx_gpio_get_value, + .set_value = mpc85xx_gpio_set_value, + .get_function = mpc85xx_gpio_get_function, +}; + +static const struct udevice_id mpc85xx_gpio_ids[] = { + { .compatible = "fsl,pq3-gpio" }, + { /* sentinel */ } +}; + +U_BOOT_DRIVER(gpio_mpc85xx) = { + .name = "gpio_mpc85xx", + .id = UCLASS_GPIO, + .ops = &gpio_mpc85xx_ops, + .ofdata_to_platdata = mpc85xx_gpio_ofdata_to_platdata, + .of_match = mpc85xx_gpio_ids, + .probe = mpc85xx_gpio_probe, + .priv_auto_alloc_size = sizeof(struct mpc85xx_gpio_data), +}; -- 2.7.0.GIT

On 23 May 2016 at 01:08, Mario Six mario.six@gdsys.cc wrote:
From: "mario.six@gdsys.cc" mario.six@gdsys.cc
This patch adds a driver for the built-in GPIO controller of the MPC85XX SoC (probably supporting other PowerQUICC III SoCs as well).
Each GPIO bank is identified by its own entry in the device tree, i.e.
gpio-controller@fc00 { #gpio-cells = <2>; compatible = "fsl,pq3-gpio"; reg = <0xfc00 0x100> }
By default, each bank is assumed to have 32 GPIOs, but the ngpios setting is honored, so the number of GPIOs for each bank in configurable to match the actual GPIO count of the SoC (e.g. the 32/32/23 banks of the P1022 SoC).
The usual functions of GPIO drivers (setting input/output mode and output value setting) are supported.
The driver has been tested on MPC85XX, but it is likely that other PowerQUICC III devices will work as well.
Signed-off-by: Mario Six mario.six@gdsys.cc
v4: None
v3:
- Added shadow for the GPDAT register, as suggested by Joakim Tjernlund
- Switched to u32 for bit masks
- Added some comments
v2:
- Added missing commit message
- Improved the Kconfig description
- Fixed and documented the mpc85xx_gpio_data members
- Introduced GPIO_MASK macro to simplify the code
- Fixed white space issues in function headers
- Removed unnecessary empty line
- Use fdtdec_get_addr_size_auto_noparent to read the register base data
arch/powerpc/include/asm/arch-mpc85xx/gpio.h | 2 + arch/powerpc/include/asm/immap_85xx.h | 2 + drivers/gpio/Kconfig | 25 ++++ drivers/gpio/Makefile | 1 + drivers/gpio/mpc85xx_gpio.c | 187 +++++++++++++++++++++++++++ 5 files changed, 217 insertions(+) create mode 100644 drivers/gpio/mpc85xx_gpio.c
Reviewed-by: Simon Glass sjg@chromium.org

From: "mario.six@gdsys.cc" mario.six@gdsys.cc
Certain GPIO devices have the capability to switch their GPIOs into open-drain mode, that is, instead of actively driving the output (Push-pull output), the pin is connected to the collector (for a NPN transistor) or the drain (for a MOSFET) of a transistor, respectively. The pin then either forms an open circuit or a connection to ground, depending on the state of the transistor.
This patch adds functions to the GPIO uclass to switch GPIOs to open-drain mode on devices that support it.
Signed-off-by: Mario Six mario.six@gdsys.cc ---
v4: None
v3: None
v2: - Added missing commit message - Fixed error return value of dm_gpio_get_open_drain - Fixed return value passing in dm_gpio_set_open_drain and added comment - Added description of open-drain mode
--- drivers/gpio/gpio-uclass.c | 32 ++++++++++++++++++++++++++++++++ include/asm-generic/gpio.h | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+)
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index b58d4e6..1be9988 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -352,6 +352,38 @@ int dm_gpio_set_value(const struct gpio_desc *desc, int value) return 0; }
+int dm_gpio_get_open_drain(struct gpio_desc *desc) +{ + struct dm_gpio_ops *ops = gpio_get_ops(desc->dev); + int ret; + + ret = check_reserved(desc, "get_open_drain"); + if (ret) + return ret; + + if (ops->set_open_drain) + return ops->get_open_drain(desc->dev, desc->offset); + else + return -ENOSYS; +} + +int dm_gpio_set_open_drain(struct gpio_desc *desc, int value) +{ + struct dm_gpio_ops *ops = gpio_get_ops(desc->dev); + int ret; + + ret = check_reserved(desc, "set_open_drain"); + if (ret) + return ret; + + if (ops->set_open_drain) + ret = ops->set_open_drain(desc->dev, desc->offset, value); + else + return 0; /* feature not supported -> ignore setting */ + + return ret; +} + int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) { struct udevice *dev = desc->dev; diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 68b5f0b..ece7552 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -241,6 +241,8 @@ struct dm_gpio_ops { int value); int (*get_value)(struct udevice *dev, unsigned offset); int (*set_value)(struct udevice *dev, unsigned offset, int value); + int (*get_open_drain)(struct udevice *dev, unsigned offset); + int (*set_open_drain)(struct udevice *dev, unsigned offset, int value); /** * get_function() Get the GPIO function * @@ -541,6 +543,38 @@ int dm_gpio_get_value(const struct gpio_desc *desc); int dm_gpio_set_value(const struct gpio_desc *desc, int value);
/** + * dm_gpio_get_open_drain() - Check if open-drain-mode of a GPIO is active + * + * This checks if open-drain-mode for a GPIO is enabled or not. This method is + * optional. + * + * @desc: GPIO description containing device, offset and flags, + * previously returned by gpio_request_by_name() + * @return Value of open drain mode for GPIO (0 for inactive, 1 for active) or + * -ve on error + */ +int dm_gpio_get_open_drain(struct gpio_desc *desc); + +/** + * dm_gpio_set_open_drain() - Switch open-drain-mode of a GPIO on or off + * + * This enables or disables open-drain mode for a GPIO. This method is + * optional; if the driver does not support it, nothing happens when the method + * is called. + * + * In open-drain mode, instead of actively driving the output (Push-pull + * output), the GPIO's pin is connected to the collector (for a NPN transistor) + * or the drain (for a MOSFET) of a transistor, respectively. The pin then + * either forms an open circuit or a connection to ground, depending on the + * state of the transistor. + * + * @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_open_drain(struct gpio_desc *desc, int value); + +/** * dm_gpio_set_dir() - Set the direction for a GPIO * * This sets up the direction according tot the provided flags. It will do -- 2.7.0.GIT

From: "mario.six@gdsys.cc" mario.six@gdsys.cc
This patch implements the open-drain setting feature for the MPC85XX GPIO controller.
Signed-off-by: Mario Six mario.six@gdsys.cc ---
v4: - Added forgotten mpc85xx_gpio_{get,set}_open_drain functions
v3: - Added comments
v2: - Added missing commit message - Fixed white space issues in function headers
--- drivers/gpio/Kconfig | 6 +++--- drivers/gpio/mpc85xx_gpio.c | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 068ee63..b250622 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -162,9 +162,9 @@ config MPC85XX_GPIO configurable to match the actual GPIO count of the SoC (e.g. the 32/32/23 banks of the P1022 SoC).
- The standard functions of input/output mode, and output value setting - are supported; the open-drain capability of the controller is not - supported yet. + Aside from the standard functions of input/output mode, and output + value setting, the open-drain feature, which can configure individual + GPIOs to work as open-drain outputs, is supported.
The driver has been tested on MPC85XX, but it is likely that other PowerQUICC III devices will work as well. diff --git a/drivers/gpio/mpc85xx_gpio.c b/drivers/gpio/mpc85xx_gpio.c index 17755df..04773e2 100644 --- a/drivers/gpio/mpc85xx_gpio.c +++ b/drivers/gpio/mpc85xx_gpio.c @@ -73,6 +73,25 @@ static inline void mpc85xx_gpio_set_high(struct ccsr_gpio *base, u32 gpios) setbits_be32(&base->gpdir, gpios); }
+static inline int mpc85xx_gpio_open_drain_val(struct ccsr_gpio *base, u32 mask) +{ + return in_be32(&base->gpodr) & mask; +} + +static inline void mpc85xx_gpio_open_drain_on(struct ccsr_gpio *base, u32 + gpios) +{ + /* GPODR register 1 -> open drain on */ + setbits_be32(&base->gpodr, gpios); +} + +static inline void mpc85xx_gpio_open_drain_off(struct ccsr_gpio *base, + u32 gpios) +{ + /* GPODR register 0 -> open drain off (actively driven) */ + clrbits_be32(&base->gpodr, gpios); +} + static int mpc85xx_gpio_direction_input(struct udevice *dev, unsigned gpio) { struct mpc85xx_gpio_data *data = dev_get_priv(dev); @@ -115,6 +134,26 @@ static int mpc85xx_gpio_get_value(struct udevice *dev, unsigned gpio) } }
+static int mpc85xx_gpio_get_open_drain(struct udevice *dev, unsigned gpio) +{ + struct mpc85xx_gpio_data *data = dev_get_priv(dev); + + return !!mpc85xx_gpio_open_drain_val(data->base, gpio_mask(gpio)); +} + +static int mpc85xx_gpio_set_open_drain(struct udevice *dev, unsigned gpio, + int value) +{ + struct mpc85xx_gpio_data *data = dev_get_priv(dev); + + if (value) { + mpc85xx_gpio_open_drain_on(data->base, gpio_mask(gpio)); + } else { + mpc85xx_gpio_open_drain_off(data->base, gpio_mask(gpio)); + } + return 0; +} + static int mpc85xx_gpio_get_function(struct udevice *dev, unsigned gpio) { struct mpc85xx_gpio_data *data = dev_get_priv(dev); @@ -168,6 +207,8 @@ static const struct dm_gpio_ops gpio_mpc85xx_ops = { .direction_output = mpc85xx_gpio_direction_output, .get_value = mpc85xx_gpio_get_value, .set_value = mpc85xx_gpio_set_value, + .get_open_drain = mpc85xx_gpio_get_open_drain, + .set_open_drain = mpc85xx_gpio_set_open_drain, .get_function = mpc85xx_gpio_get_function, };
-- 2.7.0.GIT

Add some tests for the new open drain setting feature of the GPIO uclass, and extend the capabilities of the sandbox GPIO driver accordingly.
Signed-off-by: Mario Six mario.six@gdsys.cc ---
v4: Patch added
--- drivers/gpio/sandbox.c | 35 +++++++++++++++++++++++++++++++++++ test/dm/gpio.c | 7 +++++++ 2 files changed, 42 insertions(+)
diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index a9b1efc..f6435a0 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -15,6 +15,7 @@ DECLARE_GLOBAL_DATA_PTR; /* Flags for each GPIO */ #define GPIOF_OUTPUT (1 << 0) /* Currently set as an output */ #define GPIOF_HIGH (1 << 1) /* Currently set high */ +#define GPIOF_ODR (1 << 2) /* Currently set to open drain mode */
struct gpio_state { const char *label; /* label given by requester */ @@ -70,6 +71,16 @@ int sandbox_gpio_set_value(struct udevice *dev, unsigned offset, int value) return set_gpio_flag(dev, offset, GPIOF_HIGH, value); }
+int sandbox_gpio_get_open_drain(struct udevice *dev, unsigned offset) +{ + return get_gpio_flag(dev, offset, GPIOF_ODR); +} + +int sandbox_gpio_set_open_drain(struct udevice *dev, unsigned offset, int value) +{ + return set_gpio_flag(dev, offset, GPIOF_ODR, value); +} + int sandbox_gpio_get_direction(struct udevice *dev, unsigned offset) { return get_gpio_flag(dev, offset, GPIOF_OUTPUT); @@ -124,6 +135,28 @@ static int sb_gpio_set_value(struct udevice *dev, unsigned offset, int value) return sandbox_gpio_set_value(dev, offset, value); }
+/* read GPIO ODR value of port 'offset' */ +static int sb_gpio_get_open_drain(struct udevice *dev, unsigned offset) +{ + debug("%s: offset:%u\n", __func__, offset); + + return sandbox_gpio_get_open_drain(dev, offset); +} + +/* write GPIO ODR value to port 'offset' */ +static int sb_gpio_set_open_drain(struct udevice *dev, unsigned offset, int value) +{ + debug("%s: offset:%u, value = %d\n", __func__, offset, value); + + if (!sandbox_gpio_get_direction(dev, offset)) { + printf("sandbox_gpio: error: set_open_drain on input gpio %u\n", + offset); + return -1; + } + + return sandbox_gpio_set_open_drain(dev, offset, value); +} + static int sb_gpio_get_function(struct udevice *dev, unsigned offset) { if (get_gpio_flag(dev, offset, GPIOF_OUTPUT)) @@ -154,6 +187,8 @@ static const struct dm_gpio_ops gpio_sandbox_ops = { .direction_output = sb_gpio_direction_output, .get_value = sb_gpio_get_value, .set_value = sb_gpio_set_value, + .get_open_drain = sb_gpio_get_open_drain, + .set_open_drain = sb_gpio_set_open_drain, .get_function = sb_gpio_get_function, .xlate = sb_gpio_xlate, }; diff --git a/test/dm/gpio.c b/test/dm/gpio.c index 727db18..4779b5a 100644 --- a/test/dm/gpio.c +++ b/test/dm/gpio.c @@ -75,6 +75,13 @@ static int dm_test_gpio(struct unit_test_state *uts) ut_assertok(ops->set_value(dev, offset, 1)); ut_asserteq(1, ops->get_value(dev, offset));
+ /* Make it an open drain output, and reset it */ + ut_asserteq(0, ops->get_open_drain(dev, offset)); + ut_assertok(ops->set_open_drain(dev, offset, 1)); + ut_asserteq(1, ops->get_open_drain(dev, offset)); + ut_assertok(ops->set_open_drain(dev, offset, 0)); + ut_asserteq(0, ops->get_open_drain(dev, offset)); + /* Make it an input */ ut_assertok(ops->direction_input(dev, offset)); ut_assertok(gpio_get_status(dev, offset, buf, sizeof(buf))); -- 2.7.0.GIT

Hi Mario,
On 23 May 2016 at 01:08, Mario Six mario.six@gdsys.cc wrote:
Add some tests for the new open drain setting feature of the GPIO uclass, and extend the capabilities of the sandbox GPIO driver accordingly.
Signed-off-by: Mario Six mario.six@gdsys.cc
v4: Patch added
drivers/gpio/sandbox.c | 35 +++++++++++++++++++++++++++++++++++ test/dm/gpio.c | 7 +++++++ 2 files changed, 42 insertions(+)
diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index a9b1efc..f6435a0 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -15,6 +15,7 @@ DECLARE_GLOBAL_DATA_PTR; /* Flags for each GPIO */ #define GPIOF_OUTPUT (1 << 0) /* Currently set as an output */ #define GPIOF_HIGH (1 << 1) /* Currently set high */ +#define GPIOF_ODR (1 << 2) /* Currently set to open drain mode */
struct gpio_state { const char *label; /* label given by requester */ @@ -70,6 +71,16 @@ int sandbox_gpio_set_value(struct udevice *dev, unsigned offset, int value) return set_gpio_flag(dev, offset, GPIOF_HIGH, value); }
+int sandbox_gpio_get_open_drain(struct udevice *dev, unsigned offset) +{
return get_gpio_flag(dev, offset, GPIOF_ODR);
+}
+int sandbox_gpio_set_open_drain(struct udevice *dev, unsigned offset, int value) +{
return set_gpio_flag(dev, offset, GPIOF_ODR, value);
+}
int sandbox_gpio_get_direction(struct udevice *dev, unsigned offset) { return get_gpio_flag(dev, offset, GPIOF_OUTPUT); @@ -124,6 +135,28 @@ static int sb_gpio_set_value(struct udevice *dev, unsigned offset, int value) return sandbox_gpio_set_value(dev, offset, value); }
+/* read GPIO ODR value of port 'offset' */ +static int sb_gpio_get_open_drain(struct udevice *dev, unsigned offset) +{
debug("%s: offset:%u\n", __func__, offset);
return sandbox_gpio_get_open_drain(dev, offset);
+}
+/* write GPIO ODR value to port 'offset' */ +static int sb_gpio_set_open_drain(struct udevice *dev, unsigned offset, int value) +{
debug("%s: offset:%u, value = %d\n", __func__, offset, value);
if (!sandbox_gpio_get_direction(dev, offset)) {
printf("sandbox_gpio: error: set_open_drain on input gpio %u\n",
offset);
return -1;
}
return sandbox_gpio_set_open_drain(dev, offset, value);
+}
static int sb_gpio_get_function(struct udevice *dev, unsigned offset) { if (get_gpio_flag(dev, offset, GPIOF_OUTPUT)) @@ -154,6 +187,8 @@ static const struct dm_gpio_ops gpio_sandbox_ops = { .direction_output = sb_gpio_direction_output, .get_value = sb_gpio_get_value, .set_value = sb_gpio_set_value,
.get_open_drain = sb_gpio_get_open_drain,
.set_open_drain = sb_gpio_set_open_drain, .get_function = sb_gpio_get_function, .xlate = sb_gpio_xlate,
}; diff --git a/test/dm/gpio.c b/test/dm/gpio.c index 727db18..4779b5a 100644 --- a/test/dm/gpio.c +++ b/test/dm/gpio.c @@ -75,6 +75,13 @@ static int dm_test_gpio(struct unit_test_state *uts) ut_assertok(ops->set_value(dev, offset, 1)); ut_asserteq(1, ops->get_value(dev, offset));
/* Make it an open drain output, and reset it */
ut_asserteq(0, ops->get_open_drain(dev, offset));
ut_assertok(ops->set_open_drain(dev, offset, 1));
ut_asserteq(1, ops->get_open_drain(dev, offset));
ut_assertok(ops->set_open_drain(dev, offset, 0));
ut_asserteq(0, ops->get_open_drain(dev, offset));
Instead of reading back from the driver, you should call sandbox_gpio_get_open_drain() here. The idea is to set the value through the API, then check (via the back door sandbox functions) that it happened.
/* Make it an input */ ut_assertok(ops->direction_input(dev, offset)); ut_assertok(gpio_get_status(dev, offset, buf, sizeof(buf)));
-- 2.7.0.GIT
Regards, Simon

Hi Simon,
On Mon, May 23, 2016 at 5:42 PM, Simon Glass sjg@chromium.org wrote:
Hi Mario,
On 23 May 2016 at 01:08, Mario Six mario.six@gdsys.cc wrote:
Add some tests for the new open drain setting feature of the GPIO uclass, and extend the capabilities of the sandbox GPIO driver accordingly.
Signed-off-by: Mario Six mario.six@gdsys.cc
v4: Patch added
drivers/gpio/sandbox.c | 35 +++++++++++++++++++++++++++++++++++ test/dm/gpio.c | 7 +++++++ 2 files changed, 42 insertions(+)
diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index a9b1efc..f6435a0 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -15,6 +15,7 @@ DECLARE_GLOBAL_DATA_PTR; /* Flags for each GPIO */ #define GPIOF_OUTPUT (1 << 0) /* Currently set as an output */ #define GPIOF_HIGH (1 << 1) /* Currently set high */ +#define GPIOF_ODR (1 << 2) /* Currently set to open drain mode */
struct gpio_state { const char *label; /* label given by requester */ @@ -70,6 +71,16 @@ int sandbox_gpio_set_value(struct udevice *dev, unsigned offset, int value) return set_gpio_flag(dev, offset, GPIOF_HIGH, value); }
+int sandbox_gpio_get_open_drain(struct udevice *dev, unsigned offset) +{
return get_gpio_flag(dev, offset, GPIOF_ODR);
+}
+int sandbox_gpio_set_open_drain(struct udevice *dev, unsigned offset, int value) +{
return set_gpio_flag(dev, offset, GPIOF_ODR, value);
+}
int sandbox_gpio_get_direction(struct udevice *dev, unsigned offset) { return get_gpio_flag(dev, offset, GPIOF_OUTPUT); @@ -124,6 +135,28 @@ static int sb_gpio_set_value(struct udevice *dev, unsigned offset, int value) return sandbox_gpio_set_value(dev, offset, value); }
+/* read GPIO ODR value of port 'offset' */ +static int sb_gpio_get_open_drain(struct udevice *dev, unsigned offset) +{
debug("%s: offset:%u\n", __func__, offset);
return sandbox_gpio_get_open_drain(dev, offset);
+}
+/* write GPIO ODR value to port 'offset' */ +static int sb_gpio_set_open_drain(struct udevice *dev, unsigned offset, int value) +{
debug("%s: offset:%u, value = %d\n", __func__, offset, value);
if (!sandbox_gpio_get_direction(dev, offset)) {
printf("sandbox_gpio: error: set_open_drain on input gpio %u\n",
offset);
return -1;
}
return sandbox_gpio_set_open_drain(dev, offset, value);
+}
static int sb_gpio_get_function(struct udevice *dev, unsigned offset) { if (get_gpio_flag(dev, offset, GPIOF_OUTPUT)) @@ -154,6 +187,8 @@ static const struct dm_gpio_ops gpio_sandbox_ops = { .direction_output = sb_gpio_direction_output, .get_value = sb_gpio_get_value, .set_value = sb_gpio_set_value,
.get_open_drain = sb_gpio_get_open_drain,
.set_open_drain = sb_gpio_set_open_drain, .get_function = sb_gpio_get_function, .xlate = sb_gpio_xlate,
}; diff --git a/test/dm/gpio.c b/test/dm/gpio.c index 727db18..4779b5a 100644 --- a/test/dm/gpio.c +++ b/test/dm/gpio.c @@ -75,6 +75,13 @@ static int dm_test_gpio(struct unit_test_state *uts) ut_assertok(ops->set_value(dev, offset, 1)); ut_asserteq(1, ops->get_value(dev, offset));
/* Make it an open drain output, and reset it */
ut_asserteq(0, ops->get_open_drain(dev, offset));
ut_assertok(ops->set_open_drain(dev, offset, 1));
ut_asserteq(1, ops->get_open_drain(dev, offset));
ut_assertok(ops->set_open_drain(dev, offset, 0));
ut_asserteq(0, ops->get_open_drain(dev, offset));
Instead of reading back from the driver, you should call sandbox_gpio_get_open_drain() here. The idea is to set the value through the API, then check (via the back door sandbox functions) that it happened.
Ah, OK. So, something like
ut_asserteq(0, sandbox_gpio_get_open_drain(dev, offset)); ut_assertok(ops->set_open_drain(dev, offset, 1)); ut_asserteq(1, sandbox_gpio_get_open_drain(dev, offset)); ut_assertok(ops->set_open_drain(dev, offset, 0)); ut_asserteq(0, sandbox_gpio_get_open_drain(dev, offset));
Will be enough?
Also, while adding sandbox_gpio_{get,set}_open_drain to arch/sandbox/include/asm/gpio.h, I noticed that some parameter names in the documentation seem a bit off:
/** * Return the simulated value of a GPIO (used only in sandbox test code) * * @param gp GPIO number * @return -1 on error, 0 if GPIO is low, >0 if high */ int sandbox_gpio_get_value(struct udevice *dev, unsigned int offset);
Should I fix that in a separate patch?
/* Make it an input */ ut_assertok(ops->direction_input(dev, offset)); ut_assertok(gpio_get_status(dev, offset, buf, sizeof(buf)));
-- 2.7.0.GIT
Regards, Simon
Best regards, Mario

Hi Mario,
On 24 May 2016 at 01:20, Mario Six mario.six@gdsys.cc wrote:
Hi Simon,
On Mon, May 23, 2016 at 5:42 PM, Simon Glass sjg@chromium.org wrote:
Hi Mario,
On 23 May 2016 at 01:08, Mario Six mario.six@gdsys.cc wrote:
Add some tests for the new open drain setting feature of the GPIO uclass, and extend the capabilities of the sandbox GPIO driver accordingly.
Signed-off-by: Mario Six mario.six@gdsys.cc
v4: Patch added
drivers/gpio/sandbox.c | 35 +++++++++++++++++++++++++++++++++++ test/dm/gpio.c | 7 +++++++ 2 files changed, 42 insertions(+)
diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index a9b1efc..f6435a0 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -15,6 +15,7 @@ DECLARE_GLOBAL_DATA_PTR; /* Flags for each GPIO */ #define GPIOF_OUTPUT (1 << 0) /* Currently set as an output */ #define GPIOF_HIGH (1 << 1) /* Currently set high */ +#define GPIOF_ODR (1 << 2) /* Currently set to open drain mode */
struct gpio_state { const char *label; /* label given by requester */ @@ -70,6 +71,16 @@ int sandbox_gpio_set_value(struct udevice *dev, unsigned offset, int value) return set_gpio_flag(dev, offset, GPIOF_HIGH, value); }
+int sandbox_gpio_get_open_drain(struct udevice *dev, unsigned offset) +{
return get_gpio_flag(dev, offset, GPIOF_ODR);
+}
+int sandbox_gpio_set_open_drain(struct udevice *dev, unsigned offset, int value) +{
return set_gpio_flag(dev, offset, GPIOF_ODR, value);
+}
int sandbox_gpio_get_direction(struct udevice *dev, unsigned offset) { return get_gpio_flag(dev, offset, GPIOF_OUTPUT); @@ -124,6 +135,28 @@ static int sb_gpio_set_value(struct udevice *dev, unsigned offset, int value) return sandbox_gpio_set_value(dev, offset, value); }
+/* read GPIO ODR value of port 'offset' */ +static int sb_gpio_get_open_drain(struct udevice *dev, unsigned offset) +{
debug("%s: offset:%u\n", __func__, offset);
return sandbox_gpio_get_open_drain(dev, offset);
+}
+/* write GPIO ODR value to port 'offset' */ +static int sb_gpio_set_open_drain(struct udevice *dev, unsigned offset, int value) +{
debug("%s: offset:%u, value = %d\n", __func__, offset, value);
if (!sandbox_gpio_get_direction(dev, offset)) {
printf("sandbox_gpio: error: set_open_drain on input gpio %u\n",
offset);
return -1;
}
return sandbox_gpio_set_open_drain(dev, offset, value);
+}
static int sb_gpio_get_function(struct udevice *dev, unsigned offset) { if (get_gpio_flag(dev, offset, GPIOF_OUTPUT)) @@ -154,6 +187,8 @@ static const struct dm_gpio_ops gpio_sandbox_ops = { .direction_output = sb_gpio_direction_output, .get_value = sb_gpio_get_value, .set_value = sb_gpio_set_value,
.get_open_drain = sb_gpio_get_open_drain,
.set_open_drain = sb_gpio_set_open_drain, .get_function = sb_gpio_get_function, .xlate = sb_gpio_xlate,
}; diff --git a/test/dm/gpio.c b/test/dm/gpio.c index 727db18..4779b5a 100644 --- a/test/dm/gpio.c +++ b/test/dm/gpio.c @@ -75,6 +75,13 @@ static int dm_test_gpio(struct unit_test_state *uts) ut_assertok(ops->set_value(dev, offset, 1)); ut_asserteq(1, ops->get_value(dev, offset));
/* Make it an open drain output, and reset it */
ut_asserteq(0, ops->get_open_drain(dev, offset));
ut_assertok(ops->set_open_drain(dev, offset, 1));
ut_asserteq(1, ops->get_open_drain(dev, offset));
ut_assertok(ops->set_open_drain(dev, offset, 0));
ut_asserteq(0, ops->get_open_drain(dev, offset));
Instead of reading back from the driver, you should call sandbox_gpio_get_open_drain() here. The idea is to set the value through the API, then check (via the back door sandbox functions) that it happened.
Ah, OK. So, something like
ut_asserteq(0, sandbox_gpio_get_open_drain(dev, offset)); ut_assertok(ops->set_open_drain(dev, offset, 1)); ut_asserteq(1, sandbox_gpio_get_open_drain(dev, offset)); ut_assertok(ops->set_open_drain(dev, offset, 0)); ut_asserteq(0, sandbox_gpio_get_open_drain(dev, offset));
Will be enough?
Yes that's fine.
Also, while adding sandbox_gpio_{get,set}_open_drain to arch/sandbox/include/asm/gpio.h, I noticed that some parameter names in the documentation seem a bit off:
/**
- Return the simulated value of a GPIO (used only in sandbox test code)
- @param gp GPIO number
- @return -1 on error, 0 if GPIO is low, >0 if high
*/ int sandbox_gpio_get_value(struct udevice *dev, unsigned int offset);
Should I fix that in a separate patch?
Yes please.
/* Make it an input */ ut_assertok(ops->direction_input(dev, offset)); ut_assertok(gpio_get_status(dev, offset, buf, sizeof(buf)));
-- 2.7.0.GIT
Regards, Simon
participants (2)
-
Mario Six
-
Simon Glass