mbox series

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

Message ID 20241109180038.10344-1-ansuelsmth@gmail.com
Headers show
Series led: update LED boot/activity to new property implementation | expand

Message

Christian Marangi Nov. 9, 2024, 6 p.m. UTC
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)

Christian Marangi (8):
  dm: core: implement oftree variant of parse_phandle OPs
  test: dm: fix broken dm_test_ofnode_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 |  33 ++++++++--
 arch/sandbox/dts/test.dts  |  16 +++--
 drivers/core/of_access.c   |  61 +++++++++++++-----
 drivers/core/ofnode.c      | 118 ++++++++++++++++++++++++++++++++++
 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           | 125 ++++++++++++++++++++++++++++++++-----
 9 files changed, 539 insertions(+), 55 deletions(-)

Comments

Christian Marangi Nov. 9, 2024, 8:29 p.m. UTC | #1
On Sat, Nov 09, 2024 at 07:00:25PM +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)
>

For everyone checking this, tomorrow I will push v2 of this series since
CI catched additional BUGs for flat tree scenario. (I didn't test this
with sandbox)

I also handle this and everything should be ok.
Simon Glass Nov. 20, 2024, 1:46 p.m. UTC | #2
Hi Christian,

On Sat, 9 Nov 2024 at 11:01, Christian Marangi <ansuelsmth@gmail.com> 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)

Thank you for digging into this and figuring it out.

>
> Christian Marangi (8):
>   dm: core: implement oftree variant of parse_phandle OPs
>   test: dm: fix broken dm_test_ofnode_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 |  33 ++++++++--
>  arch/sandbox/dts/test.dts  |  16 +++--
>  drivers/core/of_access.c   |  61 +++++++++++++-----
>  drivers/core/ofnode.c      | 118 ++++++++++++++++++++++++++++++++++
>  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           | 125 ++++++++++++++++++++++++++++++++-----
>  9 files changed, 539 insertions(+), 55 deletions(-)
>
> --
> 2.45.2
>

Regards,
Simon