Return-path: Received: from smtps.newmedia-net.de ([185.84.6.167]:34963 "EHLO webmail.newmedia-net.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751220AbeCGRyx (ORCPT ); Wed, 7 Mar 2018 12:54:53 -0500 Subject: Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets To: =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= Cc: Pavel Machek , "open list:LED SUBSYSTEM" , "linux-wireless@vger.kernel.org" , Kalle Valo , ath10k@lists.infradead.org, Sebastian Gottschall References: <20180226084406.2093-1-s.gottschall@dd-wrt.com> <82d8ac0c-b391-6099-4c7f-991cc35445aa@dd-wrt.com> <20180302090312.GA30267@amd> From: Sebastian Gottschall Message-ID: <890a4dcc-f549-6487-2ce5-3b62874cd266@dd-wrt.com> (sfid-20180307_185459_735501_FD71FF55) Date: Wed, 7 Mar 2018 18:54:41 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Sender: linux-wireless-owner@vger.kernel.org List-ID: Am 07.03.2018 um 17:22 schrieb Rafał Miłecki: > On 2 March 2018 at 10:22, Sebastian Gottschall wrote: >>>> 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 the >>>> 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 useless >>>> 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 limitation > 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[] = { > { > .name = "ath10k:color:function", > .active_low = 1, > .default_state = LEDS_GPIO_DEFSTATE_KEEP, > } > }; > > static struct gpio_led_platform_data bcm47xx_leds_pdata_extra = { > leds = ath10k_leds; > num_leds = ARRAY_SIZE(ath10k_leds); > }; > > ath10k_leds.gpio = ar->hw_params.led_pin; > gpio_led_register_device(0, &ath10k_leds); the problem are other architectures which have already registered gpio_led at system start like ar71xx you cannot register a second one. so a independend led driver is a requirement for direct control Sebastian > -- Mit freundlichen Grüssen / Regards Sebastian Gottschall / CTO NewMedia-NET GmbH - DD-WRT Firmensitz: Stubenwaldallee 21a, 64625 Bensheim Registergericht: Amtsgericht Darmstadt, HRB 25473 Geschäftsführer: Peter Steinhäuser, Christian Scheele http://www.dd-wrt.com email: s.gottschall@dd-wrt.com Tel.: +496251-582650 / Fax: +496251-5826565