Return-path: Received: from smtps.newmedia-net.de ([185.84.6.167]:38661 "EHLO webmail.newmedia-net.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754458AbeCHOed (ORCPT ); Thu, 8 Mar 2018 09:34:33 -0500 Subject: Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets To: Pavel Machek Cc: =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= , "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> <890a4dcc-f549-6487-2ce5-3b62874cd266@dd-wrt.com> <20180308090216.GC17761@amd> <20180308140515.GA4889@amd> From: Sebastian Gottschall Message-ID: (sfid-20180308_153437_068437_C11074CA) Date: Thu, 8 Mar 2018 15:34:23 +0100 MIME-Version: 1.0 In-Reply-To: <20180308140515.GA4889@amd> Content-Type: text/plain; charset=utf-8; format=flowed Sender: linux-wireless-owner@vger.kernel.org List-ID: Am 08.03.2018 um 15:05 schrieb Pavel Machek: > On Thu 2018-03-08 13:33:29, Sebastian Gottschall wrote: >> Am 08.03.2018 um 10:02 schrieb Pavel Machek: >>> On Wed 2018-03-07 18:54:41, Sebastian Gottschall wrote: >>>> 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 >>> If the limitation indeed exists, please fix the limitation rather than >>> working around it in each and every driver. >> see ath9k. its exact the same implementation. > Ok, so one more driver to fix. > >> in addition my variant does also work without gpiolib support. so it can be >> used even if the kernel is configured >> without gpio support. >> and not to forget, using a own led driver is more ligthweight from the call >> path for each led on / off event which is important for >> low performance embedded devices > We are not going to copy&paste code because such code works without > libraries, and we are not going to copy&paste code because that uses > less cache during calls. Sorry. > > NAK. Please fix your patch. show me a proof that its copy & paste. because its not > > Pavel -- 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