Re: [PATCH v2 0/8] led: update LED boot/activity to new property implementation

[Christian Marangi <ansuelsmth@gmail.com>]

On Wed, Nov 13, 2024 at 12:00:59PM -0600, Tom Rini wrote:
> On Sun, Nov 10, 2024 at 12:50:19PM +0100, Christian Marangi wrote:
> 
> > This series is split in 2 part.
> > 
> > While adapting the LED boot and activity code to the new property
> > accepted by Rob in dt-schema repository, a big BUG was discovered.
> > 
> > The reason wasn't clear at start and took me some days to figure it
> > out.
> > 
> > This was triggered by adding a new phandle in the test.dts to
> > introduce test for the new OPs.
> > 
> > This single addition caused the sandbox CI test to fail in the
> > dm_test_ofnode_phandle_ot test.
> > 
> > This doesn't make sense as reverting the change made the CI test
> > to correctly finish. Also moving the uboot node down
> > after the first phandle (in test.dts the gpio one) also made
> > the CI test to correctly finish.
> > 
> > A little bit of searching and debugging made me realize the
> > parse phandle OPs didn't support other.dts at all and they
> > were still referencing phandle index from test.dts.
> > (more info in the related commit)
> > 
> > In short the test was broken all along and was working by
> > pure luck. The first 4 patch address and fix the problem for good.
> > 
> > The other 4 patch expand and address the property change for
> > LED boot/activity.
> > 
> > Posting in a single series as changes are trivial and just
> > to speedup review process. (and also because the second
> > part depends on the first)
> > 
> > All CI tested with azure pipeline.
> > 
> > Changes v2:
> > - Fix handling of flat tree for phandle
> > - Fix test and other.dts changes
> > 
> > Christian Marangi (8):
> >   dm: core: implement oftree variant of parse_phandle OPs
> >   test: dm: fix broken dm_test_ofnode_phandle_ot and get_by_phandle_ot
> >   dm: core: implement ofnode/tree_parse_phandle() helper
> >   test: dm: Expand dm_test_ofnode_phandle(_ot) with new
> >     ofnode/tree_parse_phandle
> >   dm: core: implement phandle ofnode_options helper
> >   test: dm: Add test for ofnode options phandle helper
> >   led: update LED boot/activity to new property implementation
> >   test: dm: Update test for LED activity and boot
> > 
> >  arch/sandbox/dts/other.dts |  31 ++++++++-
> >  arch/sandbox/dts/test.dts  |  16 +++--
> >  drivers/core/of_access.c   |  61 ++++++++++++-----
> >  drivers/core/ofnode.c      | 124 ++++++++++++++++++++++++++++++++-
> >  drivers/led/led-uclass.c   |  30 +++++---
> >  include/dm/of_access.h     |  86 +++++++++++++++++++++++
> >  include/dm/ofnode.h        | 107 +++++++++++++++++++++++++++++
> >  test/dm/led.c              |  18 +++--
> >  test/dm/ofnode.c           | 136 ++++++++++++++++++++++++++++++++-----
> >  9 files changed, 551 insertions(+), 58 deletions(-)
> 
> My main issue with the series is a lack of documentation updates, as the
> biggest challenge thus far has been that for example Peter couldn't
> figure out how to make use of this on PinePhone. We generate
> documentation today based on include/led.h for this API, yes? Thanks.
> 

Hi Tom,

actually quite the oppisite, led.h describe how these works and was
instructed to first have the options property merged in dt-schema. [1]

Here we have all the option documented under a yaml.

I notice there is a series from Simon that is also pushing a .yaml
here locally in U-Boot and I was waiting for that to be merged to also
include the additional entry there.

Also led.h description and API have info on where to look about the
handling of /options/u-boot/

I feel the main problem with documentation is currently the fact that we
are migrating to a more robust schema and people are used to using Doc
directory? Anyway happy to get any hint on how to improve this.

[1] https://github.com/devicetree-org/dt-schema

-- 
	Ansuel