date:20180406

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

2018-04-06 Thread Sebastian Gottschall


Hello Kalle

you removed the call to leds_start from certain locations but you seem 
to have ignored the comment i wrote above the function call.
there is a reason why i reinitialize the gpio output state in these 
locations. the firmware for 9984 and 99xx resets the gpio registers
at certain points. this will lead to a non working led code. so you must 
set the gpio output to high again and this is the reason why the 
function is called "reset_led_pin" and not led_start

because it doesnt start the led in any way. it just resets the output state
there is only one work around you may do.
you set the gpio out register to high on every led trigger, but this is 
what i wanted to avoid to save cpu time since a wmi call is more 
expensive than just a register write.


so if you want to follow this up. remove ath10k_leds_start
and insert

ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin, 0,
   WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE);

in ath10k_leds_set_brightness_blocking

but in any way the function name ath10k_leds_start is wrong. it doesnt 
describe the behaviour of the function



Am 06.04.2018 um 17:17 schrieb Kalle Valo:

From: Sebastian Gottschall 

Adds LED and GPIO Control support for 988x, 9887, 9888, 99x0, 9984 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.

Signed-off-by: Sebastian Gottschall 
Reviewed-by: Steve deRosier 
[kvalo: major reorg and cleanup]
Signed-off-by: Kalle Valo 
---

v13:

* only compile tested!

* fix all checkpatch warnings

* fix commit log

* sizeof(struct ath10k_gpiocontrol) -> sizeof(*gpio)

* unsigned -> unsigned int

* remove GPIOLIB code, that should be added in a separate patch

* rename gpio.c to leds.c

* add leds.h

* rename some functions:

   ath10k_attach_led() -> ath10k_leds_register()
   ath10k_unregister_led() -> ath10k_leds_unregister()
   ath10k_reset_led_pin() -> ath10k_leds_start()

* call ath10k_leds_unregister() before ath10k_thermal_unregister() to preserve 
ordering

* call ath10k_leds_start() only from ath10k_core_start() and not from mac.c

* rename struct ath10k_gpiocontrol as anonymous function under struct
   ath10k::leds, no need for memory allocation

* merge ath10k_add_led() to ath10k_attach_led(), which is it's only caller

* remove #if IS_ENABLED() checks from most of places, memory savings from those 
were not worth it

* Kconfig help text improvement and move it lower in the menu, also don't 
enable it by default

* switch to set_brightness_blocking() so that the callback can sleep,
   then no need to use ath10k_wmi_cmd_send_nowait() and can take mutex
   to access ar->state

* don't touch ath10k_wmi_pdev_get_temperature()

* as QCA6174/QCA9377 are not (yet) supported don't add the command to WMI-TLV 
interface

* remove debugfs interface, that should be added in another patch

* cleanup includes


  drivers/net/wireless/ath/ath10k/Kconfig   |  10 +++
  drivers/net/wireless/ath/ath10k/Makefile  |   1 +
  drivers/net/wireless/ath/ath10k/core.c|  22 +++
  drivers/net/wireless/ath/ath10k/core.h|   9 ++-
  drivers/net/wireless/ath/ath10k/hw.h  |   1 +
  drivers/net/wireless/ath/ath10k/leds.c| 103 ++
  drivers/net/wireless/ath/ath10k/leds.h|  45 +
  drivers/net/wireless/ath/ath10k/mac.c |   1 +
  drivers/net/wireless/ath/ath10k/wmi-ops.h |  32 ++
  drivers/net/wireless/ath/ath10k/wmi-tlv.c |   2 +
  drivers/net/wireless/ath/ath10k/wmi.c |  54 
  drivers/net/wireless/ath/ath10k/wmi.h |  35 ++
  12 files changed, 314 insertions(+), 1 deletion(-)
  create mode 100644 drivers/net/wireless/ath/ath10k/leds.c
  create mode 100644 drivers/net/wireless/ath/ath10k/leds.h

diff --git a/drivers/net/wireless/ath/ath10k/Kconfig 
b/drivers/net/wireless/ath/ath10k/Kconfig
index deb5ae21a559..c4108c1bbe92 100644
--- a/drivers/net/wireless/ath/ath10k/Kconfig
+++ b/drivers/net/wireless/ath/ath10k/Kconfig
@@ -52,6 +52,16 @@ config ATH10K_DEBUGFS
  
  	  If unsure, say Y to make it easier to debug problems.
  
+config ATH10K_LEDS

+   bool "Atheros ath10k LED support"
+   depends on ATH10K
+   select MAC80211_LEDS
+   select LEDS_CLASS
+   select NEW_LEDS
+   default y
+   help
+ This option is necessary, if you want LED support for chipset 
connected led pins. If unsure, say N.
+
  config ATH10K_SPECTRAL
bool "Atheros ath10k spectral scan support"
depends on ATH10K_DEBUGFS
diff --git a/drivers/net/wireless/ath/ath10k/Makefile 
b/drivers/net/wireless/ath/ath10k/Makefile
index 6739ac26fd29..16c822421c39 100644
--- a/drivers/net/wireless/ath/ath10k/Makefile
+++

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

2018-04-06 Thread Kalle Valo

Kalle Valo  writes:

> From: Sebastian Gottschall 
>
> Adds LED and GPIO Control support for 988x, 9887, 9888, 99x0, 9984 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.
>
> Signed-off-by: Sebastian Gottschall 

>From and Signed-off-by fields do not match. Sebastian, which domain
should it be?

> Reviewed-by: Steve deRosier 
> [kvalo: major reorg and cleanup]
> Signed-off-by: Kalle Valo 
> ---
>
> v13:
>
> * only compile tested!

So testing is very welcome.

> --- a/drivers/net/wireless/ath/ath10k/Kconfig
> +++ b/drivers/net/wireless/ath/ath10k/Kconfig
> @@ -52,6 +52,16 @@ config ATH10K_DEBUGFS
>  
> If unsure, say Y to make it easier to debug problems.
>  
> +config ATH10K_LEDS
> + bool "Atheros ath10k LED support"
> + depends on ATH10K
> + select MAC80211_LEDS
> + select LEDS_CLASS
> + select NEW_LEDS
> + default y
> + help
> +   This option is necessary, if you want LED support for chipset 
> connected led pins. If unsure, say N.
> +

I started to wonder is this Kconfig option even needed, feels pretty
pointless. Why not just make config ATH10K depend on LED stuff, just
like ath9k already does?

> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "htt.h"
>  #include "htc.h"
> @@ -883,7 +884,6 @@ struct ath10k {
>   u32 low_5ghz_chan;
>   u32 high_5ghz_chan;
>   bool ani_enabled;
> -
>   bool p2p;
>  
>   struct {

Unrelated change, I'll fix that in v14.

-- 
Kalle Valo

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

2018-04-06 Thread Kalle Valo

From: Sebastian Gottschall 

Adds LED and GPIO Control support for 988x, 9887, 9888, 99x0, 9984 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.

Signed-off-by: Sebastian Gottschall 
Reviewed-by: Steve deRosier 
[kvalo: major reorg and cleanup]
Signed-off-by: Kalle Valo 
---

v13:

* only compile tested!

* fix all checkpatch warnings

* fix commit log

* sizeof(struct ath10k_gpiocontrol) -> sizeof(*gpio)

* unsigned -> unsigned int

* remove GPIOLIB code, that should be added in a separate patch

* rename gpio.c to leds.c

* add leds.h

* rename some functions:

  ath10k_attach_led() -> ath10k_leds_register()
  ath10k_unregister_led() -> ath10k_leds_unregister()
  ath10k_reset_led_pin() -> ath10k_leds_start()

* call ath10k_leds_unregister() before ath10k_thermal_unregister() to preserve 
ordering

* call ath10k_leds_start() only from ath10k_core_start() and not from mac.c

* rename struct ath10k_gpiocontrol as anonymous function under struct
  ath10k::leds, no need for memory allocation

* merge ath10k_add_led() to ath10k_attach_led(), which is it's only caller

* remove #if IS_ENABLED() checks from most of places, memory savings from those 
were not worth it

* Kconfig help text improvement and move it lower in the menu, also don't 
enable it by default

* switch to set_brightness_blocking() so that the callback can sleep,
  then no need to use ath10k_wmi_cmd_send_nowait() and can take mutex
  to access ar->state

* don't touch ath10k_wmi_pdev_get_temperature()

* as QCA6174/QCA9377 are not (yet) supported don't add the command to WMI-TLV 
interface

* remove debugfs interface, that should be added in another patch

* cleanup includes


 drivers/net/wireless/ath/ath10k/Kconfig   |  10 +++
 drivers/net/wireless/ath/ath10k/Makefile  |   1 +
 drivers/net/wireless/ath/ath10k/core.c|  22 +++
 drivers/net/wireless/ath/ath10k/core.h|   9 ++-
 drivers/net/wireless/ath/ath10k/hw.h  |   1 +
 drivers/net/wireless/ath/ath10k/leds.c| 103 ++
 drivers/net/wireless/ath/ath10k/leds.h|  45 +
 drivers/net/wireless/ath/ath10k/mac.c |   1 +
 drivers/net/wireless/ath/ath10k/wmi-ops.h |  32 ++
 drivers/net/wireless/ath/ath10k/wmi-tlv.c |   2 +
 drivers/net/wireless/ath/ath10k/wmi.c |  54 
 drivers/net/wireless/ath/ath10k/wmi.h |  35 ++
 12 files changed, 314 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/wireless/ath/ath10k/leds.c
 create mode 100644 drivers/net/wireless/ath/ath10k/leds.h

diff --git a/drivers/net/wireless/ath/ath10k/Kconfig 
b/drivers/net/wireless/ath/ath10k/Kconfig
index deb5ae21a559..c4108c1bbe92 100644
--- a/drivers/net/wireless/ath/ath10k/Kconfig
+++ b/drivers/net/wireless/ath/ath10k/Kconfig
@@ -52,6 +52,16 @@ config ATH10K_DEBUGFS
 
  If unsure, say Y to make it easier to debug problems.
 
+config ATH10K_LEDS
+   bool "Atheros ath10k LED support"
+   depends on ATH10K
+   select MAC80211_LEDS
+   select LEDS_CLASS
+   select NEW_LEDS
+   default y
+   help
+ This option is necessary, if you want LED support for chipset 
connected led pins. If unsure, say N.
+
 config ATH10K_SPECTRAL
bool "Atheros ath10k spectral scan support"
depends on ATH10K_DEBUGFS
diff --git a/drivers/net/wireless/ath/ath10k/Makefile 
b/drivers/net/wireless/ath/ath10k/Makefile
index 6739ac26fd29..16c822421c39 100644
--- a/drivers/net/wireless/ath/ath10k/Makefile
+++ b/drivers/net/wireless/ath/ath10k/Makefile
@@ -20,6 +20,7 @@ ath10k_core-$(CONFIG_NL80211_TESTMODE) += testmode.o
 ath10k_core-$(CONFIG_ATH10K_TRACING) += trace.o
 ath10k_core-$(CONFIG_THERMAL) += thermal.o
 ath10k_core-$(CONFIG_MAC80211_DEBUGFS) += debugfs_sta.o
+ath10k_core-$(CONFIG_ATH10K_LEDS) += leds.o
 ath10k_core-$(CONFIG_PM) += wow.o
 ath10k_core-$(CONFIG_DEV_COREDUMP) += coredump.o
 
diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index 8a3020dbd4cf..be5958e98102 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -34,6 +34,7 @@
 #include "testmode.h"
 #include "wmi-ops.h"
 #include "coredump.h"
+#include "leds.h"
 
 unsigned int ath10k_debug_mask;
 static unsigned int ath10k_cryptmode_param;
@@ -66,6 +67,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] 
= {
.id = QCA988X_HW_2_0_VERSION,
.dev_id = QCA988X_2_0_DEVICE_ID,
.name = "qca988x hw2.0",
+   .led_pin = 1,
.patch_load_addr = QCA988X_HW_2_0_PATCH_LOAD_ADDR,
.uart_pin = 7,
.cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL,
@@ -95,6

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

2018-04-06 Thread Kalle Valo

(adding back lists)

Sebastian Gottschall  writes:

> Am 06.04.2018 um 10:25 schrieb Kalle Valo:
>> Sebastian Gottschall  writes:
>>
>>> Am 06.04.2018 um 10:09 schrieb Kalle Valo:
 (adding back the lists, please don't top post and trim your quotes)

 Sebastian Gottschall  writes:

> okay. ath10k-check is buggy and doesnt work. so it doesnt help much
> with code styles
>
> root@seg-desktop:/xfs/ath10k/ath.gpio# ./ath10k-check
> global: 'drivers/net/wireless/ath/ath10k/Kconfig' is not a source file.
> Traceback (most recent call last):
>     File "./ath10k-check", line 461, in 
>       main()
>     File "./ath10k-check", line 455, in main
>       ret = run_checkpatch(args)
>     File "./ath10k-check", line 275, in run_checkpatch
>       output = subprocess.check_output(cmd, shell=True)
>     File "/usr/lib/python2.7/subprocess.py", line 219, in check_output
>       raise CalledProcessError(retcode, cmd, output=output)
> subprocess.CalledProcessError: Command 'global -f
> drivers/net/wireless/ath/ath10k/Kconfig' returned non-zero exit status 1
 You are missing gtags, see 'ath10k-check --help' for how to install:

 Requirements (all available in $PATH):

 * gcc
 * sparse
 * checkpatch.pl
 * gtags (from package global)
>>> ahm i did read this and this was all installed. but ath10k-check
>>> refuses to work
>>>
>>> root@seg-desktop:/home/seg/DEV# gtags --version
>>> gtags (GNU GLOBAL) 6.5.7
>>> Copyright (c) 1996-2017 Tama Communications Corporation
>>> License GPLv3+: GNU GPL version 3 or later
>>> 
>>> This is free software; you are free to change and redistribute it.
>>> There is NO WARRANTY, to the extent permitted by law.
>> Damn, I guess something has changed in gtags as I use older version:
>>
>> gtags - GNU GLOBAL 5.7.1
>
> okay. i will try to use a older version and see what happens

You can also run checkpatch manually (sorry for the long line):

checkpatch.pl --strict -q --terse --no-summary --max-line-length=90 
--show-types --ignore 
MSLEEP,USLEEP_RANGE,PRINTK_WITHOUT_KERN_LEVEL,NETWORKING_BLOCK_COMMENT_STYLE,LINUX_VERSION_CODE,COMPLEX_MACRO,PREFER_DEV_LEVEL,PREFER_PR_LEVEL,COMPARISON_TO_NULL,BIT_MACRO,CONSTANT_COMPARISON,MACRO_WITH_FLOW_CONTROL,CONST_STRUCT,MACRO_ARG_REUSE,MACRO_ARG_PRECEDENCE
 foo.patch

I modified ath10k-check --help to print the full commandline, just
haven't pushed it out yet.

-- 
Kalle Valo

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

2018-04-06 Thread Kalle Valo

Sebastian Gottschall  writes:

> Am 06.04.2018 um 10:09 schrieb Kalle Valo:
>> (adding back the lists, please don't top post and trim your quotes)
>>
>> Sebastian Gottschall  writes:
>>
>>> okay. ath10k-check is buggy and doesnt work. so it doesnt help much
>>> with code styles
>>>
>>> root@seg-desktop:/xfs/ath10k/ath.gpio# ./ath10k-check
>>> global: 'drivers/net/wireless/ath/ath10k/Kconfig' is not a source file.
>>> Traceback (most recent call last):
>>>    File "./ath10k-check", line 461, in 
>>>      main()
>>>    File "./ath10k-check", line 455, in main
>>>      ret = run_checkpatch(args)
>>>    File "./ath10k-check", line 275, in run_checkpatch
>>>      output = subprocess.check_output(cmd, shell=True)
>>>    File "/usr/lib/python2.7/subprocess.py", line 219, in check_output
>>>      raise CalledProcessError(retcode, cmd, output=output)
>>> subprocess.CalledProcessError: Command 'global -f
>>> drivers/net/wireless/ath/ath10k/Kconfig' returned non-zero exit status 1
>>
>> You are missing gtags, see 'ath10k-check --help' for how to install:
>>
>> Requirements (all available in $PATH):
>>
>> * gcc
>> * sparse
>> * checkpatch.pl
>> * gtags (from package global)
>
> ahm i did read this and this was all installed. but ath10k-check
> refuses to work
>
> root@seg-desktop:/home/seg/DEV# gtags --version
> gtags (GNU GLOBAL) 6.5.7
> Copyright (c) 1996-2017 Tama Communications Corporation
> License GPLv3+: GNU GPL version 3 or later
> 
> This is free software; you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.

Damn, I guess something has changed in gtags as I use older version:

gtags - GNU GLOBAL 5.7.1

-- 
Kalle Valo

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

2018-04-06 Thread Sebastian Gottschall


Am 06.04.2018 um 10:09 schrieb Kalle Valo:

(adding back the lists, please don't top post and trim your quotes)

Sebastian Gottschall  writes:


okay. ath10k-check is buggy and doesnt work. so it doesnt help much
with code styles

root@seg-desktop:/xfs/ath10k/ath.gpio# ./ath10k-check
global: 'drivers/net/wireless/ath/ath10k/Kconfig' is not a source file.
Traceback (most recent call last):
   File "./ath10k-check", line 461, in 
     main()
   File "./ath10k-check", line 455, in main
     ret = run_checkpatch(args)
   File "./ath10k-check", line 275, in run_checkpatch
     output = subprocess.check_output(cmd, shell=True)
   File "/usr/lib/python2.7/subprocess.py", line 219, in check_output
     raise CalledProcessError(retcode, cmd, output=output)
subprocess.CalledProcessError: Command 'global -f
drivers/net/wireless/ath/ath10k/Kconfig' returned non-zero exit status 1

You are missing gtags, see 'ath10k-check --help' for how to install:

Requirements (all available in $PATH):

* gcc
* sparse
* checkpatch.pl
* gtags (from package global)
ahm i did read this and this was all installed. but ath10k-check refuses 
to work


root@seg-desktop:/home/seg/DEV# gtags --version
gtags (GNU GLOBAL) 6.5.7
Copyright (c) 1996-2017 Tama Communications Corporation
License GPLv3+: GNU GPL version 3 or later 


This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.





--
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.gottsch...@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

2018-04-06 Thread Sebastian Gottschall


Am 06.04.2018 um 10:07 schrieb Kalle Valo:

Sebastian Gottschall  writes:


i have some comments about your check warnings.
some of them are bogus. for instance they advise to use "unsigned int"
instead of "unsigned". this might be better, but
the original kernel header uses "unsigned" as api definition. so i
decided to use the same declarations here.

Sure, but using "unsigned int" won't create any harm either.


for the rest like whitespaces i will make a new version and remove them

You missed my comment in my earlier mail that I'll send the next
version. I have already fixed all the warnings so let me submit it. I'll
do also some other changes.


oh okay. thank you. then forget all the rest. i will take care of these 
issues with the next patch i submit






--
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.gottsch...@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

2018-04-06 Thread Kalle Valo

(adding back the lists, please don't top post and trim your quotes)

Sebastian Gottschall  writes:

> okay. ath10k-check is buggy and doesnt work. so it doesnt help much
> with code styles
>
> root@seg-desktop:/xfs/ath10k/ath.gpio# ./ath10k-check
> global: 'drivers/net/wireless/ath/ath10k/Kconfig' is not a source file.
> Traceback (most recent call last):
>   File "./ath10k-check", line 461, in 
>     main()
>   File "./ath10k-check", line 455, in main
>     ret = run_checkpatch(args)
>   File "./ath10k-check", line 275, in run_checkpatch
>     output = subprocess.check_output(cmd, shell=True)
>   File "/usr/lib/python2.7/subprocess.py", line 219, in check_output
>     raise CalledProcessError(retcode, cmd, output=output)
> subprocess.CalledProcessError: Command 'global -f 
> drivers/net/wireless/ath/ath10k/Kconfig' returned non-zero exit status 1

You are missing gtags, see 'ath10k-check --help' for how to install:

Requirements (all available in $PATH):

* gcc
* sparse
* checkpatch.pl
* gtags (from package global)

-- 
Kalle Valo

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

2018-04-06 Thread Kalle Valo

Sebastian Gottschall  writes:

> i have some comments about your check warnings.
> some of them are bogus. for instance they advise to use "unsigned int"
> instead of "unsigned". this might be better, but
> the original kernel header uses "unsigned" as api definition. so i
> decided to use the same declarations here.

Sure, but using "unsigned int" won't create any harm either.

> for the rest like whitespaces i will make a new version and remove them

You missed my comment in my earlier mail that I'll send the next
version. I have already fixed all the warnings so let me submit it. I'll
do also some other changes.

-- 
Kalle Valo

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

2018-04-06 Thread Kalle Valo

Sebastian Gottschall  writes:

> Am 05.04.2018 um 16:44 schrieb Kalle Valo:
>> s.gottsch...@dd-wrt.com writes:
>>
>>> Adds LED and GPIO Control support for 988x, 9887, 9888, 99x0, 9984
>>> 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.
>>>
>>> Signed-off-by: Sebastian Gottschall 
>> [...]
>>
>>> @@ -1034,7 +1068,7 @@ ath10k_wmi_pdev_get_temperature(struct ath10k *ar)
>>>   if (IS_ERR(skb))
>>>   return PTR_ERR(skb);
>>>   -    return ath10k_wmi_cmd_send(ar, skb,
>>> +    return ath10k_wmi_cmd_send_nowait(ar, skb,
>>> ar->wmi.cmd->pdev_get_temperature_cmdid);
>>>   }
>> This looks odd, I don't think it belongs to this patch.
>
> thats true. but due the nature of this function i found it better to
> use nowait here. better if i split it up?

Yes, this should be done in a separate patch with a proper commit log
explaining why it's needed.

>> Also you made a some sort of record, your patch had 181 checkpatch
>> warnings! Do you use Word as your editor or what? But please do check
>> your editor settings and read the coding style documents.
>
> no? i use midnight commander for all of my code since more than 20 years
> and its the first time that i see such warnings. is there any special
> coding rule for ath10k which differs from the kernel rules?

You got even the indentation wrong in multiple functions and indentation
rules have been the same as long as I remember. And checkpatch has been
around a long time already, that should not be new to anyone submitting
patches.

> and where is ath10k-check located?

Check the link I provided:

>> https://wireless.wiki.kernel.org/en/users/drivers/ath10k/codingstyle

-- 
Kalle Valo

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

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

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

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

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

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

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

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

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

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

10 matches

Site Navigation

Mail list logo

Footer information