From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Sat, 22 Jun 2019 07:32:08 +0200 Subject: [U-Boot] [PATCH v4] gpio: add gpio-hog support In-Reply-To: <3c3cdcc2d6ca4146a314bf73d06e9368@SFHDAG6NODE3.st.com> References: <20190612041146.53854-1-hs@denx.de> <10393bfd-8a9a-9b58-839e-65327965aea1@xilinx.com> <3c3cdcc2d6ca4146a314bf73d06e9368@SFHDAG6NODE3.st.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >>> >>> --- >>> 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 >>> #include >>> #include >>> +#if defined(CONFIG_DM_GPIO_HOG) >>> +#include >>> +#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 at 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=0d15e5062b66123cd364bd9803e076db7e7dd97c;hb=77f6e2dd0551d8a825bab391a1bd6b838874bcd4#l319 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 Thanks for testing! bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de