Return-path: Received: from atrey.karlin.mff.cuni.cz ([195.113.26.193]:58297 "EHLO atrey.karlin.mff.cuni.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935099AbeCHOFS (ORCPT ); Thu, 8 Mar 2018 09:05:18 -0500 Date: Thu, 8 Mar 2018 15:05:16 +0100 From: Pavel Machek To: Sebastian Gottschall Cc: =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= , "open list:LED SUBSYSTEM" , "linux-wireless@vger.kernel.org" , Kalle Valo , ath10k@lists.infradead.org, Sebastian Gottschall Subject: Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets Message-ID: <20180308140515.GA4889@amd> (sfid-20180308_150521_395876_773B33D5) 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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="envbJBWh7q8WU6mo" In-Reply-To: Sender: linux-wireless-owner@vger.kernel.org List-ID: --envbJBWh7q8WU6mo Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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=C5=82 Mi=C5=82ecki: > >>>On 2 March 2018 at 10:22, Sebastian Gottschall wrote: > >>>>>>leds-gpio is crap and limited. you can just register one platform d= ata at > >>>>>>kernel runtime since its identified by its object name "led-gpio" b= ut the > >>>>>>kernel forbidds to register 2 platform datas with the same name > >>>>>>consider the ar71xx devices with qca988x wifi chipsets. they all ha= ve > >>>>>>already a led platform data registered > >>>>>>at boottime. a second can't be registered anymore so gpio_led is us= eless > >>>>>>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 i= t. > >>>>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 limi= tation > >>>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); > >>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 ca= ll > 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.=20 Pavel --=20 (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blo= g.html --envbJBWh7q8WU6mo Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iEYEARECAAYFAlqhQxsACgkQMOfwapXb+vLTpgCgks8J+FoLFdyIZz4Vq9wn2xuK 6ZsAmgPcfT2bOWZcTAIXWKEx1/ySKQdW =zvis -----END PGP SIGNATURE----- --envbJBWh7q8WU6mo--