* Re: QCA988x and kernel 4.13
From: Kalle Valo @ 2017-10-04 8:50 UTC (permalink / raw)
To: Ben Greear
Cc: Sebastian Gottschall, svp, ath10k@lists.infradead.org,
linux-wireless@vger.kernel.org
In-Reply-To: <48aebea8-9231-b41e-cbaa-d592d8540c53@candelatech.com>
Ben Greear <greearb@candelatech.com> writes:
> On 10/01/2017 07:06 AM, Sebastian Gottschall wrote:
>> you have to update to the latest mac80211 code. there is a well
>> known bug in mac80211 which causes a deadlock with ath10k
>
> Do you have a pointer to the patch that fixes this? I'd like to check if=
it has
> made it into 4.13 stable yet...
I think it's this one:
mac80211: fix deadlock in driver-managed RX BA session start
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?=
id=3Dbde59c475e0883e4c4294bcd9b9c7e08ae18c828
Adding linux-wireless so that someone can correct me in case I'm wrong.
--=20
Kalle Valo=
^ permalink raw reply
* Re: [PATCH 01/11] ath10k_sdio: sdio htt data transfer fixes
From: Kalle Valo @ 2017-10-04 8:55 UTC (permalink / raw)
To: Arend van Spriel
Cc: silexcommon@gmail.com, ath10k@lists.infradead.org, Alagu Sankar,
linux-wireless@vger.kernel.org
In-Reply-To: <59D1EC94.2020307@broadcom.com>
Arend van Spriel <arend.vanspriel@broadcom.com> writes:
> On 9/30/2017 7:37 PM, silexcommon@gmail.com wrote:
>> From: Alagu Sankar <alagusankar@silex-india.com>
>>
>
> [...]
>
>>
>> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
>
> Not really have a specific remark for this patch, but for the entire
> series. These patches are sent using an anonymous email address, apart
> from 'silex' being in there, which does not show up in the certificate
> of origin. Just wondering if this is acceptable?
As long as there's the "correct" From header as the first line in the
commit log, which git then uses as the author instead of the From line
from email header. And I see Alagu doing that here so there shouldn't be
any problems.
--=20
Kalle Valo=
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: add device tree binding for Allwinner XR819 SDIO Wi-Fi
From: Kalle Valo @ 2017-10-04 9:02 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Rob Herring, Maxime Ripard, Chen-Yu Tsai, linux-wireless, netdev,
devicetree, linux-kernel, linux-arm-kernel, linux-sunxi
In-Reply-To: <20171003165944.13056-2-icenowy@aosc.io>
Icenowy Zheng <icenowy@aosc.io> writes:
> Allwinner XR819 is a SDIO Wi-Fi chip, which has the functionality to use
> an out-of-band interrupt pin instead of SDIO in-band interrupt.
>
> Add the device tree binding of this chip, in order to make it possible
> to add this interrupt pin to device trees.
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
> Changes in v3:
> - Renames the node name.
> - Adds ACK from Rob.
> Changes in v2:
> - Removed status property in example.
> - Added required property reg.
>
> .../bindings/net/wireless/allwinner,xr819.txt | 38 ++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt
Like I asked already last time, AFAICS there is no upstream xr819
wireless driver in drivers/net/wireless directory. Do we still accept
bindings like this for out-of-tree drivers?
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: add device tree binding for Allwinner XR819 SDIO Wi-Fi
From: Icenowy Zheng @ 2017-10-04 9:03 UTC (permalink / raw)
To: Kalle Valo
Cc: Rob Herring, Maxime Ripard, Chen-Yu Tsai, linux-wireless, netdev,
devicetree, linux-kernel, linux-arm-kernel, linux-sunxi
In-Reply-To: <871smjxp46.fsf@kamboji.qca.qualcomm.com>
于 2017年10月4日 GMT+08:00 下午5:02:17, Kalle Valo <kvalo@codeaurora.org> 写到:
>Icenowy Zheng <icenowy@aosc.io> writes:
>
>> Allwinner XR819 is a SDIO Wi-Fi chip, which has the functionality to
>use
>> an out-of-band interrupt pin instead of SDIO in-band interrupt.
>>
>> Add the device tree binding of this chip, in order to make it
>possible
>> to add this interrupt pin to device trees.
>>
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> Acked-by: Rob Herring <robh@kernel.org>
>> ---
>> Changes in v3:
>> - Renames the node name.
>> - Adds ACK from Rob.
>> Changes in v2:
>> - Removed status property in example.
>> - Added required property reg.
>>
>> .../bindings/net/wireless/allwinner,xr819.txt | 38
>++++++++++++++++++++++
>> 1 file changed, 38 insertions(+)
>> create mode 100644
>Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt
>
>Like I asked already last time, AFAICS there is no upstream xr819
>wireless driver in drivers/net/wireless directory. Do we still accept
>bindings like this for out-of-tree drivers?
See esp8089.
There's also no in-tree driver for it.
^ permalink raw reply
* Re: ath9k: make const array reg_hole_list static, reduces object code size
From: Kalle Valo @ 2017-10-04 9:09 UTC (permalink / raw)
To: Colin Ian King
Cc: QCA ath9k Development, Kalle Valo, linux-wireless, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20170919204019.30804-1-colin.king@canonical.com>
Colin Ian King <colin.king@canonical.com> wrote:
> Don't populate the read-only array reg_hole_list on the stack, instead make
> it static. Makes the object code smaller by over 200 bytes:
>
> Before:
> text data bss dec hex filename
> 57518 15248 0 72766 11c3e debug.o
>
> After:
> text data bss dec hex filename
> 57218 15344 0 72562 11b72 debug.o
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
Patch applied to ath-next branch of ath.git, thanks.
eba0f28473b2 ath9k: make const array reg_hole_list static, reduces object code size
--
https://patchwork.kernel.org/patch/9960183/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 08/11] ath10k_sdio: common read write
From: Kalle Valo @ 2017-10-04 9:49 UTC (permalink / raw)
To: silexcommon@gmail.com
Cc: ath10k@lists.infradead.org, Alagu Sankar,
linux-wireless@vger.kernel.org
In-Reply-To: <1506793068-27445-9-git-send-email-alagusankar@silex-india.com>
silexcommon@gmail.com writes:
> From: Alagu Sankar <alagusankar@silex-india.com>
>
> convert different read write functions in sdio hif to bring it under a
> single read-write path. This helps in having a common dma bounce buffer
> implementation. Also helps in address modification that is required
> specific to change in certain mbox addresses of sdio_write.
>
> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
This didn't compile for me:
drivers/net/wireless/ath/ath10k/sdio.c:320:12: error: conflicting types for=
'ath10k_sdio_read'
drivers/net/wireless/ath/ath10k/sdio.c:39:12: note: previous declaration of=
'ath10k_sdio_read' was here
drivers/net/wireless/ath/ath10k/sdio.c:365:12: error: conflicting types for=
'ath10k_sdio_write'
drivers/net/wireless/ath/ath10k/sdio.c:41:12: note: previous declaration of=
'ath10k_sdio_write' was here
drivers/net/wireless/ath/ath10k/sdio.c:39:12: warning: 'ath10k_sdio_read' u=
sed but never defined
drivers/net/wireless/ath/ath10k/sdio.c:41:12: warning: 'ath10k_sdio_write' =
used but never defined
gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609
I fixed it like below in the pending branch. But I'll review more
carefully later, I have quite a lot of patches pending right now.
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -37,9 +37,9 @@
#define ATH10K_SDIO_DMA_BUF_SIZE (32 * 1024)
=20
static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf,
- u32 len, bool incr);
+ size_t len, bool incr);
static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void *buf,
- u32 len, bool incr);
+ size_t len, bool incr);
=20
/* inlined helper functions */
--=20
Kalle Valo=
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: add device tree binding for Allwinner XR819 SDIO Wi-Fi
From: Arend van Spriel @ 2017-10-04 10:02 UTC (permalink / raw)
To: Icenowy Zheng, Kalle Valo
Cc: Rob Herring, Maxime Ripard, Chen-Yu Tsai, linux-wireless, netdev,
devicetree, linux-kernel, linux-arm-kernel, linux-sunxi
In-Reply-To: <E46F3A20-294A-497E-AF71-C77A67AFDB8C@aosc.io>
On 10/4/2017 11:03 AM, Icenowy Zheng wrote:
>
>
> 于 2017年10月4日 GMT+08:00 下午5:02:17, Kalle Valo <kvalo@codeaurora.org> 写到:
>> Icenowy Zheng <icenowy@aosc.io> writes:
>>
>>> Allwinner XR819 is a SDIO Wi-Fi chip, which has the functionality to
>> use
>>> an out-of-band interrupt pin instead of SDIO in-band interrupt.
>>>
>>> Add the device tree binding of this chip, in order to make it
>> possible
>>> to add this interrupt pin to device trees.
>>>
>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>>> Acked-by: Rob Herring <robh@kernel.org>
>>> ---
>>> Changes in v3:
>>> - Renames the node name.
>>> - Adds ACK from Rob.
>>> Changes in v2:
>>> - Removed status property in example.
>>> - Added required property reg.
>>>
>>> .../bindings/net/wireless/allwinner,xr819.txt | 38
>> ++++++++++++++++++++++
>>> 1 file changed, 38 insertions(+)
>>> create mode 100644
>> Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt
>>
>> Like I asked already last time, AFAICS there is no upstream xr819
>> wireless driver in drivers/net/wireless directory. Do we still accept
>> bindings like this for out-of-tree drivers?
>
> See esp8089.
>
> There's also no in-tree driver for it.
The question is whether we should. The above might be a precedent, but
it may not necessarily be the way to go. The commit message for esp8089
seems to hint that there is intent to have an in-tree driver:
"""
Note that at this point there only is an out of tree driver for this
hardware, there is no clear timeline / path for merging this. Still
I believe it would be good to specify the binding for this in tree
now, so that any future migration to an in tree driver will not cause
compatiblity issues.
Cc: Icenowy Zheng <icenowy@aosc.xyz>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Rob Herring <robh@kernel.org>
"""
Regardless the bindings are in principle independent of the kernel and
just describing hardware. I think there have been discussions to move
the bindings to their own repository, but apparently it was decided
otherwise.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: add device tree binding for Allwinner XR819 SDIO Wi-Fi
From: Maxime Ripard @ 2017-10-04 10:11 UTC (permalink / raw)
To: Arend van Spriel
Cc: Icenowy Zheng, Kalle Valo, Rob Herring, Chen-Yu Tsai,
linux-wireless, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-sunxi
In-Reply-To: <59D4B1C8.8020105@broadcom.com>
[-- Attachment #1: Type: text/plain, Size: 2639 bytes --]
On Wed, Oct 04, 2017 at 10:02:48AM +0000, Arend van Spriel wrote:
> On 10/4/2017 11:03 AM, Icenowy Zheng wrote:
> >
> >
> > 于 2017年10月4日 GMT+08:00 下午5:02:17, Kalle Valo <kvalo@codeaurora.org> 写到:
> > > Icenowy Zheng <icenowy@aosc.io> writes:
> > >
> > > > Allwinner XR819 is a SDIO Wi-Fi chip, which has the functionality to
> > > use
> > > > an out-of-band interrupt pin instead of SDIO in-band interrupt.
> > > >
> > > > Add the device tree binding of this chip, in order to make it
> > > possible
> > > > to add this interrupt pin to device trees.
> > > >
> > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > > Acked-by: Rob Herring <robh@kernel.org>
> > > > ---
> > > > Changes in v3:
> > > > - Renames the node name.
> > > > - Adds ACK from Rob.
> > > > Changes in v2:
> > > > - Removed status property in example.
> > > > - Added required property reg.
> > > >
> > > > .../bindings/net/wireless/allwinner,xr819.txt | 38
> > > ++++++++++++++++++++++
> > > > 1 file changed, 38 insertions(+)
> > > > create mode 100644
> > > Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt
> > >
> > > Like I asked already last time, AFAICS there is no upstream xr819
> > > wireless driver in drivers/net/wireless directory. Do we still accept
> > > bindings like this for out-of-tree drivers?
> >
> > See esp8089.
> >
> > There's also no in-tree driver for it.
>
> The question is whether we should. The above might be a precedent, but it
> may not necessarily be the way to go. The commit message for esp8089 seems
> to hint that there is intent to have an in-tree driver:
>
> """
> Note that at this point there only is an out of tree driver for this
> hardware, there is no clear timeline / path for merging this. Still
> I believe it would be good to specify the binding for this in tree
> now, so that any future migration to an in tree driver will not cause
> compatiblity issues.
>
> Cc: Icenowy Zheng <icenowy@aosc.xyz>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Rob Herring <robh@kernel.org>
> """
>
> Regardless the bindings are in principle independent of the kernel and just
> describing hardware. I think there have been discussions to move the
> bindings to their own repository, but apparently it was decided otherwise.
Yeah, I guess especially how it could be merged with the cw1200 driver
would be very relevant to that commit log.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: add device tree binding for Allwinner XR819 SDIO Wi-Fi
From: Icenowy Zheng @ 2017-10-04 10:15 UTC (permalink / raw)
To: linux-arm-kernel, Maxime Ripard, Arend van Spriel
Cc: devicetree, netdev, linux-sunxi, linux-wireless, linux-kernel,
Chen-Yu Tsai, Rob Herring, Kalle Valo
In-Reply-To: <20171004101145.kgjufpcktodppuy3@flea>
于 2017年10月4日 GMT+08:00 下午6:11:45, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
>On Wed, Oct 04, 2017 at 10:02:48AM +0000, Arend van Spriel wrote:
>> On 10/4/2017 11:03 AM, Icenowy Zheng wrote:
>> >
>> >
>> > 于 2017年10月4日 GMT+08:00 下午5:02:17, Kalle Valo <kvalo@codeaurora.org>
>写到:
>> > > Icenowy Zheng <icenowy@aosc.io> writes:
>> > >
>> > > > Allwinner XR819 is a SDIO Wi-Fi chip, which has the
>functionality to
>> > > use
>> > > > an out-of-band interrupt pin instead of SDIO in-band interrupt.
>> > > >
>> > > > Add the device tree binding of this chip, in order to make it
>> > > possible
>> > > > to add this interrupt pin to device trees.
>> > > >
>> > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> > > > Acked-by: Rob Herring <robh@kernel.org>
>> > > > ---
>> > > > Changes in v3:
>> > > > - Renames the node name.
>> > > > - Adds ACK from Rob.
>> > > > Changes in v2:
>> > > > - Removed status property in example.
>> > > > - Added required property reg.
>> > > >
>> > > > .../bindings/net/wireless/allwinner,xr819.txt | 38
>> > > ++++++++++++++++++++++
>> > > > 1 file changed, 38 insertions(+)
>> > > > create mode 100644
>> > >
>Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt
>> > >
>> > > Like I asked already last time, AFAICS there is no upstream xr819
>> > > wireless driver in drivers/net/wireless directory. Do we still
>accept
>> > > bindings like this for out-of-tree drivers?
>> >
>> > See esp8089.
>> >
>> > There's also no in-tree driver for it.
>>
>> The question is whether we should. The above might be a precedent,
>but it
>> may not necessarily be the way to go. The commit message for esp8089
>seems
>> to hint that there is intent to have an in-tree driver:
>>
>> """
>> Note that at this point there only is an out of tree driver for
>this
>> hardware, there is no clear timeline / path for merging this.
>Still
>> I believe it would be good to specify the binding for this in
>tree
>> now, so that any future migration to an in tree driver will not
>cause
>> compatiblity issues.
>>
>> Cc: Icenowy Zheng <icenowy@aosc.xyz>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> """
>>
>> Regardless the bindings are in principle independent of the kernel
>and just
>> describing hardware. I think there have been discussions to move the
>> bindings to their own repository, but apparently it was decided
>otherwise.
>
>Yeah, I guess especially how it could be merged with the cw1200 driver
>would be very relevant to that commit log.
The cw1200 driver seems to still have some legacy platform
data. Maybe they should also be convert to DT.
(Or maybe compatible = "allwinner,xr819" is enough, as
xr819 is a specified variant of cw1200 family)
>
>Maxime
^ permalink raw reply
* Re: QCA988x and kernel 4.13
From: Sebastian Gottschall @ 2017-10-04 11:33 UTC (permalink / raw)
To: Kalle Valo, Ben Greear
Cc: svp, ath10k@lists.infradead.org, linux-wireless@vger.kernel.org
In-Reply-To: <87d163xpoe.fsf@kamboji.qca.qualcomm.com>
Am 04.10.2017 um 10:50 schrieb Kalle Valo:
> Ben Greear <greearb@candelatech.com> writes:
>
>> On 10/01/2017 07:06 AM, Sebastian Gottschall wrote:
>>> you have to update to the latest mac80211 code. there is a well
>>> known bug in mac80211 which causes a deadlock with ath10k
>> Do you have a pointer to the patch that fixes this? I'd like to check if it has
>> made it into 4.13 stable yet...
> I think it's this one:
>
> mac80211: fix deadlock in driver-managed RX BA session start
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bde59c475e0883e4c4294bcd9b9c7e08ae18c828
>
> Adding linux-wireless so that someone can correct me in case I'm wrong.
i believe thats the correct one
>
--
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
^ permalink raw reply
* [PATCH] mwifiex: double the size of chan_stats array in adapter
From: Ganapathi Bhat @ 2017-10-04 12:06 UTC (permalink / raw)
To: linux-wireless
Cc: Brian Norris, Cathy Luo, Xinming Hu, Zhiyuan Yang, James Cao,
Mangesh Malusare, Rohit Fule, Ganapathi Bhat
From: Rohit Fule <rohitf@marvell.com>
When a user requests scan, driver sends multiple scan requests
to firmware, which might be active or passive. Firmware will
send channel statistics for each channel in the request. This will
be stored in chan_stats array.
Few channels might report hidden SSIDs in passive scan results.
So, once the original scan request is finished, driver issues an
active scan request for all channels which reported hidden SSIDs.
This will cause duplicates in the chan_stats array. At worst,
every channel will have a hidden SSID, in which case the driver
can issue active scan requests for each channel. So the complete
scan statistics size will be twice of existing limit.
At present maximum number of channels returned in scan statistics
is 31(BG) + 14(A) = 45. Clearly there will be an overflow of the
chan_stats array in the above mentioned scenario. To fix this
double the size of chan_stats array.
Signed-off-by: Rohit Fule <rohitf@marvell.com>
Signed-off-by: Mangesh Malusare <mmangesh@marvell.com>
Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
---
drivers/net/wireless/marvell/mwifiex/cfg80211.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index ac01af4..f33ed79 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -4201,7 +4201,10 @@ int mwifiex_init_channel_scan_gap(struct mwifiex_adapter *adapter)
if (adapter->config_bands & BAND_A)
n_channels_a = mwifiex_band_5ghz.n_channels;
- adapter->num_in_chan_stats = n_channels_bg + n_channels_a;
+ /* allocate twice the number total channels, since the driver issues an
+ * additional active scan request for hidden SSIDs on passive channels.
+ */
+ adapter->num_in_chan_stats = 2 * (n_channels_bg + n_channels_a);
adapter->chan_stats = vmalloc(sizeof(*adapter->chan_stats) *
adapter->num_in_chan_stats);
--
1.9.1
^ permalink raw reply related
* Re: QCA988x and kernel 4.13
From: Ben Greear @ 2017-10-04 15:13 UTC (permalink / raw)
To: Kalle Valo
Cc: Sebastian Gottschall, svp, ath10k@lists.infradead.org,
linux-wireless@vger.kernel.org
In-Reply-To: <87d163xpoe.fsf@kamboji.qca.qualcomm.com>
On 10/04/2017 01:50 AM, Kalle Valo wrote:
> Ben Greear <greearb@candelatech.com> writes:
>
>> On 10/01/2017 07:06 AM, Sebastian Gottschall wrote:
>>> you have to update to the latest mac80211 code. there is a well
>>> known bug in mac80211 which causes a deadlock with ath10k
>>
>> Do you have a pointer to the patch that fixes this? I'd like to check if it has
>> made it into 4.13 stable yet...
>
> I think it's this one:
>
> mac80211: fix deadlock in driver-managed RX BA session start
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bde59c475e0883e4c4294bcd9b9c7e08ae18c828
>
> Adding linux-wireless so that someone can correct me in case I'm wrong.
That did seem to fix my problems...
Thanks,
ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH 00/11] SDIO support for ath10k
From: Erik Stromdahl @ 2017-10-04 15:53 UTC (permalink / raw)
To: Alagu Sankar, silexcommon, ath10k; +Cc: linux-wireless
In-Reply-To: <59D47E1B.5000009@silex-india.com>
On 2017-10-04 08:22, Alagu Sankar wrote:
> Hi Erik,
>
> We will work to have this support mainlined as soon as possible. Would appreciate your support
> in making sure that the patches do not affect the USB high latency path.
>
I have added the patches in my own ath-repo and I have tested with the
WUSB6100M without any problems.
> On 02-10-2017 14:32, Erik Stromdahl wrote:
>> Hi Alagu,
>>
>> It is great to see that we are finally about have fully working
>> mainline support for QCA9377 SDIO chipsets!
>>
>> Great job!
>>
>> On 2017-09-30 19:37, silexcommon@gmail.com wrote:
>>> From: Alagu Sankar <alagusankar@silex-india.com>
>>>
>>> This patchset, generated against master-pending branch, enables a fully
>>> functional SDIO interface driver for ath10k. Patches have been verified on
>>> QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station, Access Point
>>> and P2P modes.
>>>
>>> The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1
>>> with the board data from respective SDIO card vendors. Receive performance
>>> matches the QCA reference driver when used with SDIO3.0 enabled platforms.
>>> iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s
>>>
>> Can you share any scripts etc. (wrapping hostapd and wpa_supplicant stuff)
>> or provide some more info about you test setup?
>>
> I am not using any specific scripts for this. The standard ones from the ath10k configuration page
> https://wireless.wiki.kernel.org/en/users/drivers/ath10k/configuration works just fine with the
> NL80211 path.
>> I made a quick socat based test on an old laptop (I don't think it has SDIO
>> 3.0 support) and I did unfortunately not get the same figures as you did :(
>>
> If it is SDIO v1.x, then the max bus speed is only 100Mbit/s. With v2.x it is 200Mbit/s and 3.x it is
> 832 Mbit/s. Throughput primarily depends on it. We used i.MX6 SoloX Sabre SDB platform
> which supports SDIO3.x and could see the maximum performance.
>>> This patchset differs from the previous high latency patches, specific to SDIO.
>>> HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET is enabled for HI_ACS. This instructs the
>>> firmware to use HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing packets. Without
>>> this flag, the management frames are not sent out by the firmware. Possibility
>>> of management frames being sent via WMI and data frames through the reduced Tx
>>> completion needs to be probed further.
>>>
>> Ah, so that explains why I couldn't see any messages in the air.
>>
>>> Further improvements can be done on the transmit path by implementing packet
>>> bundle. Scatter Gather is another area of improvement for both Transmit and
>>> Receive, but may not work on all platforms
>>>
>>> Known issues: Surprise removal of the card, when the device is in connected
>>> state, delays sdio function remove due to delayed WMI command failures.
>>> Existing ath10k framework can not differentiate between a kernel module
>>> removal and the surprise removal of teh card.
>>>
>>> Alagu Sankar (11):
>>> ath10k_sdio: sdio htt data transfer fixes
>>> ath10k_sdio: wb396 reference card fix
>>> ath10k_sdio: DMA bounce buffers for read write
>>> ath10k_sdio: reduce transmit msdu count
>>> ath10k_sdio: use clean packet headers
>>> ath10k_sdio: high latency fixes for beacon buffer
>>> ath10k_sdio: fix rssi indication
>>> ath10k_sdio: common read write
>>> ath10k_sdio: virtual scatter gather for receive
>>> ath10k_sdio: enable firmware crash dump
>>> ath10k_sdio: hif start once addition
>>>
>>> drivers/net/wireless/ath/ath10k/core.c | 35 ++-
>>> drivers/net/wireless/ath/ath10k/debug.c | 3 +
>>> drivers/net/wireless/ath/ath10k/htc.c | 4 +-
>>> drivers/net/wireless/ath/ath10k/htc.h | 1 +
>>> drivers/net/wireless/ath/ath10k/htt_rx.c | 19 +-
>>> drivers/net/wireless/ath/ath10k/htt_tx.c | 24 +-
>>> drivers/net/wireless/ath/ath10k/hw.c | 2 +
>>> drivers/net/wireless/ath/ath10k/hw.h | 1 +
>>> drivers/net/wireless/ath/ath10k/mac.c | 31 ++-
>>> drivers/net/wireless/ath/ath10k/sdio.c | 398 ++++++++++++++++++++++--------
>>> drivers/net/wireless/ath/ath10k/sdio.h | 10 +-
>>> drivers/net/wireless/ath/ath10k/wmi-tlv.c | 2 +-
>>> 12 files changed, 403 insertions(+), 127 deletions(-)
>>>
>>
>
^ permalink raw reply
* [PATCH 0/3] iwlwifi: cosmetic fixes
From: Christoph Böhmwalder @ 2017-10-04 15:56 UTC (permalink / raw)
To: johannes.berg, emmanuel.grumbach, luciano.coelho, kvalo
Cc: linux-wireless, netdev, linux-kernel, Christoph Böhmwalder
Fix several code style issues, some of which were reported by checkpatch.pl.
The changes are:
* One instance of an `int` variable being used in a boolean context, chaned to
use the more appropriate `bool` type.
* One very minor fix, removing a newline between a function definition and its
associated `static` keyword
* One fix wrapping a macro in curly braces
Christoph Böhmwalder (3):
wireless: iwlwifi: use bool instead of int
wireless: iwlwifi: function definition cosmetic fix
wireless: iwlwifi: wrap macro into braces
drivers/net/wireless/intel/iwlwifi/iwl-io.c | 2 +-
drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 16 +++++++---------
2 files changed, 8 insertions(+), 10 deletions(-)
--
2.13.5
^ permalink raw reply
* [PATCH 1/3] wireless: iwlwifi: use bool instead of int
From: Christoph Böhmwalder @ 2017-10-04 15:56 UTC (permalink / raw)
To: johannes.berg, emmanuel.grumbach, luciano.coelho, kvalo
Cc: linux-wireless, netdev, linux-kernel, Christoph Böhmwalder
In-Reply-To: <20171004155700.18048-1-christoph@boehmwalder.at>
Change a usage of int in a boolean context to use the bool type instead, as it
makes the intent of the function clearer and helps clarify its semantics.
Also eliminate the if/else and just return the boolean result directly,
making the code more readable.
Signed-off-by: Christoph Böhmwalder <christoph@boehmwalder.at>
---
drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
index b7cd813ba70f..0eb815ae97e8 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
@@ -267,14 +267,12 @@ int iwl_phy_db_set_section(struct iwl_phy_db *phy_db,
}
IWL_EXPORT_SYMBOL(iwl_phy_db_set_section);
-static int is_valid_channel(u16 ch_id)
+static bool is_valid_channel(u16 ch_id)
{
- if (ch_id <= 14 ||
- (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
- (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
- (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1))
- return 1;
- return 0;
+ return (ch_id <= 14 ||
+ (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
+ (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
+ (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1));
}
static u8 ch_id_to_ch_index(u16 ch_id)
--
2.13.5
^ permalink raw reply related
* [PATCH 2/3] wireless: iwlwifi: function definition cosmetic fix
From: Christoph Böhmwalder @ 2017-10-04 15:56 UTC (permalink / raw)
To: johannes.berg, emmanuel.grumbach, luciano.coelho, kvalo
Cc: linux-wireless, netdev, linux-kernel, Christoph Böhmwalder
In-Reply-To: <20171004155700.18048-1-christoph@boehmwalder.at>
Separate the function from the previous definition with a newline and
put the `static` keyword on the same line, as it just looks nicer.
Signed-off-by: Christoph Böhmwalder <christoph@boehmwalder.at>
---
drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
index 0eb815ae97e8..249ee1c7b02f 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
@@ -325,8 +325,8 @@ static u16 channel_id_to_txp(struct iwl_phy_db *phy_db, u16 ch_id)
}
return 0xff;
}
-static
-int iwl_phy_db_get_section_data(struct iwl_phy_db *phy_db,
+
+static int iwl_phy_db_get_section_data(struct iwl_phy_db *phy_db,
u32 type, u8 **data, u16 *size, u16 ch_id)
{
struct iwl_phy_db_entry *entry;
--
2.13.5
^ permalink raw reply related
* [PATCH 3/3] wireless: iwlwifi: wrap macro into braces
From: Christoph Böhmwalder @ 2017-10-04 15:57 UTC (permalink / raw)
To: johannes.berg, emmanuel.grumbach, luciano.coelho, kvalo
Cc: linux-wireless, netdev, linux-kernel, Christoph Böhmwalder
In-Reply-To: <20171004155700.18048-1-christoph@boehmwalder.at>
Macros should always be wrapped in braces, so fix this instance.
Signed-off-by: Christoph Böhmwalder <christoph@boehmwalder.at>
---
drivers/net/wireless/intel/iwlwifi/iwl-io.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-io.c b/drivers/net/wireless/intel/iwlwifi/iwl-io.c
index efb1998dcabd..0211963b3e71 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-io.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-io.c
@@ -252,7 +252,7 @@ IWL_EXPORT_SYMBOL(iwl_force_nmi);
static const char *get_rfh_string(int cmd)
{
-#define IWL_CMD(x) case x: return #x
+#define IWL_CMD(x) { case x: return #x; }
#define IWL_CMD_MQ(arg, reg, q) { if (arg == reg(q)) return #reg; }
int i;
--
2.13.5
^ permalink raw reply related
* Re: [PATCH 3/3] wireless: iwlwifi: wrap macro into braces
From: Johannes Berg @ 2017-10-04 16:08 UTC (permalink / raw)
To: Christoph Böhmwalder, johannes.berg, emmanuel.grumbach,
luciano.coelho, kvalo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20171004155700.18048-4-christoph@boehmwalder.at>
Please don't send obviously broken patches.
johannes
^ permalink raw reply
* Re: [PATCH 2/3] wireless: iwlwifi: function definition cosmetic fix
From: Luciano Coelho @ 2017-10-04 16:13 UTC (permalink / raw)
To: Christoph Böhmwalder, johannes.berg, emmanuel.grumbach,
kvalo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20171004155700.18048-3-christoph@boehmwalder.at>
On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> Separate the function from the previous definition with a newline and
> put the `static` keyword on the same line, as it just looks nicer.
>
> Signed-off-by: Christoph Böhmwalder <christoph@boehmwalder.at>
> ---
> drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> index 0eb815ae97e8..249ee1c7b02f 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> @@ -325,8 +325,8 @@ static u16 channel_id_to_txp(struct iwl_phy_db *phy_db, u16 ch_id)
> }
> return 0xff;
> }
> -static
> -int iwl_phy_db_get_section_data(struct iwl_phy_db *phy_db,
> +
> +static int iwl_phy_db_get_section_data(struct iwl_phy_db *phy_db,
> u32 type, u8 **data, u16 *size, u16 ch_id)
> {
> struct iwl_phy_db_entry *entry;
Sorry, but this now looks much uglier because the second line is not
even aligned to the parenthesis.
NACK.
--
Luca.
^ permalink raw reply
* Re: [PATCH 3/3] wireless: iwlwifi: wrap macro into braces
From: Joe Perches @ 2017-10-04 16:14 UTC (permalink / raw)
To: Christoph Böhmwalder, johannes.berg, emmanuel.grumbach,
luciano.coelho, kvalo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20171004155700.18048-4-christoph@boehmwalder.at>
On Wed, 2017-10-04 at 17:57 +0200, Christoph Böhmwalder wrote:
> Macros should always be wrapped in braces, so fix this instance.
>
> Signed-off-by: Christoph Böhmwalder <christoph@boehmwalder.at>
> ---
> drivers/net/wireless/intel/iwlwifi/iwl-io.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-io.c b/drivers/net/wireless/intel/iwlwifi/iwl-io.c
> index efb1998dcabd..0211963b3e71 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-io.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-io.c
> @@ -252,7 +252,7 @@ IWL_EXPORT_SYMBOL(iwl_force_nmi);
>
> static const char *get_rfh_string(int cmd)
> {
> -#define IWL_CMD(x) case x: return #x
> +#define IWL_CMD(x) { case x: return #x; }
I think this unnecessary. Maybe:
http://lists.infradead.org/pipermail/ath10k/2017-February/009335.html
> #define IWL_CMD_MQ(arg, reg, q) { if (arg == reg(q)) return #reg; }
But this should use do { ... } while (0)
^ permalink raw reply
* Re: [PATCH 1/3] wireless: iwlwifi: use bool instead of int
From: Luciano Coelho @ 2017-10-04 16:15 UTC (permalink / raw)
To: Christoph Böhmwalder, johannes.berg, emmanuel.grumbach,
kvalo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20171004155700.18048-2-christoph@boehmwalder.at>
On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> Change a usage of int in a boolean context to use the bool type
> instead, as it
> makes the intent of the function clearer and helps clarify its
> semantics.
>
> Also eliminate the if/else and just return the boolean result
> directly,
> making the code more readable.
>
> Signed-off-by: Christoph Böhmwalder <christoph@boehmwalder.at>
> ---
> drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> index b7cd813ba70f..0eb815ae97e8 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> @@ -267,14 +267,12 @@ int iwl_phy_db_set_section(struct iwl_phy_db
> *phy_db,
> }
> IWL_EXPORT_SYMBOL(iwl_phy_db_set_section);
>
> -static int is_valid_channel(u16 ch_id)
> +static bool is_valid_channel(u16 ch_id)
> {
> - if (ch_id <= 14 ||
> - (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> - (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> - (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1))
> - return 1;
> - return 0;
> + return (ch_id <= 14 ||
> + (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> + (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> + (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1));
> }
>
> static u8 ch_id_to_ch_index(u16 ch_id)
This actually makes some sense, and I would probably apply it if it
were part of a patchset that actually does something useful.
--
Luca.
^ permalink raw reply
* Re: [PATCH 0/3] iwlwifi: cosmetic fixes
From: Luciano Coelho @ 2017-10-04 16:21 UTC (permalink / raw)
To: Christoph Böhmwalder, johannes.berg, emmanuel.grumbach,
kvalo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20171004155700.18048-1-christoph@boehmwalder.at>
On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> Fix several code style issues, some of which were reported by
> checkpatch.pl.
>
> The changes are:
> * One instance of an `int` variable being used in a boolean context,
> chaned to
> use the more appropriate `bool` type.
> * One very minor fix, removing a newline between a function
> definition and its
> associated `static` keyword
> * One fix wrapping a macro in curly braces
>
>
> Christoph Böhmwalder (3):
> wireless: iwlwifi: use bool instead of int
> wireless: iwlwifi: function definition cosmetic fix
> wireless: iwlwifi: wrap macro into braces
>
> drivers/net/wireless/intel/iwlwifi/iwl-io.c | 2 +-
> drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 16 +++++++------
> ---
> 2 files changed, 8 insertions(+), 10 deletions(-)
Sorry, but this kind of series just generates churn. Especially when 2
out of 3 patches are broken. I applied your previous patch because it
was really trivial, but I really don't want to encourage this kind of
drive-by "fixes" that only cause additional work.
I generally only accept this kind of changes when people are changing
code close or related to it.
--
Luca.
^ permalink raw reply
* Re: [PATCH 1/3] wireless: iwlwifi: use bool instead of int
From: Joe Perches @ 2017-10-04 16:26 UTC (permalink / raw)
To: Christoph Böhmwalder, johannes.berg, emmanuel.grumbach,
luciano.coelho, kvalo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20171004155700.18048-2-christoph@boehmwalder.at>
On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> Change a usage of int in a boolean context to use the bool type instead, as it
> makes the intent of the function clearer and helps clarify its semantics.
>
> Also eliminate the if/else and just return the boolean result directly,
> making the code more readable.
>
> Signed-off-by: Christoph Böhmwalder <christoph@boehmwalder.at>
> ---
> drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> index b7cd813ba70f..0eb815ae97e8 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> @@ -267,14 +267,12 @@ int iwl_phy_db_set_section(struct iwl_phy_db *phy_db,
> }
> IWL_EXPORT_SYMBOL(iwl_phy_db_set_section);
>
> -static int is_valid_channel(u16 ch_id)
> +static bool is_valid_channel(u16 ch_id)
> {
> - if (ch_id <= 14 ||
> - (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> - (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> - (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1))
> - return 1;
> - return 0;
> + return (ch_id <= 14 ||
> + (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> + (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> + (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1));
> }
This might be more intelligble as separate tests
static bool is_valid_channel(u16 ch_id)
{
if (ch_id <= 14)
return true;
if ((ch_id % 4 == 0) &&
((ch_id >= 36 && ch_id <= 64) ||
(ch_id >= 100 && ch_id <= 140)))
return true;
if ((ch_id % 4 == 1) &&
(chid >= 145 && ch_id <= 165))
return true;
return false;
}
The compiler should produce the same object code.
^ permalink raw reply
* Re: QCA988x and kernel 4.13
From: Sebastian Gottschall @ 2017-10-04 16:35 UTC (permalink / raw)
To: Ben Greear, Kalle Valo
Cc: svp, ath10k@lists.infradead.org, linux-wireless@vger.kernel.org
In-Reply-To: <78f5383d-0187-628e-cf56-de6234fdb9b0@candelatech.com>
Am 04.10.2017 um 17:13 schrieb Ben Greear:
>
>
> On 10/04/2017 01:50 AM, Kalle Valo wrote:
>> Ben Greear <greearb@candelatech.com> writes:
>>
>>> On 10/01/2017 07:06 AM, Sebastian Gottschall wrote:
>>>> you have to update to the latest mac80211 code. there is a well
>>>> known bug in mac80211 which causes a deadlock with ath10k
>>>
>>> Do you have a pointer to the patch that fixes this? I'd like to
>>> check if it has
>>> made it into 4.13 stable yet...
>>
>> I think it's this one:
>>
>> mac80211: fix deadlock in driver-managed RX BA session start
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bde59c475e0883e4c4294bcd9b9c7e08ae18c828
>>
>>
>> Adding linux-wireless so that someone can correct me in case I'm wrong.
>
> That did seem to fix my problems...
>
> Thanks,
> ben
>
consider that this bug does at least exist since may. so older kernels
might be affected too
--
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
^ permalink raw reply
* Re: [PATCH 1/3] wireless: iwlwifi: use bool instead of int
From: Luciano Coelho @ 2017-10-04 16:39 UTC (permalink / raw)
To: Joe Perches, Christoph Böhmwalder, johannes.berg,
emmanuel.grumbach, kvalo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <1507134405.4434.10.camel@perches.com>
On Wed, 2017-10-04 at 09:26 -0700, Joe Perches wrote:
> On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> > Change a usage of int in a boolean context to use the bool type
> > instead, as it
> > makes the intent of the function clearer and helps clarify its
> > semantics.
> >
> > Also eliminate the if/else and just return the boolean result
> > directly,
> > making the code more readable.
> >
> > Signed-off-by: Christoph Böhmwalder <christoph@boehmwalder.at>
> > ---
> > drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 12 +++++-------
> > 1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > index b7cd813ba70f..0eb815ae97e8 100644
> > --- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > +++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > @@ -267,14 +267,12 @@ int iwl_phy_db_set_section(struct iwl_phy_db
> > *phy_db,
> > }
> > IWL_EXPORT_SYMBOL(iwl_phy_db_set_section);
> >
> > -static int is_valid_channel(u16 ch_id)
> > +static bool is_valid_channel(u16 ch_id)
> > {
> > - if (ch_id <= 14 ||
> > - (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> > - (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> > - (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1))
> > - return 1;
> > - return 0;
> > + return (ch_id <= 14 ||
> > + (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> > + (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> > + (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1));
> > }
>
> This might be more intelligble as separate tests
>
> static bool is_valid_channel(u16 ch_id)
> {
> if (ch_id <= 14)
> return true;
>
> if ((ch_id % 4 == 0) &&
> ((ch_id >= 36 && ch_id <= 64) ||
> (ch_id >= 100 && ch_id <= 140)))
> return true;
>
> if ((ch_id % 4 == 1) &&
> (chid >= 145 && ch_id <= 165))
> return true;
>
> return false;
> }
>
> The compiler should produce the same object code.
Yeah, it may be a bit easier to read, but I don't want to start getting
"fixes" to working and reasonable code. There's nothing wrong with the
existing function (except maybe for the int vs. boolean) so let's not
change it.
A good time to change this would be the next time someone adds yet
another range of valid channels here. ;)
--
Luca.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox