Hi Andre,

On 11/23/25 12:58 PM, Andre Przywara wrote:
On Thu, 20 Nov 2025 13:02:06 +0100
Quentin Schulz <[email protected]> wrote:
From: Quentin Schulz <[email protected]>
[...]

Signed-off-by: Quentin Schulz <[email protected]>
---
Note that I do not own this device so I could only build test it.

I do have a Pinephone, and I tested this patch: it indeed works. I also
changed the GPIO number, which changed the LED (colour), so it's
positively working.

Tested-by: Andre Przywara <[email protected]>


Nice! Thanks for the quick test and feedback :)

Just one nit in the code below...

This is a follow-up to:

Are there actually dependencies on all those series? I naively applied
this patch on top of master, and it compiled and worked just fine.


I think it should be fine to apply as is without the other series I mentioned. I just initially did the removal of the legacy API in one branch and then made multiple series out of it, and it was easier not having to try to figure out what really depends on each other. But here, nothing from the legacy API is removed, so it should be safe to apply like you did.

[...]

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 2929bc17f08..799d60cf039 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
[...]
+static void status_led_init(void)
+{
+#if CONFIG_IS_ENABLED(SUNXI_LED_STATUS)
+       unsigned int state = CONFIG_SPL_SUNXI_LED_STATUS_STATE;
+       unsigned int gpio = CONFIG_SPL_SUNXI_LED_STATUS_BIT;
+
+       if (gpio_request(gpio, "gpio_led") != 0) {
+               printf("%s: failed requesting GPIO%u!\n", __func__, gpio);

Can you please just remove this error message? I see that this whole
code is written somewhat SPL agnostic (CONFIG_IS_ENABLED), but
realistically this is SPL-only code (see the two symbols above), and
there gpio_request() always returns 0, so it's just extra size in the
SPL that will never be used. Saves another 75 bytes.


Should we even call gpio_request() then?

I can use IS_ENABLED(CONFIG_SPL_SUNXI_LED_STATUS) && defined(CONFIG_SPL_BUILD) as well if you want, to make it clearer that this can only happen in SPL?

Cheers,
Quentin

Reply via email to