Convert 'pinctrl-single' using livetree functions - dev_read_prop - dev_read_u32_default - dev_read_u32_array - dev_read_bool - dev_read_addr and get rid of DECLARE_GLOBAL_DATA_PTR.

Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---

Changes in v3: None Changes in v2: - use the dev_ API instead of ofnode_ function.

drivers/pinctrl/pinctrl-single.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index 1dfc97dcea..6d8f989ebd 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -9,8 +9,6 @@ #include <linux/libfdt.h> #include <asm/io.h>

-DECLARE_GLOBAL_DATA_PTR; - struct single_pdata { fdt_addr_t base; /* first configuration register */ int offset; /* index of last configuration register */ @@ -117,13 +115,11 @@ static int single_configure_bits(struct udevice *dev, static int single_set_state(struct udevice *dev, struct udevice *config) { - const void *fdt = gd->fdt_blob; const struct single_fdt_pin_cfg *prop; const struct single_fdt_bits_cfg *prop_bits; int len;

- prop = fdt_getprop(fdt, dev_of_offset(config), "pinctrl-single,pins", - &len); + prop = dev_read_prop(dev, "pinctrl-single,pins", &len);

if (prop) { dev_dbg(dev, "configuring pins for %s\n", config->name); @@ -136,9 +132,7 @@ static int single_set_state(struct udevice *dev, }

/* pinctrl-single,pins not found so check for pinctrl-single,bits */ - prop_bits = fdt_getprop(fdt, dev_of_offset(config), - "pinctrl-single,bits", - &len); + prop_bits = dev_read_prop(dev, "pinctrl-single,bits", &len); if (prop_bits) { dev_dbg(dev, "configuring pins for %s\n", config->name); if (len % sizeof(struct single_fdt_bits_cfg)) { @@ -160,27 +154,24 @@ static int single_ofdata_to_platdata(struct udevice *dev) int res; struct single_pdata *pdata = dev->platdata;

- pdata->width = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), - "pinctrl-single,register-width", 0); + pdata->width = + dev_read_u32_default(dev, "pinctrl-single,register-width", 0);

- res = fdtdec_get_int_array(gd->fdt_blob, dev_of_offset(dev), - "reg", of_reg, 2); + res = dev_read_u32_array(dev, "reg", of_reg, 2); if (res) return res; pdata->offset = of_reg[1] - pdata->width / 8;

- addr = devfdt_get_addr(dev); + addr = dev_read_addr(dev); if (addr == FDT_ADDR_T_NONE) { dev_dbg(dev, "no valid base register address\n"); return -EINVAL; } pdata->base = addr;

- pdata->mask = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), - "pinctrl-single,function-mask", - 0xffffffff); - pdata->bits_per_mux = fdtdec_get_bool(gd->fdt_blob, dev_of_offset(dev), - "pinctrl-single,bit-per-mux"); + pdata->mask = dev_read_u32_default(dev, "pinctrl-single,function-mask", + 0xffffffff); + pdata->bits_per_mux = dev_read_bool(dev, "pinctrl-single,bit-per-mux");

return 0; }

Add functions to iterate on all property with livetree - dev_read_first_prop - dev_read_next_prop - dev_read_prop_by_prop and - ofnode_get_first_property - ofnode_get_next_property - ofnode_get_property_by_prop

And helper: dev_for_each_property

For example: struct ofprop property;

dev_for_each_property(property, config) { value = dev_read_prop_by_prop(&property, &propname, &len);

or:

for (res = ofnode_get_first_property(node, &property); !res; res = ofnode_get_next_property(&property)) { value = ofnode_get_property_by_prop(&property, &propname, &len); .... }

Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---

Changes in v3: - add test dm_test_ofnode_get_property_by_prop - udpate ofnode example in commit message - solve comment for ofnode_get_property_by_prop (and not by of_ofprop)

Changes in v2: - Identify property with a new struct ofprop as proposed by Simon Glass - Add dev_ iterate functions

drivers/core/of_access.c | 32 +++++++++++++++++++ drivers/core/ofnode.c | 48 ++++++++++++++++++++++++++++ drivers/core/read.c | 16 ++++++++++ include/dm/of_access.h | 40 ++++++++++++++++++++++++ include/dm/ofnode.h | 63 ++++++++++++++++++++++++++++++++++++- include/dm/read.h | 67 ++++++++++++++++++++++++++++++++++++++++ test/dm/Makefile | 1 + test/dm/ofread.c | 50 ++++++++++++++++++++++++++++++ 8 files changed, 316 insertions(+), 1 deletion(-) create mode 100644 test/dm/ofread.c

diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c index 945b81448c..86fe42ad14 100644 --- a/drivers/core/of_access.c +++ b/drivers/core/of_access.c @@ -170,6 +170,38 @@ const void *of_get_property(const struct device_node *np, const char *name, return pp ? pp->value : NULL; }

+const struct property *of_get_first_property(const struct device_node *np) +{ + if (!np) + return NULL; + + return np->properties; +} + +const struct property *of_get_next_property(const struct device_node *np, + const struct property *property) +{ + if (!np) + return NULL; + + return property->next; +} + +const void *of_get_property_by_prop(const struct device_node *np, + const struct property *property, + const char **name, + int *lenp) +{ + if (!np || !property) + return NULL; + if (name) + *name = property->name; + if (lenp) + *lenp = property->length; + + return property->value; +} + static const char *of_prop_next_string(struct property *prop, const char *cur) { const void *curv = cur; diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index 8f0eab2ca6..cb27562fec 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -536,6 +536,54 @@ const void *ofnode_get_property(ofnode node, const char *propname, int *lenp) propname, lenp); }

+int ofnode_get_first_property(ofnode node, struct ofprop *prop) +{ + prop->node = node; + + if (ofnode_is_np(node)) { + prop->prop = of_get_first_property(ofnode_to_np(prop->node)); + if (!prop->prop) + return -FDT_ERR_NOTFOUND; + } else { + prop->offset = + fdt_first_property_offset(gd->fdt_blob, + ofnode_to_offset(prop->node)); + if (prop->offset < 0) + return prop->offset; + } + + return 0; +} + +int ofnode_get_next_property(struct ofprop *prop) +{ + if (ofnode_is_np(prop->node)) { + prop->prop = of_get_next_property(ofnode_to_np(prop->node), + prop->prop); + if (!prop->prop) + return -FDT_ERR_NOTFOUND; + } else { + prop->offset = fdt_next_property_offset(gd->fdt_blob, + prop->offset); + if (prop->offset < 0) + return prop->offset; + } + + return 0; +} + +const void *ofnode_get_property_by_prop(const struct ofprop *prop, + const char **propname, int *lenp) +{ + if (ofnode_is_np(prop->node)) + return of_get_property_by_prop(ofnode_to_np(prop->node), + prop->prop, propname, lenp); + else + return fdt_getprop_by_offset(gd->fdt_blob, + prop->offset, + propname, lenp); +} + bool ofnode_is_available(ofnode node) { if (ofnode_is_np(node)) diff --git a/drivers/core/read.c b/drivers/core/read.c index 9602e52d1b..a9f5d147b2 100644 --- a/drivers/core/read.c +++ b/drivers/core/read.c @@ -238,6 +238,22 @@ const void *dev_read_prop(struct udevice *dev, const char *propname, int *lenp) return ofnode_get_property(dev_ofnode(dev), propname, lenp); }

+int dev_read_first_prop(struct udevice *dev, struct ofprop *prop) +{ + return ofnode_get_first_property(dev_ofnode(dev), prop); +} + +int dev_read_next_prop(struct ofprop *prop) +{ + return ofnode_get_next_property(prop); +} + +const void *dev_read_prop_by_prop(struct ofprop *prop, + const char **propname, int *lenp) +{ + return ofnode_get_property_by_prop(prop, propname, lenp); +} + int dev_read_alias_seq(struct udevice *dev, int *devnump) { ofnode node = dev_ofnode(dev); diff --git a/include/dm/of_access.h b/include/dm/of_access.h index 13fedb7cf5..1eb1ce13d0 100644 --- a/include/dm/of_access.h +++ b/include/dm/of_access.h @@ -103,6 +103,46 @@ struct property *of_find_property(const struct device_node *np, const void *of_get_property(const struct device_node *np, const char *name, int *lenp);

+/** + * of_get_first_property()- get to the pointer of the first property + * + * Get pointer to the first property of the node, it is used to iterate + * and read all the property with of_get_next_property_by_prop(). + * + * @np: Pointer to device node + * @return pointer to property or NULL if not found + */ +const struct property *of_get_first_property(const struct device_node *np); + +/** + * of_get_next_property() - get to the pointer of the next property + * + * Get pointer to the next property of the node, it is used to iterate + * and read all the property with of_get_property_by_prop(). + * + * @np: Pointer to device node + * @property: pointer of the current property + * @return pointer to next property or NULL if not found + */ +const struct property *of_get_next_property(const struct device_node *np, + const struct property *property); + +/** + * of_get_property_by_prop() - get a property value of a node property + * + * Get value for the property identified by node and property pointer. + * + * @node: node to read + * @property: pointer of the property to read + * @propname: place to property name on success + * @lenp: place to put length on success + * @return pointer to property value or NULL if error + */ +const void *of_get_property_by_prop(const struct device_node *np, + const struct property *property, + const char **name, + int *lenp); + /** * of_device_is_compatible() - Check if the node matches given constraints * @device: pointer to node diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index 5c4cbf0998..9ee5556490 100644 --- a/include/dm/ofnode.h +++ b/include/dm/ofnode.h @@ -58,6 +58,31 @@ struct ofnode_phandle_args { uint32_t args[OF_MAX_PHANDLE_ARGS]; };

+/** + * ofprop - reference to a property of a device tree node + * + * This struct hold the reference on one property of one node, + * using struct ofnode and an offset within the flat device tree or either + * a pointer to a struct property in the live device tree. + * + * Thus we can reference arguments in both the live tree and the flat tree. + * + * The property reference can also hold a null reference. This corresponds to + * a struct property NULL pointer or an offset of -1. + * + * @node: Pointer to device node + * @offset: Pointer into flat device tree, used for flat tree. + * @prop: Pointer to property, used for live treee. + */ + +struct ofprop { + ofnode node; + union { + int offset; + const struct property *prop; + }; +}; + /** * _ofnode_to_np() - convert an ofnode to a live DT node pointer * @@ -543,7 +568,7 @@ int ofnode_decode_display_timing(ofnode node, int index, struct display_timing *config);

/** - * ofnode_get_property()- - get a pointer to the value of a node property + * ofnode_get_property() - get a pointer to the value of a node property * * @node: node to read * @propname: property to read @@ -552,6 +577,42 @@ int ofnode_decode_display_timing(ofnode node, int index, */ const void *ofnode_get_property(ofnode node, const char *propname, int *lenp);

+/** + * ofnode_get_first_property()- get the reference of the first property + * + * Get reference to the first property of the node, it is used to iterate + * and read all the property with ofnode_get_property_by_prop(). + * + * @node: node to read + * @prop: place to put argument reference + * @return 0 if OK, -ve on error. -FDT_ERR_NOTFOUND if not found + */ +int ofnode_get_first_property(ofnode node, struct ofprop *prop); + +/** + * ofnode_get_next_property() - get the reference of the next property + * + * Get reference to the next property of the node, it is used to iterate + * and read all the property with ofnode_get_property_by_prop(). + * + * @prop: reference of current argument and place to put reference of next one + * @return 0 if OK, -ve on error. -FDT_ERR_NOTFOUND if not found + */ +int ofnode_get_next_property(struct ofprop *prop); + +/** + * ofnode_get_property_by_prop() - get a pointer to the value of a property + * + * Get value for the property identified by the provided reference. + * + * @prop: reference on property + * @propname: If non-NULL, place to property name on success, + * @lenp: If non-NULL, place to put length on success + * @return 0 if OK, -ve on error. -FDT_ERR_NOTFOUND if not found + */ +const void *ofnode_get_property_by_prop(const struct ofprop *prop, + const char **propname, int *lenp); + /** * ofnode_is_available() - check if a node is marked available * diff --git a/include/dm/read.h b/include/dm/read.h index d37fcb504d..3c6bed7bff 100644 --- a/include/dm/read.h +++ b/include/dm/read.h @@ -465,6 +465,42 @@ int dev_read_phandle(struct udevice *dev); */ const void *dev_read_prop(struct udevice *dev, const char *propname, int *lenp);

+/** + * dev_read_first_prop()- get the reference of the first property + * + * Get reference to the first property of the node, it is used to iterate + * and read all the property with dev_read_prop_by_prop(). + * + * @dev: device to check + * @prop: place to put argument reference + * @return 0 if OK, -ve on error. -FDT_ERR_NOTFOUND if not found + */ +int dev_read_first_prop(struct udevice *dev, struct ofprop *prop); + +/** + * ofnode_get_next_property() - get the reference of the next property + * + * Get reference to the next property of the node, it is used to iterate + * and read all the property with dev_read_prop_by_prop(). + * + * @prop: reference of current argument and place to put reference of next one + * @return 0 if OK, -ve on error. -FDT_ERR_NOTFOUND if not found + */ +int dev_read_next_prop(struct ofprop *prop); + +/** + * dev_read_prop_by_prop() - get a pointer to the value of a property + * + * Get value for the property identified by the provided reference. + * + * @prop: reference on property + * @propname: If non-NULL, place to property name on success, + * @lenp: If non-NULL, place to put length on success + * @return 0 if OK, -ve on error. -FDT_ERR_NOTFOUND if not found + */ +const void *dev_read_prop_by_prop(struct ofprop *prop, + const char **propname, int *lenp); + /** * dev_read_alias_seq() - Get the alias sequence number of a node * @@ -812,6 +848,23 @@ static inline const void *dev_read_prop(struct udevice *dev, return ofnode_get_property(dev_ofnode(dev), propname, lenp); }

+static inline int dev_read_first_prop(struct udevice *dev, struct ofprop *prop) +{ + return ofnode_get_first_property(dev_ofnode(dev), prop); +} + +static inline int dev_read_next_prop(struct ofprop *prop) +{ + return ofnode_get_next_property(prop); +} + +static inline const void *dev_read_prop_by_prop(struct ofprop *prop, + const char **propname, + int *lenp) +{ + return ofnode_get_property_by_prop(prop, propname, lenp); +} + static inline int dev_read_alias_seq(struct udevice *dev, int *devnump) { return fdtdec_get_alias_seq(gd->fdt_blob, dev->uclass->uc_drv->name, @@ -889,4 +942,18 @@ static inline int dev_read_alias_highest_id(const char *stem) ofnode_valid(subnode); \ subnode = ofnode_next_subnode(subnode))

+/** + * dev_for_each_property() - Helper function to iterate through property + * + * This creates a for() loop which works through the property in a device's + * device-tree node. + * + * @prop: struct ofprop holding the current property + * @dev: device to use for interation (struct udevice *) + */ +#define dev_for_each_property(prop, dev) \ + for (int ret_prop = dev_read_first_prop(dev, &prop); \ + !ret_prop; \ + ret_prop = dev_read_next_prop(&prop)) + #endif diff --git a/test/dm/Makefile b/test/dm/Makefile index 0c2fd5cb5e..6a1af1511a 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -29,6 +29,7 @@ obj-$(CONFIG_LED) += led.o obj-$(CONFIG_DM_MAILBOX) += mailbox.o obj-$(CONFIG_DM_MMC) += mmc.o obj-y += ofnode.o +obj-y += ofread.o obj-$(CONFIG_OSD) += osd.o obj-$(CONFIG_DM_VIDEO) += panel.o obj-$(CONFIG_DM_PCI) += pci.o diff --git a/test/dm/ofread.c b/test/dm/ofread.c new file mode 100644 index 0000000000..f2a1382259 --- /dev/null +++ b/test/dm/ofread.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0+ + +#include <common.h> +#include <dm.h> +#include <dm/test.h> +#include <test/ut.h> + +static int dm_test_ofnode_get_property_by_prop(struct unit_test_state *uts) +{ + ofnode node; + struct ofprop prop; + const void *value; + const char *propname; + int res, len, count = 0; + + node = ofnode_path("/cros-ec/flash"); + for (res = ofnode_get_first_property(node, &prop); + !res; + res = ofnode_get_next_property(&prop)) { + value = ofnode_get_property_by_prop(&prop, &propname, &len); + ut_assertnonnull(value); + switch (count) { + case 0: + ut_asserteq_str("image-pos", propname); + ut_asserteq(4, len); + break; + case 1: + ut_asserteq_str("size", propname); + ut_asserteq(4, len); + break; + case 2: + ut_asserteq_str("erase-value", propname); + ut_asserteq(4, len); + break; + case 3: + /* only for platdata */ + ut_asserteq_str("name", propname); + ut_asserteq(6, len); + ut_asserteq_str("flash", value); + break; + default: + break; + } + count++; + } + + return 0; +} +DM_TEST(dm_test_ofnode_get_property_by_prop, + DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);

Remove the define GPIOD_REQUESTED as it is never used and use BIT() macro for other defines.

Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---

Changes in v3: None Changes in v2: - remove the unused GPIOD_REQUESTED

include/asm-generic/gpio.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index d6cf18744f..7da2015805 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -117,11 +117,10 @@ struct udevice; struct gpio_desc { struct udevice *dev; /* Device, NULL for invalid GPIO */ unsigned long flags; -#define GPIOD_REQUESTED (1 << 0) /* Requested/claimed */ -#define GPIOD_IS_OUT (1 << 1) /* GPIO is an output */ -#define GPIOD_IS_IN (1 << 2) /* GPIO is an input */ -#define GPIOD_ACTIVE_LOW (1 << 3) /* value has active low */ -#define GPIOD_IS_OUT_ACTIVE (1 << 4) /* set output active */ +#define GPIOD_IS_OUT BIT(1) /* GPIO is an output */ +#define GPIOD_IS_IN BIT(2) /* GPIO is an input */ +#define GPIOD_ACTIVE_LOW BIT(3) /* value has active low */ +#define GPIOD_IS_OUT_ACTIVE BIT(4) /* set output active */

uint offset; /* GPIO offset within the device */ /*

Introduce the function _dm_gpio_set_dir_flags to set dir flags without check if the GPIO is reserved.

Separate the reserved check for "set_dir" and "set_dir_flags".

This patch is a preliminary step to add new ops.

Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---

This patch was part of v2 08/14 = gpio: add ops for configuration with dir flags

Changes in v3: - Split the previous patch [PATCH v2 08/14] to help review

Changes in v2: None

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

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index bdf7f5bb4e..94e90cb8ac 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -510,15 +510,11 @@ int dm_gpio_set_value(const struct gpio_desc *desc, int value) return 0; }

-int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) +static int _dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) { struct udevice *dev = desc->dev; struct dm_gpio_ops *ops = gpio_get_ops(dev); - int ret; - - ret = check_reserved(desc, "set_dir"); - if (ret) - return ret; + int ret = 0;

if (flags & GPIOD_IS_OUT) { int value = flags & GPIOD_IS_OUT_ACTIVE ? 1 : 0; @@ -529,20 +525,36 @@ int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) } else if (flags & GPIOD_IS_IN) { ret = ops->direction_input(dev, desc->offset); } + + return ret; +} + +int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) +{ + int ret; + + ret = check_reserved(desc, "set_dir_flags"); if (ret) return ret; - /* - * Update desc->flags here, so that GPIO_ACTIVE_LOW is honoured in - * futures - */ - desc->flags = flags;

- return 0; + ret = _dm_gpio_set_dir_flags(desc, flags); + + /* update the descriptor flags */ + if (ret) + desc->flags = flags; + + return ret; }

int dm_gpio_set_dir(struct gpio_desc *desc) { - return dm_gpio_set_dir_flags(desc, desc->flags); + int ret; + + ret = check_reserved(desc, "set_dir"); + if (ret) + return ret; + + return _dm_gpio_set_dir_flags(desc, desc->flags); }

/**

Add a macro to provide the GPIO output value according the dir flags content.

Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---

This patch was part of v2 08/14 = gpio: add ops for configuration with dir flags

Changes in v3: - Split the previous patch [PATCH v2 08/14] to help review

Changes in v2: None

drivers/gpio/gpio-uclass.c | 9 +++------ include/asm-generic/gpio.h | 6 ++++++ 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 62462dffa5..1d16d4acf9 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -541,12 +541,9 @@ static int _dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) }

if (flags & GPIOD_IS_OUT) { - int value = flags & GPIOD_IS_OUT_ACTIVE ? 1 : 0; - - if (flags & GPIOD_ACTIVE_LOW) - value = !value; - ret = ops->direction_output(dev, desc->offset, value); - } else if (flags & GPIOD_IS_IN) { + ret = ops->direction_output(dev, desc->offset, + GPIOD_FLAGS_OUTPUT(flags)); + } else if (flags & GPIOD_IS_IN) { ret = ops->direction_input(dev, desc->offset); }

diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 454578c8d2..111319f41f 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -129,6 +129,12 @@ struct gpio_desc { */ };

+/* helper to compute the value of the gpio output */ +#define GPIOD_FLAGS_OUTPUT_MASK (GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE) +#define GPIOD_FLAGS_OUTPUT(flags) \ + (((((flags) & GPIOD_FLAGS_OUTPUT_MASK) == GPIOD_IS_OUT_ACTIVE) || \ + (((flags) & GPIOD_FLAGS_OUTPUT_MASK) == GPIOD_ACTIVE_LOW))) + /** * dm_gpio_is_valid() - Check if a GPIO is valid *

This commit manages the new dir flags that can be used in gpio specifiers to indicate the pull-up or pull-down resistor configuration for output gpio (GPIO_PULL_UP, GPIO_PULL_DOWN) or the Open Drain/Open Source configuration for input gpio (GPIO_OPEN_DRAIN, GPIO_OPEN_SOURCE).

These flags are already supported in Linux kernel in gpio lib.

This patch only parse and save the direction flags in GPIO descriptor (desc->flags), it prepares the introduction of new ops to manage them. The GPIO uclass supports new GPIO flags from device-tree (GPIO_XXX define in include/dt-bindings/gpio/gpio.h) and translate them in the dir flags (GPIOD_XXX): - GPIO_PULL_UP => GPIOD_PULL_UP - GPIO_PULL_DOWN => GPIOD_PULL_DOWN - GPIO_OPEN_DRAIN => GPIOD_OPEN_DRAIN - GPIO_OPEN_SOURCE => GPIOD_OPEN_SOURCE

This patch also adds protection in the check_dir_flags function for new invalid configuration of the dir flags.

Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---

This patch was part of v2 08/14 = gpio: add ops for configuration with dir flags

Changes in v3: - Split the previous patch [PATCH v2 08/14] to help review

Changes in v2: None

drivers/gpio/gpio-uclass.c | 30 ++++++++++++++++++++++++++++++ include/asm-generic/gpio.h | 6 +++++- 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 85f0b03f81..e02ef29fed 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -144,6 +144,24 @@ int gpio_xlate_offs_flags(struct udevice *dev, struct gpio_desc *desc, if (args->args[1] & GPIO_ACTIVE_LOW) desc->flags |= GPIOD_ACTIVE_LOW;

+ /* + * need to test 2 bits for gpio output binding: + * OPEN_DRAIN (0x6) = SINGLE_ENDED (0x2) | LINE_OPEN_DRAIN (0x4) + * OPEN_SOURCE (0x2) = SINGLE_ENDED (0x2) | LINE_OPEN_SOURCE (0x0) + */ + if (args->args[1] & GPIO_SINGLE_ENDED) { + if (args->args[1] & GPIO_LINE_OPEN_DRAIN) + desc->flags |= GPIOD_OPEN_DRAIN; + else + desc->flags |= GPIOD_OPEN_SOURCE; + } + + if (args->args[1] & GPIO_PULL_UP) + desc->flags |= GPIOD_PULL_UP; + + if (args->args[1] & GPIO_PULL_DOWN) + desc->flags |= GPIOD_PULL_DOWN; + return 0; }

@@ -520,6 +538,18 @@ static int check_dir_flags(ulong flags) return -EINVAL; }

+ if ((flags & GPIOD_PULL_UP) && (flags & GPIOD_PULL_DOWN)) { + log_debug("%s: flags 0x%lx has GPIOD_PULL_UP and GPIOD_PULL_DOWN\n", + __func__, flags); + return -EINVAL; + } + + if ((flags & GPIOD_OPEN_DRAIN) && (flags & GPIOD_OPEN_SOURCE)) { + log_debug("%s: flags 0x%lx has GPIOD_OPEN_DRAIN and GPIOD_OPEN_SOURCE\n", + __func__, flags); + return -EINVAL; + } + return 0; }

diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 3dd0002b0b..4f7b43f163 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -119,8 +119,12 @@ struct gpio_desc { unsigned long flags; #define GPIOD_IS_OUT BIT(1) /* GPIO is an output */ #define GPIOD_IS_IN BIT(2) /* GPIO is an input */ -#define GPIOD_ACTIVE_LOW BIT(3) /* value has active low */ +#define GPIOD_ACTIVE_LOW BIT(3) /* GPIO is active when value is low */ #define GPIOD_IS_OUT_ACTIVE BIT(4) /* set output active */ +#define GPIOD_OPEN_DRAIN BIT(5) /* GPIO is open drain type */ +#define GPIOD_OPEN_SOURCE BIT(6) /* GPIO is open source type */ +#define GPIOD_PULL_UP BIT(7) /* GPIO has pull-up enabled */ +#define GPIOD_PULL_DOWN BIT(8) /* GPIO has pull-down enabled */

uint offset; /* GPIO offset within the device */ /*

Add the ops for GPIO driver set_dir_flags() to set the dir flags. The user can update the direction and configuration of each GPIO with a only call to dm_gpio_set_dir_flags() or dm_gpio_set_dir() and respecting the configuration provided by device tree (saved in desc->flags).

When these optional ops are absent, the gpio uclass use the mandatory ops (direction_output, direction_input, get_value) and desc->flags to manage only the main dir flags: - GPIOD_IS_IN - GPIOD_IS_OUT - GPIOD_IS_OUT_ACTIVE - GPIOD_ACTIVE_LOW

Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---

This patch was part of v2 08/14 = gpio: add ops for configuration with dir flags

Changes in v3: - Split the previous patch [PATCH v2 08/14] to help review

Changes in v2: - change the proposed ops for pin config to set_dir_flags/get_dir_flags - reused the existing API dm_gpio_set_dir_flags/dm_gpio_set_dir - add a new API dm_gpio_get_dir_flags

drivers/gpio/gpio-uclass.c | 17 ++++++++++++----- include/asm-generic/gpio.h | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 89af37a4ea..5da28e6ed9 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -571,11 +571,16 @@ static int _dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) return ret; }

- if (flags & GPIOD_IS_OUT) { - ret = ops->direction_output(dev, desc->offset, - GPIOD_FLAGS_OUTPUT(flags)); - } else if (flags & GPIOD_IS_IN) { - ret = ops->direction_input(dev, desc->offset); + /* GPIOD_ are directly managed by driver in set_dir_flags*/ + if (ops->set_dir_flags) { + ret = ops->set_dir_flags(dev, desc->offset, flags); + } else { + if (flags & GPIOD_IS_OUT) { + ret = ops->direction_output(dev, desc->offset, + GPIOD_FLAGS_OUTPUT(flags)); + } else if (flags & GPIOD_IS_IN) { + ret = ops->direction_input(dev, desc->offset); + } }

return ret; @@ -1145,6 +1150,8 @@ static int gpio_post_bind(struct udevice *dev) ops->get_function += gd->reloc_off; if (ops->xlate) ops->xlate += gd->reloc_off; + if (ops->set_dir_flags) + ops->set_dir_flags += gd->reloc_off; if (ops->get_dir_flags) ops->get_dir_flags += gd->reloc_off;

diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index ac30c3ee4e..ec4877037a 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -298,6 +298,22 @@ struct dm_gpio_ops { int (*xlate)(struct udevice *dev, struct gpio_desc *desc, struct ofnode_phandle_args *args);

+ /** + * set_dir_flags() - Set GPIO dir flags + * + * This function should set up the GPIO configuration according to the + * information provide by the direction flags bitfield. + * + * This method is optional. + * + * @dev: GPIO device + * @offset: GPIO offset within that device + * @flags: GPIO configuration to use + * @return 0 if OK, -ve on error + */ + int (*set_dir_flags)(struct udevice *dev, unsigned int offset, + ulong flags); + /** * get_dir_flags() - Get GPIO dir flags *

Add param information in pin information output. This update prepare unitary test for pin configuration in pinctrl node.

Signed-off-by: Patrick Delaunay patrick.delaunay@st.com Reviewed-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

drivers/pinctrl/pinctrl-sandbox.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-sandbox.c b/drivers/pinctrl/pinctrl-sandbox.c index 0786afe747..d1a21f0f19 100644 --- a/drivers/pinctrl/pinctrl-sandbox.c +++ b/drivers/pinctrl/pinctrl-sandbox.c @@ -54,6 +54,10 @@ static const struct pinconf_param sandbox_conf_params[] = { { "input-disable", PIN_CONFIG_INPUT_ENABLE, 0 }, };

+/* bitfield used to save param and value of each pin/selector */ +static unsigned int sandbox_pins_param[ARRAY_SIZE(sandbox_pins)]; +static unsigned int sandbox_pins_value[ARRAY_SIZE(sandbox_pins)]; + static int sandbox_get_pins_count(struct udevice *dev) { return ARRAY_SIZE(sandbox_pins); @@ -68,8 +72,25 @@ static int sandbox_get_pin_muxing(struct udevice *dev, unsigned int selector, char *buf, int size) { + const struct pinconf_param *p; + int i; + snprintf(buf, size, "%s", sandbox_pins_muxing[selector]);

+ if (sandbox_pins_param[selector]) { + for (i = 0, p = sandbox_conf_params; + i < ARRAY_SIZE(sandbox_conf_params); + i++, p++) { + if ((sandbox_pins_param[selector] & BIT(p->param)) && + (!!(sandbox_pins_value[selector] & BIT(p->param)) == + p->default_value)) { + strncat(buf, " ", size); + strncat(buf, p->property, size); + } + } + } + strncat(buf, ".", size); + return 0; }

@@ -102,6 +123,9 @@ static int sandbox_pinmux_set(struct udevice *dev, unsigned pin_selector, pin_selector, sandbox_get_pin_name(dev, pin_selector), func_selector, sandbox_get_function_name(dev, func_selector));

+ sandbox_pins_param[pin_selector] = 0; + sandbox_pins_value[pin_selector] = 0; + return 0; }

@@ -123,6 +147,12 @@ static int sandbox_pinconf_set(struct udevice *dev, unsigned pin_selector, pin_selector, sandbox_get_pin_name(dev, pin_selector), param, argument);

+ sandbox_pins_param[pin_selector] |= BIT(param); + if (argument) + sandbox_pins_value[pin_selector] |= BIT(param); + else + sandbox_pins_value[pin_selector] &= ~BIT(param); + return 0; }

Add test for "pins" configuration in gpio uclass with set_state() ops and test for generic parsing of pinconf_param array).

set_state() is called by: - pinctrl_generic_set_state |- pinctrl_generic_set_state_subnode

Signed-off-by: Patrick Delaunay patrick.delaunay@st.com Reviewed-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

arch/sandbox/dts/test.dts | 25 +++++++++++++++++++++++++ drivers/pinctrl/pinctrl-sandbox.c | 14 +++++++++++++- test/py/tests/test_pinmux.py | 28 ++++++++++++++++++---------- 3 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index fdb08f2111..b57f152fa3 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -851,6 +851,31 @@

pinctrl { compatible = "sandbox,pinctrl"; + + pinctrl-names = "default"; + pinctrl-0 = <&gpios>; + + gpios: gpios { + gpio0 { + pins = "GPIO0"; + bias-pull-up; + input-disable; + }; + gpio1 { + pins = "GPIO1"; + output-high; + drive-open-drain; + }; + gpio2 { + pins = "GPIO2"; + bias-pull-down; + input-enable; + }; + gpio3 { + pins = "GPIO3"; + bias-disable; + }; + }; };

hwspinlock@0 { diff --git a/drivers/pinctrl/pinctrl-sandbox.c b/drivers/pinctrl/pinctrl-sandbox.c index d1a21f0f19..3ee75fbbee 100644 --- a/drivers/pinctrl/pinctrl-sandbox.c +++ b/drivers/pinctrl/pinctrl-sandbox.c @@ -14,7 +14,11 @@ static const char * const sandbox_pins[] = { "SDA", "TX", "RX", - "W1" + "W1", + "GPIO0", + "GPIO1", + "GPIO2", + "GPIO3", };

static const char * const sandbox_pins_muxing[] = { @@ -23,6 +27,10 @@ static const char * const sandbox_pins_muxing[] = { "Uart TX", "Uart RX", "1-wire gpio", + "gpio", + "gpio", + "gpio", + "gpio", };

static const char * const sandbox_groups[] = { @@ -38,6 +46,10 @@ static const char * const sandbox_functions[] = { "serial", "spi", "w1", + "gpio", + "gpio", + "gpio", + "gpio", };

static const struct pinconf_param sandbox_conf_params[] = { diff --git a/test/py/tests/test_pinmux.py b/test/py/tests/test_pinmux.py index 25394f1faf..5ca0b4b630 100644 --- a/test/py/tests/test_pinmux.py +++ b/test/py/tests/test_pinmux.py @@ -22,11 +22,15 @@ def test_pinmux_usage_2(u_boot_console): def test_pinmux_status_all(u_boot_console): """Test that 'pinmux status -a' displays pin's muxing.""" output = u_boot_console.run_command('pinmux status -a') - assert ('SCL : I2C SCL' in output) - assert ('SDA : I2C SDA' in output) - assert ('TX : Uart TX' in output) - assert ('RX : Uart RX' in output) - assert ('W1 : 1-wire gpio' in output) + assert ('SCL : I2C SCL.' in output) + assert ('SDA : I2C SDA.' in output) + assert ('TX : Uart TX.' in output) + assert ('RX : Uart RX.' in output) + assert ('W1 : 1-wire gpio.' in output) + assert ('GPIO0 : gpio bias-pull-up input-disable.' in output) + assert ('GPIO1 : gpio drive-open-drain.' in output) + assert ('GPIO2 : gpio bias-pull-down input-enable.' in output) + assert ('GPIO3 : gpio bias-disable.' in output)

@pytest.mark.buildconfigspec('cmd_pinmux') @pytest.mark.boardspec('sandbox') @@ -59,8 +63,12 @@ def test_pinmux_status(u_boot_console): """Test that 'pinmux status' displays selected pincontroller's pin muxing descriptions.""" output = u_boot_console.run_command('pinmux status') - assert ('SCL : I2C SCL' in output) - assert ('SDA : I2C SDA' in output) - assert ('TX : Uart TX' in output) - assert ('RX : Uart RX' in output) - assert ('W1 : 1-wire gpio' in output) + assert ('SCL : I2C SCL.' in output) + assert ('SDA : I2C SDA.' in output) + assert ('TX : Uart TX.' in output) + assert ('RX : Uart RX.' in output) + assert ('W1 : 1-wire gpio.' in output) + assert ('GPIO0 : gpio bias-pull-up input-disable.' in output) + assert ('GPIO1 : gpio drive-open-drain.' in output) + assert ('GPIO2 : gpio bias-pull-down input-enable.' in output) + assert ('GPIO3 : gpio bias-disable.' in output)

Cleanup binding support, use the generic binding by default (test u-class gpio_xlate_offs_flags function) and add specific binding for added value.

Signed-off-by: Patrick Delaunay patrick.delaunay@st.com Reviewed-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

arch/sandbox/dts/test.dts | 14 ++++++++++---- drivers/gpio/sandbox.c | 13 ++++++++----- include/dt-bindings/gpio/sandbox-gpio.h | 24 ++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 9 deletions(-) create mode 100644 include/dt-bindings/gpio/sandbox-gpio.h

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index b57f152fa3..df58a0b488 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -1,5 +1,8 @@ /dts-v1/;

+#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/gpio/sandbox-gpio.h> + / { model = "sandbox"; compatible = "sandbox"; @@ -86,11 +89,14 @@ ping-expect = <0>; ping-add = <0>; u-boot,dm-pre-reloc; - test-gpios = <&gpio_a 1>, <&gpio_a 4>, <&gpio_b 5 0 3 2 1>, + test-gpios = <&gpio_a 1>, <&gpio_a 4>, + <&gpio_b 5 GPIO_ACTIVE_HIGH 3 2 1>, <0>, <&gpio_a 12>; - test2-gpios = <&gpio_a 1>, <&gpio_a 4>, <&gpio_b 6 1 3 2 1>, - <&gpio_b 7 2 3 2 1>, <&gpio_b 8 4 3 2 1>, - <&gpio_b 9 0xc 3 2 1>; + test2-gpios = <&gpio_a 1>, <&gpio_a 4>, + <&gpio_b 6 GPIO_ACTIVE_LOW 3 2 1>, + <&gpio_b 7 GPIO_IN 3 2 1>, + <&gpio_b 8 GPIO_OUT 3 2 1>, + <&gpio_b 9 (GPIO_OUT|GPIO_OUT_ACTIVE) 3 2 1>; int-value = <1234>; uint-value = <(-1234)>; }; diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index 91e8e0677e..c2a8adc647 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -10,6 +10,7 @@ #include <asm/gpio.h> #include <dm/of.h> #include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/gpio/sandbox-gpio.h>

/* Flags for each GPIO */ #define GPIOF_OUTPUT (1 << 0) /* Currently set as an output */ @@ -136,13 +137,15 @@ static int sb_gpio_xlate(struct udevice *dev, struct gpio_desc *desc, desc->offset = args->args[0]; if (args->args_count < 2) return 0; - if (args->args[1] & GPIO_ACTIVE_LOW) - desc->flags |= GPIOD_ACTIVE_LOW; - if (args->args[1] & 2) + /* treat generic binding with gpio uclass */ + gpio_xlate_offs_flags(dev, desc, args); + + /* sandbox test specific, not defined in gpio.h */ + if (args->args[1] & GPIO_IN) desc->flags |= GPIOD_IS_IN; - if (args->args[1] & 4) + if (args->args[1] & GPIO_OUT) desc->flags |= GPIOD_IS_OUT; - if (args->args[1] & 8) + if (args->args[1] & GPIO_OUT_ACTIVE) desc->flags |= GPIOD_IS_OUT_ACTIVE;

return 0; diff --git a/include/dt-bindings/gpio/sandbox-gpio.h b/include/dt-bindings/gpio/sandbox-gpio.h new file mode 100644 index 0000000000..e4bfdb3ce1 --- /dev/null +++ b/include/dt-bindings/gpio/sandbox-gpio.h @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * This header provides constants for binding sandbox,gpio + * + */ +#ifndef _DT_BINDINGS_GPIO_SANDBOX_GPIO_H +#define _DT_BINDINGS_GPIO_SANDBOX_GPIO_H + +/* + * Add a specific binding for sandbox gpio. + * The value need to be after the generic defines of + * dt-bindings/gpio/gpio.h + */ + +/* Bit 16 express GPIO input mode */ +#define GPIO_IN 0x10000 + +/* Bit 17 express GPIO output mode */ +#define GPIO_OUT 0x20000 + +/* Bit 18 express GPIO output is active */ +#define GPIO_OUT_ACTIVE 0x40000 + +#endif

Add tests for new API set_dir_flags and set_dir_flags and associated code in gpio uclass.

Test support for new flags GPIO_OPEN_DRAIN, GPIO_OPEN_SOURCE GPIO_PULL_UP and GPIO_PULL_DOWN.

Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---

Changes in v3: None Changes in v2: - simplify sandbox GPIO driver: save dir_flags (GPIOD_...) and remove internal flags (GPIOF_...); the previous path "gpio: sandbox: cleanup flag support" is no more needed - add unitary test in dm_test_gpio for set_dir_flags - add some function check in dm_test_gpio_phandles - dm_test_gpio_pin_config change to dm_test_gpio_get_dir_flags

arch/sandbox/dts/test.dts | 16 ++++++ arch/sandbox/include/asm/gpio.h | 20 ++++++++ drivers/gpio/sandbox.c | 86 +++++++++++++++++++++++++-------- test/dm/gpio.c | 66 ++++++++++++++++++++++--- test/dm/test-fdt.c | 2 +- 5 files changed, 163 insertions(+), 27 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index df58a0b488..8ab22c0c81 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -16,6 +16,7 @@ eth5 = &eth_5; gpio1 = &gpio_a; gpio2 = &gpio_b; + gpio3 = &gpio_c; i2c0 = "/i2c@0"; mmc0 = "/mmc0"; mmc1 = "/mmc1"; @@ -97,6 +98,13 @@ <&gpio_b 7 GPIO_IN 3 2 1>, <&gpio_b 8 GPIO_OUT 3 2 1>, <&gpio_b 9 (GPIO_OUT|GPIO_OUT_ACTIVE) 3 2 1>; + test3-gpios = + <&gpio_c 0 (GPIO_OUT|GPIO_OPEN_DRAIN)>, + <&gpio_c 1 (GPIO_OUT|GPIO_OPEN_SOURCE)>, + <&gpio_c 2 GPIO_OUT>, + <&gpio_c 3 (GPIO_IN|GPIO_PULL_UP)>, + <&gpio_c 4 (GPIO_IN|GPIO_PULL_DOWN)>, + <&gpio_c 5 GPIO_IN>; int-value = <1234>; uint-value = <(-1234)>; }; @@ -296,6 +304,14 @@ sandbox,gpio-count = <10>; };

+ gpio_c: extra2-gpios { + compatible = "sandbox,gpio"; + gpio-controller; + #gpio-cells = <2>; + gpio-bank-name = "c"; + sandbox,gpio-count = <10>; + }; + i2c@0 { #address-cells = <1>; #size-cells = <0>; diff --git a/arch/sandbox/include/asm/gpio.h b/arch/sandbox/include/asm/gpio.h index cfb803bb3b..df4ba4fb5f 100644 --- a/arch/sandbox/include/asm/gpio.h +++ b/arch/sandbox/include/asm/gpio.h @@ -62,4 +62,24 @@ int sandbox_gpio_get_direction(struct udevice *dev, unsigned int offset); int sandbox_gpio_set_direction(struct udevice *dev, unsigned int offset, int output);

+/** + * Return the simulated flags of a GPIO (used only in sandbox test code) + * + * @param dev device to use + * @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); + +/** + * 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 + * @return -1 on error, 0 if ok + */ +int sandbox_gpio_set_dir_flags(struct udevice *dev, unsigned int offset, + ulong flags); + #endif diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index c2a8adc647..a9c470ee5e 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -8,43 +8,43 @@ #include <fdtdec.h> #include <malloc.h> #include <asm/gpio.h> +#include <dm/lists.h> #include <dm/of.h> +#include <dm/pinctrl.h> #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/gpio/sandbox-gpio.h>

-/* Flags for each GPIO */ -#define GPIOF_OUTPUT (1 << 0) /* Currently set as an output */ -#define GPIOF_HIGH (1 << 1) /* Currently set high */

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

-/* Access routines for GPIO state */ -static u8 *get_gpio_flags(struct udevice *dev, unsigned offset) +/* Access routines for GPIO dir flags */ +static ulong *get_gpio_dir_flags(struct udevice *dev, unsigned int 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 u8 invalid_flags; + static ulong invalid_dir_flags; printf("sandbox_gpio: error: invalid gpio %u\n", offset); - return &invalid_flags; + return &invalid_dir_flags; }

- return &state[offset].flags; + return &state[offset].dir_flags; + }

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

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

if (value) *gpio |= flag; @@ -60,24 +60,40 @@ static int set_gpio_flag(struct udevice *dev, unsigned offset, int flag,

int sandbox_gpio_get_value(struct udevice *dev, unsigned offset) { - if (get_gpio_flag(dev, offset, GPIOF_OUTPUT)) + 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, GPIOF_HIGH); + return get_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE); }

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

int sandbox_gpio_get_direction(struct udevice *dev, unsigned offset) { - return get_gpio_flag(dev, offset, GPIOF_OUTPUT); + return get_gpio_flag(dev, offset, GPIOD_IS_OUT); }

int sandbox_gpio_set_direction(struct udevice *dev, unsigned offset, int output) { - return set_gpio_flag(dev, offset, GPIOF_OUTPUT, output); + set_gpio_flag(dev, offset, GPIOD_IS_OUT, output); + set_gpio_flag(dev, offset, GPIOD_IS_IN, !(output)); + + return 0; +} + +ulong sandbox_gpio_get_dir_flags(struct udevice *dev, unsigned int offset) +{ + return *get_gpio_dir_flags(dev, offset); +} + +int sandbox_gpio_set_dir_flags(struct udevice *dev, unsigned int offset, + ulong flags) +{ + *get_gpio_dir_flags(dev, offset) = flags; + + return 0; }

/* @@ -126,9 +142,12 @@ static int sb_gpio_set_value(struct udevice *dev, unsigned offset, int value)

static int sb_gpio_get_function(struct udevice *dev, unsigned offset) { - if (get_gpio_flag(dev, offset, GPIOF_OUTPUT)) + if (get_gpio_flag(dev, offset, GPIOD_IS_OUT)) return GPIOF_OUTPUT; - return GPIOF_INPUT; + if (get_gpio_flag(dev, offset, GPIOD_IS_IN)) + return GPIOF_INPUT; + + return GPIOF_INPUT; /*GPIO is not configurated */ }

static int sb_gpio_xlate(struct udevice *dev, struct gpio_desc *desc, @@ -143,14 +162,39 @@ static int sb_gpio_xlate(struct udevice *dev, struct gpio_desc *desc, /* sandbox test specific, not defined in gpio.h */ if (args->args[1] & GPIO_IN) desc->flags |= GPIOD_IS_IN; + if (args->args[1] & GPIO_OUT) desc->flags |= GPIOD_IS_OUT; + if (args->args[1] & GPIO_OUT_ACTIVE) desc->flags |= GPIOD_IS_OUT_ACTIVE;

return 0; }

+static int sb_gpio_set_dir_flags(struct udevice *dev, unsigned int offset, + ulong flags) +{ + ulong *dir_flags; + + debug("%s: offset:%u, dir_flags = %lx\n", __func__, offset, flags); + + dir_flags = get_gpio_dir_flags(dev, offset); + + *dir_flags = flags; + + return 0; +} + +static int sb_gpio_get_dir_flags(struct udevice *dev, unsigned int offset, + ulong *flags) +{ + debug("%s: offset:%u\n", __func__, offset); + *flags = *get_gpio_dir_flags(dev, offset); + + return 0; +} + static const struct dm_gpio_ops gpio_sandbox_ops = { .direction_input = sb_gpio_direction_input, .direction_output = sb_gpio_direction_output, @@ -158,6 +202,8 @@ static const struct dm_gpio_ops gpio_sandbox_ops = { .set_value = sb_gpio_set_value, .get_function = sb_gpio_get_function, .xlate = sb_gpio_xlate, + .set_dir_flags = sb_gpio_set_dir_flags, + .get_dir_flags = sb_gpio_get_dir_flags, };

static int sandbox_gpio_ofdata_to_platdata(struct udevice *dev) diff --git a/test/dm/gpio.c b/test/dm/gpio.c index 8abf0cd8c8..5992d938d7 100644 --- a/test/dm/gpio.c +++ b/test/dm/gpio.c @@ -23,9 +23,9 @@ static int dm_test_gpio(struct unit_test_state *uts) char buf[80];

/* - * We expect to get 3 banks. One is anonymous (just numbered) and - * comes from platdata. The other two are named a (20 gpios) - * and b (10 gpios) and come from the device tree. See + * We expect to get 4 banks. One is anonymous (just numbered) and + * comes from platdata. The other are named a (20 gpios), + * b (10 gpios) and c (10 gpios) and come from the device tree. See * test/dm/test.dts. */ ut_assertok(gpio_lookup_name("b4", &dev, &offset, &gpio)); @@ -72,6 +72,18 @@ 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(GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE, + sandbox_gpio_get_dir_flags(dev, offset)); + ut_assertok(ops->set_dir_flags(dev, offset, + GPIOD_IS_OUT | GPIOD_OPEN_DRAIN)); + ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_DRAIN, + sandbox_gpio_get_dir_flags(dev, offset)); + ut_assertok(ops->set_dir_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)); + /* Make it an input */ ut_assertok(ops->direction_input(dev, offset)); ut_assertok(gpio_get_status(dev, offset, buf, sizeof(buf))); @@ -207,11 +219,14 @@ static int dm_test_gpio_phandles(struct unit_test_state *uts) desc_list2, ARRAY_SIZE(desc_list2), 0)); + ut_asserteq(GPIOF_INPUT, gpio_get_function(gpio_a, 4, NULL)); ut_assertok(gpio_free_list(dev, desc_list, 3)); + ut_asserteq(GPIOF_UNUSED, gpio_get_function(gpio_a, 4, NULL)); ut_asserteq(3, gpio_request_list_by_name(dev, "test-gpios", desc_list, ARRAY_SIZE(desc_list), GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE)); + ut_asserteq(GPIOF_OUTPUT, gpio_get_function(gpio_a, 4, NULL)); ut_asserteq_ptr(gpio_a, desc_list[0].dev); ut_asserteq(1, desc_list[0].offset); ut_asserteq_ptr(gpio_a, desc_list[1].dev); @@ -221,10 +236,14 @@ static int dm_test_gpio_phandles(struct unit_test_state *uts) ut_asserteq(1, dm_gpio_get_value(desc_list)); 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)); ut_asserteq(6, gpio_request_list_by_name(dev, "test2-gpios", desc_list, ARRAY_SIZE(desc_list), 0)); - /* This was set to output previously, so still will be */ - ut_asserteq(GPIOF_OUTPUT, gpio_get_function(gpio_a, 1, NULL)); + + /* 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(GPIOF_INPUT, gpio_get_function(gpio_a, 1, NULL));

/* Active low should invert the input value */ ut_asserteq(GPIOF_INPUT, gpio_get_function(gpio_b, 6, NULL)); @@ -236,7 +255,42 @@ static int dm_test_gpio_phandles(struct unit_test_state *uts) ut_asserteq(GPIOF_OUTPUT, gpio_get_function(gpio_b, 9, NULL)); ut_asserteq(1, dm_gpio_get_value(&desc_list[5]));

- return 0; } DM_TEST(dm_test_gpio_phandles, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + +/* Check the gpio pin configuration get from device tree information */ +static int dm_test_gpio_get_dir_flags(struct unit_test_state *uts) +{ + struct gpio_desc desc_list[6]; + struct udevice *dev; + ulong flags; + + ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev)); + + ut_asserteq(6, gpio_request_list_by_name(dev, "test3-gpios", desc_list, + ARRAY_SIZE(desc_list), 0)); + + ut_assertok(dm_gpio_get_dir_flags(&desc_list[0], &flags)); + ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_DRAIN, flags); + + ut_assertok(dm_gpio_get_dir_flags(&desc_list[1], &flags)); + ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_SOURCE, flags); + + ut_assertok(dm_gpio_get_dir_flags(&desc_list[2], &flags)); + ut_asserteq(GPIOD_IS_OUT, flags); + + ut_assertok(dm_gpio_get_dir_flags(&desc_list[3], &flags)); + ut_asserteq(GPIOD_IS_IN | GPIOD_PULL_UP, flags); + + ut_assertok(dm_gpio_get_dir_flags(&desc_list[4], &flags)); + ut_asserteq(GPIOD_IS_IN | GPIOD_PULL_DOWN, flags); + + ut_assertok(dm_gpio_get_dir_flags(&desc_list[5], &flags)); + ut_asserteq(GPIOD_IS_IN, flags); + + ut_assertok(gpio_free_list(dev, desc_list, 6)); + + return 0; +} +DM_TEST(dm_test_gpio_get_dir_flags, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 1fb8b5c248..3451146d04 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -227,7 +227,7 @@ static int dm_test_alias_highest_id(struct unit_test_state *uts) ut_asserteq(5, ret);

ret = dev_read_alias_highest_id("gpio"); - ut_asserteq(2, ret); + ut_asserteq(3, ret);

ret = dev_read_alias_highest_id("pci"); ut_asserteq(2, ret);

Add a simple pincontrol associated to the sandbox gpio driver, that allows to check pin configuration with the command pinmux.

The pinmux test is also updated to test behavior with 2 pincontrols.

Example to check LED pin configuration:

=> pinmux list | Device | Driver | Parent | pinctrl-gpio | sandbox_pinctrl_gpio | root_driver | pinctrl | sandbox_pinctrl | root_driver

=> pinmux dev pinctrl-gpio

=> pinmux status

a0 : gpio input . a1 : gpio input . a2 : gpio input . a3 : gpio input . a4 : gpio input . a5 : gpio output . a6 : gpio output . ...

Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---

Changes in v3: None Changes in v2: - Adapt sandbox_pinctrl_gpio driver with the saved dir_flags in sandbox gpio driver - rebase on v2020.01-rc3

arch/sandbox/dts/test.dts | 48 +++++---- drivers/gpio/sandbox.c | 195 +++++++++++++++++++++++++++++++++++ test/py/tests/test_pinmux.py | 10 ++ 3 files changed, 231 insertions(+), 22 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 8ab22c0c81..b69f0da871 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -288,28 +288,32 @@ }; };

- gpio_a: base-gpios { - compatible = "sandbox,gpio"; - gpio-controller; - #gpio-cells = <1>; - gpio-bank-name = "a"; - sandbox,gpio-count = <20>; - }; - - gpio_b: extra-gpios { - compatible = "sandbox,gpio"; - gpio-controller; - #gpio-cells = <5>; - gpio-bank-name = "b"; - sandbox,gpio-count = <10>; - }; - - gpio_c: extra2-gpios { - compatible = "sandbox,gpio"; - gpio-controller; - #gpio-cells = <2>; - gpio-bank-name = "c"; - sandbox,gpio-count = <10>; + pinctrl-gpio { + compatible = "sandbox,pinctrl-gpio"; + + gpio_a: base-gpios { + compatible = "sandbox,gpio"; + gpio-controller; + #gpio-cells = <1>; + gpio-bank-name = "a"; + sandbox,gpio-count = <20>; + }; + + gpio_b: extra-gpios { + compatible = "sandbox,gpio"; + gpio-controller; + #gpio-cells = <5>; + gpio-bank-name = "b"; + sandbox,gpio-count = <10>; + }; + + gpio_c: pinmux-gpios { + compatible = "sandbox,gpio"; + gpio-controller; + #gpio-cells = <2>; + gpio-bank-name = "c"; + sandbox,gpio-count = <10>; + }; };

i2c@0 { diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index a9c470ee5e..bf6287f029 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -251,3 +251,198 @@ U_BOOT_DRIVER(gpio_sandbox) = { .remove = gpio_sandbox_remove, .ops = &gpio_sandbox_ops, }; + +/* pincontrol: used only to check GPIO pin configuration (pinmux command) */ + +struct sb_pinctrl_priv { + int pinctrl_ngpios; + struct list_head gpio_dev; +}; + +struct sb_gpio_bank { + struct udevice *gpio_dev; + struct list_head list; +}; + +static int sb_populate_gpio_dev_list(struct udevice *dev) +{ + struct sb_pinctrl_priv *priv = dev_get_priv(dev); + struct udevice *gpio_dev; + struct udevice *child; + struct sb_gpio_bank *gpio_bank; + int ret; + + /* + * parse pin-controller sub-nodes (ie gpio bank nodes) and fill + * a list with all gpio device reference which belongs to the + * current pin-controller. This list is used to find pin_name and + * pin muxing + */ + list_for_each_entry(child, &dev->child_head, sibling_node) { + ret = uclass_get_device_by_name(UCLASS_GPIO, child->name, + &gpio_dev); + if (ret < 0) + continue; + + gpio_bank = malloc(sizeof(*gpio_bank)); + if (!gpio_bank) { + dev_err(dev, "Not enough memory\n"); + return -ENOMEM; + } + + gpio_bank->gpio_dev = gpio_dev; + list_add_tail(&gpio_bank->list, &priv->gpio_dev); + } + + return 0; +} + +static int sb_pinctrl_get_pins_count(struct udevice *dev) +{ + struct sb_pinctrl_priv *priv = dev_get_priv(dev); + struct gpio_dev_priv *uc_priv; + struct sb_gpio_bank *gpio_bank; + + /* + * if get_pins_count has already been executed once on this + * pin-controller, no need to run it again + */ + if (priv->pinctrl_ngpios) + return priv->pinctrl_ngpios; + + if (list_empty(&priv->gpio_dev)) + sb_populate_gpio_dev_list(dev); + /* + * walk through all banks to retrieve the pin-controller + * pins number + */ + list_for_each_entry(gpio_bank, &priv->gpio_dev, list) { + uc_priv = dev_get_uclass_priv(gpio_bank->gpio_dev); + + priv->pinctrl_ngpios += uc_priv->gpio_count; + } + + return priv->pinctrl_ngpios; +} + +static struct udevice *sb_pinctrl_get_gpio_dev(struct udevice *dev, + unsigned int selector, + unsigned int *idx) +{ + struct sb_pinctrl_priv *priv = dev_get_priv(dev); + struct sb_gpio_bank *gpio_bank; + struct gpio_dev_priv *uc_priv; + int pin_count = 0; + + if (list_empty(&priv->gpio_dev)) + sb_populate_gpio_dev_list(dev); + + /* look up for the bank which owns the requested pin */ + list_for_each_entry(gpio_bank, &priv->gpio_dev, list) { + uc_priv = dev_get_uclass_priv(gpio_bank->gpio_dev); + + if (selector < (pin_count + uc_priv->gpio_count)) { + /* + * we found the bank, convert pin selector to + * gpio bank index + */ + *idx = selector - pin_count; + + return gpio_bank->gpio_dev; + } + pin_count += uc_priv->gpio_count; + } + + return NULL; +} + +static const char *sb_pinctrl_get_pin_name(struct udevice *dev, + unsigned int selector) +{ + struct gpio_dev_priv *uc_priv; + struct udevice *gpio_dev; + unsigned int gpio_idx; + static char pin_name[PINNAME_SIZE]; + + /* look up for the bank which owns the requested pin */ + gpio_dev = sb_pinctrl_get_gpio_dev(dev, selector, &gpio_idx); + if (!gpio_dev) { + snprintf(pin_name, PINNAME_SIZE, "Error"); + } else { + uc_priv = dev_get_uclass_priv(gpio_dev); + + snprintf(pin_name, PINNAME_SIZE, "%s%d", + uc_priv->bank_name, + gpio_idx); + } + + return pin_name; +} + +static char *get_dir_flags_string(ulong flags) +{ + if (flags & GPIOD_OPEN_DRAIN) + return "drive-open-drain"; + if (flags & GPIOD_OPEN_SOURCE) + return "drive-open-source"; + if (flags & GPIOD_PULL_UP) + return "bias-pull-up"; + if (flags & GPIOD_PULL_DOWN) + return "bias-pull-down"; + return "."; +} + +static int sb_pinctrl_get_pin_muxing(struct udevice *dev, + unsigned int selector, + char *buf, int size) +{ + struct udevice *gpio_dev; + unsigned int gpio_idx; + ulong dir_flags; + int function; + + /* look up for the bank which owns the requested pin */ + gpio_dev = sb_pinctrl_get_gpio_dev(dev, selector, &gpio_idx); + if (!gpio_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); + + snprintf(buf, size, "gpio %s %s", + function == GPIOF_OUTPUT ? "output" : "input", + get_dir_flags_string(dir_flags)); + } + + return 0; +} + +static int sandbox_pinctrl_probe(struct udevice *dev) +{ + struct sb_pinctrl_priv *priv = dev_get_priv(dev); + + INIT_LIST_HEAD(&priv->gpio_dev); + + return 0; +} + +static struct pinctrl_ops sandbox_pinctrl_gpio_ops = { + .get_pin_name = sb_pinctrl_get_pin_name, + .get_pins_count = sb_pinctrl_get_pins_count, + .get_pin_muxing = sb_pinctrl_get_pin_muxing, +}; + +static const struct udevice_id sandbox_pinctrl_gpio_match[] = { + { .compatible = "sandbox,pinctrl-gpio" }, + { /* sentinel */ } +}; + +U_BOOT_DRIVER(sandbox_pinctrl_gpio) = { + .name = "sandbox_pinctrl_gpio", + .id = UCLASS_PINCTRL, + .of_match = sandbox_pinctrl_gpio_match, + .ops = &sandbox_pinctrl_gpio_ops, + .bind = dm_scan_fdt_dev, + .probe = sandbox_pinctrl_probe, + .priv_auto_alloc_size = sizeof(struct sb_pinctrl_priv), +}; diff --git a/test/py/tests/test_pinmux.py b/test/py/tests/test_pinmux.py index 5ca0b4b630..4e6df992a4 100644 --- a/test/py/tests/test_pinmux.py +++ b/test/py/tests/test_pinmux.py @@ -22,6 +22,12 @@ def test_pinmux_usage_2(u_boot_console): def test_pinmux_status_all(u_boot_console): """Test that 'pinmux status -a' displays pin's muxing.""" output = u_boot_console.run_command('pinmux status -a') + + assert ('pinctrl-gpio:' in output) + assert ('a5 : gpio output .' in output) + assert ('a6 : gpio output .' in output) + + assert ('pinctrl:' in output) assert ('SCL : I2C SCL.' in output) assert ('SDA : I2C SDA.' in output) assert ('TX : Uart TX.' in output) @@ -63,6 +69,10 @@ def test_pinmux_status(u_boot_console): """Test that 'pinmux status' displays selected pincontroller's pin muxing descriptions.""" output = u_boot_console.run_command('pinmux status') + + assert (not 'pinctrl-gpio:' in output) + assert (not 'pinctrl:' in output) + assert ('SCL : I2C SCL.' in output) assert ('SDA : I2C SDA.' in output) assert ('TX : Uart TX.' in output)