Return-path: Received: from smtps.newmedia-net.de ([185.84.6.167]:56596 "EHLO webmail.newmedia-net.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751285AbeDESCP (ORCPT ); Thu, 5 Apr 2018 14:02:15 -0400 Subject: Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets To: Kalle Valo Cc: linux-wireless@vger.kernel.org, Sebastian Gottschall , ath10k@lists.infradead.org References: <20180226084406.2093-1-s.gottschall@dd-wrt.com> <871sftemru.fsf@kamboji.qca.qualcomm.com> From: Sebastian Gottschall Message-ID: (sfid-20180405_200219_363163_6F4740BC) Date: Thu, 5 Apr 2018 20:01:47 +0200 MIME-Version: 1.0 In-Reply-To: <871sftemru.fsf@kamboji.qca.qualcomm.com> Content-Type: text/plain; charset=utf-8; format=flowed Sender: linux-wireless-owner@vger.kernel.org List-ID: Am 05.04.2018 um 16:44 schrieb Kalle Valo: > s.gottschall@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? > > 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? and where is ath10k-check located? > > https://wireless.wiki.kernel.org/en/users/drivers/ath10k/codingstyle > > I'll send v13. > > Here are the warnings from ath10k-check: > > drivers/net/wireless/ath/ath10k/gpio.c:19: Please don't use multiple > blank lines > drivers/net/wireless/ath/ath10k/gpio.c:30: Please don't use multiple > blank lines > drivers/net/wireless/ath/ath10k/gpio.c:36: Prefer 'unsigned int' to > bare use of 'unsigned' > drivers/net/wireless/ath/ath10k/gpio.c:38: line over 90 characters > drivers/net/wireless/ath/ath10k/gpio.c:39: line over 90 characters > drivers/net/wireless/ath/ath10k/gpio.c:39: Missing a blank line after > declarations > drivers/net/wireless/ath/ath10k/gpio.c:45: Prefer 'unsigned int' to > bare use of 'unsigned' > drivers/net/wireless/ath/ath10k/gpio.c:46: Alignment should match open > parenthesis > drivers/net/wireless/ath/ath10k/gpio.c:48: line over 90 characters > drivers/net/wireless/ath/ath10k/gpio.c:50: line over 90 characters > drivers/net/wireless/ath/ath10k/gpio.c:58: Prefer 'unsigned int' to > bare use of 'unsigned' > drivers/net/wireless/ath/ath10k/gpio.c:60: line over 90 characters > drivers/net/wireless/ath/ath10k/gpio.c:66: Prefer 'unsigned int' to > bare use of 'unsigned' > drivers/net/wireless/ath/ath10k/gpio.c:68: line over 90 characters > drivers/net/wireless/ath/ath10k/gpio.c:74: Prefer 'unsigned int' to > bare use of 'unsigned' > drivers/net/wireless/ath/ath10k/gpio.c:75: Alignment should match open > parenthesis > drivers/net/wireless/ath/ath10k/gpio.c:77: line over 90 characters > drivers/net/wireless/ath/ath10k/gpio.c:86: trailing whitespace > drivers/net/wireless/ath/ath10k/gpio.c:86: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/gpio.c:86: Blank lines aren't > necessary after an open brace '{' > drivers/net/wireless/ath/ath10k/gpio.c:88: Missing a blank line after > declarations > drivers/net/wireless/ath/ath10k/gpio.c:88: braces {} are not necessary > for single statement blocks > drivers/net/wireless/ath/ath10k/gpio.c:113: trailing whitespace > drivers/net/wireless/ath/ath10k/gpio.c:114: Missing a blank line after > declarations > drivers/net/wireless/ath/ath10k/gpio.c:114: braces {} are not > necessary for single statement blocks > drivers/net/wireless/ath/ath10k/gpio.c:121: line over 90 characters > drivers/net/wireless/ath/ath10k/gpio.c:130: Please don't use multiple > blank lines > drivers/net/wireless/ath/ath10k/gpio.c:132: Alignment should match > open parenthesis > drivers/net/wireless/ath/ath10k/gpio.c:134: line over 90 characters > drivers/net/wireless/ath/ath10k/gpio.c:136: Missing a blank line after > declarations > drivers/net/wireless/ath/ath10k/gpio.c:145: trailing whitespace > drivers/net/wireless/ath/ath10k/gpio.c:146: Missing a blank line after > declarations > drivers/net/wireless/ath/ath10k/gpio.c:159: trailing whitespace > drivers/net/wireless/ath/ath10k/gpio.c:160: Missing a blank line after > declarations > drivers/net/wireless/ath/ath10k/gpio.c:166: trailing whitespace > drivers/net/wireless/ath/ath10k/gpio.c:169: trailing whitespace > drivers/net/wireless/ath/ath10k/gpio.c:170: line over 90 characters > drivers/net/wireless/ath/ath10k/gpio.c:178: Missing a blank line after > declarations > drivers/net/wireless/ath/ath10k/gpio.c:181: Prefer > kzalloc(sizeof(*gpio)...) over kzalloc(sizeof(struct > ath10k_gpiocontrol)...) > drivers/net/wireless/ath/ath10k/gpio.c:182: braces {} are not > necessary for single statement blocks > drivers/net/wireless/ath/ath10k/gpio.c:192: trailing whitespace > drivers/net/wireless/ath/ath10k/gpio.c:194: line over 90 characters > drivers/net/wireless/ath/ath10k/gpio.c:196: trailing whitespace > drivers/net/wireless/ath/ath10k/core.h:865: trailing whitespace > drivers/net/wireless/ath/ath10k/core.h:865: line over 90 characters > drivers/net/wireless/ath/ath10k/core.h:871: trailing whitespace > drivers/net/wireless/ath/ath10k/core.h:890: Blank lines aren't > necessary after an open brace '{' > drivers/net/wireless/ath/ath10k/core.h:891: Blank lines aren't > necessary before a close brace '}' > drivers/net/wireless/ath/ath10k/core.h:892: Please use a blank line > after function/struct/union/enum declarations > drivers/net/wireless/ath/ath10k/core.h:894: Blank lines aren't > necessary after an open brace '{' > drivers/net/wireless/ath/ath10k/core.h:895: Blank lines aren't > necessary before a close brace '}' > drivers/net/wireless/ath/ath10k/core.h:896: Please use a blank line > after function/struct/union/enum declarations > drivers/net/wireless/ath/ath10k/core.h:905: trailing whitespace > drivers/net/wireless/ath/ath10k/core.h:907: Blank lines aren't > necessary after an open brace '{' > drivers/net/wireless/ath/ath10k/core.h:908: Blank lines aren't > necessary before a close brace '}' > drivers/net/wireless/ath/ath10k/core.c:29: Please don't use multiple > blank lines > drivers/net/wireless/ath/ath10k/core.c:74: trailing whitespace > drivers/net/wireless/ath/ath10k/core.c:105: trailing whitespace > drivers/net/wireless/ath/ath10k/core.c:136: trailing whitespace > drivers/net/wireless/ath/ath10k/core.c:282: trailing whitespace > drivers/net/wireless/ath/ath10k/core.c:318: trailing whitespace > drivers/net/wireless/ath/ath10k/core.c:359: trailing whitespace > drivers/net/wireless/ath/ath10k/core.c:2396: trailing whitespace > drivers/net/wireless/ath/ath10k/core.c:2396: Please don't use multiple > blank lines > drivers/net/wireless/ath/ath10k/core.c:2403: trailing whitespace > drivers/net/wireless/ath/ath10k/debug.c:1089: Alignment should match > open parenthesis > drivers/net/wireless/ath/ath10k/debug.c:1092: trailing whitespace > drivers/net/wireless/ath/ath10k/debug.c:1095: Missing a blank line > after declarations > drivers/net/wireless/ath/ath10k/debug.c:1098: line over 90 characters > drivers/net/wireless/ath/ath10k/debug.c:1103: Please don't use > multiple blank lines > drivers/net/wireless/ath/ath10k/debug.c:1105: Alignment should match > open parenthesis > drivers/net/wireless/ath/ath10k/debug.c:1109: trailing whitespace > drivers/net/wireless/ath/ath10k/debug.c:1113: Missing a blank line > after declarations > drivers/net/wireless/ath/ath10k/debug.c:1130: Please don't use > multiple blank lines > drivers/net/wireless/ath/ath10k/debug.c:1133: Please don't use > multiple blank lines > drivers/net/wireless/ath/ath10k/debug.c:1149: Alignment should match > open parenthesis > drivers/net/wireless/ath/ath10k/debug.c:1152: trailing whitespace > drivers/net/wireless/ath/ath10k/debug.c:1155: Missing a blank line > after declarations > drivers/net/wireless/ath/ath10k/debug.c:1163: Please don't use > multiple blank lines > drivers/net/wireless/ath/ath10k/debug.c:1165: Alignment should match > open parenthesis > drivers/net/wireless/ath/ath10k/debug.c:1169: trailing whitespace > drivers/net/wireless/ath/ath10k/debug.c:1173: Missing a blank line > after declarations > drivers/net/wireless/ath/ath10k/debug.c:1188: Please don't use > multiple blank lines > drivers/net/wireless/ath/ath10k/debug.c:1191: Please don't use > multiple blank lines > drivers/net/wireless/ath/ath10k/debug.c:1375: Please don't use > multiple blank lines > drivers/net/wireless/ath/ath10k/wmi-ops.h:211: Please don't use > multiple blank lines > drivers/net/wireless/ath/ath10k/wmi-ops.h:986: trailing whitespace > drivers/net/wireless/ath/ath10k/wmi-ops.h:986: Please use a blank line > after function/struct/union/enum declarations > drivers/net/wireless/ath/ath10k/wmi-ops.h:987: line over 90 characters > drivers/net/wireless/ath/ath10k/wmi-ops.h:1001: trailing whitespace > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3340: line over 90 characters > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3342: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3343: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3344: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3345: trailing whitespace > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3345: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3346: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3348: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3349: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3350: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3353: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3354: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3355: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3356: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3358: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3359: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3360: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3361: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3362: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3364: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3365: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3367: line over 90 characters > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3367: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3368: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3374: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3375: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3376: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3377: trailing whitespace > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3377: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3378: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3380: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3381: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3382: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3385: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3386: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3387: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3388: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3390: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3391: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3392: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3394: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3395: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3397: line over 90 characters > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3397: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3398: please, no spaces at > the start of a line > drivers/net/wireless/ath/ath10k/wmi-tlv.c:3401: Please don't use > multiple blank lines > drivers/net/wireless/ath/ath10k/mac.c:4622: trailing whitespace > drivers/net/wireless/ath/ath10k/mac.c:4623: line over 90 characters > drivers/net/wireless/ath/ath10k/mac.c:4625: trailing whitespace > drivers/net/wireless/ath/ath10k/mac.c:4626: trailing whitespace > drivers/net/wireless/ath/ath10k/wmi.h:2934: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2935: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2936: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2940: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2941: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2942: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2943: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2944: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2945: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2950: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2951: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2952: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2953: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2958: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2959: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2964: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.h:2967: Please don't use multiple > blank lines > drivers/net/wireless/ath/ath10k/wmi.c:6957: line over 90 characters > drivers/net/wireless/ath/ath10k/wmi.c:6959: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6960: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6962: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6963: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6965: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6966: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6967: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6968: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6969: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6971: line over 90 characters > drivers/net/wireless/ath/ath10k/wmi.c:6971: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6972: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6978: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6979: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6981: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6982: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6984: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6985: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6986: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6987: line over 90 characters > drivers/net/wireless/ath/ath10k/wmi.c:6987: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:6988: please, no spaces at the > start of a line > drivers/net/wireless/ath/ath10k/wmi.c:8583: trailing whitespace > -- 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