[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
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 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
[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 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
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][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] 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
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 "
[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
[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 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 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 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 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 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
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
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 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
[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 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
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] 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 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 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 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 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
[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] 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
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
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: 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 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 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 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] 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] 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] 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] 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 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][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
[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 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: 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
[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)) {
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