Hi Heiko,
From: Heiko Schocher hs@denx.de Sent: samedi 22 juin 2019 07:32
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...
Ok. It is safe....
and anyway any board can choose the call the function earlier if the hog was required (for nay initcall before board_late_init : initr_nand, initr_mmc ....)
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():
Fine.
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=0d15e5062b66123cd364bd9803e076db7 e7dd97c;hb=77f6e2dd0551d8a825bab391a1bd6b838874bcd4#l319
May you can test if this works for you?
I have the same idea, it is why I say is was no't blocking for me.
Already tested and it is working (gpio hog is called only one time even if gpio_hog_probe_all several time).
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
Regards Patrick