Hello Patrick,
Am 21.06.2019 um 18:09 schrieb Patrick DELAUNAY:
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 hs@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 ?
Not really, but we need to check at some point if all gpio hogs are processed, and I thought at a late time it does not break anything...
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 ?...
Hmm... I do not like this, because if you enable gpio-hog support, you don;t want to think to call gpio_hog_probe_all().
But it should be possible to call gpio_hog_probe_all() earlier from board code, as gpio_hog_probe_all() calls device_probe():
212 int gpio_hog_probe_all(void) 213 { 214 struct udevice *dev; 215 int ret; 216 217 for (uclass_first_device(UCLASS_NOP, &dev); 218 dev; 219 uclass_find_next_device(&dev)) { 220 if (dev->driver == DM_GET_DRIVER(gpio_hog)) { 221 ret = device_probe(dev);
device_probe() checks if it is already probed, and simply returns if so, see:
http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/core/device.c;h=0d15e5062b...
May you can test if this works for you?
Anyway it is not blocking for me, it just a question.
Fine.
#ifdef CONFIG_BOARD_LATE_INIT board_late_init,
[...]
For the patch Review and tested on stm32mp1 board.
Tested-by: Patrick Delaunay patrick.delaunay@st.com
Thanks for testing!
bye, Heiko