Re: [staging:staging-testing 41/59] drivers/staging/wfx/main.c:47:14-21: ERROR: PTR_ERR applied after initialization to constant on line 42
On 10/7/19 4:36 PM, Jerome Pouiller wrote:
On Friday 4 October 2019 12:48:32 CEST kbuild test robot wrote:
[...]
drivers/staging/wfx/main.c:47:14-21: ERROR: PTR_ERR applied after
initialization to constant on line 42
vim +47 drivers/staging/wfx/main.c
30
31 struct gpio_desc *wfx_get_gpio(struct device *dev, int override, const
char *label)
32 {
33 struct gpio_desc *ret;
34 char label_buf[256];
35
36 if (override >= 0) {
37 snprintf(label_buf, sizeof(label_buf), "wfx_%s",
label);
38 ret = ERR_PTR(devm_gpio_request_one(dev, override,
GPIOF_OUT_INIT_LOW, label_buf));
39 if (!ret)
40 ret = gpio_to_desc(override);
41 } else if (override == -1) {
> 42 ret = NULL;
43 } else {
44 ret = devm_gpiod_get(dev, label, GPIOD_OUT_LOW);
45 }
46 if (IS_ERR(ret) || !ret) {
> 47 if (!ret || PTR_ERR(ret) == -ENOENT)
48 dev_warn(dev, "gpio %s is not defined\n",
label);
49 else
50 dev_warn(dev, "error while requesting gpio
%s\n", label);
51 ret = NULL;
52 } else {
53 dev_dbg(dev, "using gpio %d for %s\n",
desc_to_gpio(ret), label);
54 }
55 return ret;
56 }
57
I think that this report is a false positive or I missed something?
Hi,
Sorry for the inconvenience, but we confirmed that the error first
appeared since commit 0096214a59.
Best Regards,
Rong Chen
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] KPC2000: kpc2000_spi.c: Fix alignment and style problems.
Fixed alignment and style issues raised by checkpatch.pl
Signed-off-by: Chandra Annamaneni
---
drivers/staging/kpc2000/kpc2000_spi.c | 49 ---
1 file changed, 28 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_spi.c
b/drivers/staging/kpc2000/kpc2000_spi.c
index 3be33c4..a20f2d7 100644
--- a/drivers/staging/kpc2000/kpc2000_spi.c
+++ b/drivers/staging/kpc2000/kpc2000_spi.c
@@ -30,19 +30,27 @@
#include "kpc.h"
static struct mtd_partition p2kr0_spi0_parts[] = {
- { .name = "SLOT_0", .size = 7798784,.offset = 0,
},
- { .name = "SLOT_1", .size = 7798784,.offset =
MTDPART_OFS_NXTBLK},
- { .name = "SLOT_2", .size = 7798784,.offset =
MTDPART_OFS_NXTBLK},
- { .name = "SLOT_3", .size = 7798784,.offset =
MTDPART_OFS_NXTBLK},
- { .name = "CS0_EXTRA", .size = MTDPART_SIZ_FULL, .offset =
MTDPART_OFS_NXTBLK},
+ { .name = "SLOT_0", .size = 7798784,.offset = 0,},
+ { .name = "SLOT_1", .size = 7798784,.offset =
+MTDPART_OFS_NXTBLK},
+ { .name = "SLOT_2", .size = 7798784,.offset =
+MTDPART_OFS_NXTBLK},
+ { .name = "SLOT_3", .size = 7798784,.offset =
+MTDPART_OFS_NXTBLK},
+ { .name = "CS0_EXTRA", .size = MTDPART_SIZ_FULL, .offset =
+MTDPART_OFS_NXTBLK},
};
static struct mtd_partition p2kr0_spi1_parts[] = {
- { .name = "SLOT_4", .size = 7798784,.offset = 0,
},
- { .name = "SLOT_5", .size = 7798784,.offset =
MTDPART_OFS_NXTBLK},
- { .name = "SLOT_6", .size = 7798784,.offset =
MTDPART_OFS_NXTBLK},
- { .name = "SLOT_7", .size = 7798784,.offset =
MTDPART_OFS_NXTBLK},
- { .name = "CS1_EXTRA", .size = MTDPART_SIZ_FULL, .offset =
MTDPART_OFS_NXTBLK},
+ { .name = "SLOT_4", .size = 7798784,.offset = 0,},
+ { .name = "SLOT_5", .size = 7798784,.offset =
+ MTDPART_OFS_NXTBLK},
+ { .name = "SLOT_6", .size = 7798784,.offset =
+ MTDPART_OFS_NXTBLK},
+ { .name = "SLOT_7", .size = 7798784,.offset =
+ MTDPART_OFS_NXTBLK},
+ { .name = "CS1_EXTRA", .size = MTDPART_SIZ_FULL, .offset =
+ MTDPART_OFS_NXTBLK},
};
static struct flash_platform_data p2kr0_spi0_pdata = {
@@ -50,6 +58,7 @@ static struct flash_platform_data p2kr0_spi0_pdata = {
.nr_parts = ARRAY_SIZE(p2kr0_spi0_parts),
.parts =p2kr0_spi0_parts,
};
+
static struct flash_platform_data p2kr0_spi1_pdata = {
.name = "SPI1",
.nr_parts = ARRAY_SIZE(p2kr0_spi1_parts),
@@ -165,7 +174,7 @@ kp_spi_read_reg(struct kp_spi_controller_state *cs, int idx)
u64 val;
addr += idx;
- if ((idx == KP_SPI_REG_CONFIG) && (cs->conf_cache >= 0))
+ if (idx == KP_SPI_REG_CONFIG && cs->conf_cache >= 0)
return cs->conf_cache;
val = readq(addr);
@@ -227,8 +236,7 @@ kp_spi_txrx_pio(struct spi_device *spidev, struct
spi_transfer *transfer)
kp_spi_write_reg(cs, KP_SPI_REG_TXDATA, val);
processed++;
}
- }
- else if (rx) {
+ } else if (rx) {
for (i = 0 ; i < c ; i++) {
char test = 0;
@@ -315,19 +323,18 @@ kp_spi_transfer_one_message(struct spi_master *master,
struct spi_message *m)
if (transfer->speed_hz > KP_SPI_CLK ||
(len && !(rx_buf || tx_buf))) {
dev_dbg(kpspi->dev, " transfer: %d Hz, %d %s%s, %d
bpw\n",
- transfer->speed_hz,
- len,
- tx_buf ? "tx" : "",
- rx_buf ? "rx" : "",
- transfer->bits_per_word);
+ transfer->speed_hz,
+ len,
+ tx_buf ? "tx" : "",
+ rx_buf ? "rx" : "",
+ transfer->bits_per_word);
dev_dbg(kpspi->dev, " transfer -EINVAL\n");
return -EINVAL;
}
if (transfer->speed_hz &&
transfer->speed_hz < (KP_SPI_CLK >> 15)) {
[PATCH v3] staging: vc04_services: Avoid NULL comparison
Remove NULL comparison. Issue found using checkpatch.pl
Signed-off-by: Nachammai Karuppiah
---
Changes in V2
- Remove all NULL comparisons in vc04_services/interface directory.
---
changes in V3
- Fixed warnings. Reported-by: kbuild test robot
---
Signed-off-by: Nachammai Karuppiah
---
.../interface/vchiq_arm/vchiq_2835_arm.c | 4 ++--
.../vc04_services/interface/vchiq_arm/vchiq_arm.c | 22 +++---
.../vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ++--
.../vc04_services/interface/vchiq_arm/vchiq_shim.c | 2 +-
4 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 8dc730c..7cdb21e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -526,11 +526,11 @@ create_pagelist(char __user *buf, size_t count, unsigned
short type)
return NULL;
}
- WARN_ON(g_free_fragments == NULL);
+ WARN_ON(!g_free_fragments);
down(&g_free_fragments_mutex);
fragments = g_free_fragments;
- WARN_ON(fragments == NULL);
+ WARN_ON(!fragments);
g_free_fragments = *(char **) g_free_fragments;
up(&g_free_fragments_mutex);
pagelist->type = PAGELIST_READ_WITH_FRAGMENTS +
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index b1595b1..d7d9c7c 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -827,7 +827,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
/* Remove all services */
i = 0;
while ((service = next_service_by_instance(instance->state,
- instance, &i)) != NULL) {
+ instance, &i))) {
status = vchiq_remove_service(service->handle);
unlock_service(service);
if (status != VCHIQ_SUCCESS)
@@ -907,7 +907,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
&args.params, srvstate,
instance, user_service_free);
- if (service != NULL) {
+ if (service) {
user_service->service = service;
user_service->userdata = userdata;
user_service->instance = instance;
@@ -988,7 +988,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
service = find_service_for_instance(instance, handle);
- if (service != NULL) {
+ if (service) {
status = (cmd == VCHIQ_IOC_USE_SERVICE) ?
vchiq_use_service_internal(service) :
vchiq_release_service_internal(service);
@@ -1021,7 +1021,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
service = find_service_for_instance(instance, args.handle);
- if ((service != NULL) && (args.count <= MAX_ELEMENTS)) {
+ if (service && (args.count <= MAX_ELEMENTS)) {
/* Copy elements into kernel space */
struct vchiq_element elements[MAX_ELEMENTS];
@@ -1343,11 +1343,11 @@ vchiq_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
spin_unlock(&msg_queue_spinlock);
complete(&user_service->remove_event);
- if (header == NULL)
+ if (!header)
ret = -ENOTCONN;
else if (header->size <= args.bufsize) {
/* Copy to user space if msgbuf is not NULL */
- if ((args.buf == NULL) ||
+ if (!args.buf ||
(copy_to_user((void __user *)args.buf,
header->data,
header->size) == 0)) {
@@ -1426,7 +1426,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
service = find_closed_service_for_instance(instance, handle);
- if (service != NULL) {
+ if (service) {
struct user_service *user_service =
(struct user_service *)service->base.userdata;
close_delivered(user_service);
@@ -2223,13 +2223,13 @@ struct vchiq_st
RE: [PATCH v2] PCI: PM: Move to D0 before calling pci_legacy_resume_early()
> From: Bjorn Helgaas > Sent: Tuesday, October 8, 2019 12:56 PM > ... > Wordsmithing nit: what the patch does is not "fix the error message"; > what it does is fix the *problem*, i.e., the fact that we can't > operate the device because we can't enable MSI-X. The message is only > a symptom. I totally agree. :-) > IIUC the relevant part of the system hibernation sequence is: > > pci_pm_freeze_noirq > pci_pm_thaw_noirq > pci_pm_thaw > > And the execution flow is: > > pci_pm_freeze_noirq > if (pci_has_legacy_pm_support(pci_dev)) # true for mlx4 > pci_legacy_suspend_late(dev, PMSG_FREEZE) > pci_pm_set_unknown_state > dev->current_state = PCI_UNKNOWN # <--- > pci_pm_thaw_noirq > if (pci_has_legacy_pm_support(pci_dev)) # true > pci_legacy_resume_early(dev) # noop; mlx4 doesn't > implement > pci_pm_thaw # returns -95 > EOPNOTSUPP > if (pci_has_legacy_pm_support(pci_dev)) # true > pci_legacy_resume > drv->resume > mlx4_resume # mlx4_driver.resume (legacy) > mlx4_load_one > mlx4_enable_msi_x > pci_enable_msix_range > __pci_enable_msix_range > __pci_enable_msix > if (!pci_msi_supported()) > if (dev->current_state != PCI_D0) # <--- > return 0 > return -EINVAL > err = -EOPNOTSUPP > "INTx is not supported ..." > > (These are just my notes; you don't need to put them all into the > commit message. I'm just sharing them in case I'm not understanding > correctly.) Yes, these notes are accurate. > > > > > When the system starts again, a fresh kernel starts to run, and when > > > > > the > > > > > kernel detects that a hibernation image was saved, the kernel > "quiesces" > > > > > the devices, and then "restores" the devices from the saved image. In > this > > > > > path: > > > > > device_resume_noirq() -> ... -> > > > > >pci_pm_restore_noirq() -> > > > > > pci_pm_default_resume_early() -> > > > > >pci_power_up() moves the device states back to PCI_D0. This > path is > > > > > not broken and doesn't need my patch. > > > > > > > The cc list suggests that this might be a fix for a user-reported > problem. Is there a launchpad or similar link you could include here? I guess I'm the first one to notice the issue and there is not any bug link AFAIK. The hibernation process usually saves the states into a local disk (before the system is powered off), and the Mellanox NIC is not needed during the process, so it's not a real issue that the NIC can not work between pci_pm_thaw() and power_down(). This may explain why nobody else noticed the issue. I happened to see the error message, and hence investigated the issue. > Should this be marked for stable? I think we should do it. > > > > > --- a/drivers/pci/pci-driver.c > > > > > +++ b/drivers/pci/pci-driver.c > > > > > @@ -1074,15 +1074,16 @@ static int pci_pm_thaw_noirq(struct device > > > > *dev) > > > > > return error; > > > > > } > > > > > > > > > > - if (pci_has_legacy_pm_support(pci_dev)) > > > > > - return pci_legacy_resume_early(dev); > > > > > - > > > > > /* > > > > >* pci_restore_state() requires the device to be in D0 (because > of MSI > > > > >* restoration among other things), so force it into D0 in case > the > > > > >* driver's "freeze" callbacks put it into a low-power state > directly. > > > > >*/ > > > > > pci_set_power_state(pci_dev, PCI_D0); > > > > > + > > > > > + if (pci_has_legacy_pm_support(pci_dev)) > > > > > + return pci_legacy_resume_early(dev); > > > > > + > > > > > pci_restore_state(pci_dev); > > > > > > > > > > if (drv && drv->pm && drv->pm->thaw_noirq) > > > > > -- > > > > > 2.19.1 > > > > > > > The patch looks reasonable to me, but the comment above the > > pci_set_power_state() call needs to be updated too IMO. > > Hmm. > > 1) pci_restore_state() mainly writes config space, which doesn't > require the device to be in D0. The only thing I see that would > require D0 is the MSI-X MMIO space, so to be more specific, the > comment could say "restoring the MSI-X *MMIO* state requires the > device to be in D0". > > But I think you meant some other comment change. Did you mean > something along the lines of "a legacy drv->resume_early() callback > and pci_restore_state() both require the device to be in D0"? > > If something else, maybe you could propose some text? > > 2) I assume pci_pm_thaw_noirq() should leave the device in a > functionally equivalent state, whether it uses legacy PM or not. Do > we want something like the patch below instead? If we *do* want to > skip pci_restore_state() for legacy PM, maybe we should add a comment. > > 3) Documentation/power/pci.rst says: > >
[PATCH v3] staging: rtl8723bs: Remove set but not used variable 'i'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/staging/rtl8723bs/core/rtw_xmit.c: In function update_attrib:
drivers/staging/rtl8723bs/core/rtw_xmit.c:680:7: warning: variable i set but
not used [-Wunused-but-set-variable]
It is not used since commit 554c0a3abf21 ("staging:
Add rtl8723bs sdio wifi driver")
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
v1->v2: remove unnecessary (void) of (void)_rtw_pktfile_read(...)
v2->v3: add the changelog for v1->v2
drivers/staging/rtl8723bs/core/rtw_xmit.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c
b/drivers/staging/rtl8723bs/core/rtw_xmit.c
index b5dcb78..c50d05e 100644
--- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
+++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
@@ -675,7 +675,6 @@ static void set_qos(struct pkt_file *ppktfile, struct
pkt_attrib *pattrib)
static s32 update_attrib(struct adapter *padapter, _pkt *pkt, struct
pkt_attrib *pattrib)
{
- uint i;
struct pkt_file pktfile;
struct sta_info *psta = NULL;
struct ethhdr etherhdr;
@@ -689,7 +688,7 @@ static s32 update_attrib(struct adapter *padapter, _pkt
*pkt, struct pkt_attrib
DBG_COUNTER(padapter->tx_logs.core_tx_upd_attrib);
_rtw_open_pktfile(pkt, &pktfile);
- i = _rtw_pktfile_read(&pktfile, (u8 *)ðerhdr, ETH_HLEN);
+ _rtw_pktfile_read(&pktfile, (u8 *)ðerhdr, ETH_HLEN);
pattrib->ether_type = ntohs(etherhdr.h_proto);
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH][next] staging: wfx: fix spelling mistake "hexdecimal" -> "hexadecimal"
On Tuesday 8 October 2019 10:22:05 CEST Colin King wrote:
> From: Colin Ian King
>
> There is a spelling mistake in the documentation and a module parameter
> description. Fix these.
>
> Signed-off-by: Colin Ian King
> ---
> .../devicetree/bindings/net/wireless/siliabs,wfx.txt| 2 +-
> drivers/staging/wfx/main.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git
> a/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt
>
> b/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt
> index 15965c9b4180..26de6762b942 100644
> ---
> a/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt
> +++
> b/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt
> @@ -89,7 +89,7 @@ Some properties are recognized either by SPI and SDIO
> versions:
> this property, driver will disable most of power saving features.
> - config-file: Use an alternative file as PDS. Default is `wf200.pds`. Only
> necessary for development/debug purpose.
> - - slk_key: String representing hexdecimal value of secure link key to use.
> + - slk_key: String representing hexadecimal value of secure link key to use.
> Must contains 64 hexadecimal digits. Not supported in current version.
>
> WFx driver also supports `mac-address` and `local-mac-address` as described
> in
> diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
> index fe9a89703897..d2508bc950fa 100644
> --- a/drivers/staging/wfx/main.c
> +++ b/drivers/staging/wfx/main.c
> @@ -48,7 +48,7 @@ MODULE_PARM_DESC(gpio_wakeup, "gpio number for wakeup. -1
> for none.");
>
> static char *slk_key;
> module_param(slk_key, charp, 0600);
> -MODULE_PARM_DESC(slk_key, "secret key for secure link (expect 64 hexdecimal
> digits).");
> +MODULE_PARM_DESC(slk_key, "secret key for secure link (expect 64 hexadecimal
> digits).");
>
> #define RATETAB_ENT(_rate, _rateid, _flags) { \
> .bitrate = (_rate), \
> --
> 2.20.1
>
Thank you!
Acked-by: Jérôme Pouiller
--
Jérôme Pouiller
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] PCI: PM: Move to D0 before calling pci_legacy_resume_early()
On Tue, Oct 08, 2019 at 07:32:27PM +0200, Rafael J. Wysocki wrote:
> On 10/7/2019 8:57 PM, Dexuan Cui wrote:
> > > -Original Message-
> > > From: Bjorn Helgaas
> > > Sent: Monday, October 7, 2019 6:24 AM
> > > To: Dexuan Cui
> > > Cc: lorenzo.pieral...@arm.com; linux-...@vger.kernel.org; Michael Kelley
> > > ; linux-hyp...@vger.kernel.org;
> > > linux-ker...@vger.kernel.org; driverdev-devel@linuxdriverproject.org;
> > > Sasha
> > > Levin ; Haiyang Zhang
> > > ; KY Srinivasan ;
> > > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; vkuznets
> > > ; marcelo.ce...@canonical.com; Stephen Hemminger
> > > ; ja...@mellanox.com
> > > Subject: Re: [PATCH v2] PCI: PM: Move to D0 before calling
> > > pci_legacy_resume_early()
> > >
> > > On Wed, Aug 14, 2019 at 01:06:55AM +, Dexuan Cui wrote:
> > > > In pci_legacy_suspend_late(), the device state is moved to PCI_UNKNOWN.
> > > >
> > > > In pci_pm_thaw_noirq(), the state is supposed to be moved back to
> > > > PCI_D0,
> > > > but the current code misses the pci_legacy_resume_early() path, so the
> > > > state remains in PCI_UNKNOWN in that path. As a result, in the resume
> > > > phase of hibernation, this causes an error for the Mellanox VF driver,
> > > > which fails to enable MSI-X because pci_msi_supported() is false due
> > > > to dev->current_state != PCI_D0:
> > > >
> > > > mlx4_core a6d1:00:02.0: Detected virtual function - running in slave
> > > > mode
> > > > mlx4_core a6d1:00:02.0: Sending reset
> > > > mlx4_core a6d1:00:02.0: Sending vhcr0
> > > > mlx4_core a6d1:00:02.0: HCA minimum page size:512
> > > > mlx4_core a6d1:00:02.0: Timestamping is not supported in slave mode
> > > > mlx4_core a6d1:00:02.0: INTx is not supported in multi-function mode,
> > > aborting
> > > > PM: dpm_run_callback(): pci_pm_thaw+0x0/0xd7 returns -95
> > > > PM: Device a6d1:00:02.0 failed to thaw: error -95
> > > >
> > > > To be more accurate, the "resume" phase means the "thaw" callbacks which
> > > > run before the system enters hibernation: when the user runs the command
> > > > "echo disk > /sys/power/state" for hibernation, first the kernel
> > > > "freezes"
> > > > all the devices and creates a hibernation image, then the kernel "thaws"
> > > > the devices including the disk/NIC, writes the memory to the disk, and
> > > > powers down. This patch fixes the error message for the Mellanox VF
> > > > driver
> > > > in this phase.
Wordsmithing nit: what the patch does is not "fix the error message";
what it does is fix the *problem*, i.e., the fact that we can't
operate the device because we can't enable MSI-X. The message is only
a symptom.
IIUC the relevant part of the system hibernation sequence is:
pci_pm_freeze_noirq
pci_pm_thaw_noirq
pci_pm_thaw
And the execution flow is:
pci_pm_freeze_noirq
if (pci_has_legacy_pm_support(pci_dev)) # true for mlx4
pci_legacy_suspend_late(dev, PMSG_FREEZE)
pci_pm_set_unknown_state
dev->current_state = PCI_UNKNOWN # <---
pci_pm_thaw_noirq
if (pci_has_legacy_pm_support(pci_dev)) # true
pci_legacy_resume_early(dev) # noop; mlx4 doesn't implement
pci_pm_thaw # returns -95 EOPNOTSUPP
if (pci_has_legacy_pm_support(pci_dev)) # true
pci_legacy_resume
drv->resume
mlx4_resume # mlx4_driver.resume (legacy)
mlx4_load_one
mlx4_enable_msi_x
pci_enable_msix_range
__pci_enable_msix_range
__pci_enable_msix
if (!pci_msi_supported())
if (dev->current_state != PCI_D0) # <---
return 0
return -EINVAL
err = -EOPNOTSUPP
"INTx is not supported ..."
(These are just my notes; you don't need to put them all into the
commit message. I'm just sharing them in case I'm not understanding
correctly.)
> > > > When the system starts again, a fresh kernel starts to run, and when the
> > > > kernel detects that a hibernation image was saved, the kernel "quiesces"
> > > > the devices, and then "restores" the devices from the saved image. In
> > > > this
> > > > path:
> > > > device_resume_noirq() -> ... ->
> > > >pci_pm_restore_noirq() ->
> > > > pci_pm_default_resume_early() ->
> > > >pci_power_up() moves the device states back to PCI_D0. This path
> > > > is
> > > > not broken and doesn't need my patch.
> > > >
The cc list suggests that this might be a fix for a user-reported
problem. Is there a launchpad or similar link you could include here?
Should this be marked for stable?
> > > > Signed-off-by: Dexuan Cui
> > > This looks like a bugfix for 5839ee7389e8 ("PCI / PM: Force devices to
> > > D0 in pci_pm_thaw_noirq()") so maybe it should be marked for stable as
> > > 5839ee7389e8 was?
> > >
> > > Rafael, could you confirm?
>
> No, it is not a bug fix for that commit. T
Re: [PATCH] staging: wlan-ng: p80211wep.c: use lib/crc32
Dan Carpenter writes:
> On Mon, Oct 07, 2019 at 06:15:23PM +0200, Thomas Meyer wrote:
>> Dan Carpenter writes:
>>
>> so... what do you think?
>
> Could you send your test program?
sure, here you go:
cc crc32.c -o crc32 lib/crc32.o
#include
#include
#include
typedef uint8_t u8;
typedef uint32_t u32;
static const u32 wep_crc32_table[256] = {
0xL, 0x77073096L, 0xee0e612cL, 0x990951baL, 0x076dc419L,
0x706af48fL, 0xe963a535L, 0x9e6495a3L, 0x0edb8832L, 0x79dcb8a4L,
0xe0d5e91eL, 0x97d2d988L, 0x09b64c2bL, 0x7eb17cbdL, 0xe7b82d07L,
0x90bf1d91L, 0x1db71064L, 0x6ab020f2L, 0xf3b97148L, 0x84be41deL,
0x1adad47dL, 0x6ddde4ebL, 0xf4d4b551L, 0x83d385c7L, 0x136c9856L,
0x646ba8c0L, 0xfd62f97aL, 0x8a65c9ecL, 0x14015c4fL, 0x63066cd9L,
0xfa0f3d63L, 0x8d080df5L, 0x3b6e20c8L, 0x4c69105eL, 0xd56041e4L,
0xa2677172L, 0x3c03e4d1L, 0x4b04d447L, 0xd20d85fdL, 0xa50ab56bL,
0x35b5a8faL, 0x42b2986cL, 0xdbbbc9d6L, 0xacbcf940L, 0x32d86ce3L,
0x45df5c75L, 0xdcd60dcfL, 0xabd13d59L, 0x26d930acL, 0x51de003aL,
0xc8d75180L, 0xbfd06116L, 0x21b4f4b5L, 0x56b3c423L, 0xcfba9599L,
0xb8bda50fL, 0x2802b89eL, 0x5f058808L, 0xc60cd9b2L, 0xb10be924L,
0x2f6f7c87L, 0x58684c11L, 0xc1611dabL, 0xb6662d3dL, 0x76dc4190L,
0x01db7106L, 0x98d220bcL, 0xefd5102aL, 0x71b18589L, 0x06b6b51fL,
0x9fbfe4a5L, 0xe8b8d433L, 0x7807c9a2L, 0x0f00f934L, 0x9609a88eL,
0xe10e9818L, 0x7f6a0dbbL, 0x086d3d2dL, 0x91646c97L, 0xe6635c01L,
0x6b6b51f4L, 0x1c6c6162L, 0x856530d8L, 0xf262004eL, 0x6c0695edL,
0x1b01a57bL, 0x8208f4c1L, 0xf50fc457L, 0x65b0d9c6L, 0x12b7e950L,
0x8bbeb8eaL, 0xfcb9887cL, 0x62dd1ddfL, 0x15da2d49L, 0x8cd37cf3L,
0xfbd44c65L, 0x4db26158L, 0x3ab551ceL, 0xa3bc0074L, 0xd4bb30e2L,
0x4adfa541L, 0x3dd895d7L, 0xa4d1c46dL, 0xd3d6f4fbL, 0x4369e96aL,
0x346ed9fcL, 0xad678846L, 0xda60b8d0L, 0x44042d73L, 0x33031de5L,
0xaa0a4c5fL, 0xdd0d7cc9L, 0x5005713cL, 0x270241aaL, 0xbe0b1010L,
0xc90c2086L, 0x5768b525L, 0x206f85b3L, 0xb966d409L, 0xce61e49fL,
0x5edef90eL, 0x29d9c998L, 0xb0d09822L, 0xc7d7a8b4L, 0x59b33d17L,
0x2eb40d81L, 0xb7bd5c3bL, 0xc0ba6cadL, 0xedb88320L, 0x9abfb3b6L,
0x03b6e20cL, 0x74b1d29aL, 0xead54739L, 0x9dd277afL, 0x04db2615L,
0x73dc1683L, 0xe3630b12L, 0x94643b84L, 0x0d6d6a3eL, 0x7a6a5aa8L,
0xe40ecf0bL, 0x9309ff9dL, 0x0a00ae27L, 0x7d079eb1L, 0xf00f9344L,
0x8708a3d2L, 0x1e01f268L, 0x6906c2feL, 0xf762575dL, 0x806567cbL,
0x196c3671L, 0x6e6b06e7L, 0xfed41b76L, 0x89d32be0L, 0x10da7a5aL,
0x67dd4accL, 0xf9b9df6fL, 0x8ebeeff9L, 0x17b7be43L, 0x60b08ed5L,
0xd6d6a3e8L, 0xa1d1937eL, 0x38d8c2c4L, 0x4fdff252L, 0xd1bb67f1L,
0xa6bc5767L, 0x3fb506ddL, 0x48b2364bL, 0xd80d2bdaL, 0xaf0a1b4cL,
0x36034af6L, 0x41047a60L, 0xdf60efc3L, 0xa867df55L, 0x316e8eefL,
0x4669be79L, 0xcb61b38cL, 0xbc66831aL, 0x256fd2a0L, 0x5268e236L,
0xcc0c7795L, 0xbb0b4703L, 0x220216b9L, 0x5505262fL, 0xc5ba3bbeL,
0xb2bd0b28L, 0x2bb45a92L, 0x5cb36a04L, 0xc2d7ffa7L, 0xb5d0cf31L,
0x2cd99e8bL, 0x5bdeae1dL, 0x9b64c2b0L, 0xec63f226L, 0x756aa39cL,
0x026d930aL, 0x9c0906a9L, 0xeb0e363fL, 0x72076785L, 0x05005713L,
0x95bf4a82L, 0xe2b87a14L, 0x7bb12baeL, 0x0cb61b38L, 0x92d28e9bL,
0xe5d5be0dL, 0x7cdcefb7L, 0x0bdbdf21L, 0x86d3d2d4L, 0xf1d4e242L,
0x68ddb3f8L, 0x1fda836eL, 0x81be16cdL, 0xf6b9265bL, 0x6fb077e1L,
0x18b74777L, 0x88085ae6L, 0xff0f6a70L, 0x66063bcaL, 0x11010b5cL,
0x8f659effL, 0xf862ae69L, 0x616bffd3L, 0x166ccf45L, 0xa00ae278L,
0xd70dd2eeL, 0x4e048354L, 0x3903b3c2L, 0xa7672661L, 0xd06016f7L,
0x4969474dL, 0x3e6e77dbL, 0xaed16a4aL, 0xd9d65adcL, 0x40df0b66L,
0x37d83bf0L, 0xa9bcae53L, 0xdebb9ec5L, 0x47b2cf7fL, 0x30b5ffe9L,
0xbdbdf21cL, 0xcabac28aL, 0x53b39330L, 0x24b4a3a6L, 0xbad03605L,
0xcdd70693L, 0x54de5729L, 0x23d967bfL, 0xb3667a2eL, 0xc4614ab8L,
0x5d681b02L, 0x2a6f2b94L, 0xb40bbe37L, 0xc30c8ea1L, 0x5a05df1bL,
0x2d02ef8dL
};
u32 crc32_le(u32 crc, unsigned char const *p, size_t len);
u32 crc32_be(u32 crc, unsigned char const *p, size_t len);
void main(void) {
u8 buf[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
int len = sizeof(buf);
printf("sizeof buf=%d\n", len);
u32 crc = ~0;
u32 k;
// loop1
// for (k = 0; k < len; k++) {
// crc = wep_crc32_table[(crc ^ buf[k]) & 0xff] ^ (crc >> 8);
// // i = (i + 1) & 0xff;
// // j = (j + s[i]) & 0xff;
// // swap(i, j);
// // dst[k] = buf[k] ^ s[(s[i] + s[j]) & 0xff];
// }
// loop2
for (k = 0; k < len; k++) {
//i = (i + 1) & 0xff;
//j = (j + s[i]) & 0xff;
//swap(i, j);
//buf[k] ^= s[(s[i] + s[j]) & 0xff];
crc = wep_crc32_table[(crc ^ buf[k]) & 0xff] ^ (crc >> 8);
}
crc = ~crc;
printf("crc32_homemade= %x\n", crc);
crc = ~crc32_le(~0, buf, len);
printf("crc32_le= %x\n", crc);
crc = ~crc32_be(~0, buf, len);
print
Re: [PATCH] binder: prevent UAF read in print_binder_transaction_log_entry()
On Tue, Oct 08, 2019 at 03:01:59PM +0200, Christian Brauner wrote:
> When a binder transaction is initiated on a binder device coming from a
> binderfs instance, a pointer to the name of the binder device is stashed
> in the binder_transaction_log_entry's context_name member. Later on it
> is used to print the name in print_binder_transaction_log_entry(). By
> the time print_binder_transaction_log_entry() accesses context_name
> binderfs_evict_inode() might have already freed the associated memory
> thereby causing a UAF. Do the simple thing and prevent this by copying
> the name of the binder device instead of stashing a pointer to it.
>
> Reported-by: Jann Horn
> Fixes: 03e2e07e3814 ("binder: Make transaction_log available in binderfs")
> Link:
> https://lore.kernel.org/r/cag48ez14q0-f8lqsvcnbyr2o6gpw8shxsm4u5jmd9mpstem...@mail.gmail.com
> Cc: Joel Fernandes
> Cc: Todd Kjos
> Cc: Hridya Valsaraju
> Signed-off-by: Christian Brauner
> ---
> drivers/android/binder.c | 4 +++-
> drivers/android/binder_internal.h | 2 +-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index c0a491277aca..5b9ac2122e89 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -57,6 +57,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -66,6 +67,7 @@
> #include
>
> #include
> +#include
>
> #include
>
> @@ -2876,7 +2878,7 @@ static void binder_transaction(struct binder_proc *proc,
> e->target_handle = tr->target.handle;
> e->data_size = tr->data_size;
> e->offsets_size = tr->offsets_size;
> - e->context_name = proc->context->name;
> + strscpy(e->context_name, proc->context->name, BINDERFS_MAX_NAME);
Strictly speaking, proc-context->name can also be initialized for !BINDERFS
so the BINDERFS in the MAX_NAME macro is misleading. So probably there should
be a BINDER_MAX_NAME (and associated checks for whether non BINDERFS names
fit within the MAX.
> if (reply) {
> binder_inner_proc_lock(proc);
> diff --git a/drivers/android/binder_internal.h
> b/drivers/android/binder_internal.h
> index bd47f7f72075..ae991097d14d 100644
> --- a/drivers/android/binder_internal.h
> +++ b/drivers/android/binder_internal.h
> @@ -130,7 +130,7 @@ struct binder_transaction_log_entry {
> int return_error_line;
> uint32_t return_error;
> uint32_t return_error_param;
> - const char *context_name;
> + char context_name[BINDERFS_MAX_NAME + 1];
Same comment here, context_name can be used for non-BINDERFS transactions as
well such as default binder devices.
One more thought, this can be made dependent on CONFIG_BINDERFS since regular
binder devices cannot be unregistered AFAICS and as Jann said, the problem is
BINDERFS specific. That way we avoid the memcpy for _every_ transaction.
These can be thundering when Android starts up.
(I secretly wish C strings could be refcounted to avoid exactly this issue,
that should not be hard to develop but I am not sure if it is worth it for
this problem :) - For one, it will avoid having to do the strcpy for _every_
transaction).
Other than these nits, please add my tag on whichever is the final solution:
Reviewed-by: Joel Fernandes (Google)
thanks,
- Joel
> };
>
> struct binder_transaction_log {
> --
> 2.23.0
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] PCI: PM: Move to D0 before calling pci_legacy_resume_early()
On 10/7/2019 8:57 PM, Dexuan Cui wrote:
-Original Message-
From: Bjorn Helgaas
Sent: Monday, October 7, 2019 6:24 AM
To: Dexuan Cui
Cc: lorenzo.pieral...@arm.com; linux-...@vger.kernel.org; Michael Kelley
; linux-hyp...@vger.kernel.org;
linux-ker...@vger.kernel.org; driverdev-devel@linuxdriverproject.org; Sasha
Levin ; Haiyang Zhang
; KY Srinivasan ;
o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; vkuznets
; marcelo.ce...@canonical.com; Stephen Hemminger
; ja...@mellanox.com
Subject: Re: [PATCH v2] PCI: PM: Move to D0 before calling
pci_legacy_resume_early()
On Wed, Aug 14, 2019 at 01:06:55AM +, Dexuan Cui wrote:
In pci_legacy_suspend_late(), the device state is moved to PCI_UNKNOWN.
In pci_pm_thaw_noirq(), the state is supposed to be moved back to PCI_D0,
but the current code misses the pci_legacy_resume_early() path, so the
state remains in PCI_UNKNOWN in that path. As a result, in the resume
phase of hibernation, this causes an error for the Mellanox VF driver,
which fails to enable MSI-X because pci_msi_supported() is false due
to dev->current_state != PCI_D0:
mlx4_core a6d1:00:02.0: Detected virtual function - running in slave mode
mlx4_core a6d1:00:02.0: Sending reset
mlx4_core a6d1:00:02.0: Sending vhcr0
mlx4_core a6d1:00:02.0: HCA minimum page size:512
mlx4_core a6d1:00:02.0: Timestamping is not supported in slave mode
mlx4_core a6d1:00:02.0: INTx is not supported in multi-function mode,
aborting
PM: dpm_run_callback(): pci_pm_thaw+0x0/0xd7 returns -95
PM: Device a6d1:00:02.0 failed to thaw: error -95
To be more accurate, the "resume" phase means the "thaw" callbacks which
run before the system enters hibernation: when the user runs the command
"echo disk > /sys/power/state" for hibernation, first the kernel "freezes"
all the devices and creates a hibernation image, then the kernel "thaws"
the devices including the disk/NIC, writes the memory to the disk, and
powers down. This patch fixes the error message for the Mellanox VF driver
in this phase.
When the system starts again, a fresh kernel starts to run, and when the
kernel detects that a hibernation image was saved, the kernel "quiesces"
the devices, and then "restores" the devices from the saved image. In this
path:
device_resume_noirq() -> ... ->
pci_pm_restore_noirq() ->
pci_pm_default_resume_early() ->
pci_power_up() moves the device states back to PCI_D0. This path is
not broken and doesn't need my patch.
Signed-off-by: Dexuan Cui
This looks like a bugfix for 5839ee7389e8 ("PCI / PM: Force devices to
D0 in pci_pm_thaw_noirq()") so maybe it should be marked for stable as
5839ee7389e8 was?
Rafael, could you confirm?
No, it is not a bug fix for that commit. The underlying issue would be
there without that commit too.
---
changes in v2:
Updated the changelog with more details.
drivers/pci/pci-driver.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 36dbe960306b..27dfc68db9e7 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -1074,15 +1074,16 @@ static int pci_pm_thaw_noirq(struct device
*dev)
return error;
}
- if (pci_has_legacy_pm_support(pci_dev))
- return pci_legacy_resume_early(dev);
-
/*
* pci_restore_state() requires the device to be in D0 (because of MSI
* restoration among other things), so force it into D0 in case the
* driver's "freeze" callbacks put it into a low-power state directly.
*/
pci_set_power_state(pci_dev, PCI_D0);
+
+ if (pci_has_legacy_pm_support(pci_dev))
+ return pci_legacy_resume_early(dev);
+
pci_restore_state(pci_dev);
if (drv && drv->pm && drv->pm->thaw_noirq)
--
2.19.1
The patch looks reasonable to me, but the comment above the
pci_set_power_state() call needs to be updated too IMO.
Re: [PATCH] staging: vchiq: don't leak kernel address
On Tue, Oct 08, 2019 at 05:25:17PM +0300, Dan Carpenter wrote: > On Tue, Oct 08, 2019 at 04:21:54PM +0200, Matteo Croce wrote: > > On Tue, Oct 8, 2019 at 3:16 PM Dan Carpenter > > wrote: > > > > > > The subject doesn't match the patch. It should just be "remove useless > > > printk". > > > > > > regards, > > > dan carpenter > > > > > > > Well, it avoids leaking an address by removing an useless printk. > > It seems that GKH already picked the patch in his staging tree, but > > I'm fine with both subjects, really, > > The address wasn't leaked because it was already %pK. The subject > says there is an info leak security problem, when the opposite is true. I've edited the subject line now. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 5/6] staging: comedi: Remove set but not used variable 'hi'
On Tue, Oct 08, 2019 at 01:56:49PM +0100, Ian Abbott wrote:
> On 08/10/2019 08:41, zhengbin wrote:
> > Fixes gcc '-Wunused-but-set-variable' warning:
> >
> > drivers/staging/comedi/drivers/dt2815.c: In function dt2815_ao_insn:
> > drivers/staging/comedi/drivers/dt2815.c:91:19: warning: variable hi set but
> > not used [-Wunused-but-set-variable]
> >
> > It is not used since commit d6a929b7608a ("Staging:
> > comedi: add dt2815 driver")
> >
> > Reported-by: Hulk Robot
> > Signed-off-by: zhengbin
> > ---
> > drivers/staging/comedi/drivers/dt2815.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/comedi/drivers/dt2815.c
> > b/drivers/staging/comedi/drivers/dt2815.c
> > index 83026ba..bcf85ec 100644
> > --- a/drivers/staging/comedi/drivers/dt2815.c
> > +++ b/drivers/staging/comedi/drivers/dt2815.c
> > @@ -88,12 +88,11 @@ static int dt2815_ao_insn(struct comedi_device *dev,
> > struct comedi_subdevice *s,
> > struct dt2815_private *devpriv = dev->private;
> > int i;
> > int chan = CR_CHAN(insn->chanspec);
> > - unsigned int lo, hi;
> > + unsigned int lo;
> > int ret;
> >
> > for (i = 0; i < insn->n; i++) {
> > lo = ((data[i] & 0x0f) << 4) | (chan << 1) | 0x01;
> > - hi = (data[i] & 0xff0) >> 4;
> >
> > ret = comedi_timeout(dev, s, insn, dt2815_ao_status, 0x00);
> > if (ret)
> > --
> > 2.7.4
> >
>
> There is something fishy going on in this one too. It should be writing the
> 'hi' value to the card registers.
>
> Please could you omit this patch from the series so I can investigate?
Now dropped, thanks.
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/6] staging: comedi: Remove set but not used variable 'data'
On Tue, Oct 08, 2019 at 01:55:01PM +0100, Ian Abbott wrote:
> On 08/10/2019 08:41, zhengbin wrote:
> > Fixes gcc '-Wunused-but-set-variable' warning:
> >
> > drivers/staging/comedi/drivers/dt2814.c: In function dt2814_interrupt:
> > drivers/staging/comedi/drivers/dt2814.c:193:6: warning: variable data set
> > but not used [-Wunused-but-set-variable]
> >
> > It is not used since commit 7806012e97ed ("staging:
> > comedi: refactor dt2814 driver and use module_comedi_driver")
> >
> > Reported-by: Hulk Robot
> > Signed-off-by: zhengbin
> > ---
> > drivers/staging/comedi/drivers/dt2814.c | 3 ---
> > 1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/staging/comedi/drivers/dt2814.c
> > b/drivers/staging/comedi/drivers/dt2814.c
> > index d2c7157..4825168 100644
> > --- a/drivers/staging/comedi/drivers/dt2814.c
> > +++ b/drivers/staging/comedi/drivers/dt2814.c
> > @@ -190,7 +190,6 @@ static irqreturn_t dt2814_interrupt(int irq, void *d)
> > struct comedi_device *dev = d;
> > struct dt2814_private *devpriv = dev->private;
> > struct comedi_subdevice *s = dev->read_subdev;
> > - int data;
> >
> > if (!dev->attached) {
> > dev_err(dev->class_dev, "spurious interrupt\n");
> > @@ -200,8 +199,6 @@ static irqreturn_t dt2814_interrupt(int irq, void *d)
> > hi = inb(dev->iobase + DT2814_DATA);
> > lo = inb(dev->iobase + DT2814_DATA);
> >
> > - data = (hi << 4) | (lo >> 4);
> > -
> > if (!(--devpriv->ntrig)) {
> > int i;
> >
> > --
> > 2.7.4
> >
>
> There is something fishy going on there. The driver ought to be writing the
> data to a buffer.
>
> Can I suggest omitting this patch from the series so I can investigate and
> supply a proper fix?
Now dropped.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/7] Fix various compilation issues with wfx driver
On Tue, Oct 08, 2019 at 09:42:47AM +, Jerome Pouiller wrote: > From: Jérôme Pouiller > > Most of problems are related to big-endian architectures. kbuild still reports 2 errors with these patches applied: Regressions in current branch: drivers/staging/wfx/hif_tx.c:82:2-8: preceding lock on line 65 drivers/staging/wfx/main.c:188:14-21: ERROR: PTR_ERR applied after initialization to constant on line 183 Can you please fix those up as well? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vchiq: don't leak kernel address
On Tue, Oct 08, 2019 at 04:21:54PM +0200, Matteo Croce wrote: > On Tue, Oct 8, 2019 at 3:16 PM Dan Carpenter wrote: > > > > The subject doesn't match the patch. It should just be "remove useless > > printk". > > > > regards, > > dan carpenter > > > > Well, it avoids leaking an address by removing an useless printk. > It seems that GKH already picked the patch in his staging tree, but > I'm fine with both subjects, really, The address wasn't leaked because it was already %pK. The subject says there is an info leak security problem, when the opposite is true. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vchiq: don't leak kernel address
On Tue, Oct 8, 2019 at 3:16 PM Dan Carpenter wrote: > > The subject doesn't match the patch. It should just be "remove useless > printk". > > regards, > dan carpenter > Well, it avoids leaking an address by removing an useless printk. It seems that GKH already picked the patch in his staging tree, but I'm fine with both subjects, really, Greg? -- Matteo Croce per aspera ad upstream ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: fieldbus: make use of devm_platform_ioremap_resource
On Tue, Oct 8, 2019 at 2:11 AM hariprasad Kelam wrote: > > From: Hariprasad Kelam > > fix below issues reported by coccicheck > drivers/staging//fieldbus/anybuss/arcx-anybus.c:135:1-5: WARNING: Use > devm_platform_ioremap_resource for base > drivers/staging//fieldbus/anybuss/arcx-anybus.c:248:1-14: WARNING: Use > devm_platform_ioremap_resource for cd -> cpld_base > > Signed-off-by: Hariprasad Kelam > --- > drivers/staging/fieldbus/anybuss/arcx-anybus.c | 8 ++-- > 1 file changed, 2 insertions(+), 6 deletions(-) Reviewed-by: Sven Van Asbroeck Tested-by: Sven Van Asbroeck ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vchiq: don't leak kernel address
The subject doesn't match the patch. It should just be "remove useless printk". regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] binder: prevent UAF read in print_binder_transaction_log_entry()
When a binder transaction is initiated on a binder device coming from a
binderfs instance, a pointer to the name of the binder device is stashed
in the binder_transaction_log_entry's context_name member. Later on it
is used to print the name in print_binder_transaction_log_entry(). By
the time print_binder_transaction_log_entry() accesses context_name
binderfs_evict_inode() might have already freed the associated memory
thereby causing a UAF. Do the simple thing and prevent this by copying
the name of the binder device instead of stashing a pointer to it.
Reported-by: Jann Horn
Fixes: 03e2e07e3814 ("binder: Make transaction_log available in binderfs")
Link:
https://lore.kernel.org/r/cag48ez14q0-f8lqsvcnbyr2o6gpw8shxsm4u5jmd9mpstem...@mail.gmail.com
Cc: Joel Fernandes
Cc: Todd Kjos
Cc: Hridya Valsaraju
Signed-off-by: Christian Brauner
---
drivers/android/binder.c | 4 +++-
drivers/android/binder_internal.h | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index c0a491277aca..5b9ac2122e89 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -57,6 +57,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -66,6 +67,7 @@
#include
#include
+#include
#include
@@ -2876,7 +2878,7 @@ static void binder_transaction(struct binder_proc *proc,
e->target_handle = tr->target.handle;
e->data_size = tr->data_size;
e->offsets_size = tr->offsets_size;
- e->context_name = proc->context->name;
+ strscpy(e->context_name, proc->context->name, BINDERFS_MAX_NAME);
if (reply) {
binder_inner_proc_lock(proc);
diff --git a/drivers/android/binder_internal.h
b/drivers/android/binder_internal.h
index bd47f7f72075..ae991097d14d 100644
--- a/drivers/android/binder_internal.h
+++ b/drivers/android/binder_internal.h
@@ -130,7 +130,7 @@ struct binder_transaction_log_entry {
int return_error_line;
uint32_t return_error;
uint32_t return_error_param;
- const char *context_name;
+ char context_name[BINDERFS_MAX_NAME + 1];
};
struct binder_transaction_log {
--
2.23.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 6/6] staging: comedi: Remove set but not used variable 'aref'
On 08/10/2019 08:41, zhengbin wrote:
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/staging/comedi/drivers/dt3000.c: In function dt3k_ai_insn_read:
drivers/staging/comedi/drivers/dt3000.c:511:27: warning: variable aref set but
not used [-Wunused-but-set-variable]
It is not used since commit 2e310235ca8f ("staging:
comedi: dt3000: rename dt3k_ai_insn()")
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
drivers/staging/comedi/drivers/dt3000.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/dt3000.c
b/drivers/staging/comedi/drivers/dt3000.c
index caf4d4d..f7c365b 100644
--- a/drivers/staging/comedi/drivers/dt3000.c
+++ b/drivers/staging/comedi/drivers/dt3000.c
@@ -508,12 +508,11 @@ static int dt3k_ai_insn_read(struct comedi_device *dev,
unsigned int *data)
{
int i;
- unsigned int chan, gain, aref;
+ unsigned int chan, gain;
chan = CR_CHAN(insn->chanspec);
gain = CR_RANGE(insn->chanspec);
/* XXX docs don't explain how to select aref */
- aref = CR_AREF(insn->chanspec);
for (i = 0; i < insn->n; i++)
data[i] = dt3k_readsingle(dev, DPR_SUBSYS_AI, chan, gain);
--
2.7.4
That looks fine, thanks.
Reviewed-by: Ian Abbott
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 5/6] staging: comedi: Remove set but not used variable 'hi'
On 08/10/2019 08:41, zhengbin wrote:
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/staging/comedi/drivers/dt2815.c: In function dt2815_ao_insn:
drivers/staging/comedi/drivers/dt2815.c:91:19: warning: variable hi set but not
used [-Wunused-but-set-variable]
It is not used since commit d6a929b7608a ("Staging:
comedi: add dt2815 driver")
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
drivers/staging/comedi/drivers/dt2815.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/dt2815.c
b/drivers/staging/comedi/drivers/dt2815.c
index 83026ba..bcf85ec 100644
--- a/drivers/staging/comedi/drivers/dt2815.c
+++ b/drivers/staging/comedi/drivers/dt2815.c
@@ -88,12 +88,11 @@ static int dt2815_ao_insn(struct comedi_device *dev, struct
comedi_subdevice *s,
struct dt2815_private *devpriv = dev->private;
int i;
int chan = CR_CHAN(insn->chanspec);
- unsigned int lo, hi;
+ unsigned int lo;
int ret;
for (i = 0; i < insn->n; i++) {
lo = ((data[i] & 0x0f) << 4) | (chan << 1) | 0x01;
- hi = (data[i] & 0xff0) >> 4;
ret = comedi_timeout(dev, s, insn, dt2815_ao_status, 0x00);
if (ret)
--
2.7.4
There is something fishy going on in this one too. It should be writing
the 'hi' value to the card registers.
Please could you omit this patch from the series so I can investigate?
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/6] staging: comedi: Remove set but not used variable 'data'
On 08/10/2019 08:41, zhengbin wrote:
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/staging/comedi/drivers/dt2814.c: In function dt2814_interrupt:
drivers/staging/comedi/drivers/dt2814.c:193:6: warning: variable data set but
not used [-Wunused-but-set-variable]
It is not used since commit 7806012e97ed ("staging:
comedi: refactor dt2814 driver and use module_comedi_driver")
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
drivers/staging/comedi/drivers/dt2814.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/dt2814.c
b/drivers/staging/comedi/drivers/dt2814.c
index d2c7157..4825168 100644
--- a/drivers/staging/comedi/drivers/dt2814.c
+++ b/drivers/staging/comedi/drivers/dt2814.c
@@ -190,7 +190,6 @@ static irqreturn_t dt2814_interrupt(int irq, void *d)
struct comedi_device *dev = d;
struct dt2814_private *devpriv = dev->private;
struct comedi_subdevice *s = dev->read_subdev;
- int data;
if (!dev->attached) {
dev_err(dev->class_dev, "spurious interrupt\n");
@@ -200,8 +199,6 @@ static irqreturn_t dt2814_interrupt(int irq, void *d)
hi = inb(dev->iobase + DT2814_DATA);
lo = inb(dev->iobase + DT2814_DATA);
- data = (hi << 4) | (lo >> 4);
-
if (!(--devpriv->ntrig)) {
int i;
--
2.7.4
There is something fishy going on there. The driver ought to be writing
the data to a buffer.
Can I suggest omitting this patch from the series so I can investigate
and supply a proper fix?
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: rtl8723bs: Remove set but not used variable 'i'
On Tue, Oct 08, 2019 at 09:25:03AM +0800, zhengbin wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/staging/rtl8723bs/core/rtw_xmit.c: In function update_attrib:
> drivers/staging/rtl8723bs/core/rtw_xmit.c:680:7: warning: variable i set but
> not used [-Wunused-but-set-variable]
>
> It is not used since commit 554c0a3abf21 ("staging:
> Add rtl8723bs sdio wifi driver")
>
> Reported-by: Hulk Robot
> Signed-off-by: zhengbin
> ---
> drivers/staging/rtl8723bs/core/rtw_xmit.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
What changed from v1? Always put that below the --- line.
Please fix that up and resend a v3.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: media: sunxi: make use of devm_platform_ioremap_resource
On Tue, Oct 08, 2019 at 12:29:34PM +0530, haripra...@osuosl.org wrote: > From: Hariprasad Kelam > > fix below issue reported by coccicheck > drivers/staging//media/sunxi/cedrus/cedrus_hw.c:229:1-10: WARNING: Use > devm_platform_ioremap_resource for dev -> base > > Signed-off-by: Hariprasad Kelam > --- > drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) I've dropped all of your patches, please fix up your tool and resend this as a patch series so we know what order to apply them in. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] rtl8723bs: core: Remove code valid only for 5GHZ
On Sun, Sep 29, 2019 at 07:24:57PM +0530, haripra...@osuosl.org wrote: > From: Hariprasad Kelam > > As per TODO ,remove code valid only for 5 GHz(channel > 14). > > Signed-off-by: Hariprasad Kelam > --- > drivers/staging/rtl8723bs/core/rtw_ieee80211.c | 19 ++- > 1 file changed, 6 insertions(+), 13 deletions(-) Your email client is really messed up and got the From: line wrong in your tool. Please fix up and resend. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: vc04_services: Avoid NULL comparison
On Mon, Oct 07, 2019 at 03:29:28PM -0700, Nachammai Karuppiah wrote:
> Remove NULL comparison. Issue found using checkpatch.pl
>
> Signed-off-by: Nachammai Karuppiah
>
> ---
>
> Changes in V2
>- Remove all NULL comparisons in vc04_services/interface directory.
> ---
> .../interface/vchiq_arm/vchiq_2835_arm.c | 4 ++--
> .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 28
> +++---
> .../vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ++--
> .../vc04_services/interface/vchiq_arm/vchiq_shim.c | 2 +-
> 4 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git
> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> index 8dc730c..7cdb21e 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> @@ -526,11 +526,11 @@ create_pagelist(char __user *buf, size_t count,
> unsigned short type)
> return NULL;
> }
>
> - WARN_ON(g_free_fragments == NULL);
> + WARN_ON(!g_free_fragments);
>
> down(&g_free_fragments_mutex);
> fragments = g_free_fragments;
> - WARN_ON(fragments == NULL);
> + WARN_ON(!fragments);
> g_free_fragments = *(char **) g_free_fragments;
> up(&g_free_fragments_mutex);
> pagelist->type = PAGELIST_READ_WITH_FRAGMENTS +
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index b1595b1..b930148 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -826,8 +826,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned
> long arg)
>
> /* Remove all services */
> i = 0;
> - while ((service = next_service_by_instance(instance->state,
> - instance, &i)) != NULL) {
> + while (service = next_service_by_instance(instance->state,
> + instance, &i)) {
As you can see, this added a build warning. Please always be careful
about your patches to not have them do that :(
Please fix up and resend.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vchiq: don't leak kernel address
Since commit ad67b74d2469d9b8 ("printk: hash addresses printed with %p"),
an obfuscated kernel pointer is printed at boot:
vchiq: vchiq_init_state: slot_zero = (ptrval)
Remove the the print completely, as it's useless without the address.
Signed-off-by: Matteo Croce
---
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 56a23a297fa4..084cd4b0f07c 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -2132,9 +2132,6 @@ vchiq_init_state(struct vchiq_state *state, struct
vchiq_slot_zero *slot_zero)
char threadname[16];
int i;
- vchiq_log_warning(vchiq_core_log_level,
- "%s: slot_zero = %pK", __func__, slot_zero);
-
if (vchiq_states[0]) {
pr_err("%s: VCHIQ state already initialized\n", __func__);
return VCHIQ_ERROR;
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/7] staging: wfx: drop calls to BUG_ON()
On Tue, Oct 08, 2019 at 09:43:01AM +, Jerome Pouiller wrote:
> @@ -56,9 +56,9 @@ static uint8_t fill_tkip_pair(struct hif_tkip_pairwise_key
> *msg,
> {
> uint8_t *keybuf = key->key;
>
> - WARN_ON(key->keylen != sizeof(msg->tkip_key_data)
> -+ sizeof(msg->tx_mic_key)
> -+ sizeof(msg->rx_mic_key));
> + WARN(key->keylen != sizeof(msg->tkip_key_data)
> + + sizeof(msg->tx_mic_key)
> + + sizeof(msg->rx_mic_key), "inconsistent data");
This is not a comment on the patch since the code was like that
originally, but the " +" should go of the first line:
WARN(key->keylen != sizeof(msg->tkip_key_data) +
sizeof(msg->tx_mic_key) +
sizeof(msg->rx_mic_key),
"inconsistent data");
That doesn't look too good still... The error message is sort of
rubbish also. Anyway the operator goes on the first line.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/7] staging: wfx: correctly cast data on big-endian targets
On Tue, Oct 08, 2019 at 09:43:00AM +, Jerome Pouiller wrote:
> From: Jérôme Pouiller
>
> When built for a big-endian target, original code caused error:
>
> include/uapi/linux/swab.h:242:29: note: expected '__u32 * {aka unsigned
> int *}' but argument is of type 'struct hif_mib_protected_mgmt_policy *'
>
> Fixes: f95a29d40782 ("staging: wfx: add HIF commands helpers")
> Reported-by: kbuild test robot
> Reported-by: Stephen Rothwell
> Signed-off-by: Jérôme Pouiller
> ---
> drivers/staging/wfx/hif_tx_mib.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/wfx/hif_tx_mib.h
> b/drivers/staging/wfx/hif_tx_mib.h
> index 167c5dec009f..4f132348f5fa 100644
> --- a/drivers/staging/wfx/hif_tx_mib.h
> +++ b/drivers/staging/wfx/hif_tx_mib.h
> @@ -145,7 +145,7 @@ static inline int hif_set_mfp(struct wfx_vif *wvif, bool
> capable, bool required)
> }
> if (!required)
> val.unpmf_allowed = 1;
> - cpu_to_le32s(&val);
> + cpu_to_le32s((uint32_t *) &val);
Again, this is fine for now, but in the future there shouldn't be a
space after the cast. It's to mark that it's a high precedence
operation.
cpu_to_le32s((uint32_t *)&val);
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/7] staging: wfx: simplify memory allocation in wfx_update_filtering()
These patches are good. I just have a few nits to point out for future
reference.
On Tue, Oct 08, 2019 at 09:42:58AM +, Jerome Pouiller wrote:
> static inline int hif_set_beacon_filter_table(struct wfx_vif *wvif,
> - struct hif_mib_bcn_filter_table
> *ft)
> + int tbl_len,
> + struct hif_ie_table_entry *tbl)
> {
> - size_t buf_len = struct_size(ft, ie_table, ft->num_of_info_elmts);
> + int ret;
> + struct hif_mib_bcn_filter_table *val;
> + int buf_len = struct_size(val, ie_table, tbl_len);
This is fine for now, but since this is networking code, in the future
could you write your declarations in reverse Christmas tree order?
long laskdfjasldfj asldfkj aslkdfj alskdfj askldfj;
mid asdfasdflkajdflasjdf lkasjdf;
short asdfasd;
shortest i;
>
> - cpu_to_le32s(&ft->num_of_info_elmts);
> - return hif_write_mib(wvif->wdev, wvif->id,
> - HIF_MIB_ID_BEACON_FILTER_TABLE, ft, buf_len);
[ snip ]
> diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
> index 2855d14a709c..12198b8f3685 100644
> --- a/drivers/staging/wfx/sta.c
> +++ b/drivers/staging/wfx/sta.c
> @@ -217,14 +217,13 @@ void wfx_update_filtering(struct wfx_vif *wvif)
> bool filter_bssid = wvif->filter_bssid;
> bool fwd_probe_req = wvif->fwd_probe_req;
> struct hif_mib_bcn_filter_enable bf_ctrl;
> - struct hif_mib_bcn_filter_table *bf_tbl;
> - struct hif_ie_table_entry ie_tbl[] = {
> + struct hif_ie_table_entry filter_ies[] = {
> {
> .ie_id= WLAN_EID_VENDOR_SPECIFIC,
> .has_changed = 1,
> .no_longer= 1,
> .has_appeared = 1,
> - .oui = { 0x50, 0x6F, 0x9A},
> + .oui = { 0x50, 0x6F, 0x9A },
Please don't do unrelated white space changes in their own patches.
> }, {
> .ie_id= WLAN_EID_HT_OPERATION,
> .has_changed = 1,
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/7] staging: wfx: drop calls to BUG_ON()
From: Jérôme Pouiller
Most of calls to BUG_ON() could replaced by WARN().
By the way, this patch also try to favor WARN() (that include a comment
about the problem) instead of WARN_ON().
Reported-by: Andrew Lunn
Signed-off-by: Jérôme Pouiller
---
drivers/staging/wfx/bh.c | 4 ++--
drivers/staging/wfx/bus_sdio.c | 4 ++--
drivers/staging/wfx/data_tx.c| 4 ++--
drivers/staging/wfx/hif_tx_mib.h | 2 +-
drivers/staging/wfx/key.c| 32
drivers/staging/wfx/queue.c | 6 +++---
drivers/staging/wfx/scan.c | 2 +-
drivers/staging/wfx/sta.c| 2 +-
8 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index 3715bb18bd78..3355183fc86c 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -56,7 +56,7 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len,
int *is_cnf)
int release_count;
int piggyback = 0;
- WARN_ON(read_len < 4);
+ WARN(read_len < 4, "corrupted read");
WARN(read_len > round_down(0xFFF, 2) * sizeof(u16),
"%s: request exceed WFx capability", __func__);
@@ -173,7 +173,7 @@ static void tx_helper(struct wfx_dev *wdev, struct hif_msg
*hif)
bool is_encrypted = false;
size_t len = le16_to_cpu(hif->len);
- BUG_ON(len < sizeof(*hif));
+ WARN(len < sizeof(*hif), "try to send corrupted data");
hif->seqnum = wdev->hif.tx_seqnum;
wdev->hif.tx_seqnum = (wdev->hif.tx_seqnum + 1) % (HIF_COUNTER_MAX + 1);
diff --git a/drivers/staging/wfx/bus_sdio.c b/drivers/staging/wfx/bus_sdio.c
index 05f02c278782..f97360513150 100644
--- a/drivers/staging/wfx/bus_sdio.c
+++ b/drivers/staging/wfx/bus_sdio.c
@@ -37,7 +37,7 @@ static int wfx_sdio_copy_from_io(void *priv, unsigned int
reg_id,
unsigned int sdio_addr = reg_id << 2;
int ret;
- BUG_ON(reg_id > 7);
+ WARN(reg_id > 7, "chip only has 7 registers");
WARN(((uintptr_t) dst) & 3, "unaligned buffer size");
WARN(count & 3, "unaligned buffer address");
@@ -58,7 +58,7 @@ static int wfx_sdio_copy_to_io(void *priv, unsigned int
reg_id,
unsigned int sdio_addr = reg_id << 2;
int ret;
- BUG_ON(reg_id > 7);
+ WARN(reg_id > 7, "chip only has 7 registers");
WARN(((uintptr_t) src) & 3, "unaligned buffer size");
WARN(count & 3, "unaligned buffer address");
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 1891bcaaf9fc..b2ca3986c6d0 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -44,7 +44,7 @@ static void tx_policy_build(struct wfx_vif *wvif, struct
tx_policy *policy,
size_t count;
struct wfx_dev *wdev = wvif->wdev;
- BUG_ON(rates[0].idx < 0);
+ WARN(rates[0].idx < 0, "invalid rate policy");
memset(policy, 0, sizeof(*policy));
for (i = 1; i < IEEE80211_TX_MAX_RATES; i++)
if (rates[i].idx < 0)
@@ -162,7 +162,7 @@ static int tx_policy_get(struct wfx_vif *wvif, struct
ieee80211_tx_rate *rates,
tx_policy_build(wvif, &wanted, rates);
spin_lock_bh(&cache->lock);
- if (WARN_ON_ONCE(list_empty(&cache->free))) {
+ if (WARN_ON(list_empty(&cache->free))) {
spin_unlock_bh(&cache->lock);
return WFX_INVALID_RATE_ID;
}
diff --git a/drivers/staging/wfx/hif_tx_mib.h b/drivers/staging/wfx/hif_tx_mib.h
index 4f132348f5fa..3339ad95f732 100644
--- a/drivers/staging/wfx/hif_tx_mib.h
+++ b/drivers/staging/wfx/hif_tx_mib.h
@@ -138,7 +138,7 @@ static inline int hif_set_mfp(struct wfx_vif *wvif, bool
capable, bool required)
{
struct hif_mib_protected_mgmt_policy val = { };
- WARN_ON(required && !capable);
+ WARN(required && !capable, "incoherent arguments");
if (capable) {
val.pmf_enable = 1;
val.host_enc_auth_frames = 1;
diff --git a/drivers/staging/wfx/key.c b/drivers/staging/wfx/key.c
index 4e7d2b510a9c..6d03abec20e4 100644
--- a/drivers/staging/wfx/key.c
+++ b/drivers/staging/wfx/key.c
@@ -26,7 +26,7 @@ static int wfx_alloc_key(struct wfx_dev *wdev)
static void wfx_free_key(struct wfx_dev *wdev, int idx)
{
- BUG_ON(!(wdev->key_map & BIT(idx)));
+ WARN(!(wdev->key_map & BIT(idx)), "inconsistent key allocation");
memset(&wdev->keys[idx], 0, sizeof(wdev->keys[idx]));
wdev->key_map &= ~BIT(idx);
}
@@ -34,7 +34,7 @@ static void wfx_free_key(struct wfx_dev *wdev, int idx)
static uint8_t fill_wep_pair(struct hif_wep_pairwise_key *msg,
struct ieee80211_key_conf *key, u8 *peer_addr)
{
- WARN_ON(key->keylen > sizeof(msg->key_data));
+ WARN(key->keylen > sizeof(msg->key_data), "inconsistent data");
msg->key_length = key->keylen;
memcpy(msg->key_data, key->key, key->keylen);
ether_addr_copy(msg->peer_addr
[PATCH 5/7] staging: wfx: fix copy_{to,from}_user() usage
From: Jérôme Pouiller
On error, copy_to_user() returns number of bytes remaining. Driver
should return -EFAULT.
Fixes: 4f8b7fabb15d ("staging: wfx: allow to send commands to chip")
Reported-by: kbuild test robot
Reported-by: Dan Carpenter
Signed-off-by: Jérôme Pouiller
---
drivers/staging/wfx/debug.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/wfx/debug.c b/drivers/staging/wfx/debug.c
index 3261b267c385..8de16ad7c710 100644
--- a/drivers/staging/wfx/debug.c
+++ b/drivers/staging/wfx/debug.c
@@ -256,9 +256,8 @@ static ssize_t wfx_send_hif_msg_read(struct file *file,
char __user *user_buf,
return context->ret;
// Be carefull, write() is waiting for a full message while read()
// only return a payload
- ret = copy_to_user(user_buf, context->reply, count);
- if (ret)
- return ret;
+ if (copy_to_user(user_buf, context->reply, count))
+ return -EFAULT;
return count;
}
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/7] staging: wfx: avoid namespace contamination
From: Jérôme Pouiller
tx_policy_init() was already defined in driver cw1200. So, compilation
failed when wfx and cw1200 were both built-in.
In order to keep a coherent naming scheme, this patch prefixes all
"tx_policy_*" functions with "wfx_".
Fixes: 9bca45f3d692 ("staging: wfx: allow to send 802.11 frames")
Reported-by: kbuild test robot
Signed-off-by: Jérôme Pouiller
---
drivers/staging/wfx/data_tx.c | 34 +-
drivers/staging/wfx/data_tx.h | 2 +-
drivers/staging/wfx/sta.c | 2 +-
3 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index b2ca3986c6d0..6e4dd4ac5544 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -37,7 +37,7 @@ static int wfx_get_hw_rate(struct wfx_dev *wdev, const struct
ieee80211_tx_rate
/* TX policy cache implementation */
-static void tx_policy_build(struct wfx_vif *wvif, struct tx_policy *policy,
+static void wfx_tx_policy_build(struct wfx_vif *wvif, struct tx_policy *policy,
struct ieee80211_tx_rate *rates)
{
int i;
@@ -124,7 +124,7 @@ static bool tx_policy_is_equal(const struct tx_policy *a,
const struct tx_policy
return !memcmp(a->rates, b->rates, sizeof(a->rates));
}
-static int tx_policy_find(struct tx_policy_cache *cache, struct tx_policy
*wanted)
+static int wfx_tx_policy_find(struct tx_policy_cache *cache, struct tx_policy
*wanted)
{
struct tx_policy *it;
@@ -137,13 +137,13 @@ static int tx_policy_find(struct tx_policy_cache *cache,
struct tx_policy *wante
return -1;
}
-static void tx_policy_use(struct tx_policy_cache *cache, struct tx_policy
*entry)
+static void wfx_tx_policy_use(struct tx_policy_cache *cache, struct tx_policy
*entry)
{
++entry->usage_count;
list_move(&entry->link, &cache->used);
}
-static int tx_policy_release(struct tx_policy_cache *cache, struct tx_policy
*entry)
+static int wfx_tx_policy_release(struct tx_policy_cache *cache, struct
tx_policy *entry)
{
int ret = --entry->usage_count;
@@ -152,21 +152,21 @@ static int tx_policy_release(struct tx_policy_cache
*cache, struct tx_policy *en
return ret;
}
-static int tx_policy_get(struct wfx_vif *wvif, struct ieee80211_tx_rate *rates,
+static int wfx_tx_policy_get(struct wfx_vif *wvif, struct ieee80211_tx_rate
*rates,
bool *renew)
{
int idx;
struct tx_policy_cache *cache = &wvif->tx_policy_cache;
struct tx_policy wanted;
- tx_policy_build(wvif, &wanted, rates);
+ wfx_tx_policy_build(wvif, &wanted, rates);
spin_lock_bh(&cache->lock);
if (WARN_ON(list_empty(&cache->free))) {
spin_unlock_bh(&cache->lock);
return WFX_INVALID_RATE_ID;
}
- idx = tx_policy_find(cache, &wanted);
+ idx = wfx_tx_policy_find(cache, &wanted);
if (idx >= 0) {
*renew = false;
} else {
@@ -181,7 +181,7 @@ static int tx_policy_get(struct wfx_vif *wvif, struct
ieee80211_tx_rate *rates,
entry->usage_count = 0;
idx = entry - cache->cache;
}
- tx_policy_use(cache, &cache->cache[idx]);
+ wfx_tx_policy_use(cache, &cache->cache[idx]);
if (list_empty(&cache->free)) {
/* Lock TX queues. */
wfx_tx_queues_lock(wvif->wdev);
@@ -190,14 +190,14 @@ static int tx_policy_get(struct wfx_vif *wvif, struct
ieee80211_tx_rate *rates,
return idx;
}
-static void tx_policy_put(struct wfx_vif *wvif, int idx)
+static void wfx_tx_policy_put(struct wfx_vif *wvif, int idx)
{
int usage, locked;
struct tx_policy_cache *cache = &wvif->tx_policy_cache;
spin_lock_bh(&cache->lock);
locked = list_empty(&cache->free);
- usage = tx_policy_release(cache, &cache->cache[idx]);
+ usage = wfx_tx_policy_release(cache, &cache->cache[idx]);
if (locked && !usage) {
/* Unlock TX queues. */
wfx_tx_queues_unlock(wvif->wdev);
@@ -205,7 +205,7 @@ static void tx_policy_put(struct wfx_vif *wvif, int idx)
spin_unlock_bh(&cache->lock);
}
-static int tx_policy_upload(struct wfx_vif *wvif)
+static int wfx_tx_policy_upload(struct wfx_vif *wvif)
{
int i;
struct tx_policy_cache *cache = &wvif->tx_policy_cache;
@@ -238,18 +238,18 @@ static int tx_policy_upload(struct wfx_vif *wvif)
return 0;
}
-static void tx_policy_upload_work(struct work_struct *work)
+static void wfx_tx_policy_upload_work(struct work_struct *work)
{
struct wfx_vif *wvif =
container_of(work, struct wfx_vif, tx_policy_upload_work);
- tx_policy_upload(wvif);
+ wfx_tx_policy_upload(wvif);
wfx_tx_unlock(wvif->wdev);
wfx_tx_queues_unlock(wvif->wdev);
}
-void tx_policy_init(struct wfx_vif *wvif)
+void wfx_tx_policy_init
[PATCH 3/7] staging: wfx: le16_to_cpus() takes a reference as parameter
From: Jérôme Pouiller
Original code caused an (100% reproducible) invalid memory access on
big-endian targets.
Fixes: b0998f0c040d "staging: wfx: add IRQ handling"
Reported-by: kbuild test robot
Reported-by: Stephen Rothwell
Signed-off-by: Jérôme Pouiller
---
drivers/staging/wfx/bh.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index 6000c03bb658..3715bb18bd78 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -83,12 +83,12 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len,
int *is_cnf)
// piggyback is probably correct.
return piggyback;
}
- le16_to_cpus(hif->len);
+ le16_to_cpus(&hif->len);
computed_len = round_up(hif->len - sizeof(hif->len), 16)
+ sizeof(struct hif_sl_msg)
+ sizeof(struct hif_sl_tag);
} else {
- le16_to_cpus(hif->len);
+ le16_to_cpus(&hif->len);
computed_len = round_up(hif->len, 2);
}
if (computed_len != read_len) {
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/7] staging: wfx: remove misused call to cpu_to_le16()
From: Jérôme Pouiller
Indeed, hif_msg->id is a uint8_t, so use of cpu_to_le16() is a madness.
Fixes: 9bca45f3d692 ("staging: wfx: allow to send 802.11 frames")
Reported-by: kbuild test robot
Reported-by: Stephen Rothwell
Signed-off-by: Jérôme Pouiller
---
drivers/staging/wfx/data_tx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 7f2799fbdafe..1891bcaaf9fc 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -620,7 +620,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct
ieee80211_sta *sta, struct
memset(skb->data, 0, wmsg_len);
hif_msg = (struct hif_msg *) skb->data;
hif_msg->len = cpu_to_le16(skb->len);
- hif_msg->id = cpu_to_le16(HIF_REQ_ID_TX);
+ hif_msg->id = HIF_REQ_ID_TX;
hif_msg->interface = wvif->id;
if (skb->len > wvif->wdev->hw_caps.size_inp_ch_buf) {
dev_warn(wvif->wdev->dev, "requested frame size (%d) is larger
than maximum supported (%d)\n",
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/7] staging: wfx: simplify memory allocation in wfx_update_filtering()
From: Jérôme Pouiller
Original code did not handle case where kmalloc failed. By the way, it
is more convenient to allocate and build HIF message in
hif_set_beacon_filter_table() instead of to ask to caller function to
build it.
Fixes: 40115bbc40e2 ("staging: wfx: implement the rest of mac80211 API")
Reported-by: kbuild test robot
Signed-off-by: Jérôme Pouiller
---
drivers/staging/wfx/hif_tx_mib.h | 19 ++-
drivers/staging/wfx/sta.c| 17 +++--
2 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/wfx/hif_tx_mib.h b/drivers/staging/wfx/hif_tx_mib.h
index f6624a403016..167c5dec009f 100644
--- a/drivers/staging/wfx/hif_tx_mib.h
+++ b/drivers/staging/wfx/hif_tx_mib.h
@@ -86,13 +86,22 @@ static inline int hif_set_rx_filter(struct wfx_vif *wvif,
bool filter_bssid,
}
static inline int hif_set_beacon_filter_table(struct wfx_vif *wvif,
- struct hif_mib_bcn_filter_table
*ft)
+ int tbl_len,
+ struct hif_ie_table_entry *tbl)
{
- size_t buf_len = struct_size(ft, ie_table, ft->num_of_info_elmts);
+ int ret;
+ struct hif_mib_bcn_filter_table *val;
+ int buf_len = struct_size(val, ie_table, tbl_len);
- cpu_to_le32s(&ft->num_of_info_elmts);
- return hif_write_mib(wvif->wdev, wvif->id,
-HIF_MIB_ID_BEACON_FILTER_TABLE, ft, buf_len);
+ val = kzalloc(buf_len, GFP_KERNEL);
+ if (!val)
+ return -ENOMEM;
+ val->num_of_info_elmts = cpu_to_le32(tbl_len);
+ memcpy(val->ie_table, tbl, tbl_len * sizeof(*tbl));
+ ret = hif_write_mib(wvif->wdev, wvif->id,
+ HIF_MIB_ID_BEACON_FILTER_TABLE, val, buf_len);
+ kfree(val);
+ return ret;
}
static inline int hif_beacon_filter_control(struct wfx_vif *wvif,
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 2855d14a709c..12198b8f3685 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -217,14 +217,13 @@ void wfx_update_filtering(struct wfx_vif *wvif)
bool filter_bssid = wvif->filter_bssid;
bool fwd_probe_req = wvif->fwd_probe_req;
struct hif_mib_bcn_filter_enable bf_ctrl;
- struct hif_mib_bcn_filter_table *bf_tbl;
- struct hif_ie_table_entry ie_tbl[] = {
+ struct hif_ie_table_entry filter_ies[] = {
{
.ie_id= WLAN_EID_VENDOR_SPECIFIC,
.has_changed = 1,
.no_longer= 1,
.has_appeared = 1,
- .oui = { 0x50, 0x6F, 0x9A},
+ .oui = { 0x50, 0x6F, 0x9A },
}, {
.ie_id= WLAN_EID_HT_OPERATION,
.has_changed = 1,
@@ -237,36 +236,34 @@ void wfx_update_filtering(struct wfx_vif *wvif)
.has_appeared = 1,
}
};
+ int n_filter_ies;
if (wvif->state == WFX_STATE_PASSIVE)
return;
- bf_tbl = kmalloc(sizeof(struct hif_mib_bcn_filter_table) +
sizeof(ie_tbl), GFP_KERNEL);
- memcpy(bf_tbl->ie_table, ie_tbl, sizeof(ie_tbl));
if (wvif->disable_beacon_filter) {
bf_ctrl.enable = 0;
bf_ctrl.bcn_count = 1;
- bf_tbl->num_of_info_elmts = 0;
+ n_filter_ies = 0;
} else if (!is_sta) {
bf_ctrl.enable = HIF_BEACON_FILTER_ENABLE |
HIF_BEACON_FILTER_AUTO_ERP;
bf_ctrl.bcn_count = 0;
- bf_tbl->num_of_info_elmts = 2;
+ n_filter_ies = 2;
} else {
bf_ctrl.enable = HIF_BEACON_FILTER_ENABLE;
bf_ctrl.bcn_count = 0;
- bf_tbl->num_of_info_elmts = 3;
+ n_filter_ies = 3;
}
ret = hif_set_rx_filter(wvif, filter_bssid, fwd_probe_req);
if (!ret)
- ret = hif_set_beacon_filter_table(wvif, bf_tbl);
+ ret = hif_set_beacon_filter_table(wvif, n_filter_ies,
filter_ies);
if (!ret)
ret = hif_beacon_filter_control(wvif, bf_ctrl.enable,
bf_ctrl.bcn_count);
if (!ret)
ret = wfx_set_mcast_filter(wvif, &wvif->mcast_filter);
if (ret)
dev_err(wvif->wdev->dev, "update filtering failed: %d\n", ret);
- kfree(bf_tbl);
}
void wfx_update_filtering_work(struct work_struct *work)
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/7] Fix various compilation issues with wfx driver
From: Jérôme Pouiller
Most of problems are related to big-endian architectures.
Jérôme Pouiller (7):
staging: wfx: simplify memory allocation in wfx_update_filtering()
staging: wfx: remove misused call to cpu_to_le16()
staging: wfx: le16_to_cpus() takes a reference as parameter
staging: wfx: correctly cast data on big-endian targets
staging: wfx: fix copy_{to,from}_user() usage
staging: wfx: drop calls to BUG_ON()
staging: wfx: avoid namespace contamination
drivers/staging/wfx/bh.c | 8 +++
drivers/staging/wfx/bus_sdio.c | 4 ++--
drivers/staging/wfx/data_tx.c| 40
drivers/staging/wfx/data_tx.h| 2 +-
drivers/staging/wfx/debug.c | 5 ++--
drivers/staging/wfx/hif_tx_mib.h | 23 --
drivers/staging/wfx/key.c| 32 -
drivers/staging/wfx/queue.c | 6 ++---
drivers/staging/wfx/scan.c | 2 +-
drivers/staging/wfx/sta.c| 21 +++--
10 files changed, 74 insertions(+), 69 deletions(-)
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/7] staging: wfx: correctly cast data on big-endian targets
From: Jérôme Pouiller
When built for a big-endian target, original code caused error:
include/uapi/linux/swab.h:242:29: note: expected '__u32 * {aka unsigned int
*}' but argument is of type 'struct hif_mib_protected_mgmt_policy *'
Fixes: f95a29d40782 ("staging: wfx: add HIF commands helpers")
Reported-by: kbuild test robot
Reported-by: Stephen Rothwell
Signed-off-by: Jérôme Pouiller
---
drivers/staging/wfx/hif_tx_mib.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/wfx/hif_tx_mib.h b/drivers/staging/wfx/hif_tx_mib.h
index 167c5dec009f..4f132348f5fa 100644
--- a/drivers/staging/wfx/hif_tx_mib.h
+++ b/drivers/staging/wfx/hif_tx_mib.h
@@ -145,7 +145,7 @@ static inline int hif_set_mfp(struct wfx_vif *wvif, bool
capable, bool required)
}
if (!required)
val.unpmf_allowed = 1;
- cpu_to_le32s(&val);
+ cpu_to_le32s((uint32_t *) &val);
return hif_write_mib(wvif->wdev, wvif->id,
HIF_MIB_ID_PROTECTED_MGMT_POLICY,
&val, sizeof(val));
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: vc04_services: Avoid NULL comparison
Hi Nachammai,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on staging/staging-testing]
url:
https://github.com/0day-ci/linux/commits/Nachammai-Karuppiah/staging-vc04_services-Avoid-NULL-comparison/20191008-143400
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-13) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
All warnings (new ones prefixed by >>):
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: In function
'vchiq_ioctl':
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:829:10:
>> warning: suggest parentheses around assignment used as truth value
>> [-Wparentheses]
while (service = next_service_by_instance(instance->state,
^~~
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: In function
'vchiq_instance_get_use_count':
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:2926:9:
warning: suggest parentheses around assignment used as truth value
[-Wparentheses]
while (service = next_service_by_instance(instance->state,
^~~
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: In function
'vchiq_instance_set_trace':
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:2953:9:
warning: suggest parentheses around assignment used as truth value
[-Wparentheses]
while (service = next_service_by_instance(instance->state,
^~~
vim +829 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
798
799
/
800 *
801 * vchiq_ioctl
802 *
803
***/
804 static long
805 vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
806 {
807 VCHIQ_INSTANCE_T instance = file->private_data;
808 VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
809 struct vchiq_service *service = NULL;
810 long ret = 0;
811 int i, rc;
812
813 DEBUG_INITIALISE(g_state.local)
814
815 vchiq_log_trace(vchiq_arm_log_level,
816 "%s - instance %pK, cmd %s, arg %lx",
817 __func__, instance,
818 ((_IOC_TYPE(cmd) == VCHIQ_IOC_MAGIC) &&
819 (_IOC_NR(cmd) <= VCHIQ_IOC_MAX)) ?
820 ioctl_names[_IOC_NR(cmd)] : "", arg);
821
822 switch (cmd) {
823 case VCHIQ_IOC_SHUTDOWN:
824 if (!instance->connected)
825 break;
826
827 /* Remove all services */
828 i = 0;
> 829 while (service =
> next_service_by_instance(instance->state,
830 instance, &i)) {
831 status = vchiq_remove_service(service->handle);
832 unlock_service(service);
833 if (status != VCHIQ_SUCCESS)
834 break;
835 }
836 service = NULL;
837
838 if (status == VCHIQ_SUCCESS) {
839 /* Wake the completion thread and ask it to
exit */
840 instance->closing = 1;
841 complete(&instance->insert_event);
842 }
843
844 break;
845
846 case VCHIQ_IOC_CONNECT:
847 if (instance->connected) {
848 ret = -EINVAL;
849 break;
850 }
851 rc = mutex_lock_killable(&instance->state->mutex);
852 if (rc) {
853 vchiq_log_error(vchiq_arm_log_level,
854 "vchiq: connect: could not lock mutex
for "
855 "state %d: %d",
856 instance->state->id, rc);
857 ret = -EINTR;
858 break;
859 }
860 status = vchiq_connect_internal(instance->state,
instance);
861 mutex_unlock(&instance->state->mutex);
862
863 if (status == VCHIQ_SUCCESS)
864 instance->connected = 1;
865 else
866 vchiq_log_error(vchiq_arm_log_level,
867 "
Re: [PATCH] staging: media: imx: make use devm_platform_ioremap_resource
Hi Hariprasad,
Thanks for the patch
On Tue 08 Oct 2019 at 07:17, nobody wrote:
> From: Hariprasad Kelam
>
Something went wrong formating the patch email, no To: nor From:
>
> fix below issue reported by coccicheck
> drivers/staging//media/imx/imx7-mipi-csis.c:973:1-12: WARNING: Use
> devm_platform_ioremap_resource for state -> regs
>
Sorry, but someone else, Jeeeun, already sent a patch for this
[0]. Thanks anyway.
---
Cheers,
Rui
[0]: https://lore.kernel.org/linux-media/m3wodvgec4@gmail.com/
>
> Signed-off-by: Hariprasad Kelam
> ---
> drivers/staging/media/imx/imx7-mipi-csis.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c
> b/drivers/staging/media/imx/imx7-mipi-csis.c
> index 73d8354..bf21db3 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -947,7 +947,6 @@ static void mipi_csis_debugfs_exit(struct csi_state
> *state)
> static int mipi_csis_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> - struct resource *mem_res;
> struct csi_state *state;
> int ret;
>
> @@ -969,8 +968,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
> mipi_csis_phy_init(state);
> mipi_csis_phy_reset(state);
>
> - mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - state->regs = devm_ioremap_resource(dev, mem_res);
> + state->regs = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(state->regs))
> return PTR_ERR(state->regs);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH][next] staging: wfx: fix spelling mistake "hexdecimal" -> "hexadecimal"
From: Colin Ian King
There is a spelling mistake in the documentation and a module parameter
description. Fix these.
Signed-off-by: Colin Ian King
---
.../devicetree/bindings/net/wireless/siliabs,wfx.txt| 2 +-
drivers/staging/wfx/main.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git
a/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt
b/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt
index 15965c9b4180..26de6762b942 100644
---
a/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt
+++
b/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt
@@ -89,7 +89,7 @@ Some properties are recognized either by SPI and SDIO
versions:
this property, driver will disable most of power saving features.
- config-file: Use an alternative file as PDS. Default is `wf200.pds`. Only
necessary for development/debug purpose.
- - slk_key: String representing hexdecimal value of secure link key to use.
+ - slk_key: String representing hexadecimal value of secure link key to use.
Must contains 64 hexadecimal digits. Not supported in current version.
WFx driver also supports `mac-address` and `local-mac-address` as described in
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index fe9a89703897..d2508bc950fa 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -48,7 +48,7 @@ MODULE_PARM_DESC(gpio_wakeup, "gpio number for wakeup. -1 for
none.");
static char *slk_key;
module_param(slk_key, charp, 0600);
-MODULE_PARM_DESC(slk_key, "secret key for secure link (expect 64 hexdecimal
digits).");
+MODULE_PARM_DESC(slk_key, "secret key for secure link (expect 64 hexadecimal
digits).");
#define RATETAB_ENT(_rate, _rateid, _flags) { \
.bitrate = (_rate), \
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/6] staging: Remove a bounch set of set but not used variables
Looks good. Thanks! Reviewed-by: Dan Carpenter regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/6] staging: bcm2835-audio: Need to judge the return value of vchi_msg_dequeue in audio_vchi_callback
If vchi_msg_dequeue return -1, variable m is not assigined,
need to return.
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index c6f9cf1..5f6a73a 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -103,6 +103,9 @@ static void audio_vchi_callback(void *param,
status = vchi_msg_dequeue(instance->vchi_handle,
&m, sizeof(m), &msg_len, VCHI_FLAGS_NONE);
+ if (status)
+ return;
+
if (m.type == VC_AUDIO_MSG_TYPE_RESULT) {
instance->result = m.result.success;
complete(&instance->msg_avail_comp);
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 6/6] staging: comedi: Remove set but not used variable 'aref'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/staging/comedi/drivers/dt3000.c: In function dt3k_ai_insn_read:
drivers/staging/comedi/drivers/dt3000.c:511:27: warning: variable aref set but
not used [-Wunused-but-set-variable]
It is not used since commit 2e310235ca8f ("staging:
comedi: dt3000: rename dt3k_ai_insn()")
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
drivers/staging/comedi/drivers/dt3000.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/dt3000.c
b/drivers/staging/comedi/drivers/dt3000.c
index caf4d4d..f7c365b 100644
--- a/drivers/staging/comedi/drivers/dt3000.c
+++ b/drivers/staging/comedi/drivers/dt3000.c
@@ -508,12 +508,11 @@ static int dt3k_ai_insn_read(struct comedi_device *dev,
unsigned int *data)
{
int i;
- unsigned int chan, gain, aref;
+ unsigned int chan, gain;
chan = CR_CHAN(insn->chanspec);
gain = CR_RANGE(insn->chanspec);
/* XXX docs don't explain how to select aref */
- aref = CR_AREF(insn->chanspec);
for (i = 0; i < insn->n; i++)
data[i] = dt3k_readsingle(dev, DPR_SUBSYS_AI, chan, gain);
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/6] staging: comedi: Remove set but not used variable 'data'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/staging/comedi/drivers/dt2814.c: In function dt2814_interrupt:
drivers/staging/comedi/drivers/dt2814.c:193:6: warning: variable data set but
not used [-Wunused-but-set-variable]
It is not used since commit 7806012e97ed ("staging:
comedi: refactor dt2814 driver and use module_comedi_driver")
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
drivers/staging/comedi/drivers/dt2814.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/dt2814.c
b/drivers/staging/comedi/drivers/dt2814.c
index d2c7157..4825168 100644
--- a/drivers/staging/comedi/drivers/dt2814.c
+++ b/drivers/staging/comedi/drivers/dt2814.c
@@ -190,7 +190,6 @@ static irqreturn_t dt2814_interrupt(int irq, void *d)
struct comedi_device *dev = d;
struct dt2814_private *devpriv = dev->private;
struct comedi_subdevice *s = dev->read_subdev;
- int data;
if (!dev->attached) {
dev_err(dev->class_dev, "spurious interrupt\n");
@@ -200,8 +199,6 @@ static irqreturn_t dt2814_interrupt(int irq, void *d)
hi = inb(dev->iobase + DT2814_DATA);
lo = inb(dev->iobase + DT2814_DATA);
- data = (hi << 4) | (lo >> 4);
-
if (!(--devpriv->ntrig)) {
int i;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/6] staging: Remove a bounch set of set but not used variables
v1->v2: need to judge the value of status, If status != 0, just return zhengbin (6): staging: bcm2835-audio: Need to judge the return value of vchi_msg_dequeue in audio_vchi_callback staging: sm750fb: Remove set but not used variable 'uiActualPixelClk' staging: sm750fb: Remove set but not used variable 'actual_mx_clk' staging: comedi: Remove set but not used variable 'data' staging: comedi: Remove set but not used variable 'hi' staging: comedi: Remove set but not used variable 'aref' drivers/staging/comedi/drivers/dt2814.c | 3 --- drivers/staging/comedi/drivers/dt2815.c | 3 +-- drivers/staging/comedi/drivers/dt3000.c | 3 +-- drivers/staging/sm750fb/ddk750_chip.c | 3 +-- drivers/staging/sm750fb/ddk750_mode.c | 3 +-- drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 3 +++ 6 files changed, 7 insertions(+), 11 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 5/6] staging: comedi: Remove set but not used variable 'hi'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/staging/comedi/drivers/dt2815.c: In function dt2815_ao_insn:
drivers/staging/comedi/drivers/dt2815.c:91:19: warning: variable hi set but not
used [-Wunused-but-set-variable]
It is not used since commit d6a929b7608a ("Staging:
comedi: add dt2815 driver")
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
drivers/staging/comedi/drivers/dt2815.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/dt2815.c
b/drivers/staging/comedi/drivers/dt2815.c
index 83026ba..bcf85ec 100644
--- a/drivers/staging/comedi/drivers/dt2815.c
+++ b/drivers/staging/comedi/drivers/dt2815.c
@@ -88,12 +88,11 @@ static int dt2815_ao_insn(struct comedi_device *dev, struct
comedi_subdevice *s,
struct dt2815_private *devpriv = dev->private;
int i;
int chan = CR_CHAN(insn->chanspec);
- unsigned int lo, hi;
+ unsigned int lo;
int ret;
for (i = 0; i < insn->n; i++) {
lo = ((data[i] & 0x0f) << 4) | (chan << 1) | 0x01;
- hi = (data[i] & 0xff0) >> 4;
ret = comedi_timeout(dev, s, insn, dt2815_ao_status, 0x00);
if (ret)
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/6] staging: sm750fb: Remove set but not used variable 'actual_mx_clk'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/staging/sm750fb/ddk750_chip.c: In function set_chip_clock:
drivers/staging/sm750fb/ddk750_chip.c:59:15: warning: variable actual_mx_clk
set but not used [-Wunused-but-set-variable]
It is not used since commit f0977109a577 ("staging:
sm750fb: lower case to fix camelcase checkpatch warning")
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
drivers/staging/sm750fb/ddk750_chip.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_chip.c
b/drivers/staging/sm750fb/ddk750_chip.c
index e5988813..02860d3 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -56,7 +56,6 @@ static unsigned int get_mxclk_freq(void)
static void set_chip_clock(unsigned int frequency)
{
struct pll_value pll;
- unsigned int actual_mx_clk;
/* Cheok_0509: For SM750LE, the chip clock is fixed. Nothing to set. */
if (sm750_get_chip_type() == SM750LE)
@@ -76,7 +75,7 @@ static void set_chip_clock(unsigned int frequency)
* Return value of sm750_calc_pll_value gives the actual
* possible clock.
*/
- actual_mx_clk = sm750_calc_pll_value(frequency, &pll);
+ sm750_calc_pll_value(frequency, &pll);
/* Master Clock Control: MXCLK_PLL */
poke32(MXCLK_PLL_CTRL, sm750_format_pll_reg(&pll));
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/6] staging: sm750fb: Remove set but not used variable 'uiActualPixelClk'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/staging/sm750fb/ddk750_mode.c: In function ddk750_setModeTiming:
drivers/staging/sm750fb/ddk750_mode.c:212:15: warning: variable
uiActualPixelClk set but not used [-Wunused-but-set-variable]
It is not used since commit 81dee67e215b ("staging:
sm750fb: add sm750 to staging")
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
drivers/staging/sm750fb/ddk750_mode.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_mode.c
b/drivers/staging/sm750fb/ddk750_mode.c
index 9722692..e0230f4 100644
--- a/drivers/staging/sm750fb/ddk750_mode.c
+++ b/drivers/staging/sm750fb/ddk750_mode.c
@@ -209,12 +209,11 @@ static int programModeRegisters(struct mode_parameter
*pModeParam,
int ddk750_setModeTiming(struct mode_parameter *parm, enum clock_type clock)
{
struct pll_value pll;
- unsigned int uiActualPixelClk;
pll.input_freq = DEFAULT_INPUT_CLOCK;
pll.clock_type = clock;
- uiActualPixelClk = sm750_calc_pll_value(parm->pixel_clock, &pll);
+ sm750_calc_pll_value(parm->pixel_clock, &pll);
if (sm750_get_chip_type() == SM750LE) {
/* set graphic mode via IO method */
outb_p(0x88, 0x3d4);
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: media: sunxi: make use of devm_platform_ioremap_resource
Hi,
On Tue 08 Oct 19, 12:29, hariprasadkelamhariprasad.ke...@gmail.com wrote:
> From: Hariprasad Kelam
>
> fix below issue reported by coccicheck
> drivers/staging//media/sunxi/cedrus/cedrus_hw.c:229:1-10: WARNING: Use
> devm_platform_ioremap_resource for dev -> base
Looks good, thanks!
Acked-by: Paul Kocialkowski
Cheers,
Paul
> Signed-off-by: Hariprasad Kelam
> ---
> drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> index a942cd9..f19b87c 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> @@ -146,7 +146,6 @@ static irqreturn_t cedrus_irq(int irq, void *data)
> int cedrus_hw_probe(struct cedrus_dev *dev)
> {
> const struct cedrus_variant *variant;
> - struct resource *res;
> int irq_dec;
> int ret;
>
> @@ -225,8 +224,7 @@ int cedrus_hw_probe(struct cedrus_dev *dev)
> goto err_sram;
> }
>
> - res = platform_get_resource(dev->pdev, IORESOURCE_MEM, 0);
> - dev->base = devm_ioremap_resource(dev->dev, res);
> + dev->base = devm_platform_ioremap_resource(dev->pdev, 0);
> if (IS_ERR(dev->base)) {
> dev_err(dev->dev, "Failed to map registers\n");
>
> --
> 2.7.4
>
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: media: sunxi: make use of devm_platform_ioremap_resource
From: Hariprasad Kelam
fix below issue reported by coccicheck
drivers/staging//media/sunxi/cedrus/cedrus_hw.c:229:1-10: WARNING: Use
devm_platform_ioremap_resource for dev -> base
Signed-off-by: Hariprasad Kelam
---
drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
index a942cd9..f19b87c 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
@@ -146,7 +146,6 @@ static irqreturn_t cedrus_irq(int irq, void *data)
int cedrus_hw_probe(struct cedrus_dev *dev)
{
const struct cedrus_variant *variant;
- struct resource *res;
int irq_dec;
int ret;
@@ -225,8 +224,7 @@ int cedrus_hw_probe(struct cedrus_dev *dev)
goto err_sram;
}
- res = platform_get_resource(dev->pdev, IORESOURCE_MEM, 0);
- dev->base = devm_ioremap_resource(dev->dev, res);
+ dev->base = devm_platform_ioremap_resource(dev->pdev, 0);
if (IS_ERR(dev->base)) {
dev_err(dev->dev, "Failed to map registers\n");
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
