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

[Steve deRosier]

On Fri, Feb 16, 2018 at 2:30 AM,  <s.gottschall at dd-wrt.com> wrote:
> From: Sebastian Gottschall <s.gottschall at newmedia-net.de>
>
> Adds LED and GPIO Control support for 988x, 9887, 9888, 99x0, 9984 and ipq4019 based chipsets with on chipset connected led's
> using WMI Firmware API.
> The LED device will get available named as "ath10k-phyX" at sysfs and can be controlled with various triggers.
> adds also debugfs interface for gpio control.
>

Hi Sebastian,

First off, let me say I love this and the effort to make the gpios
hanging off the wifi chip as proper system gpios. Years ago I had to
do a hack to make some wifi-chip provided pins available to a customer
who needed more GPIOs on our SoM. It would have been nice if provided
hardware functionality had already been properly supported by the
driver.

In looking through, it looks like you're hooking into the standard
gpio-chip driver interface. Which is great. So, why are you providing
a separate out-of-band debugfs access interface for gpio control?
Seems to me to be a duplicate of the interface in sysfs that provides
gpio access (`/sys/class/gpio`). My preference is we don't duplicate
functionality and that we use the standard provided features. That way
we don't possibly confuse users, and we also reduce our maintenance
effort.

Additionally, what happens when the relevant GPIO configuration
options aren't enabled? Do we have a compilation issue? I'm more
familiar with being a _consumer_ of gpiolib in my drivers and I know
if CONFIG_GPIOLIB isn't checked I end up with problems. I'm unsure
what happens when you're presenting a gpio-chip interface without the
relevant CONFIG_ being enabled. Note that I didn't test this, I'm just
asking the question to be sure we don't forget anything.

- Steve