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

From: Sebastian Gottschall <s.gottschall@dd-wrt.com>
To: Pavel Machek <pavel@ucw.cz>
Cc: Rafa?? Mi??ecki <zajec5@gmail.com>,
	"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: Thu, 8 Mar 2018 16:31:52 +0100	[thread overview]
Message-ID: <d7f09cd5-c434-a2bb-c721-68e90c0c72b6@dd-wrt.com> (raw)
In-Reply-To: <20180308150431.GA12475@localhost>

Am 08.03.2018 um 16:04 schrieb Pavel Machek:
> Hi!
>
>> show me a proof that its copy & paste. because its not
> I don't have to prove you anything. Sorry.
then i will deny your argument because its false.
>
> But you said:
>
>>>> see ath9k. its exact the same implementation.
> We don't want to have exact same code multiple times in the tree.
it isnt the exact same code, its just the way its done. i mean 
registering a led driver function happens multiple times in the kernel
you cannot say that calling led_classdev_register with the required 
parameters and function implementation is a case of code duplication.
then i would just say using of "printk" is a case of code duplication.
registering a led driver is nothing unusual, the implementation of the 
led driver is different each time. the implementation for led_brightness 
is very different
there are many led drivers in the kernel. all are going the same way.

i checked the kernel drivers just right now. almost all major wireless 
drivers are comming with a led driver without using gpiolib
your way is a case of codebloating since a registering gpio-leds 
requires a gpio driver for each wireless driver, even if its sometimes 
just a single register write for a led  and no real gpio.
a gpio driver is more complex and bigger than just a led driver. i just 
wrote a optional gpio driver to get access to all gpios available on the 
card. some vendors are using these in a unusual way
i have seen that vendors used them for reset button input etc. this is 
why i made it. so dont take this as a argument for going a impossible 
way (again. the kernel does not support multiple platform datas with the 
same name
THE KERNEL not leds-gpio. so once a leds-gpio platform data is 
registered, no other driver can register a new one.
in addition the kernel must have gpiolib support which increases the 
kernel size. the best way is always the most simple way and the smallest 
performant way.

and again. i did not duplicate the code of ath9k, i just used it as 
documentation to write a own led driver in a simple way

now a list of wireless drivers with a led driver
intersil
carl9170
ath5k
rt2x00
b43legacy
b43
iwlegacy
rtl8187
brcmsmac
iwlwifi

Sebastian

>   
> 									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