[U-Boot] [PATCH v4 0/5] dm: led: remove auto probe in binding function

* [U-Boot] [PATCH v4 0/5] dm: led: remove auto probe in binding function
@ 2018-07-27 14:37 Patrick Delaunay
  2018-07-27 14:37 ` [U-Boot] [PATCH v4 1/5] stm32mp1: add gpio led support Patrick Delaunay
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Patrick Delaunay @ 2018-07-27 14:37 UTC (permalink / raw)
  To: u-boot

Hi,

The commit bc882f5d5c7b4d6ed5e927bf838863af43c786e7
introduce auto probe of LED in binding function
but that cause issue on my board.

This first patch of this patchset activateis the LED on my board
to explain the issue, the second patch revert this commit and
the third one propose an other solution.

For me, this commit is a error because in README of doc/driver-model/
it is indicated:

  The device's bind() method is permitted to perform simple actions, but
  should not scan the device tree node, not initialise hardware, nor set up
  structures or allocate memory. All of these tasks should be left for
  the probe() method.

And in the patch the LED driver is probed during the binding scan.

When I activated the LED in my ARM target with the patch
"stm32mp1: add gpio led support", I have the sequence:

dm_init_and_scan() :

1/ dm_scan_fdt_node()
	=> loop on all the node

2/ scan for LED node
	=> probe of LED driver is forced by "default-state" detection
		LED1 - "red"
        => probe of father of "red" node
		A - led
		B - soc
		C - root
	=> probe of node needed by GPIO
		1 - pin-controller
		2 - gpio at 50002000
		3 - rcc-clk at 50000000
		4 - rcc at 50000000

	=> probe forced by default state for other led
		LED2 - green
		LED3 - orange

	=> probe of node needed by GPIO (other bank)
		5 - gpio at 50009000

3/ dm_extended_scan_fdt scan continue...
   scan node "fixed-clock" under "/clocks"
	clk-hse
	clk-hsi
	clk-lse
	clk-lsi
	clk-csi

4/ probe of all the used devices.... after dm_extended_scan_fdt()

So many driver are probed before the end of the scan binding loop !

And that cause issue in my board (boot failed) because the rcc-clk clock
driver found the input frequency with these fixed-clock, which are binded
only after the stm32mp1 clock driver probe.

For me probe in forbidden in binding function and
thus uclass_get_device_tail() is not allowed in the commit
bc882f5d5c7b4d6ed5e927bf838863af43c786e7 which need to be reverted.

In the third patch I proposed an other solution based
on the existing solution in u-class regulator used to enable
regulator with "boot on" property (see regulators_enable_boot_on function).

I think that adding a function is the  better solution in the driver model
to force probe for some node according binding information
(after dm_init_and_scan call).

This new function should be called in board_init function for each board
but it could be also added in init_sequence_r[] in future.

Changes in v4:
  - simplify the uclass impact after Patrick Bruenn review
  - remove #ifdef CONFIG_LED for sandbox

Changes in v3:
- add motivation in revert commit
- minor update after Simon review
- include led.h to avoid compilation warning on stm32mp1 board

Changes in v2:
  - add sandbox impact and test update

Patrick Delaunay (5):
  stm32mp1: add gpio led support
  Revert "dm: led: auto probe() LEDs with "default-state""
  dm: led: move default state support in led uclass
  stm32mp1: use new function led default state
  sandbox: led: use new function to configure default state

 arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi | 24 ++++++++++++++++++++++++
 board/sandbox/sandbox.c                  |  9 +++++++++
 board/st/stm32mp1/stm32mp1.c             |  4 ++++
 common/board_r.c                         |  3 ++-
 configs/stm32mp15_basic_defconfig        |  2 ++
 drivers/led/led-uclass.c                 | 30 ++++++++++++++++++++++++++++++
 drivers/led/led_gpio.c                   | 17 -----------------
 include/led.h                            |  9 +++++++++
 test/dm/led.c                            |  3 +++
 9 files changed, 83 insertions(+), 18 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread