Hi Mickael
> Subject: Re: [PATCH v4] gpio: add gpio-hog support
> Importance: High
>
> On 12. 06. 19 6:11, Heiko Schocher wrote:
> > add gpio-hog support. GPIO hogging is a mechanism providing automatic
> > GPIO request and configuration as part of the gpio-controller's driver
> > probe function.
> >
> > for more infos see:
> > doc/device-tree-bindings/gpio/gpio.txt
> >
> > Signed-off-by: Heiko Schocher <h...@denx.de>
> >
> > ---
> > clean travis build, see result:
> > https://travis-ci.org/hsdenx/u-boot-test/builds/544040419
> >
> > Changes in v4:
> > - rework as suggested by Patrick
> > based on patch:
> > http://patchwork.ozlabs.org/patch/1098913/
> > use new UCLASS_NOP
> > hog detection + DT parsing in gpio_post_bind() .... info saved in platdata
> > gpio configuration for hog (request and set_dir) => probe function
> >
> > Changes in v3:
> > - probe now all gpio devices with gpio-hogs
> > from board_r before board_late_init() call
> > or if someone wants to access gpio-hog
> > based gpios with gpio_hog_lookup_name()
> > This is needed, because we cannot be sure,
> > if a gpio device which contains gpio-hogs
> > is probed.
> > - add line-name property as Michal recommended
> > - renamed gpio_priv_one into gpio_priv_one_hog
> > and protected through CONFIG_DM_GPIO_HOG
> >
> > Changes in v2:
> > - move gpio_hog() call from post_probe() to post_bind()
> > call. Check if current gpio device has gpio-hog
> > definitions, if so, probe it.
> >
> > common/board_r.c | 6 +
> > doc/device-tree-bindings/gpio/gpio.txt | 55 ++++++++
> > drivers/gpio/Kconfig | 10 ++
> > drivers/gpio/gpio-uclass.c | 181 ++++++++++++++++++++++---
> > include/asm-generic/gpio.h | 32 +++++
> > 5 files changed, 268 insertions(+), 16 deletions(-)
> >
> > diff --git a/common/board_r.c b/common/board_r.c index
> > 150e8cd424..507da4c74a 100644
> > --- a/common/board_r.c
> > +++ b/common/board_r.c
> > @@ -49,6 +49,9 @@
> > #include <linux/err.h>
> > #include <efi_loader.h>
> > #include <wdt.h>
> > +#if defined(CONFIG_DM_GPIO_HOG)
> > +#include <asm/gpio.h>
> > +#endif
> >
> > DECLARE_GLOBAL_DATA_PTR;
> >
> > @@ -796,6 +799,9 @@ static init_fnc_t init_sequence_r[] = { #ifdef
> > CONFIG_CMD_NET
> > initr_ethaddr,
> > #endif
> > +#if defined(CONFIG_DM_GPIO_HOG)
> > + gpio_hog_probe_all,
> > +#endif
Any reason to have hog at this place ?
Before board_late_init....
I expect more before board_init() for my future use-case,
to replace the current (dirty) implementation with pinctrl-0
#if defined(CONFIG_WDT)
initr_watchdog,
#endif
+#if defined(CONFIG_DM_GPIO_HOG)
+ gpio_hog_probe_all,
+#endif
#if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || defined(CONFIG_RISCV) || \
defined(CONFIG_SANDBOX)
board_init, /* Setup chipselects */
#endif
In my device tree I have :
stmfx: stmfx@42 {
compatible = "st,stmfx-0300";
reg = <0x42>;
interrupts = <8 IRQ_TYPE_EDGE_RISING>;
interrupt-parent = <&gpioi>;
vdd-supply = <&v3v3>;
stmfx_pinctrl: stmfx-pin-controller {
compatible = "st,stmfx-0300-pinctrl";
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
gpio-ranges = <&stmfx_pinctrl 0 0 24>;
pinctrl-names = "default";
pinctrl-0 = <&hog_pins>;
hog_pins: hog {
pins = "gpio14";
drive-push-pull;
bias-pull-down;
};
};
And in board, I force probe to configure HOG....before to initialize the
DISPLAY, so in
./board/st/stm32mp1/stm32mp1.c : board _init()
/* board dependent setup after realloc */
int board_init(void)
{
.....
/* probe all PINCTRL for hog */
for (uclass_first_device(UCLASS_PINCTRL, &dev);
dev;
uclass_next_device(&dev)) {
pr_debug("probe pincontrol = %s\n", dev->name);
}
Or you can let each board choose when call the function
as it is already done for other function as regulators_enable_boot_on()
So don't add it in init_sequence_r[] / board_r.c ?...
Anyway it is not blocking for me, it just a question.
> > #ifdef CONFIG_BOARD_LATE_INIT
> > board_late_init,
> > #endif
> > diff --git a/doc/device-tree-bindings/gpio/gpio.txt
> > b/doc/device-tree-bindings/gpio/gpio.txt
> > index f7a158d858..e774439369 100644
> > --- a/doc/device-tree-bindings/gpio/gpio.txt
> > +++ b/doc/device-tree-bindings/gpio/gpio.txt
> > @@ -210,3 +210,58 @@ Example 2:
> > Here, three GPIO ranges are defined wrt. two pin controllers.
> > pinctrl1 GPIO ranges are defined using pin numbers whereas the GPIO
> > ranges wrt. pinctrl2 are named "foo" and "bar".
> > +
> > +3) GPIO hog definitions
> > +-----------------------
> > +
> > +The GPIO chip may contain GPIO hog definitions. GPIO hogging is a
> > +mechanism providing automatic GPIO request and configuration as part
> > +of the gpio-controller's driver probe function.
> > +
> > +Each GPIO hog definition is represented as a child node of the GPIO
> controller.
> > +Required properties:
> > +- gpio-hog: A property specifying that this child node represents a GPIO
> > hog.
> > +- gpios: Store the GPIO information (id, flags) for the GPIO to
> > + affect.
> > +
> > + ! Not yet support more than one gpio !
> > +
> > +Only one of the following properties scanned in the order shown below.
> > +- input: A property specifying to set the GPIO direction as input.
> > +- output-low A property specifying to set the GPIO direction as output
> > with
> > + the value low.
> > +- output-high A property specifying to set the GPIO direction as output
> > with
> > + the value high.
> > +
> > +Optional properties:
> > +- line-name: The GPIO label name. If not present the node name is used.
> > +
> > +Example:
> > +
> > + tca6416@20 {
> > + compatible = "ti,tca6416";
> > + reg = <0x20>;
> > + #gpio-cells = <2>;
> > + gpio-controller;
> > +
> > + env_reset {
> > + gpio-hog;
> > + input;
> > + gpios = <6 GPIO_ACTIVE_LOW>;
> > + };
> > + boot_rescue {
> > + gpio-hog;
> > + input;
> > + gpios = <7 GPIO_ACTIVE_LOW>;
> > + };
> > + };
> > +
> > +For the above Example you can than access the gpio in your boardcode
> > +with:
> > +
> > + desc = gpio_hog_lookup_name("boot_rescue.gpio-hog");
> > + if (desc) {
> > + if (dm_gpio_get_value(desc))
> > + printf("\nBooting into Rescue System\n");
> > + else
> > + printf("\nBoot normal\n");
> > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index
> > e36a8abc42..fa1c99700f 100644
> > --- a/drivers/gpio/Kconfig
> > +++ b/drivers/gpio/Kconfig
> > @@ -14,6 +14,16 @@ config DM_GPIO
> > particular GPIOs that they provide. The uclass interface
> > is defined in include/asm-generic/gpio.h.
> >
> > +config DM_GPIO_HOG
> > + bool "Enable GPIO hog support"
> > + depends on DM_GPIO
> > + default n
> > + help
> > + Enable gpio hog support
> > + The GPIO chip may contain GPIO hog definitions. GPIO hogging
> > + is a mechanism providing automatic GPIO request and config-
> > + uration as part of the gpio-controller's driver probe function.
> > +
> > config ALTERA_PIO
> > bool "Altera PIO driver"
> > depends on DM_GPIO
> > diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
> > index da5e9ba6e5..308d0863ad 100644
> > --- a/drivers/gpio/gpio-uclass.c
> > +++ b/drivers/gpio/gpio-uclass.c
> > @@ -5,6 +5,9 @@
> >
> > #include <common.h>
> > #include <dm.h>
> > +#include <dm/device-internal.h>
> > +#include <dm/lists.h>
> > +#include <dm/uclass-internal.h>
> > #include <dt-bindings/gpio/gpio.h>
> > #include <errno.h>
> > #include <fdtdec.h>
> > @@ -141,6 +144,118 @@ static int gpio_find_and_xlate(struct gpio_desc
> *desc,
> > return gpio_xlate_offs_flags(desc->dev, desc, args); }
> >
> > +#if defined(CONFIG_DM_GPIO_HOG)
> > +
> > +struct gpio_hog_priv {
> > + struct gpio_desc gpiod;
> > +};
> > +
> > +struct gpio_hog_data {
> > + int gpiod_flags;
> > + int value;
> > + u32 val[2];
> > +};
> > +
> > +static int gpio_hog_ofdata_to_platdata(struct udevice *dev) {
> > + struct gpio_hog_data *plat = dev_get_platdata(dev);
> > + const char *nodename;
> > + int ret;
> > +
> > + plat->value = 0;
> > + if (dev_read_bool(dev, "input")) {
> > + plat->gpiod_flags = GPIOD_IS_IN;
> > + } else if (dev_read_bool(dev, "output-high")) {
> > + plat->value = 1;
> > + plat->gpiod_flags = GPIOD_IS_OUT;
> > + } else if (dev_read_bool(dev, "output-low")) {
> > + plat->gpiod_flags = GPIOD_IS_OUT;
> > + } else {
> > + printf("%s: missing gpio-hog state.\n", __func__);
> > + return -EINVAL;
> > + }
> > + ret = dev_read_u32_array(dev, "gpios", plat->val, 2);
> > + if (ret) {
> > + printf("%s: wrong gpios property, 2 values needed %d\n",
> > + __func__, ret);
> > + return ret;
> > + }
> > + nodename = dev_read_string(dev, "line-name");
> > + if (!nodename)
> > + nodename = dev_read_name(dev);
> > + device_set_name(dev, nodename);
> > +
> > + return 0;
> > +}
> > +
> > +static int gpio_hog_probe(struct udevice *dev) {
> > + struct gpio_hog_data *plat = dev_get_platdata(dev);
> > + struct gpio_hog_priv *priv = dev_get_priv(dev);
> > + int ret;
> > +
> > + ret = gpio_dev_request_index(dev->parent, dev->name, "gpio-hog",
> > + plat->val[0], plat->gpiod_flags,
> > + plat->val[1], &priv->gpiod);
> > + if (ret < 0) {
> > + debug("%s: node %s could not get gpio.\n", __func__,
> > + dev->name);
> > + return ret;
> > + }
> > + dm_gpio_set_dir(&priv->gpiod);
> > + if (plat->gpiod_flags == GPIOD_IS_OUT)
> > + dm_gpio_set_value(&priv->gpiod, plat->value);
> > +
> > + return 0;
> > +}
> > +
> > +int gpio_hog_probe_all(void)
> > +{
> > + struct udevice *dev;
> > + int ret;
> > +
> > + for (uclass_first_device(UCLASS_NOP, &dev);
> > + dev;
> > + uclass_find_next_device(&dev)) {
> > + if (dev->driver == DM_GET_DRIVER(gpio_hog)) {
> > + ret = device_probe(dev);
> > + if (ret)
> > + return ret;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +struct gpio_desc *gpio_hog_lookup_name(const char *name) {
> > + struct udevice *dev;
> > +
> > + gpio_hog_probe_all();
> > + if (!uclass_get_device_by_name(UCLASS_NOP, name, &dev)) {
> > + struct gpio_hog_priv *priv = dev_get_priv(dev);
> > +
> > + return &priv->gpiod;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +U_BOOT_DRIVER(gpio_hog) = {
> > + .name = "gpio_hog",
> > + .id = UCLASS_NOP,
> > + .ofdata_to_platdata = gpio_hog_ofdata_to_platdata,
> > + .probe = gpio_hog_probe,
> > + .priv_auto_alloc_size = sizeof(struct gpio_hog_priv),
> > + .platdata_auto_alloc_size = sizeof(struct gpio_hog_data), }; #else
> > +struct gpio_desc *gpio_hog_lookup_name(const char *name) {
> > + return NULL;
> > +}
> > +#endif
> > +
> > int dm_gpio_request(struct gpio_desc *desc, const char *label) {
> > struct udevice *dev = desc->dev;
> > @@ -640,22 +755,25 @@ int dm_gpio_get_values_as_int(const struct
> gpio_desc *desc_list, int count)
> > return vector;
> > }
> >
> > -static int gpio_request_tail(int ret, ofnode node,
> > +static int gpio_request_tail(int ret, const char *nodename,
> > struct ofnode_phandle_args *args,
> > const char *list_name, int index,
> > - struct gpio_desc *desc, int flags, bool add_index)
> > + struct gpio_desc *desc, int flags,
> > + bool add_index, struct udevice *dev)
> > {
> > - desc->dev = NULL;
> > + desc->dev = dev;
> > desc->offset = 0;
> > desc->flags = 0;
> > if (ret)
> > goto err;
> >
> > - ret = uclass_get_device_by_ofnode(UCLASS_GPIO, args->node,
> > - &desc->dev);
> > - if (ret) {
> > - debug("%s: uclass_get_device_by_ofnode failed\n", __func__);
> > - goto err;
> > + if (!desc->dev) {
> > + ret = uclass_get_device_by_ofnode(UCLASS_GPIO, args->node,
> > + &desc->dev);
> > + if (ret) {
> > + debug("%s: uclass_get_device_by_ofnode failed\n",
> __func__);
> > + goto err;
> > + }
> > }
> > ret = gpio_find_and_xlate(desc, args);
> > if (ret) {
> > @@ -663,8 +781,7 @@ static int gpio_request_tail(int ret, ofnode node,
> > goto err;
> > }
> > ret = dm_gpio_requestf(desc, add_index ? "%s.%s%d" : "%s.%s",
> > - ofnode_get_name(node),
> > - list_name, index);
> > + nodename, list_name, index);
> > if (ret) {
> > debug("%s: dm_gpio_requestf failed\n", __func__);
> > goto err;
> > @@ -678,7 +795,7 @@ static int gpio_request_tail(int ret, ofnode node,
> > return 0;
> > err:
> > debug("%s: Node '%s', property '%s', failed to request GPIO index %d:
> %d\n",
> > - __func__, ofnode_get_name(node), list_name, index, ret);
> > + __func__, nodename, list_name, index, ret);
> > return ret;
> > }
> >
> > @@ -692,8 +809,8 @@ static int _gpio_request_by_name_nodev(ofnode node,
> const char *list_name,
> > ret = ofnode_parse_phandle_with_args(node, list_name, "#gpio-cells", 0,
> > index, &args);
> >
> > - return gpio_request_tail(ret, node, &args, list_name, index, desc,
> > - flags, add_index);
> > + return gpio_request_tail(ret, ofnode_get_name(node), &args, list_name,
> > + index, desc, flags, add_index, NULL);
> > }
> >
> > int gpio_request_by_name_nodev(ofnode node, const char *list_name,
> > int index, @@ -707,13 +824,14 @@ int gpio_request_by_name(struct udevice
> *dev, const char *list_name, int index,
> > struct gpio_desc *desc, int flags) {
> > struct ofnode_phandle_args args;
> > + ofnode node;
> > int ret;
> >
> > ret = dev_read_phandle_with_args(dev, list_name, "#gpio-cells", 0,
> > index, &args);
> > -
> > - return gpio_request_tail(ret, dev_ofnode(dev), &args, list_name,
> > - index, desc, flags, index > 0);
> > + node = dev_ofnode(dev);
> > + return gpio_request_tail(ret, ofnode_get_name(node), &args, list_name,
> > + index, desc, flags, index > 0, NULL);
> > }
> >
> > int gpio_request_list_by_name_nodev(ofnode node, const char
> > *list_name, @@ -854,8 +972,28 @@ static int gpio_pre_remove(struct udevice
> *dev)
> > return gpio_renumber(dev);
> > }
> >
> > +int gpio_dev_request_index(struct udevice *dev, const char *nodename,
> > + char *list_name, int index, int flags,
> > + int dtflags, struct gpio_desc *desc) {
> > + struct ofnode_phandle_args args;
> > +
> > + args.node = ofnode_null();
> > + args.args_count = 2;
> > + args.args[0] = index;
> > + args.args[1] = dtflags;
> > +
> > + return gpio_request_tail(0, nodename, &args, list_name, index, desc,
> > + flags, 0, dev);
> > +}
> > +
> > static int gpio_post_bind(struct udevice *dev) {
> > +#if defined(CONFIG_DM_GPIO_HOG)
> > + struct udevice *child;
> > + ofnode node;
> > +#endif
> > +
> > #if defined(CONFIG_NEEDS_MANUAL_RELOC)
> > struct dm_gpio_ops *ops = (struct dm_gpio_ops *)device_get_ops(dev);
> > static int reloc_done;
> > @@ -885,6 +1023,17 @@ static int gpio_post_bind(struct udevice *dev)
> > reloc_done++;
> > }
> > #endif
> > +
> > +#if defined(CONFIG_DM_GPIO_HOG)
> > + dev_for_each_subnode(node, dev) {
> > + if (ofnode_read_bool(node, "gpio-hog")) {
> > + const char *name = ofnode_get_name(node);
> > +
> > + device_bind_driver_to_node(dev, "gpio_hog", name,
> > + node, &child);
> > + }
> > + }
> > +#endif
> > return 0;
> > }
> >
> > diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> > index d03602696f..37f71e5708 100644
> > --- a/include/asm-generic/gpio.h
> > +++ b/include/asm-generic/gpio.h
> > @@ -348,6 +348,22 @@ const char *gpio_get_bank_info(struct udevice *dev,
> int *offset_count);
> > */
> > int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc);
> >
> > +/**
> > + * gpio_hog_lookup_name() - Look up a named GPIO and return the gpio
> descr.
> > + *
> > + * @name: Name to look up
> > + * @return: Returns gpio_desc for gpio
> > + */
> > +struct gpio_desc *gpio_hog_lookup_name(const char *name);
> > +
> > +/**
> > + * gpio_hog_probe_all() - probe all gpio devices with
> > + * gpio-hog subnodes.
> > + *
> > + * @return: Returns return value from device_probe()
> > + */
> > +int gpio_hog_probe_all(void);
> > +
> > /**
> > * gpio_lookup_name - Look up a GPIO name and return its details
> > *
> > @@ -503,6 +519,22 @@ int gpio_request_list_by_name_nodev(ofnode node,
> const char *list_name,
> > struct gpio_desc *desc_list, int max_count,
> > int flags);
> >
> > +/**
> > + * gpio_dev_request_index() - request single GPIO from gpio device
> > + *
> > + * @dev: GPIO device
> > + * @nodename: Name of node
> > + * @list_name: Name of GPIO list (e.g. "board-id-gpios")
> > + * @index: Index number of the GPIO in that list use request (0=first)
> > + * @flags: GPIOD_* flags
> > + * @dtflags: GPIO flags read from DT
> > + * @desc: GPIO descriotor filled from this function
> > + * @return: return value from gpio_request_tail()
> > + */
> > +int gpio_dev_request_index(struct udevice *dev, const char *nodename,
> > + char *list_name, int index, int flags,
> > + int dtflags, struct gpio_desc *desc);
> > +
> > /**
> > * dm_gpio_free() - Free a single GPIO
> > *
> >
>
> Tested-by: Michal Simek <michal.si...@xilinx.com> (zcu102)
>
> Thanks,
> Michal
For the patch Review and tested on stm32mp1 board.
Tested-by: Patrick Delaunay <patrick.delau...@st.com>
Regards
PAtrick
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
