* [PATCHv8] wlcore: spi: add wl18xx support
From: Reizer, Eyal @ 2016-07-20 6:58 UTC (permalink / raw)
To: Kalle Valo
Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-spi@vger.kernel.org, Rob Herring
In-Reply-To: <1468994768-23609-1-git-send-email-eyalr@ti.com>
Add support for using with both wl12xx and wl18xx.
- all wilink family needs special init command for entering wspi mode.
extra clock cycles should be sent after the spi init command while the
cs pin is high.
- Use inverted chip select for sending a dummy 4 bytes command that
completes the init stage.
Signed-off-by: Eyal Reizer <eyalr@ti.com>
Acked-by: Rob Herring <robh@kernel.org>
---
v1->v2:update device tree bindings configuration
v2->v3:revert from manual gpio manipulation. use inverted chip select instead
for sending the extra init cycle which, achieves the same hardware purpose.
update device tree bindings docucmentation accordingly
v3->v4: Remove redundant data form binding documentation and fix chip select
number mismatch in wl1271 example
v4->v5: Rebase on top of head of wireless-drivers-next
v5->v6: Add ACKs
v6->v7: Mail format issues
v7->v8: Remove redundant varaible from wlcore_probe_of
.../bindings/net/wireless/ti,wlcore,spi.txt | 41 +++++--
drivers/net/wireless/ti/wlcore/spi.c | 124 +++++++++++++++++----
2 files changed, 137 insertions(+), 28 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
index 9180724..8f9ced0 100644
--- a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
@@ -1,19 +1,30 @@
-* Texas Instruments wl1271 wireless lan controller
+* Texas Instruments wl12xx/wl18xx wireless lan controller
-The wl1271 chip can be connected via SPI or via SDIO. This
+The wl12xx/wl18xx chips can be connected via SPI or via SDIO. This
document describes the binding for the SPI connected chip.
Required properties:
-- compatible : Should be "ti,wl1271"
+- compatible : Should be one of the following:
+ * "ti,wl1271"
+ * "ti,wl1273"
+ * "ti,wl1281"
+ * "ti,wl1283"
+ * "ti,wl1801"
+ * "ti,wl1805"
+ * "ti,wl1807"
+ * "ti,wl1831"
+ * "ti,wl1835"
+ * "ti,wl1837"
- reg : Chip select address of device
- spi-max-frequency : Maximum SPI clocking speed of device in Hz
-- ref-clock-frequency : Reference clock frequency
- interrupt-parent, interrupts :
Should contain parameters for 1 interrupt line.
Interrupt parameters: parent, line number, type.
-- vwlan-supply : Point the node of the regulator that powers/enable the wl1271 chip
+- vwlan-supply : Point the node of the regulator that powers/enable the
+ wl12xx/wl18xx chip
Optional properties:
+- ref-clock-frequency : Reference clock frequency (should be set for wl12xx)
- clock-xtal : boolean, clock is generated from XTAL
- Please consult Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -21,16 +32,28 @@ Optional properties:
Examples:
+For wl12xx family:
&spi1 {
- wl1271@1 {
+ wlcore: wlcore@1 {
compatible = "ti,wl1271";
-
reg = <1>;
spi-max-frequency = <48000000>;
- clock-xtal;
- ref-clock-frequency = <38400000>;
interrupt-parent = <&gpio3>;
interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
vwlan-supply = <&vwlan_fixed>;
+ clock-xtal;
+ ref-clock-frequency = <38400000>;
+ };
+};
+
+For wl18xx family:
+&spi0 {
+ wlcore: wlcore@0 {
+ compatible = "ti,wl1835";
+ reg = <0>;
+ spi-max-frequency = <48000000>;
+ interrupt-parent = <&gpio0>;
+ interrupts = <27 IRQ_TYPE_EDGE_RISING>;
+ vwlan-supply = <&vwlan_fixed>;
};
};
diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
index cea9443..6d24040 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -70,16 +70,30 @@
#define WSPI_MAX_CHUNK_SIZE 4092
/*
- * only support SPI for 12xx - this code should be reworked when 18xx
- * support is introduced
+ * wl18xx driver aggregation buffer size is (13 * PAGE_SIZE) compared to
+ * (4 * PAGE_SIZE) for wl12xx, so use the larger buffer needed for wl18xx
*/
-#define SPI_AGGR_BUFFER_SIZE (4 * PAGE_SIZE)
+#define SPI_AGGR_BUFFER_SIZE (13 * PAGE_SIZE)
/* Maximum number of SPI write chunks */
#define WSPI_MAX_NUM_OF_CHUNKS \
((SPI_AGGR_BUFFER_SIZE / WSPI_MAX_CHUNK_SIZE) + 1)
+struct wilink_familiy_data {
+ char name[8];
+};
+
+const struct wilink_familiy_data *wilink_data;
+
+static const struct wilink_familiy_data wl18xx_data = {
+ .name = "wl18xx",
+};
+
+static const struct wilink_familiy_data wl12xx_data = {
+ .name = "wl12xx",
+};
+
struct wl12xx_spi_glue {
struct device *dev;
struct platform_device *core;
@@ -119,6 +133,7 @@ static void wl12xx_spi_init(struct device *child)
struct wl12xx_spi_glue *glue = dev_get_drvdata(child->parent);
struct spi_transfer t;
struct spi_message m;
+ struct spi_device *spi = to_spi_device(glue->dev);
u8 *cmd = kzalloc(WSPI_INIT_CMD_LEN, GFP_KERNEL);
if (!cmd) {
@@ -151,6 +166,7 @@ static void wl12xx_spi_init(struct device *child)
cmd[6] |= WSPI_INIT_CMD_EN_FIXEDBUSY;
cmd[7] = crc7_be(0, cmd+2, WSPI_INIT_CMD_CRC_LEN) | WSPI_INIT_CMD_END;
+
/*
* The above is the logical order; it must actually be stored
* in the buffer byte-swapped.
@@ -163,6 +179,28 @@ static void wl12xx_spi_init(struct device *child)
spi_message_add_tail(&t, &m);
spi_sync(to_spi_device(glue->dev), &m);
+
+ /* Send extra clocks with inverted CS (high). this is required
+ * by the wilink family in order to successfully enter WSPI mode.
+ */
+ spi->mode ^= SPI_CS_HIGH;
+ memset(&m, 0, sizeof(m));
+ spi_message_init(&m);
+
+ cmd[0] = 0xff;
+ cmd[1] = 0xff;
+ cmd[2] = 0xff;
+ cmd[3] = 0xff;
+ __swab32s((u32 *)cmd);
+
+ t.tx_buf = cmd;
+ t.len = 4;
+ spi_message_add_tail(&t, &m);
+
+ spi_sync(to_spi_device(glue->dev), &m);
+
+ /* Restore chip select configration to normal */
+ spi->mode ^= SPI_CS_HIGH;
kfree(cmd);
}
@@ -270,22 +308,25 @@ static int __must_check wl12xx_spi_raw_read(struct device *child, int addr,
return 0;
}
-static int __must_check wl12xx_spi_raw_write(struct device *child, int addr,
- void *buf, size_t len, bool fixed)
+static int __wl12xx_spi_raw_write(struct device *child, int addr,
+ void *buf, size_t len, bool fixed)
{
struct wl12xx_spi_glue *glue = dev_get_drvdata(child->parent);
- /* SPI write buffers - 2 for each chunk */
- struct spi_transfer t[2 * WSPI_MAX_NUM_OF_CHUNKS];
+ struct spi_transfer *t;
struct spi_message m;
u32 commands[WSPI_MAX_NUM_OF_CHUNKS]; /* 1 command per chunk */
u32 *cmd;
u32 chunk_len;
int i;
+ /* SPI write buffers - 2 for each chunk */
+ t = kzalloc(sizeof(*t) * 2 * WSPI_MAX_NUM_OF_CHUNKS, GFP_KERNEL);
+ if (!t)
+ return -ENOMEM;
+
WARN_ON(len > SPI_AGGR_BUFFER_SIZE);
spi_message_init(&m);
- memset(t, 0, sizeof(t));
cmd = &commands[0];
i = 0;
@@ -318,9 +359,26 @@ static int __must_check wl12xx_spi_raw_write(struct device *child, int addr,
spi_sync(to_spi_device(glue->dev), &m);
+ kfree(t);
return 0;
}
+static int __must_check wl12xx_spi_raw_write(struct device *child, int addr,
+ void *buf, size_t len, bool fixed)
+{
+ int ret;
+
+ /* The ELP wakeup write may fail the first time due to internal
+ * hardware latency. It is safer to send the wakeup command twice to
+ * avoid unexpected failures.
+ */
+ if (addr == HW_ACCESS_ELP_CTRL_REG)
+ ret = __wl12xx_spi_raw_write(child, addr, buf, len, fixed);
+ ret = __wl12xx_spi_raw_write(child, addr, buf, len, fixed);
+
+ return ret;
+}
+
/**
* wl12xx_spi_set_power - power on/off the wl12xx unit
* @child: wl12xx device handle.
@@ -349,17 +407,38 @@ static int wl12xx_spi_set_power(struct device *child, bool enable)
return ret;
}
+/**
+ * wl12xx_spi_set_block_size
+ *
+ * This function is not needed for spi mode, but need to be present.
+ * Without it defined the wlcore fallback to use the wrong packet
+ * allignment on tx.
+ */
+static void wl12xx_spi_set_block_size(struct device *child,
+ unsigned int blksz)
+{
+}
+
static struct wl1271_if_operations spi_ops = {
.read = wl12xx_spi_raw_read,
.write = wl12xx_spi_raw_write,
.reset = wl12xx_spi_reset,
.init = wl12xx_spi_init,
.power = wl12xx_spi_set_power,
- .set_block_size = NULL,
+ .set_block_size = wl12xx_spi_set_block_size,
};
static const struct of_device_id wlcore_spi_of_match_table[] = {
- { .compatible = "ti,wl1271" },
+ { .compatible = "ti,wl1271", .data = &wl12xx_data},
+ { .compatible = "ti,wl1273", .data = &wl12xx_data},
+ { .compatible = "ti,wl1281", .data = &wl12xx_data},
+ { .compatible = "ti,wl1283", .data = &wl12xx_data},
+ { .compatible = "ti,wl1801", .data = &wl18xx_data},
+ { .compatible = "ti,wl1805", .data = &wl18xx_data},
+ { .compatible = "ti,wl1807", .data = &wl18xx_data},
+ { .compatible = "ti,wl1831", .data = &wl18xx_data},
+ { .compatible = "ti,wl1835", .data = &wl18xx_data},
+ { .compatible = "ti,wl1837", .data = &wl18xx_data},
{ }
};
MODULE_DEVICE_TABLE(of, wlcore_spi_of_match_table);
@@ -375,18 +454,24 @@ static int wlcore_probe_of(struct spi_device *spi, struct wl12xx_spi_glue *glue,
struct wlcore_platdev_data *pdev_data)
{
struct device_node *dt_node = spi->dev.of_node;
- int ret;
+ const struct of_device_id *of_id;
+
+ of_id = of_match_node(wlcore_spi_of_match_table, dt_node);
+ if (!of_id)
+ return -ENODEV;
+
+ wilink_data = of_id->data;
+ dev_info(&spi->dev, "selected chip familiy is %s\n",
+ wilink_data->name);
if (of_find_property(dt_node, "clock-xtal", NULL))
pdev_data->ref_clock_xtal = true;
- ret = of_property_read_u32(dt_node, "ref-clock-frequency",
- &pdev_data->ref_clock_freq);
- if (ret) {
- dev_err(glue->dev,
- "can't get reference clock frequency (%d)\n", ret);
- return ret;
- }
+ /* optional clock frequency params */
+ of_property_read_u32(dt_node, "ref-clock-frequency",
+ &pdev_data->ref_clock_freq);
+ of_property_read_u32(dt_node, "tcxo-clock-frequency",
+ &pdev_data->tcxo_clock_freq);
return 0;
}
@@ -437,7 +522,8 @@ static int wl1271_probe(struct spi_device *spi)
return ret;
}
- glue->core = platform_device_alloc("wl12xx", PLATFORM_DEVID_AUTO);
+ glue->core = platform_device_alloc(wilink_data->name,
+ PLATFORM_DEVID_AUTO);
if (!glue->core) {
dev_err(glue->dev, "can't allocate platform_device\n");
return -ENOMEM;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2 2/3] mac80211: mesh: improve path resolving time
From: Yeoh Chun-Yeow @ 2016-07-20 6:45 UTC (permalink / raw)
To: Machani, Yaniv
Cc: Bob Copeland, linux-wireless@vger.kernel.org, Hahn, Maital,
Johannes Berg
In-Reply-To: <AE1C82FB3D0EC64DB1F752C81CBD110139206BD6@DFRE01.ent.ti.com>
On Wed, Jul 20, 2016 at 4:01 AM, Machani, Yaniv <yanivma@ti.com> wrote:
> On Tue, Jul 19, 2016 at 19:01:54, Yeoh Chun-Yeow wrote:
>> Johannes Berg
>> Subject: Re: [PATCH v2 2/3] mac80211: mesh: improve path resolving
>> time
>>
>> On Tue, Jul 19, 2016 at 9:43 PM, Bob Copeland <me@bobcopeland.com>
>> wrote:
>> > On Tue, Jul 19, 2016 at 01:02:13PM +0000, Machani, Yaniv wrote:
>> >> On Tue, Jul 19, 2016 at 15:44:56, Bob Copeland wrote:
>> >
>> > This wording seems to indicate that it is not per path. Perhaps
>> > this should be clarified in the standard. (If the intent turns out
>> > to be per path, then I guess we should fix it by storing last_preq
>> > per path
>> > instead.)
>> >
>> >
>> > Ignoring the standard for a second, let's explore this: can you give
>> > some idea on how many stations are in your target network, how
>> > frequently a given pair of nodes unpeer, what sort of improvements
>> > you see with the patch? It should then be pretty easy to run some
>> > simulations to see the scenarios where this helps and where it hurts.
>> >
>>
>
> Bob, Chun-Yeow,
> Thanks for your inputs.
>
> Let take a simple scenario, where you have a,b,c,d nodes connected to each other as shown below.
>
> A~ ~ ~~~~C- - - D
> \ /
> \ /
> B
>
> A would like to pass data to D.
> A-C matric is worse than A-B-C, so path is constructed via B.
> We are in idle state, and PREQ are sent every dot11MeshHWMPpreqMinInterval.
> Now, node B have been disconnected (ran out of battery/shut down/suddenly went out of range)
> As A has a path to D via B, he cleans up his path table.
> Now he needs to build a new path, in the WCS, he have just sent a PREQ.
> And now he needs to wait dot11MeshHWMPpreqMinInterval seconds, until he can rebuild the path.
Did you reduce the dot11MeshHWMPactivePathTimeout to see whether it
produces positive impact to your network? Once the path to A - C - D
has established, it needs to wait till the active path timer to expire
before establishing a new path. This active path time is default to
5000 TUs (or 5s).
> As we wouldn't like to flood the network with PREQ, we can assume that the dot11MeshHWMPpreqMinInterval is few seconds, for us, it seemed un-acceptable.
>
But your patch is indeed generating "more" PREQ frame.
> In cases where you need to have a reliable data link, passing audio/video you usually can't afford few seconds w/o traffic.
>
>> In addition to Bob's comment, you probably can try to reduce the
>> dot11MeshHWMPpreqMinInterval to 1 TU (1ms) instead of sticking to
>> default value 10 TUs. Besides, you can also reduce the
>> mesh_path_refresh_time which is currently default to 1000 ms and check
>> whether you can improve on your network scenarios.
>>
>
> We did tried to play with these values, but again, we don't want to flood the network.
> We just want to recover ASAP.
>
>> When you mentioned "next hop peer disconnect", it could also be the
>> time taken to re-established the mesh peering before your mesh STA can
>> transmit the data to your peer mesh STA.
>>
>
> Link establishment takes no more than few 100s of ms usually,
> And in the example above, there is no new link establishment, just path generation.
>
Suggest that you simulate your scenario and validate the improvement first.
---
Chun-Yeow
^ permalink raw reply
* RE: linux-next: build warning after merge of the wireless-drivers-next tree
From: Reizer, Eyal @ 2016-07-20 6:18 UTC (permalink / raw)
To: Stephen Rothwell, Kalle Valo, linux-wireless@vger.kernel.org
Cc: linux-next@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20160720122503.09a46d7b@canb.auug.org.au>
Hi Stephen,
>
> After merging the wireless-drivers-next tree, today's linux-next build
> (x86_64 allmodconfig) produced this warning:
>
> drivers/net/wireless/ti/wlcore/spi.c: In function 'wlcore_probe_of':
> drivers/net/wireless/ti/wlcore/spi.c:457:6: warning: unused variable 'ret' [-
> Wunused-variable]
> int ret;
> ^
>
> Introduced by commit
>
> 01efe65aba65 ("wlcore: spi: add wl18xx support")
>
Of course you are right. It is indeed redundant now.
Don't know how I have missed it. Don't remember seeing a warning from the tool chain I have used.
Anyway ,will submit an amended patch soon.
Best Regards,
Eyal
^ permalink raw reply
* [PATCH] NFC: pn533: Avoid a double free in error handling code
From: Christophe JAILLET @ 2016-07-20 5:46 UTC (permalink / raw)
To: lauro.venancio, aloisio.almeida, sameo, michael.thalmeier
Cc: linux-wireless, linux-kernel, kernel-janitors, Christophe JAILLET
'phy' has been allocated with 'devm_kzalloc', so we should not free it
using an explicit 'kfree'. It would result in a double free if the
allocation of 'in_buf' fails.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/nfc/pn533/usb.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
index 8ca0603..33ed78b 100644
--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -464,10 +464,8 @@ static int pn533_usb_probe(struct usb_interface *interface,
return -ENOMEM;
in_buf = kzalloc(in_buf_len, GFP_KERNEL);
- if (!in_buf) {
- rc = -ENOMEM;
- goto out_free_phy;
- }
+ if (!in_buf)
+ return -ENOMEM;
phy->udev = usb_get_dev(interface_to_usbdev(interface));
phy->interface = interface;
@@ -554,8 +552,7 @@ error:
usb_free_urb(phy->out_urb);
usb_put_dev(phy->udev);
kfree(in_buf);
-out_free_phy:
- kfree(phy);
+
return rc;
}
--
2.7.4
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
^ permalink raw reply related
* linux-next: manual merge of the l2-mtd tree with the wireless-drivers-next tree
From: Stephen Rothwell @ 2016-07-20 2:31 UTC (permalink / raw)
To: Brian Norris, Kalle Valo, linux-wireless
Cc: linux-next, linux-kernel, Rafał Miłecki
Hi Brian,
Today's linux-next merge of the l2-mtd tree got a conflict in:
drivers/mtd/devices/Kconfig
between commit:
efacc699139e ("mtd: add arch dependency for MTD_BCM47XXSFLASH symbol")
from the wireless-drivers-next tree and commit:
0a526341fee0 ("mtd: update description of MTD_BCM47XXSFLASH symbol")
from the l2-mtd tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/mtd/devices/Kconfig
index 64a248556d29,bf8238fb2e12..000000000000
--- a/drivers/mtd/devices/Kconfig
+++ b/drivers/mtd/devices/Kconfig
@@@ -113,8 -113,8 +113,8 @@@ config MTD_SST25
if you want to specify device partitioning.
config MTD_BCM47XXSFLASH
- tristate "R/O support for serial flash on BCMA bus"
+ tristate "Support for serial flash on BCMA bus"
- depends on BCMA_SFLASH
+ depends on BCMA_SFLASH && (MIPS || ARM)
help
BCMA bus can have various flash memories attached, they are
registered by bcma as platform devices. This enables driver for
^ permalink raw reply
* linux-next: build warning after merge of the wireless-drivers-next tree
From: Stephen Rothwell @ 2016-07-20 2:25 UTC (permalink / raw)
To: Kalle Valo, linux-wireless; +Cc: linux-next, linux-kernel, Eyal Reizer
Hi all,
After merging the wireless-drivers-next tree, today's linux-next build
(x86_64 allmodconfig) produced this warning:
drivers/net/wireless/ti/wlcore/spi.c: In function 'wlcore_probe_of':
drivers/net/wireless/ti/wlcore/spi.c:457:6: warning: unused variable 'ret' [-Wunused-variable]
int ret;
^
Introduced by commit
01efe65aba65 ("wlcore: spi: add wl18xx support")
--
Cheers,
Stephen Rothwell
^ permalink raw reply
* Re: linux-next: build failure after merge of the wireless-drivers-next tree
From: Stephen Rothwell @ 2016-07-19 23:36 UTC (permalink / raw)
To: Brian Norris
Cc: Kalle Valo, Rafał Miłecki,
linux-mtd@lists.infradead.org, linux-wireless@vger.kernel.org,
linux-next, Linux Kernel Mailing List
In-Reply-To: <20160719183913.GA88186@google.com>
Hi Brian,
On Tue, 19 Jul 2016 11:39:13 -0700 Brian Norris <computersforpeace@gmail.com> wrote:
>
> I applied a trivial change to this same Kconfig entry:
>
> Subject: mtd: update description of MTD_BCM47XXSFLASH symbol
> http://git.infradead.org/l2-mtd.git/commitdiff/0a526341fee054c1e2b9f0e4b2b424ae81707d4c
>
> It's a trivial conflict, so should we just let Linus work it out? I can
> note it my MTD pull request, if wireless lands first.
Linus and I should be able to cope. I will send my usual notification today.
--
Cheers,
Stephen Rothwell
^ permalink raw reply
* Re: [PATCH] NFC: null-ify info->ram_patch when firmware is released
From: Samuel Ortiz @ 2016-07-19 21:28 UTC (permalink / raw)
To: Colin King
Cc: Lauro Ramos Venancio, Aloisio Almeida Jr, Christophe Ricard,
Robert Dolca, linux-wireless, linux-kernel
In-Reply-To: <1468587633-25766-1-git-send-email-colin.king@canonical.com>
Hi Colin,
On Fri, Jul 15, 2016 at 02:00:33PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> When the firmware is released for info->ram_patch currently
> info->otp_patch is being nullified and not info->ram_patch.
> This looks like a cut-n-paste coding error. Fix this by
> nullifing the correct field.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/nfc/fdp/fdp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to nfc-next, thanks.
Cheers,
Samuel.
^ permalink raw reply
* Re: [PATCH 0/4] brcm80211: Misc coverity fixes
From: Florian Fainelli @ 2016-07-19 20:09 UTC (permalink / raw)
To: Arend Van Spriel, brcm80211-dev-list.pdl
Cc: linux-wireless, pieterpg, kvalo, hante.meuleman
In-Reply-To: <9774b991-ef09-66c9-c997-b2d043db5f0f@broadcom.com>
On 07/19/2016 12:36 PM, Arend Van Spriel wrote:
> On 19-7-2016 20:30, Florian Fainelli wrote:
>> On 07/19/2016 11:21 AM, Arend Van Spriel wrote:
>>> On 19-7-2016 18:41, Florian Fainelli wrote:
>>>> On 07/19/2016 02:20 AM, Arend Van Spriel wrote:
>>>>> + Bob
>>>>>
>>>>> On 19-7-2016 1:24, Florian Fainelli wrote:
>>>>>> Hi,
>>>>>>
>>>>>> This patch series addresses several coverity issues, they all seemed relevant
>>>>>> to me.
>>>>>
>>>>> Hi Florian,
>>>>>
>>>>> Been a while so nice to see coverity fixes popping up. Actually
>>>>> something that I have on my todo list to add our brcm80211 to coverity
>>>>> within Broadcom. So being curious as to whether this comes from a public
>>>>> coverity server like scan.coverity.com. Maybe bit redundant to setup
>>>>> internally if there is a good coverity analysis publicly available.
>>>>
>>>> This is coming from the public coverity instance, if you create an
>>>> account there I could transfer to you the other bugs that affect the
>>>> brcm80211 drivers (hint: there is a ton of of them because of
>>>> brcmf_fil_iovar_int_get and friends).
>>>
>>> I did create an account and noticed "Broadcom Linux Kernel Open Source
>>> Repository" project. Anyways, let me have it :-p
>>>
>>>>>
>>>>>> There is also a ton of warnings in Coverity caused by brcmf_fil_iovar_int_get()
>>>>>> and friends because of the initial access:
>>>>>>
>>>>>> __le32 data_le = cpu_to_le32(*data) which can utilize unitialized memory. I am
>>>>>> not sure if we actually care about any kind of initial, value, but if we don't,
>>>>>> then the fix should be fairly obvious.
>>>>>
>>>>> If we are talking only about "get" variant than we mostly don't care.
>>>>> Some getters support filter variables to be passed towards firmware. I
>>>>> have not looked at the analysis to give any judgement here.
>>>>
>>>> Alright, do you have a good way to test a patch that would just zero
>>>> initialize the data variable in brcmf_fil_iovar_int_get()? If so, I will
>>>> submit one with the appropriate CID references.
>>>
>>> But why would we care to zero the data when firmware simply overwrites
>>> it. These control messages are not high-prio so we can burn some cycles
>>> to initialize the variable, but to me this is a false positive.
>>
>> We have something like this all over the place:
>>
>> func() {
>> u32 val;
>>
>> brcmf_fil_iovar_int_get(..., &val, sizeof(val));
>
> If I am not mistaken the iovar_int_get only passes &val.
We pass data and len:
err = brcmf_fil_iovar_data_get(ifp, name, &data_le,
sizeof(data_le));
if (err == 0)
>
>> }
>>
>> brcmf_fil_iovar_int_get(.., const u32 *data, size_t len)
>> {
>> __le32 data_le = cpu_to_le32(*data);
>> ...
>> }
>>
>> So here data_le also has an uninitialized value, and later on, this:
>>
>>
>> buflen = brcmf_create_iovar(name, data, len, drvr->proto_buf,
>> sizeof(drvr->proto_buf));
>
> create_iovar is used for both get and set...
>
>> }
>>
>> And this function does this:
>>
>> static u32
>> brcmf_create_iovar(char *name, const char *data, u32 datalen,
>> char *buf, u32 buflen)
>> {
>> u32 len;
>>
>> len = strlen(name) + 1;
>>
>> if ((len + datalen) > buflen)
>> return 0;
>>
>> memcpy(buf, name, len);
>>
>> /* append data onto the end of the name string */
>> if (data && datalen)
>> memcpy(&buf[len], data, datalen);
>
> but it does not have that info so it does the memcpy. We could add an
> extra parameter to avoid the memcpy for get, but as said there are
> instances where get passes query data to firmware.
OK.
>
>> return len + datalen;
>> }
>>
>>
>> We are essentially copying into buf an uninitialized value coming from
>> the stack, which seems like a problem to me.
>
> Functionally I still don't see the problem. For the instances where this
> is true, the buf will be overwritten with data from firmware. It may be
> a security issue as stack content is passed to an external device.
That's the part that I am mostly concerned with, and the fact that this
is a genuine uninitialized value, not under direct control, but still.
--
Florian
^ permalink raw reply
* RE: [PATCH v2 2/3] mac80211: mesh: improve path resolving time
From: Machani, Yaniv @ 2016-07-19 20:01 UTC (permalink / raw)
To: Yeoh Chun-Yeow, Bob Copeland
Cc: linux-wireless@vger.kernel.org, Hahn, Maital, Johannes Berg
In-Reply-To: <CAEFj987V0r3aVn+JhQOX-40bheKyEJdrppF75OCaLXhU=skmNw@mail.gmail.com>
T24gVHVlLCBKdWwgMTksIDIwMTYgYXQgMTk6MDE6NTQsIFllb2ggQ2h1bi1ZZW93IHdyb3RlOg0K
PiBKb2hhbm5lcyBCZXJnDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggdjIgMi8zXSBtYWM4MDIxMTog
bWVzaDogaW1wcm92ZSBwYXRoIHJlc29sdmluZyANCj4gdGltZQ0KPiANCj4gT24gVHVlLCBKdWwg
MTksIDIwMTYgYXQgOTo0MyBQTSwgQm9iIENvcGVsYW5kIDxtZUBib2Jjb3BlbGFuZC5jb20+DQo+
IHdyb3RlOg0KPiA+IE9uIFR1ZSwgSnVsIDE5LCAyMDE2IGF0IDAxOjAyOjEzUE0gKzAwMDAsIE1h
Y2hhbmksIFlhbml2IHdyb3RlOg0KPiA+PiBPbiBUdWUsIEp1bCAxOSwgMjAxNiBhdCAxNTo0NDo1
NiwgQm9iIENvcGVsYW5kIHdyb3RlOg0KPiA+DQo+ID4gVGhpcyB3b3JkaW5nIHNlZW1zIHRvIGlu
ZGljYXRlIHRoYXQgaXQgaXMgbm90IHBlciBwYXRoLiAgUGVyaGFwcyANCj4gPiB0aGlzIHNob3Vs
ZCBiZSBjbGFyaWZpZWQgaW4gdGhlIHN0YW5kYXJkLiAgKElmIHRoZSBpbnRlbnQgdHVybnMgb3V0
IA0KPiA+IHRvIGJlIHBlciBwYXRoLCB0aGVuIEkgZ3Vlc3Mgd2Ugc2hvdWxkIGZpeCBpdCBieSBz
dG9yaW5nIGxhc3RfcHJlcSANCj4gPiBwZXIgcGF0aA0KPiA+IGluc3RlYWQuKQ0KPiA+DQo+ID4N
Cj4gPiBJZ25vcmluZyB0aGUgc3RhbmRhcmQgZm9yIGEgc2Vjb25kLCBsZXQncyBleHBsb3JlIHRo
aXM6IGNhbiB5b3UgZ2l2ZSANCj4gPiBzb21lIGlkZWEgb24gaG93IG1hbnkgc3RhdGlvbnMgYXJl
IGluIHlvdXIgdGFyZ2V0IG5ldHdvcmssIGhvdyANCj4gPiBmcmVxdWVudGx5IGEgZ2l2ZW4gcGFp
ciBvZiBub2RlcyB1bnBlZXIsIHdoYXQgc29ydCBvZiBpbXByb3ZlbWVudHMgDQo+ID4geW91IHNl
ZSB3aXRoIHRoZSBwYXRjaD8gIEl0IHNob3VsZCB0aGVuIGJlIHByZXR0eSBlYXN5IHRvIHJ1biBz
b21lIA0KPiA+IHNpbXVsYXRpb25zIHRvIHNlZSB0aGUgc2NlbmFyaW9zIHdoZXJlIHRoaXMgaGVs
cHMgYW5kIHdoZXJlIGl0IGh1cnRzLg0KPiA+DQo+IA0KDQpCb2IsIENodW4tWWVvdywNClRoYW5r
cyBmb3IgeW91ciBpbnB1dHMuDQoNCkxldCB0YWtlIGEgc2ltcGxlIHNjZW5hcmlvLCB3aGVyZSB5
b3UgaGF2ZSBhLGIsYyxkIG5vZGVzIGNvbm5lY3RlZCB0byBlYWNoIG90aGVyIGFzIHNob3duIGJl
bG93Lg0KDQpBfiB+IH5+fn5DLSAtIC0gRA0KICAgXCAgICAgICAgICAvDQogICAgICBcICAgICAv
ICAgDQogICAgICAgICBCDQoNCkEgd291bGQgbGlrZSB0byBwYXNzIGRhdGEgdG8gRC4NCkEtQyBt
YXRyaWMgaXMgd29yc2UgdGhhbiBBLUItQywgc28gcGF0aCBpcyBjb25zdHJ1Y3RlZCB2aWEgQi4N
CldlIGFyZSBpbiBpZGxlIHN0YXRlLCBhbmQgUFJFUSBhcmUgc2VudCBldmVyeSBkb3QxMU1lc2hI
V01QcHJlcU1pbkludGVydmFsLg0KTm93LCBub2RlIEIgaGF2ZSBiZWVuIGRpc2Nvbm5lY3RlZCAo
cmFuIG91dCBvZiBiYXR0ZXJ5L3NodXQgZG93bi9zdWRkZW5seSB3ZW50IG91dCBvZiByYW5nZSkN
CkFzIEEgaGFzIGEgcGF0aCB0byBEIHZpYSBCLCBoZSBjbGVhbnMgdXAgaGlzIHBhdGggdGFibGUu
DQpOb3cgaGUgbmVlZHMgdG8gYnVpbGQgYSBuZXcgcGF0aCwgaW4gdGhlIFdDUywgaGUgaGF2ZSBq
dXN0IHNlbnQgYSBQUkVRLg0KQW5kIG5vdyBoZSBuZWVkcyB0byB3YWl0IGRvdDExTWVzaEhXTVBw
cmVxTWluSW50ZXJ2YWwgc2Vjb25kcywgdW50aWwgaGUgY2FuIHJlYnVpbGQgdGhlIHBhdGguDQpB
cyB3ZSB3b3VsZG4ndCBsaWtlIHRvIGZsb29kIHRoZSBuZXR3b3JrIHdpdGggUFJFUSwgd2UgY2Fu
IGFzc3VtZSB0aGF0IHRoZSBkb3QxMU1lc2hIV01QcHJlcU1pbkludGVydmFsIGlzIGZldyBzZWNv
bmRzLCBmb3IgdXMsIGl0IHNlZW1lZCB1bi1hY2NlcHRhYmxlLg0KDQpJbiBjYXNlcyB3aGVyZSB5
b3UgbmVlZCB0byBoYXZlIGEgcmVsaWFibGUgZGF0YSBsaW5rLCBwYXNzaW5nIGF1ZGlvL3ZpZGVv
IHlvdSB1c3VhbGx5IGNhbid0IGFmZm9yZCBmZXcgc2Vjb25kcyB3L28gdHJhZmZpYy4NCg0KPiBJ
biBhZGRpdGlvbiB0byBCb2IncyBjb21tZW50LCB5b3UgcHJvYmFibHkgY2FuIHRyeSB0byByZWR1
Y2UgdGhlIA0KPiBkb3QxMU1lc2hIV01QcHJlcU1pbkludGVydmFsIHRvIDEgVFUgKDFtcykgaW5z
dGVhZCBvZiBzdGlja2luZyB0byANCj4gZGVmYXVsdCB2YWx1ZSAxMCBUVXMuIEJlc2lkZXMsIHlv
dSBjYW4gYWxzbyByZWR1Y2UgdGhlIA0KPiBtZXNoX3BhdGhfcmVmcmVzaF90aW1lIHdoaWNoIGlz
IGN1cnJlbnRseSBkZWZhdWx0IHRvIDEwMDAgbXMgYW5kIGNoZWNrIA0KPiB3aGV0aGVyIHlvdSBj
YW4gaW1wcm92ZSBvbiB5b3VyIG5ldHdvcmsgc2NlbmFyaW9zLg0KPg0KDQpXZSBkaWQgdHJpZWQg
dG8gcGxheSB3aXRoIHRoZXNlIHZhbHVlcywgYnV0IGFnYWluLCB3ZSBkb24ndCB3YW50IHRvIGZs
b29kIHRoZSBuZXR3b3JrLg0KV2UganVzdCB3YW50IHRvIHJlY292ZXIgQVNBUC4NCiANCj4gV2hl
biB5b3UgbWVudGlvbmVkICJuZXh0IGhvcCBwZWVyIGRpc2Nvbm5lY3QiLCBpdCBjb3VsZCBhbHNv
IGJlIHRoZSANCj4gdGltZSB0YWtlbiB0byByZS1lc3RhYmxpc2hlZCB0aGUgbWVzaCBwZWVyaW5n
IGJlZm9yZSB5b3VyIG1lc2ggU1RBIGNhbiANCj4gdHJhbnNtaXQgdGhlIGRhdGEgdG8geW91ciBw
ZWVyIG1lc2ggU1RBLg0KPiANCg0KTGluayBlc3RhYmxpc2htZW50IHRha2VzIG5vIG1vcmUgdGhh
biBmZXcgMTAwcyBvZiBtcyB1c3VhbGx5LA0KQW5kIGluIHRoZSBleGFtcGxlIGFib3ZlLCB0aGVy
ZSBpcyBubyBuZXcgbGluayBlc3RhYmxpc2htZW50LCBqdXN0IHBhdGggZ2VuZXJhdGlvbi4NCg0K
DQpUaGFua3MsDQpZYW5pdg0KDQo=
^ permalink raw reply
* Re: [PATCH 0/4] brcm80211: Misc coverity fixes
From: Arend Van Spriel @ 2016-07-19 19:36 UTC (permalink / raw)
To: Florian Fainelli, brcm80211-dev-list.pdl
Cc: linux-wireless, pieterpg, kvalo, hante.meuleman
In-Reply-To: <578E71C8.1050003@gmail.com>
On 19-7-2016 20:30, Florian Fainelli wrote:
> On 07/19/2016 11:21 AM, Arend Van Spriel wrote:
>> On 19-7-2016 18:41, Florian Fainelli wrote:
>>> On 07/19/2016 02:20 AM, Arend Van Spriel wrote:
>>>> + Bob
>>>>
>>>> On 19-7-2016 1:24, Florian Fainelli wrote:
>>>>> Hi,
>>>>>
>>>>> This patch series addresses several coverity issues, they all seemed relevant
>>>>> to me.
>>>>
>>>> Hi Florian,
>>>>
>>>> Been a while so nice to see coverity fixes popping up. Actually
>>>> something that I have on my todo list to add our brcm80211 to coverity
>>>> within Broadcom. So being curious as to whether this comes from a public
>>>> coverity server like scan.coverity.com. Maybe bit redundant to setup
>>>> internally if there is a good coverity analysis publicly available.
>>>
>>> This is coming from the public coverity instance, if you create an
>>> account there I could transfer to you the other bugs that affect the
>>> brcm80211 drivers (hint: there is a ton of of them because of
>>> brcmf_fil_iovar_int_get and friends).
>>
>> I did create an account and noticed "Broadcom Linux Kernel Open Source
>> Repository" project. Anyways, let me have it :-p
>>
>>>>
>>>>> There is also a ton of warnings in Coverity caused by brcmf_fil_iovar_int_get()
>>>>> and friends because of the initial access:
>>>>>
>>>>> __le32 data_le = cpu_to_le32(*data) which can utilize unitialized memory. I am
>>>>> not sure if we actually care about any kind of initial, value, but if we don't,
>>>>> then the fix should be fairly obvious.
>>>>
>>>> If we are talking only about "get" variant than we mostly don't care.
>>>> Some getters support filter variables to be passed towards firmware. I
>>>> have not looked at the analysis to give any judgement here.
>>>
>>> Alright, do you have a good way to test a patch that would just zero
>>> initialize the data variable in brcmf_fil_iovar_int_get()? If so, I will
>>> submit one with the appropriate CID references.
>>
>> But why would we care to zero the data when firmware simply overwrites
>> it. These control messages are not high-prio so we can burn some cycles
>> to initialize the variable, but to me this is a false positive.
>
> We have something like this all over the place:
>
> func() {
> u32 val;
>
> brcmf_fil_iovar_int_get(..., &val, sizeof(val));
If I am not mistaken the iovar_int_get only passes &val.
> }
>
> brcmf_fil_iovar_int_get(.., const u32 *data, size_t len)
> {
> __le32 data_le = cpu_to_le32(*data);
> ...
> }
>
> So here data_le also has an uninitialized value, and later on, this:
>
>
> buflen = brcmf_create_iovar(name, data, len, drvr->proto_buf,
> sizeof(drvr->proto_buf));
create_iovar is used for both get and set...
> }
>
> And this function does this:
>
> static u32
> brcmf_create_iovar(char *name, const char *data, u32 datalen,
> char *buf, u32 buflen)
> {
> u32 len;
>
> len = strlen(name) + 1;
>
> if ((len + datalen) > buflen)
> return 0;
>
> memcpy(buf, name, len);
>
> /* append data onto the end of the name string */
> if (data && datalen)
> memcpy(&buf[len], data, datalen);
but it does not have that info so it does the memcpy. We could add an
extra parameter to avoid the memcpy for get, but as said there are
instances where get passes query data to firmware.
> return len + datalen;
> }
>
>
> We are essentially copying into buf an uninitialized value coming from
> the stack, which seems like a problem to me.
Functionally I still don't see the problem. For the instances where this
is true, the buf will be overwritten with data from firmware. It may be
a security issue as stack content is passed to an external device.
Regards,
Arend
^ permalink raw reply
* Re: linux-next: build failure after merge of the wireless-drivers-next tree
From: Brian Norris @ 2016-07-19 18:39 UTC (permalink / raw)
To: Kalle Valo
Cc: Rafał Miłecki, linux-mtd@lists.infradead.org,
Stephen Rothwell, linux-wireless@vger.kernel.org, linux-next,
Linux Kernel Mailing List
In-Reply-To: <87h9blvhxg.fsf@kamboji.qca.qualcomm.com>
Just to head this off, since I noticed it...
On Tue, Jul 19, 2016 at 05:08:59PM +0300, Kalle Valo wrote:
> Rafał Miłecki <zajec5@gmail.com> writes:
> > I sent a patch seconds ago, you may just take a look at it. If you
> > still prefer to revert my commit, go ahead.
>
> Ok, let's try your fix. We still have few days. I applied the patch[1]
> to the pending branch this morning and if I don't see any errors I apply
> it later tonight and hopefully it will be in tomorrow's linux-next
> build. Thanks for the quick fix, very much appreciated.
>
> [1] "mtd: add arch dependency for MTD_BCM47XXSFLASH symbol"
> https://patchwork.kernel.org/patch/9236053/
I applied a trivial change to this same Kconfig entry:
Subject: mtd: update description of MTD_BCM47XXSFLASH symbol
http://git.infradead.org/l2-mtd.git/commitdiff/0a526341fee054c1e2b9f0e4b2b424ae81707d4c
It's a trivial conflict, so should we just let Linus work it out? I can
note it my MTD pull request, if wireless lands first.
Brian
^ permalink raw reply
* Re: [PATCH 0/4] brcm80211: Misc coverity fixes
From: Florian Fainelli @ 2016-07-19 18:30 UTC (permalink / raw)
To: Arend Van Spriel, brcm80211-dev-list.pdl
Cc: linux-wireless, pieterpg, kvalo, hante.meuleman
In-Reply-To: <53a36e85-37ad-10a5-81fc-40ce4efa6a65@broadcom.com>
On 07/19/2016 11:21 AM, Arend Van Spriel wrote:
> On 19-7-2016 18:41, Florian Fainelli wrote:
>> On 07/19/2016 02:20 AM, Arend Van Spriel wrote:
>>> + Bob
>>>
>>> On 19-7-2016 1:24, Florian Fainelli wrote:
>>>> Hi,
>>>>
>>>> This patch series addresses several coverity issues, they all seemed relevant
>>>> to me.
>>>
>>> Hi Florian,
>>>
>>> Been a while so nice to see coverity fixes popping up. Actually
>>> something that I have on my todo list to add our brcm80211 to coverity
>>> within Broadcom. So being curious as to whether this comes from a public
>>> coverity server like scan.coverity.com. Maybe bit redundant to setup
>>> internally if there is a good coverity analysis publicly available.
>>
>> This is coming from the public coverity instance, if you create an
>> account there I could transfer to you the other bugs that affect the
>> brcm80211 drivers (hint: there is a ton of of them because of
>> brcmf_fil_iovar_int_get and friends).
>
> I did create an account and noticed "Broadcom Linux Kernel Open Source
> Repository" project. Anyways, let me have it :-p
>
>>>
>>>> There is also a ton of warnings in Coverity caused by brcmf_fil_iovar_int_get()
>>>> and friends because of the initial access:
>>>>
>>>> __le32 data_le = cpu_to_le32(*data) which can utilize unitialized memory. I am
>>>> not sure if we actually care about any kind of initial, value, but if we don't,
>>>> then the fix should be fairly obvious.
>>>
>>> If we are talking only about "get" variant than we mostly don't care.
>>> Some getters support filter variables to be passed towards firmware. I
>>> have not looked at the analysis to give any judgement here.
>>
>> Alright, do you have a good way to test a patch that would just zero
>> initialize the data variable in brcmf_fil_iovar_int_get()? If so, I will
>> submit one with the appropriate CID references.
>
> But why would we care to zero the data when firmware simply overwrites
> it. These control messages are not high-prio so we can burn some cycles
> to initialize the variable, but to me this is a false positive.
We have something like this all over the place:
func() {
u32 val;
brcmf_fil_iovar_int_get(..., &val, sizeof(val));
}
brcmf_fil_iovar_int_get(.., const u32 *data, size_t len)
{
__le32 data_le = cpu_to_le32(*data);
...
}
So here data_le also has an uninitialized value, and later on, this:
buflen = brcmf_create_iovar(name, data, len, drvr->proto_buf,
sizeof(drvr->proto_buf));
}
And this function does this:
static u32
brcmf_create_iovar(char *name, const char *data, u32 datalen,
char *buf, u32 buflen)
{
u32 len;
len = strlen(name) + 1;
if ((len + datalen) > buflen)
return 0;
memcpy(buf, name, len);
/* append data onto the end of the name string */
if (data && datalen)
memcpy(&buf[len], data, datalen);
return len + datalen;
}
We are essentially copying into buf an uninitialized value coming from
the stack, which seems like a problem to me.
--
Florian
^ permalink raw reply
* Re: [PATCH 3/4] brcmsmac: Fix invalid memcpy() size in brcms_c_d11hdrs_mac80211
From: Arend Van Spriel @ 2016-07-19 18:26 UTC (permalink / raw)
To: Florian Fainelli, brcm80211-dev-list.pdl
Cc: linux-wireless, pieterpg, kvalo, hante.meuleman
In-Reply-To: <578E5864.3040309@gmail.com>
On 19-7-2016 18:42, Florian Fainelli wrote:
> On 07/19/2016 03:38 AM, Arend Van Spriel wrote:
>> On 19-7-2016 1:24, Florian Fainelli wrote:
>>> struct ieee80211_rts::ra is only ETH_ALEN wide, yet we attempt to copy 2
>>> * ETH_ALEN, which will potentially overrun the destination buffer.
>>
>> NACK - this is intentional. Have to admit it is a bit of trickery.
>
> This seems fragile, if there is any kind of re-ordering of fields within
> that structure your trick breaks apart.
Well, that would be an 802.11 standard update and given that it would
break legacy devices I think it is unlikely that reordering would happen
in this case.
>> struct ieee80211_rts is mapped over struct d11txh which is sent to
>> hardware. The struct is used for both RTS and CTS. Transmitting CTS will
>> only fill 802.11 addr2 in struct ieee80211_rts::ra. Transmitting RTS
>> fills 802.11 addr1 in ra and 802.11 addr2 in ta using single memcpy().
>> Not very clear, but your change is not the way to go here.
>
> Fair enough, as Kalle suggested, this deserves a comment explaining why,
> I would not be surprised other people would trip over that.
Agree. I am also fine with two separate memcpy() statements if that
gives enough clarification.
Regards,
Arend
>>
>> Regards,
>> Arend
>>
>>> Reported-by: coverity (CID 145657)
>>> Fixes: 5b435de0d7868 ("net: wireless: add brcm80211 drivers")
>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>> ---
>>> drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c
>>> index c2a938b59044..59813a3666eb 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c
>>> @@ -6671,7 +6671,7 @@ brcms_c_d11hdrs_mac80211(struct brcms_c_info *wlc, struct ieee80211_hw *hw,
>>> rts->frame_control = cpu_to_le16(IEEE80211_FTYPE_CTL |
>>> IEEE80211_STYPE_RTS);
>>>
>>> - memcpy(&rts->ra, &h->addr1, 2 * ETH_ALEN);
>>> + memcpy(&rts->ra, &h->addr1, ETH_ALEN);
>>> }
>>>
>>> /* mainrate
>>>
>
>
^ permalink raw reply
* Re: [PATCH 0/4] brcm80211: Misc coverity fixes
From: Arend Van Spriel @ 2016-07-19 18:21 UTC (permalink / raw)
To: Florian Fainelli, brcm80211-dev-list.pdl
Cc: linux-wireless, pieterpg, kvalo, hante.meuleman
In-Reply-To: <578E5845.6030100@gmail.com>
On 19-7-2016 18:41, Florian Fainelli wrote:
> On 07/19/2016 02:20 AM, Arend Van Spriel wrote:
>> + Bob
>>
>> On 19-7-2016 1:24, Florian Fainelli wrote:
>>> Hi,
>>>
>>> This patch series addresses several coverity issues, they all seemed relevant
>>> to me.
>>
>> Hi Florian,
>>
>> Been a while so nice to see coverity fixes popping up. Actually
>> something that I have on my todo list to add our brcm80211 to coverity
>> within Broadcom. So being curious as to whether this comes from a public
>> coverity server like scan.coverity.com. Maybe bit redundant to setup
>> internally if there is a good coverity analysis publicly available.
>
> This is coming from the public coverity instance, if you create an
> account there I could transfer to you the other bugs that affect the
> brcm80211 drivers (hint: there is a ton of of them because of
> brcmf_fil_iovar_int_get and friends).
I did create an account and noticed "Broadcom Linux Kernel Open Source
Repository" project. Anyways, let me have it :-p
>>
>>> There is also a ton of warnings in Coverity caused by brcmf_fil_iovar_int_get()
>>> and friends because of the initial access:
>>>
>>> __le32 data_le = cpu_to_le32(*data) which can utilize unitialized memory. I am
>>> not sure if we actually care about any kind of initial, value, but if we don't,
>>> then the fix should be fairly obvious.
>>
>> If we are talking only about "get" variant than we mostly don't care.
>> Some getters support filter variables to be passed towards firmware. I
>> have not looked at the analysis to give any judgement here.
>
> Alright, do you have a good way to test a patch that would just zero
> initialize the data variable in brcmf_fil_iovar_int_get()? If so, I will
> submit one with the appropriate CID references.
But why would we care to zero the data when firmware simply overwrites
it. These control messages are not high-prio so we can burn some cycles
to initialize the variable, but to me this is a false positive.
Regards,
Arend
^ permalink raw reply
* Re: [1/3] rtlwifi: don't add include path for rtl8188ee
From: Kalle Valo @ 2016-07-19 18:17 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-wireless, Larry Finger, netdev, Jes Sorensen, Arnd Bergmann,
Chaoming Li, linux-kernel
In-Reply-To: <20160719153403.2967812-1-arnd@arndb.de>
Arnd Bergmann <arnd@arndb.de> wrote:
> For rtl8188ee, we pass -Idrivers/net/wireless/rtlwifi/ to gcc,
> however that directy no longer exists, so evidently this option
> is no longer required here and can be removed to avoid a warning
> when building with 'make W=1' or 'gcc -Wmissing-include-dirs'
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Thanks, 1 patch applied to wireless-drivers-next.git:
944c07a7aac9 rtlwifi: don't add include path for rtl8188ee
--
Sent by pwcli
https://patchwork.kernel.org/patch/9237723/
^ permalink raw reply
* Re: mtd: add arch dependency for MTD_BCM47XXSFLASH symbol
From: Kalle Valo @ 2016-07-19 18:17 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-wireless, Rafał Miłecki, Brian Norris,
David Woodhouse, open list:MEMORY TECHNOLOGY DEVICES (MTD),
open list
In-Reply-To: <1468912123-14899-1-git-send-email-zajec5@gmail.com>
Rafał Miłecki wrote:
> We dropped strict MIPS dependency for bcm47xxsflash driver in:
> commit 5651d6aaf489 ("mtd: bcm47xxsflash: use ioremap_cache() instead of
> KSEG0ADDR()") but using ioremap_cache still limits building it to few
> selected architectures only.
>
> A recent commit 57d8f7dd2132 ("bcma: allow enabling serial flash support
> on non-MIPS SoCs") automatically dropped MIPS dependency for
> MTD_BCM47XXSFLASH which broke building e.g. on powerpc and cris.
>
> The bcma change is alright as it doesn't break building bcma code in any
> way. MTD_BCM47XXSFLASH on the other hand should be limited to archs
> which need it and can build it (by providing ioremap_cache).
>
> Fixes: 57d8f7dd2132 ("bcma: allow enabling serial flash support on non-MIPS SoCs")
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Acked-by: Brian Norris <computersforpeace@gmail.com>
Thanks, 1 patch applied to wireless-drivers-next.git:
efacc699139e mtd: add arch dependency for MTD_BCM47XXSFLASH symbol
--
Sent by pwcli
https://patchwork.kernel.org/patch/9236053/
^ permalink raw reply
* Re: [v7] wlcore: spi: add wl18xx support
From: Kalle Valo @ 2016-07-19 18:16 UTC (permalink / raw)
To: Eyal Reizer
Cc: linux-wireless, netdev, linux-kernel, devicetree, linux-spi, robh,
Eyal Reizer, Eyal Reizer
In-Reply-To: <1468911482-32560-1-git-send-email-eyalr@ti.com>
Eyal Reizer <eyalreizer@gmail.com> wrote:
> From: Eyal Reizer <eyalr@ti.com>
>
> Add support for using with both wl12xx and wl18xx.
>
> - all wilink family needs special init command for entering wspi mode.
> extra clock cycles should be sent after the spi init command while the
> cs pin is high.
> - Use inverted chip select for sending a dummy 4 bytes command that
> completes the init stage.
>
> Signed-off-by: Eyal Reizer <eyalr@ti.com>
> Acked-by: Rob Herring <robh@kernel.org>
Thanks, 1 patch applied to wireless-drivers-next.git:
01efe65aba65 wlcore: spi: add wl18xx support
--
Sent by pwcli
https://patchwork.kernel.org/patch/9235983/
^ permalink raw reply
* Re: [1/4] brcmfmac: Fix glob_skb leak in brcmf_sdiod_recv_chain
From: Kalle Valo @ 2016-07-19 18:14 UTC (permalink / raw)
To: Florian Fainelli
Cc: brcm80211-dev-list.pdl, linux-wireless, pieterpg, arend.vanspriel,
hante.meuleman, Florian Fainelli
In-Reply-To: <1468884277-18606-2-git-send-email-f.fainelli@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> wrote:
> In case brcmf_sdiod_recv_chain() cannot complete a succeful call to
> brcmf_sdiod_buffrw, we would be leaking glom_skb and not free it as we
> should, fix this.
>
> Reported-by: coverity (CID 1164856)
> Fixes: a413e39a38573 ("brcmfmac: fix brcmf_sdcard_recv_chain() for host without sg support")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Thanks, 3 patches applied to wireless-drivers-next.git:
3bdae810721b brcmfmac: Fix glob_skb leak in brcmf_sdiod_recv_chain
5c5fa1f464ac brcmsmac: Free packet if dma_mapping_error() fails in dma_rxfill
f823a2aa8f46 brcmsmac: Initialize power in brcms_c_stf_ss_algo_channel_get()
--
Sent by pwcli
https://patchwork.kernel.org/patch/9235673/
^ permalink raw reply
* Re: [V2] bcma: define ChipCommon B MII registers
From: Kalle Valo @ 2016-07-19 18:13 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Rafał Miłecki,
open list:BROADCOM SPECIFIC AMBA DRIVER (BCMA), open list
In-Reply-To: <1467990860-10663-1-git-send-email-zajec5@gmail.com>
Rafał Miłecki wrote:
> We don't have access to datasheets to document all the bits but we can
> name these registers at least.
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Thanks, 1 patch applied to wireless-drivers-next.git:
cc2d1de06f05 bcma: define ChipCommon B MII registers
--
Sent by pwcli
https://patchwork.kernel.org/patch/9221183/
^ permalink raw reply
* Re: [PATCH v2 05/11] Kbuild: don't add obj tree in additional includes
From: Kalle Valo @ 2016-07-19 18:07 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Greg Kroah-Hartman, linux-arm-kernel, Michal Marek, netdev,
dri-devel, linux-kernel, linux-kbuild, akpm, linux-wireless
In-Reply-To: <3906958.MrNt71DzrU@wuerfel>
Arnd Bergmann <arnd@arndb.de> writes:
>> > I think that's fine, a couple were already picked up, and what I have
>> > left now is
>> >
>> > a281bfa5713a [SUBMITTED 20160615] [EXPERIMENTAL] Kbuild: enable -Wmissing-include-dirs by default
>> > 83934921e68e [SUBMITTED 20160615] rtlwifi: don't add include path for rtl8188ee
>>
>> Apparently[1] you didn't CC linux-wireless and that's why I didn't see
>> the rtlwifi patch in wireless patchwork. Care to resend?
>>
>> [1] https://patchwork.kernel.org/patch/9178861/
>>
>
> Done.
Thanks.
> I've also thrown in two patches for drivers/staging/rtl8*/ that I
> submitted a while ago, but I'm not sure if they should get merged
> through the staging tree or the wireless tree. I had previously
> submitted those two as a combined patch along with a third one that
> turned out to be unnecessary.
Greg applies drivers/staging patches to his staging tree, but I'll take
the rtlwifi patch.
--
Kalle Valo
^ permalink raw reply
* Re: [1/4] ath9k_hw: fix duplicate (and partially wrong) definition of AR_CH0_THERM
From: Kalle Valo @ 2016-07-19 17:59 UTC (permalink / raw)
To: Felix Fietkau; +Cc: linux-wireless, kvalo
In-Reply-To: <20160711100248.68678-1-nbd@nbd.name>
Felix Fietkau <nbd@nbd.name> wrote:
> AR_PHY_65NM_CH0_THERM and AR_CH0_THERM were supposed to refer to the
> same register, however they had different SREV checks.
>
> Remove the duplicate and use the checks. Since there were other SREV
> checks present in the only place that uses this, this will probaby not
> affect runtime behavior.
>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
Thanks, 4 patches applied to ath-next branch of ath.git:
3ff25093cbdd ath9k_hw: fix duplicate (and partially wrong) definition of AR_CH0_THERM
8f778c72ac8f ath9k_hw: simplify ar9003_hw_per_calibration
feaacb174892 ath9k_hw: get rid of some duplicate code in calibration init
171f6402e4aa ath9k_hw: implement temperature compensation support for AR9003+
--
Sent by pwcli
https://patchwork.kernel.org/patch/9223141/
^ permalink raw reply
* Re: ath9k_hw: fix spectral scan on AR9285 and newer
From: Kalle Valo @ 2016-07-19 17:58 UTC (permalink / raw)
To: Felix Fietkau; +Cc: linux-wireless, kvalo
In-Reply-To: <20160711083539.58649-1-nbd@nbd.name>
Felix Fietkau <nbd@nbd.name> wrote:
> The register layout of AR_PHY_SPECTRAL_SCAN has changed, only AR9280
> uses the old layout
>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
Thanks, 1 patch applied to ath-next branch of ath.git:
7d6c2d1e34ef ath9k_hw: fix spectral scan on AR9285 and newer
--
Sent by pwcli
https://patchwork.kernel.org/patch/9223035/
^ permalink raw reply
* Re: [v2] ath9k: simplify the code-paths when not using the built-in EEPROM
From: Kalle Valo @ 2016-07-19 17:57 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: ath9k-devel, linux-wireless, ath9k-devel, nbd,
Martin Blumenstingl
In-Reply-To: <20160709230955.27266-1-martin.blumenstingl@googlemail.com>
Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
> There were two paths in the code for "external" eeprom sources. The code
> in eeprom.c only handled the cases where the eeprom data was loaded via
> request_firmware. ahb.c and pci.c on the other hand had some duplicate
> code which was only used when the eeprom data was passed via
> ath9k_platform_data.
> With this change all eeprom data handling is now unified in eeprom.c.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Thanks, 1 patch applied to ath-next branch of ath.git:
563985199693 ath9k: simplify the code-paths when not using the built-in EEPROM
--
Sent by pwcli
https://patchwork.kernel.org/patch/9222187/
^ permalink raw reply
* Re: ath6kl: sme_state shortcut to SME_DISCONNECTED removed
From: Kalle Valo @ 2016-07-19 17:56 UTC (permalink / raw)
To: engineering; +Cc: ath6kl, linux-wireless
In-Reply-To: <1468026259-25148-1-git-send-email-dan.kephart@lairdtech.com>
engineering@keppy.com wrote:
> From: Dan Kephart <dan.kephart@lairdtech.com>
>
> When an NL80211_DISCONNECT is sent to cfg80211, the driver's cfg80211
> disconnect function sets the sme_state to SME_DISCONNECTED before receiving
> a WMI_DISCONNECT_EVENT from the firmware. This caused cfg80211 to not know
> that the connection is disconnected because the driver fails to notify
> cfg80211 upon receiving WMI_DISCONNECT_EVENT from the firmware believing
> it is already disconnected.
>
> Signed-off-by: Dan Kephart <dan.kephart@lairdtech.com>
Thanks, 1 patch applied to ath-next branch of ath.git:
42e54152e707 ath6kl: sme_state shortcut to SME_DISCONNECTED removed
--
Sent by pwcli
https://patchwork.kernel.org/patch/9221927/
^ 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