Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets

From: "Rafał Miłecki" <zajec5@gmail.com>
To: Sebastian Gottschall <s.gottschall@dd-wrt.com>
Cc: Pavel Machek <pavel@ucw.cz>,
	"open list:LED SUBSYSTEM" <linux-leds@vger.kernel.org>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	Kalle Valo <kvalo@codeaurora.org>,
	ath10k@lists.infradead.org,
	Sebastian Gottschall <s.gottschall@newmedia-net.de>
Subject: Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets
Date: Wed, 7 Mar 2018 17:22:53 +0100	[thread overview]
Message-ID: <CACna6rys85VEkzRGDVUAkBy7KgHS2y0DET7YvedboX-YEQHrkg@mail.gmail.com> (raw)
In-Reply-To: <e77d25d4-a824-380a-c51a-e81175ba6205@dd-wrt.com>

On 2 March 2018 at 10:22, Sebastian Gottschall <s.gottschall@dd-wrt.com> wr=
ote:
>>> leds-gpio is crap and limited. you can just register one platform data =
at
>>> kernel runtime since its identified by its object name "led-gpio" but t=
he
>>> kernel forbidds to register 2 platform datas with the same name
>>> consider the ar71xx devices with qca988x wifi chipsets. they all have
>>> already a led platform data registered
>>> at boottime. a second can't be registered anymore so gpio_led is useles=
s
>>> at
>>> all for most developers on such platforms. its mainly used for early
>>> kernel
>>> platform data initialisation for system leds.
>>
>> If leds-gpio has limitations, please fix those, rather then
>> introducing duplicated code.
>
> there is no duplicated code introduced and there is no solution for it.
> consider that all wifi drivers with softled support
> are going that way with registering a own led driver. see ath9k for
> instance. gpio-led cannot be used for it and there is no way to
> support multiple platform datas with the same name. its a kernel limitati=
on

I just reviewed some mips arch patch adding support for more LEDs for
selected devices:
[PATCH] MIPS: BCM47XX: Add Luxul XAP1500/XWR1750 WiFi LEDs
https://patchwork.linux-mips.org/patch/18681/

It seems to be simply adding another call to the
gpio_led_register_device(). It seems to me you can call that function
multiple times and register multiple structs with LEDs.

Isn't all you need something like this?

static const struct gpio_led ath10k_leds[] =3D {
        {
                .name =3D "ath10k:color:function",
                .active_low =3D 1,
                .default_state =3D LEDS_GPIO_DEFSTATE_KEEP,
        }
};

static struct gpio_led_platform_data bcm47xx_leds_pdata_extra =3D {
        leds =3D ath10k_leds;
        num_leds =3D ARRAY_SIZE(ath10k_leds);
};

ath10k_leds.gpio =3D ar->hw_params.led_pin;
gpio_led_register_device(0, &ath10k_leds);

--=20
Rafa=C5=82