Re: Re: [PATCH] staging: media: davinci_vpfe: add error handling on kmalloc failure
On Mon, Mar 19, 2018 at 01:24:57PM +0900, Ji-Hun Kim wrote: > > 1294 } else if (to && !from && size) { > > 1295 rval = module_if->set(ipipe, NULL); > > 1296 if (rval) > > 1297 goto error; > > > > And here again goto free_params. > > > > 1298 } > > 1299 kfree(params); > > 1300 } > > 1301 } > > 1302 error: > > 1303 return rval; > > > > > > Change this to: > > > > return 0; > Instead of returning rval, returning 0 would be fine? It looks that should > return rval in normal case. > In the proposed code, the errors all do a return or a goto so "rval" would be zero here. Then the error path would look like: err_free_params: kfree(params); return rval; } regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: o2iblnd: Stop MLX5 triggering a dump_cqe
I don't really understand this patch... On Fri, Mar 16, 2018 at 04:40:21PM -0700, Doug Oucharek wrote: > We have found that MLX5 will trigger a dump_cqe if we don't > invalidate the rkey on a newly alloated MR for FastReg usage. > > This fix just tags the MR as invalid on its creation if we are > using FastReg and that will force it to do an invalidate of the > rkey on first usage. This paragraph makes the change seem like a limited workaround for a bug in the MLX5 code. Why can't the MLX5 code be fixed instead? Looking at the patch it doesn't seem like a limitted solution at all. Now frd->frd_valid is *always* set to false. Why don't we instead just delete ->frd_valid along with the newly dead code? regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback
On Sun, Mar 18, 2018 at 10:53:02PM +0800, Jia-Ju Bai wrote: > hv_pci_onchannelcallback() is not called in atomic context. > > The call chain ending up at hv_pci_onchannelcallback() is: > [1] hv_pci_onchannelcallback() <- hv_pci_probe() > hv_pci_probe() is only set as ".probe" in hv_driver > structure "hv_pci_drv". > Your static analysis tool is faulty and apparently so is Smatch. $ smdb function_ptrs hv_pci_onchannelcallback Says it can't find a caller. When I look for function pointers I get: $ smdb function_ptr hv_pci_onchannelcallback hv_pci_onchannelcallback = 'hv_pci_onchannelcallback' , 'vmbus_open param 5' , '(struct vmbus_channel)->onchannel_callback' , '__read_once_size param 0' Anyway the call tree is: vmbus_chan_sched() <-- takes rcu_read_lock(); -> vmbus_channel_isr() -> channel->onchannel_callback() -> which is hv_pci_onchannelcallback() regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in new_pcichild_device
Looks ok. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback
On 2018/3/19 16:38, Dan Carpenter wrote: On Sun, Mar 18, 2018 at 10:53:02PM +0800, Jia-Ju Bai wrote: hv_pci_onchannelcallback() is not called in atomic context. The call chain ending up at hv_pci_onchannelcallback() is: [1] hv_pci_onchannelcallback() <- hv_pci_probe() hv_pci_probe() is only set as ".probe" in hv_driver structure "hv_pci_drv". Your static analysis tool is faulty and apparently so is Smatch. $ smdb function_ptrs hv_pci_onchannelcallback Says it can't find a caller. When I look for function pointers I get: $ smdb function_ptr hv_pci_onchannelcallback hv_pci_onchannelcallback = 'hv_pci_onchannelcallback' , 'vmbus_open param 5' , '(struct vmbus_channel)->onchannel_callback' , '__read_once_size param 0' Anyway the call tree is: vmbus_chan_sched() <-- takes rcu_read_lock(); -> vmbus_channel_isr() -> channel->onchannel_callback() -> which is hv_pci_onchannelcallback( Thanks for your reply :) I admit my tool produces a false positive for this code... Sorry for my incorrect patch. Anyway, I find that function pointers are quite hard to analyze in the Linux kernel code, because their usages are often flexible. Have you found a good way to handle function pointers? Or can you recommend some good tools to handle them? Best wishes, Jia-Ju Bai ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[staging] 184ecc5ceb: BUG:unable_to_handle_kernel
FYI, we noticed the following commit (built with gcc-6): commit: 184ecc5ceb379b43b29fc373f497fca88c73ad38 ("staging: lustre: allow monolithic builds") https://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git staging-testing in testcase: trinity with following parameters: runtime: 300s test-description: Trinity is a linux system call fuzz tester. test-url: http://codemonkey.org.uk/projects/trinity/ on test machine: qemu-system-i386 -enable-kvm -smp 2 -m 320M caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace): +--+++ | | 26f7a294e5 | 184ecc5ceb | +--+++ | boot_successes | 14 | 4 | | boot_failures| 0 | 12 | | BUG:unable_to_handle_kernel | 0 | 12 | | Oops:#[##] | 0 | 12 | | EIP:cfs_cpt_spread_node | 0 | 12 | | Kernel_panic-not_syncing:Fatal_exception | 0 | 12 | +--+++ [ 123.548190] BUG: unable to handle kernel NULL pointer dereference at 0008 [ 123.551023] IP: cfs_cpt_spread_node+0x31/0x100 [ 123.552032] *pdpt = *pde = f000ff53f000ff53 [ 123.552032] Oops: [#1] PREEMPT SMP [ 123.552032] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.16.0-rc3-00311-g184ecc5 #115 [ 123.552032] EIP: cfs_cpt_spread_node+0x31/0x100 [ 123.552032] EFLAGS: 00210202 CPU: 1 [ 123.552032] EAX: EBX: ECX: EDX: [ 123.552032] ESI: cf503ad8 EDI: 0080 EBP: c02bded8 ESP: c02bded0 [ 123.552032] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 [ 123.552032] CR0: 80050033 CR2: 0008 CR3: 024b3000 CR4: 06b0 [ 123.552032] Call Trace: [ 123.552032] cfs_percpt_alloc+0x62/0xb0 [ 123.552032] cfs_percpt_lock_create+0x3a/0x1a0 [ 123.552032] lnet_lib_init+0x28f/0x460 [ 123.552032] ? goldfish_nand_driver_init+0x16/0x16 [ 123.552032] lnet_init+0x22/0x16b [ 123.552032] ? goldfish_nand_driver_init+0x16/0x16 [ 123.581926] do_one_initcall+0x90/0x13c [ 123.581926] ? do_early_param+0x7a/0x7a [ 123.581926] ? parse_args+0x16e/0x2e0 [ 123.581926] kernel_init_freeable+0x18a/0x20d [ 123.581926] ? rest_init+0x120/0x120 [ 123.581926] kernel_init+0xd/0x100 [ 123.581926] ret_from_fork+0x19/0x24 [ 123.581926] Code: 85 d2 89 e5 56 53 78 05 39 50 08 77 15 8b 50 04 8b 70 18 8d 4a 01 89 48 04 eb 1c 8d b4 26 00 00 00 00 8b 40 0c 8d 14 52 8d 04 90 <8b> 50 08 8b 70 04 8d 4a 01 89 48 08 0f b6 06 e8 ab d7 9f ff 85 [ 123.581926] EIP: cfs_cpt_spread_node+0x31/0x100 SS:ESP: 0068:c02bded0 [ 123.581926] CR2: 0008 [ 123.581926] ---[ end trace d8c2581529145c01 ]--- To reproduce: git clone https://github.com/intel/lkp-tests.git cd lkp-tests bin/lkp qemu -k job-script # job-script is attached in this email Thanks, lkp # # Automatically generated file; DO NOT EDIT. # Linux/i386 4.16.0-rc3 Kernel Configuration # # CONFIG_64BIT is not set CONFIG_X86_32=y CONFIG_X86=y CONFIG_INSTRUCTION_DECODER=y CONFIG_OUTPUT_FORMAT="elf32-i386" CONFIG_ARCH_DEFCONFIG="arch/x86/configs/i386_defconfig" CONFIG_LOCKDEP_SUPPORT=y CONFIG_STACKTRACE_SUPPORT=y CONFIG_MMU=y CONFIG_ARCH_MMAP_RND_BITS_MIN=8 CONFIG_ARCH_MMAP_RND_BITS_MAX=16 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16 CONFIG_NEED_DMA_MAP_STATE=y CONFIG_NEED_SG_DMA_LENGTH=y CONFIG_GENERIC_ISA_DMA=y CONFIG_GENERIC_BUG=y CONFIG_GENERIC_HWEIGHT=y CONFIG_ARCH_MAY_HAVE_PC_FDC=y CONFIG_RWSEM_XCHGADD_ALGORITHM=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_ARCH_HAS_CPU_RELAX=y CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y CONFIG_HAVE_SETUP_PER_CPU_AREA=y CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y CONFIG_ARCH_HIBERNATION_POSSIBLE=y CONFIG_ARCH_SUSPEND_POSSIBLE=y CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y CONFIG_ARCH_WANT_GENERAL_HUGETLB=y CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y CONFIG_X86_32_SMP=y CONFIG_ARCH_SUPPORTS_UPROBES=y CONFIG_FIX_EARLYCON_MEM=y CONFIG_PGTABLE_LEVELS=3 CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config" CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_EXTABLE_SORT=y CONFIG_THREAD_INFO_IN_TASK=y # # General setup # CONFIG_INIT_ENV_ARG_LIMIT=32 CONFIG_CROSS_COMPILE="" # CONFIG_COMPILE_TEST is not set CONFIG_LOCALVERSION="" CONFIG_LOCALVERSION_AUTO=y CONFIG_HAVE_KERNEL_GZIP=y CONFIG_HAVE_KERNEL_BZIP2=y CONFIG_HAVE_KERNEL_LZMA=y CONFIG_HAVE_KERNEL_XZ=y CONFIG_HAVE_KERNEL_LZO=y CONFIG_HAVE_KERNEL_LZ4=y CONFIG_KERNEL_GZIP=y # CONFIG_KERNEL_BZIP2 is not set # CONFIG_KERNEL_LZMA is not set # CONFIG_KERNEL_XZ is not set # CONFIG_KERNEL_LZO is not set # CONFIG_KERNEL_LZ4 is not set CONFIG_DEFAULT_HOSTNAME="(none)" #
Re: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback
Smatch tracks information about every function call. When a function pointer is called it maybe looks something like this: kernel/module.c | SYSC_delete_module | (struct module)->exit | INTERNAL | -1 | | void(*)() So then we just have to know what functions are assigned to module->exit. I also filter based on the function signature "void(*)()" because there are a couple places where we cut and pasted so the structs can have the same name and function pointer name as a member but take different arguments. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] ncpfs: memory corruption in ncp_read_kernel()
If the server is malicious then *bytes_read could be larger than the size of the "target" buffer. It would lead to memory corruption when we do the memcpy(). Reported-by: Dr Silvio Cesare of InfoSect Signed-off-by: Dan Carpenter diff --git a/drivers/staging/ncpfs/ncplib_kernel.c b/drivers/staging/ncpfs/ncplib_kernel.c index 804adfebba2f..3e047eb4cc7c 100644 --- a/drivers/staging/ncpfs/ncplib_kernel.c +++ b/drivers/staging/ncpfs/ncplib_kernel.c @@ -981,6 +981,10 @@ ncp_read_kernel(struct ncp_server *server, const char *file_id, goto out; } *bytes_read = ncp_reply_be16(server, 0); + if (*bytes_read > to_read) { + result = -EINVAL; + goto out; + } source = ncp_reply_data(server, 2 + (offset & 1)); memcpy(target, source, *bytes_read); ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: media: davinci_vpfe: fix spelling of resizer_configure_in_continious_mode
From: Colin Ian King Trivial fix: rename function resizer_configure_in_continious_mode to resizer_configure_in_continuous_mode to fix spelling mistake. Signed-off-by: Colin Ian King --- drivers/staging/media/davinci_vpfe/dm365_resizer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/davinci_vpfe/dm365_resizer.c b/drivers/staging/media/davinci_vpfe/dm365_resizer.c index 857b0e847c5e..1ee216d71d42 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_resizer.c +++ b/drivers/staging/media/davinci_vpfe/dm365_resizer.c @@ -480,7 +480,7 @@ resizer_configure_common_in_params(struct vpfe_resizer_device *resizer) return 0; } static int -resizer_configure_in_continious_mode(struct vpfe_resizer_device *resizer) +resizer_configure_in_continuous_mode(struct vpfe_resizer_device *resizer) { struct device *dev = resizer->crop_resizer.subdev.v4l2_dev->dev; struct resizer_params *param = &resizer->config; @@ -1242,7 +1242,7 @@ static int resizer_do_hw_setup(struct vpfe_resizer_device *resizer) ipipeif_source == IPIPEIF_OUTPUT_RESIZER) ret = resizer_configure_in_single_shot_mode(resizer); else - ret = resizer_configure_in_continious_mode(resizer); + ret = resizer_configure_in_continuous_mode(resizer); if (ret) return ret; ret = config_rsz_hw(resizer, param); -- 2.15.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] ncpfs: memory corruption in ncp_read_kernel()
On Mon, Mar 19, 2018 at 02:07:45PM +0300, Dan Carpenter wrote: > If the server is malicious then *bytes_read could be larger than the > size of the "target" buffer. It would lead to memory corruption when we > do the memcpy(). > > Reported-by: Dr Silvio Cesare of InfoSect > Signed-off-by: Dan Carpenter > > diff --git a/drivers/staging/ncpfs/ncplib_kernel.c > b/drivers/staging/ncpfs/ncplib_kernel.c > index 804adfebba2f..3e047eb4cc7c 100644 > --- a/drivers/staging/ncpfs/ncplib_kernel.c > +++ b/drivers/staging/ncpfs/ncplib_kernel.c Ugh, I have like 2 more months before I delete this code :) Anyway, nice find, and fix, I'll go queue it up now, thanks. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/5] Staging:rtl8723bs:os_dep braces not necessary
WARNING: braces {} are not necessary for any arm of this statement Signed-off-by: Paul McQuade --- drivers/staging/rtl8723bs/os_dep/recv_linux.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/rtl8723bs/os_dep/recv_linux.c b/drivers/staging/rtl8723bs/os_dep/recv_linux.c index e804b430931c..117fda4fa1bf 100644 --- a/drivers/staging/rtl8723bs/os_dep/recv_linux.c +++ b/drivers/staging/rtl8723bs/os_dep/recv_linux.c @@ -181,11 +181,11 @@ void rtw_os_recv_indicate_pkt(struct adapter *padapter, _pkt *pkt, struct rx_pkt pkt->dev = padapter->pnetdev; #ifdef CONFIG_TCP_CSUM_OFFLOAD_RX - if ((pattrib->tcpchk_valid == 1) && (pattrib->tcp_chkrpt == 1)) { + if ((pattrib->tcpchk_valid == 1) && (pattrib->tcp_chkrpt == 1)) pkt->ip_summed = CHECKSUM_UNNECESSARY; - } else { + else pkt->ip_summed = CHECKSUM_NONE; - } + #else /* !CONFIG_TCP_CSUM_OFFLOAD_RX */ pkt->ip_summed = CHECKSUM_NONE; #endif /* CONFIG_TCP_CSUM_OFFLOAD_RX */ -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/5] Staging: rtl8723bs: os_dep
ERROR: do not initialise statics to NULL Signed-off-by: Paul McQuade --- drivers/staging/rtl8723bs/os_dep/rtw_proc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8723bs/os_dep/rtw_proc.c b/drivers/staging/rtl8723bs/os_dep/rtw_proc.c index 9a885e626d1c..37ee8d90709a 100644 --- a/drivers/staging/rtl8723bs/os_dep/rtw_proc.c +++ b/drivers/staging/rtl8723bs/os_dep/rtw_proc.c @@ -19,7 +19,7 @@ #ifdef PROC_DEBUG -static struct proc_dir_entry *rtw_proc = NULL; +static struct proc_dir_entry *rtw_proc; #define RTW_PROC_NAME "rtl8723bs" -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/5] Staging:rtl8723bs:os_dep blank line needed
WARNING: Missing a blank line after declarations Signed-off-by: Paul McQuade --- drivers/staging/rtl8723bs/os_dep/recv_linux.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/rtl8723bs/os_dep/recv_linux.c b/drivers/staging/rtl8723bs/os_dep/recv_linux.c index 117fda4fa1bf..b43e24c3a23a 100644 --- a/drivers/staging/rtl8723bs/os_dep/recv_linux.c +++ b/drivers/staging/rtl8723bs/os_dep/recv_linux.c @@ -43,6 +43,7 @@ void rtw_os_recv_resource_free(struct recv_priv *precvpriv) { sint i; union recv_frame *precvframe; + precvframe = (union recv_frame*) precvpriv->precv_frame_buf; for (i = 0; i < NR_RECVFRAME; i++) -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/5] Staging: rtl8723bs: os_dep Spacing issue
ERROR: spaces required around that '<' (ctx:VxV) Signed-off-by: Paul McQuade --- drivers/staging/rtl8723bs/os_dep/rtw_proc.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/staging/rtl8723bs/os_dep/rtw_proc.c b/drivers/staging/rtl8723bs/os_dep/rtw_proc.c index 37ee8d90709a..49c8684dc25b 100644 --- a/drivers/staging/rtl8723bs/os_dep/rtw_proc.c +++ b/drivers/staging/rtl8723bs/os_dep/rtw_proc.c @@ -142,7 +142,7 @@ int rtw_drv_proc_init(void) goto exit; } - for (i = 0;icam_cache[i].ctrl != 0) DBG_871X_SEL_NL(m, "%2u 0x%04x "MAC_FMT" "KEY_FMT" %3u %-7s" /* %2u %2u 0x%02x %5u" */ @@ -663,7 +663,7 @@ static struct proc_dir_entry *rtw_odm_proc_init(struct net_device *dev) adapter->dir_odm = dir_odm; - for (i = 0;idir_dev); @@ -721,7 +721,7 @@ struct proc_dir_entry *rtw_adapter_proc_init(struct net_device *dev) adapter->dir_dev = dir_dev; - for (i = 0;ihttp://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/5] Staging:rtl8723bs:os_dep Fixed braces
ERROR: that open brace { should be on the previous line Signed-off-by: Paul McQuade --- drivers/staging/rtl8723bs/os_dep/recv_linux.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/staging/rtl8723bs/os_dep/recv_linux.c b/drivers/staging/rtl8723bs/os_dep/recv_linux.c index b43e24c3a23a..e6de8d2eabb7 100644 --- a/drivers/staging/rtl8723bs/os_dep/recv_linux.c +++ b/drivers/staging/rtl8723bs/os_dep/recv_linux.c @@ -20,8 +20,7 @@ void rtw_os_free_recvframe(union recv_frame *precvframe) { - if (precvframe->u.hdr.pkt) - { + if (precvframe->u.hdr.pkt) { dev_kfree_skb_any(precvframe->u.hdr.pkt);/* free skb by driver */ precvframe->u.hdr.pkt = NULL; @@ -46,10 +45,8 @@ void rtw_os_recv_resource_free(struct recv_priv *precvpriv) precvframe = (union recv_frame*) precvpriv->precv_frame_buf; - for (i = 0; i < NR_RECVFRAME; i++) - { - if (precvframe->u.hdr.pkt) - { + for (i = 0; i < NR_RECVFRAME; i++) { + if (precvframe->u.hdr.pkt) { dev_kfree_skb_any(precvframe->u.hdr.pkt);/* free skb by driver */ precvframe->u.hdr.pkt = NULL; } -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/5] Staging: rtl8723bs: os_dep
On Mon, Mar 19, 2018 at 02:34:56PM +, Paul McQuade wrote: > ERROR: do not initialise statics to NULL You are going to have to do a bit more writing in the changelog than just dumping in random output from checkpatch.pl. I know that's where that line came from, but no one else will. And your subject lines also need a lot of work. Please fix up for all of these patches and resend the whole series. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 06/21] fpga: Remove depends on HAS_DMA in case of platform dependency
On Fri, Mar 16, 2018 at 8:51 AM, Geert Uytterhoeven wrote: Hi Geert, This essentially removes this commit commit 1c8cb409491403036919dd1c6b45013dc8835a44 Author: Sudip Mukherjee Date: Wed Aug 3 13:45:46 2016 -0700 drivers/fpga/Kconfig: fix build failure While building m32r allmodconfig the build is failing with the error: ERROR: "bad_dma_ops" [drivers/fpga/zynq-fpga.ko] undefined! Xilinx Zynq FPGA is using DMA but there was no dependency while building. Link: http://lkml.kernel.org/r/1464346526-13913-1-git-send-email-sudipm.mukher...@gmail.com Signed-off-by: Sudip Mukherjee Acked-by: Moritz Fischer Cc: Alan Tull Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds Alan > Remove dependencies on HAS_DMA where a Kconfig symbol depends on another > symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST". > In most cases this other symbol is an architecture or platform specific > symbol, or PCI. > > Generic symbols and drivers without platform dependencies keep their > dependencies on HAS_DMA, to prevent compiling subsystems or drivers that > cannot work anyway. > > This simplifies the dependencies, and allows to improve compile-testing. > > Signed-off-by: Geert Uytterhoeven > Reviewed-by: Mark Brown > Acked-by: Robin Murphy > --- > v2: > - Add Reviewed-by, Acked-by, > - Drop RFC state, > - Split per subsystem. > --- > drivers/fpga/Kconfig | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig > index f47ef848bcd056d5..fd539132542e30ee 100644 > --- a/drivers/fpga/Kconfig > +++ b/drivers/fpga/Kconfig > @@ -53,7 +53,6 @@ config FPGA_MGR_ALTERA_CVP > config FPGA_MGR_ZYNQ_FPGA > tristate "Xilinx Zynq FPGA" > depends on ARCH_ZYNQ || COMPILE_TEST > - depends on HAS_DMA > help > FPGA manager driver support for Xilinx Zynq FPGAs. > > -- > 2.7.4 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: o2iblnd: Enable Multiple OPA Endpoints between Nodes
On Thu, Mar 15, 2018 at 02:53:46PM -0700, Doug Oucharek wrote: > OPA driver optimizations are based on the MPI model where it is > expected to have multiple endpoints between two given nodes. To > enable this optimization for Lustre, we need to make it possible, > via an LND-specific tuneable, to create multiple endpoints and to > balance the traffic over them. > > Both sides of a connection must have this patch for it to work. > Only the active side of the connection (usually the client) > needs to have the new tuneable set > 1. > > Signed-off-by: Doug Oucharek > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8943 > Reviewed-on: https://review.whamcloud.com/25168 > Reviewed-by: Amir Shehata > Reviewed-by: Dmitry Eremin > Reviewed-by: James Simmons > Reviewed-by: Oleg Drokin > Signed-off-by: Doug Oucharek > --- > .../lustre/include/uapi/linux/lnet/lnet-dlc.h | 3 ++- > .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h| 17 --- > .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 25 +++--- > .../lustre/lnet/klnds/o2iblnd/o2iblnd_modparams.c | 9 > 4 files changed, 42 insertions(+), 12 deletions(-) Does not apply to my tree at all :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: o2iblnd: Fix FastReg map/unmap for MLX5
On Wed, Mar 14, 2018 at 11:18:30PM -0700, Doug Oucharek wrote: > The FastReg support in ko2iblnd was not unmapping pool items > causing the items to leak. In addition, the mapping code > is not growing the pool like we do with FMR. > > This patch makes sure we are unmapping FastReg pool elements > when we are done with them. It also makes sure the pool > will grow when we depleat the pool. > > Signed-off-by: Doug Oucharek > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-9472 > Reviewed-on: https://review.whamcloud.com/27015 > Reviewed-by: Andrew Perepechko > Reviewed-by: Dmitry Eremin > Reviewed-by: James Simmons > Reviewed-by: Oleg Drokin > Signed-off-by: Doug Oucharek > --- > drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c| 2 +- > drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 12 > 2 files changed, 5 insertions(+), 9 deletions(-) Also does not apply :( Please rebase and resend. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Salut l'ami.
Salut l'ami. S'il vous plaît allumer contactez-moi, il y a un problème important dont j'ai besoin pour discuter avec vous concernant mon client décédé une nationalité de votre pays qui a quitté le fonds immobilier non réclamé évalué USD 5,3 millions de dollars avant sa mort. S'il vous plaît écrivez à (mrwu...@gmail.com) et attendez pour plus de détails à ce sujet. Meilleures salutations, A. Wuso Esq. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/7] staging: wilc1000: replace switch statement by simple if condition
On Wed, Mar 14, 2018 at 06:15:03PM +0530, hariprasath.ela...@gmail.com wrote: > From: HariPrasath Elango > > In this case,there is only a single switch case statement.So replacing > by a simple if condition. > > Signed-off-by: HariPrasath Elango > --- > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 8 +--- > 1 file changed, 1 insertion(+), 7 deletions(-) Does not apply to my tree :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/4] staging: ks7010: some cleanups and replaces
On Thu, Mar 15, 2018 at 08:09:20PM +0100, Sergio Paracuellos wrote: > The following patch series includes some cleanups of useless traces as well > as some replaces in order to use preferred macros for debugging and others. > > v2: > * Review deleted DPRINTK traces > * Minor checkpatch warning cleanup > > Sergio Paracuellos (4): > staging: ks7010: remove useless DPRINTK traces > staging: ks7010: replace DPRINTK traces in favour of netdev_* > staging: ks7010: replace KS_WLAN_DEBUG with DEBUG preprocessor > directive > staging: ks7010: remove unnecessary brackets in single statement block I didn't get patch 3/4 here, so I only applied the first 2 :( Please rebase and resend the remaining patches. thanks, greg k-h- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: ks7010: Factor out repeated code.
On Thu, Mar 15, 2018 at 11:26:59PM -0700, Quytelda Kahja wrote: > Some of the code for reading IEs is replicated multiple times in the > switch statement for get_ap_information(). Factor that code out into > read_ie(). > > Signed-off-by: Quytelda Kahja > --- > drivers/staging/ks7010/ks_hostif.c | 48 > +- > 1 file changed, 22 insertions(+), 26 deletions(-) These patches do not apply to my tree at all :( Can you please rebase and resend? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ashmem: Fix possible deadlock in ashmem_ioctl
On Tue, Feb 27, 2018 at 10:59 PM, Yisheng Xie wrote: > ashmem_mutex may create a chain of dependencies like: > > CPU0CPU1 > mmap syscall ioctl syscall > -> mmap_sem (acquired) -> ashmem_ioctl > -> ashmem_mmap-> ashmem_mutex (acquired) > -> ashmem_mutex (try to acquire) -> copy_from_user > -> mmap_sem (try to acquire) > > There is a lock odering problem between mmap_sem and ashmem_mutex causing > a lockdep splat[1] during a syzcaller test. This patch fixes the problem > by move copy_from_user out of ashmem_mutex. > > [1] https://www.spinics.net/lists/kernel/msg2733200.html > > Fixes: ce8a3a9e76d0 (staging: android: ashmem: Fix a race condition in pin > ioctls) > Reported-by: syzbot+d7a918a7a8e1c952b...@syzkaller.appspotmail.com > Signed-off-by: Yisheng Xie Greg, Could you take this patch for the stable trees? I do see it in staging already. I couldn't find it in stable so wanted to bring it to your attention. If you already aware of it, please ignore my note. Thanks, - Joel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2 1/3] staging: xm2mvscale: Driver support for Xilinx M2M Video Scaler
Hi Hans, Thanks for taking the time to take a look at this. > This should definitely use the V4L2 API. I guess it could be added > to staging/media with a big fat TODO that this should be converted to > the V4L2 mem2mem framework. > > But it makes no sense to re-invent the V4L2 streaming API :-) > > drivers/media/platform/mx2_emmaprp.c does something similar to this. > It's a little bit outdated (not using the latest m2m helper functions) > but it is a good starting point. I looked at the mx2_emmaprp.c and the Samsung G-Scaler M2M driver. IMHO, the main difference between the Hardware registers/capabilities is that mx2_emmaprp driver or the gsc driver, have one scaling "channel" if we might call it. Whereas the HW/IP I have in mind has 4-8 scaling channels. By a scaling channel, I mean an entity of the HW or IP, that can take the following parameters : - Input height, stride , width, color format, input Y and Cb/Cr physically contiguous memory pointers - Output height, stride, width, color format, output Y and Cb/Cr physically contiguous memory pointers Based on the above parameters, when the above are provided and the IP is started, we get an interrupt on completion. I'm sure you are familiar with this model. However, in the case of this IP, there could be 4-8 such channels and a single interrupt on the completion of the all 4-8 scaling operations. In this IP, we are trying to have 4-8 input sources being scaled by this single piece of hardware, by time multiplexing. An example use case is : Four applications (sources) will feed (enqueue) 4 input buffers to the scaler, the scaler driver will synchronize the programming of these buffers, when the number of buffers received by the driver meets our batch size (say a batch size of 4), it will kick start the IP. The four applications will poll on the fd, upon receiving an interrupt from the hardware the poll will unblock. And all four applications can dequeue their respective buffers and display them on a sink. But each "channel" can be set to do accept its own individual input and output formats. When I went through : https://www.kernel.org/doc/html/v4.14/media/uapi/v4l/open.html#multiple-opens It appears, once an application has invoked VIDIOC_REQBUFS or VIDIOC_CREATE_BUFS, other applications cannot VIDIOC_S_FMT on them. However to maximize the available number of channels, it would be necessary to allow several applications to be able to perform VIDIOC_S_FMT on the device node in the case of this hardware as different channels can be expected to deal with different scaling operations. One option is to create a logical /dev/videoX node for each such channel, and have a parent driver perform the interrupt handling, batch size setting and other such common functionalities. Is there a way to allow multiple applications talk to the same video device node/file handle without creating logical video nodes for each channel ? Please let me know if the description of HW is not clear. I will look forward to hear comments from you. > > So for this series: > > Nacked-by: Hans Verkuil > > If this was added to drivers/staging/media instead and with an updated > TODO, then we can accept it, but we need to see some real effort afterwards > to switch this to the right API. Otherwise it will be removed again > after a few kernel cycles. > Many thanks for providing a pathway to get this into drivers/staging/media I will drop this series, and re-send with the driver being placed in drivers/staging/media. I'll add some references to this conversation, so a new reviewer gets some context of what was discussed. In the meanwhile I will look into re-writing this to utilize the M2M V4L2 API. > Regards, > > Hans Best Regards, Rohit > -Original Message- > From: Hans Verkuil [mailto:hverk...@xs4all.nl] > Sent: Friday, March 09, 2018 3:58 AM > To: Greg KH ; Rohit Athavale > > Cc: de...@driverdev.osuosl.org; linux-me...@vger.kernel.org > Subject: Re: [PATCH v2 1/3] staging: xm2mvscale: Driver support for Xilinx M2M > Video Scaler > > On 22/02/18 14:46, Greg KH wrote: > > On Wed, Feb 21, 2018 at 02:43:14PM -0800, Rohit Athavale wrote: > >> This commit adds driver support for the pre-release Xilinx M2M Video > >> Scaler IP. There are three parts to this driver : > >> > >> - The Hardware/IP layer that reads and writes register of the IP > >>contained in the scaler_hw_xm2m.c > >> - The set of ioctls that applications would need to know contained > >>in ioctl_xm2mvsc.h > >> - The char driver that consumes the IP layer in xm2m_vscale.c > >> > >> Signed-off-by: Rohit Athavale > >> --- > > > > I need an ack from the linux-media maintainers before I can consider > > this for staging, as this really looks like an "odd" video driver... > > This should definitely use the V4L2 API. I guess it could be added > to staging/media with a big fat TODO that this should be converted to > the V4L2 mem2mem framework. > >
Re: [PATCH] staging: lustre: Fix unneeded byte-ordering cast
On Sat, Mar 17 2018, Justin Skists wrote: > Fix sparse warning: > > CHECK drivers/staging//lustre/lnet/lnet/acceptor.c > drivers/staging//lustre/lnet/lnet/acceptor.c:243:30: warning: cast to > restricted __le32 > > LNET_PROTO_TCP_MAGIC, as a define, is already CPU byte-ordered when > compared to 'magic', so no need for a cast. > > Signed-off-by: Justin Skists > --- > drivers/staging/lustre/lnet/lnet/acceptor.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/lustre/lnet/lnet/acceptor.c > b/drivers/staging/lustre/lnet/lnet/acceptor.c > index fb478e20e204..13e981781b9a 100644 > --- a/drivers/staging/lustre/lnet/lnet/acceptor.c > +++ b/drivers/staging/lustre/lnet/lnet/acceptor.c > @@ -240,7 +240,7 @@ lnet_accept(struct socket *sock, __u32 magic) > return -EPROTO; > } > > - if (magic == le32_to_cpu(LNET_PROTO_TCP_MAGIC)) > + if (magic == LNET_PROTO_TCP_MAGIC) > str = "'old' socknal/tcpnal"; > else > str = "unrecognised"; This code is almost completely irrelevant (it just choose which error message to use when failing), but we may as well get it right and I cannot see why your change is a fix. "magic" was passed as an argument from lnet_acceptor() to lnet_accept(), and lnet_acceptor() got it by reading bytes off the network with lnet_sock_read(). My knowledge is far from complete, but from what I've seen, lustre sends data in host-byte-order on the sender, and expects the receiver to determine which byte-order that is (often by looking at a "magic" word like this) and do any byte-swap that is necessary. While I agree that LNET_PROTO_TCP_MAGIC is in host-byte-order so calling le32_to_cpu() on it makes no sense, I don't agree that "magic" is also host byte-ordered. I suspect a more correct fix would be to use lnet_accept_magic(magic, LNET_PROTO_TCP_MAGIC) as the condition of the if(). This is consistent with other code that tests magic, and it is consistent with the general understanding that "magic" should be in host-byte-order for the peer which sent the message. Could you resubmit with that change? Thanks, NeilBrown signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/3] staging:iio:ad2s1210: Add channel for fclkin and fexcit
Hi, On 03/17, Jonathan Cameron wrote: > On Tue, 13 Mar 2018 13:05:28 -0300 > Rodrigo Siqueira wrote: > > > The ad2s1210 does not contain any channel for the fclkin and fexcit. As > > a result, it uses IIO_DEVICE_ATTR to expose this information. This patch > > adds one channel for fclkin and another for fexcit. It also adds an enum > > to easily address the correct channel. > > > > Signed-off-by: Rodrigo Siqueira > Take a step back. What are these new channels actually for? > We aren't looking at a general purpose frequency input and output. > (mind you they are both currently inputs which makes even less sense!) > > These are controls for the excitation frequency for a resolver. > What is userspace going to do with them? Nothing of any use certainly. > > So what do they actually change that matters to an application? > 1) The speed at which we can detect a loss of signal condition. > 2) The achievable resolution of the sensor. > > So how would userspace know how to configure it? I'm not sure it > would, but it is these things that should be driving the choice > not the actual value of the frequency (which is really just > a weird internal value in the way the resolver system works). > > There is a pretty strong argument that we should leave the excitation > frequency alone at 10kHz unless the platform designer knows better, > in which case they should supply it via devicetree rather than > from userspace. > > Anyhow, it needs a rethink - exposing these as channels is not > the way to go! > > Jonathan Thanks for the feedback. Now I am thinking about this module (and IIO subsystem) from another perspective :) I will rethink and came with another approach. > > > --- > > drivers/staging/iio/resolver/ad2s1210.c | 43 > > ++--- > > 1 file changed, 29 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/staging/iio/resolver/ad2s1210.c > > b/drivers/staging/iio/resolver/ad2s1210.c > > index ac13b99bd9cb..28c3fd439663 100644 > > --- a/drivers/staging/iio/resolver/ad2s1210.c > > +++ b/drivers/staging/iio/resolver/ad2s1210.c > > @@ -67,6 +67,11 @@ enum ad2s1210_mode { > > MOD_RESERVED, > > }; > > > > +enum ad2s1210_frequency_channel { > > + FCLKIN = 0, > > + FEXCIT, > > +}; > > + > > static const unsigned int ad2s1210_resolution_value[] = { 10, 12, 14, 16 }; > > > > struct ad2s1210_state { > > @@ -88,6 +93,30 @@ static const int ad2s1210_mode_vals[4][2] = { > > [MOD_CONFIG] = { 1, 0 }, > > }; > > > > +static const struct iio_chan_spec ad2s1210_channels[] = { > > + { > > + .type = IIO_ANGL, > > + .indexed = 1, > > + .channel = 0, > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > > + }, { > > + .type = IIO_ANGL_VEL, > > + .indexed = 1, > > + .channel = 0, > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > > + }, { > > + .type = IIO_CHAN_INFO_FREQUENCY, > > + .indexed = 1, > > + .channel = FCLKIN, > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > > + }, { > > + .type = IIO_CHAN_INFO_FREQUENCY, > > + .indexed = 1, > > + .channel = FEXCIT, > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > > + }, > > +}; > > + > This seems broken, you can't just add a channel and not support > it until the following patches. > > > static inline void ad2s1210_set_mode(enum ad2s1210_mode mode, > > struct ad2s1210_state *st) > > { > > @@ -552,20 +581,6 @@ static IIO_DEVICE_ATTR(lot_low_thrd, 0644, > >ad2s1210_show_reg, ad2s1210_store_reg, > >AD2S1210_REG_LOT_LOW_THRD); > > > > -static const struct iio_chan_spec ad2s1210_channels[] = { > > - { > > - .type = IIO_ANGL, > > - .indexed = 1, > > - .channel = 0, > > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > > - }, { > > - .type = IIO_ANGL_VEL, > > - .indexed = 1, > > - .channel = 0, > > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > > - } > > -}; > > - > > static struct attribute *ad2s1210_attributes[] = { > > &iio_dev_attr_fclkin.dev_attr.attr, > > &iio_dev_attr_fexcit.dev_attr.attr, > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/3] staging: xm2mvscale: Driver support for Xilinx M2M Video Scaler
Le mardi 20 mars 2018 à 00:46 +, Rohit Athavale a écrit : > Hi Hans, > > Thanks for taking the time to take a look at this. > > > This should definitely use the V4L2 API. I guess it could be added > > to staging/media with a big fat TODO that this should be converted > > to > > the V4L2 mem2mem framework. > > > > But it makes no sense to re-invent the V4L2 streaming API :-) > > > > drivers/media/platform/mx2_emmaprp.c does something similar to > > this. > > It's a little bit outdated (not using the latest m2m helper > > functions) > > but it is a good starting point. > > I looked at the mx2_emmaprp.c and the Samsung G-Scaler M2M driver. > IMHO, the main difference between > the Hardware registers/capabilities is that mx2_emmaprp driver or the > gsc driver, have one scaling "channel" > if we might call it. Whereas the HW/IP I have in mind has 4-8 scaling > channels. > > By a scaling channel, I mean an entity of the HW or IP, that can take > the following parameters : > - Input height, stride , width, color format, input Y and Cb/Cr > physically contiguous memory pointers > - Output height, stride, width, color format, output Y and Cb/Cr > physically contiguous memory pointers > > Based on the above parameters, when the above are provided and the IP > is started, we get an interrupt on completion. > I'm sure you are familiar with this model. However, in the case of > this IP, there could be 4-8 such channels and a single interrupt > on the completion of the all 4-8 scaling operations. > > In this IP, we are trying to have 4-8 input sources being scaled by > this single piece of hardware, by time multiplexing. > > An example use case is : > > Four applications (sources) will feed (enqueue) 4 input buffers to > the scaler, the scaler driver will synchronize the programming of > these buffers, when the number of buffers received by the driver > meets our batch size (say a batch size of 4), it will kick start the > IP. The four applications will poll on the fd, upon receiving an > interrupt from the hardware the poll will unblock. And all four > applications can dequeue their respective buffers and display them on > a sink. You should think of a better scheduling model, it will be really hard to design userspace that collaborate in order to optimize the IP usage. I think a better approach would be to queue while the IP is busy. These queues can then be sorted and prioritized. > > But each "channel" can be set to do accept its own individual input > and output formats. When I went through : > https://www.kernel.org/doc/html/v4.14/media/uapi/v4l/open.html#multip > le-opens > > It appears, once an application has invoked VIDIOC_REQBUFS or > VIDIOC_CREATE_BUFS, other applications cannot VIDIOC_S_FMT on them. > However to maximize the available number of channels, it would be > necessary to allow several applications to be able to > perform VIDIOC_S_FMT on the device node in the case of this hardware > as different channels can be expected to deal with different scaling > operations. This does not apply to M2M devices. Each time userspace open an M2M device, it will get a different instance (unless there is no more resource available). What drivers like Samsung FIMC, GSCALER, MFC. etc. do, is that they limit the number of instances (open calls) to the number of streams they can handle in parallel. They don't seem to share an IRQ when doing batch though. > > One option is to create a logical /dev/videoX node for each such > channel, and have a parent driver perform the interrupt handling, > batch size setting and other such common functionalities. Is there a > way to allow multiple applications talk to the same video device > node/file handle without creating logical video nodes for each > channel ? FIMC used to expose a node per instance and it was terribly hard to use. I don't think this is a good idea. > > Please let me know if the description of HW is not clear. I will look > forward to hear comments from you. > > > > > So for this series: > > > > Nacked-by: Hans Verkuil > > > > If this was added to drivers/staging/media instead and with an > > updated > > TODO, then we can accept it, but we need to see some real effort > > afterwards > > to switch this to the right API. Otherwise it will be removed again > > after a few kernel cycles. > > > > Many thanks for providing a pathway to get this into > drivers/staging/media > > I will drop this series, and re-send with the driver being placed in > drivers/staging/media. > I'll add some references to this conversation, so a new reviewer gets > some context of what > was discussed. In the meanwhile I will look into re-writing this to > utilize the M2M V4L2 API. > > > Regards, > > > > Hans > > > Best Regards, > Rohit > > > > -Original Message- > > From: Hans Verkuil [mailto:hverk...@xs4all.nl] > > Sent: Friday, March 09, 2018 3:58 AM > > To: Greg KH ; Rohit Athavale > > > > Cc: de...@driverdev.osuosl.org; linux-me.
Re: [PATCH v2 1/8] staging:iio:ade7854: Fix error handling on read/write
On 03/18, Jonathan Cameron wrote: > On Fri, 16 Mar 2018 19:48:33 -0300 > Rodrigo Siqueira wrote: > > > The original code does not correctly handle the error related to I2C > > read and write. This patch fixes the error handling related to all > > read/write functions for I2C. This patch is an adaptation of the John > > Syne patches. > > > > Signed-off-by: Rodrigo Siqueira > > Signed-off-by: John Syne > Hi Rodrigo, > > I'm not sure what the chain of authorship was here. If this is fundamentally > John's original patch he should still be the author and his sign off should be > first. You then sign off afterwards to indicate that you 'handled' the patch > and believe the work to be John's (you are trusting his sign off). This > is 'fun' legal stuff - read the docs on developers certificate of origin. > > If the patch has changed 'enough' (where that is a fuzzy definition) > then you should as you have here take the authorship, but John's sign off is > no longer true (it's a different patch). If John has reviewed the code > it is fine to have a reviewed-by or acked-by from John there to reflect > that. > > Anyhow, please clarify the situation as I shouldn't take a patch where > I'm applying my sign-off without knowing the origins etc. Hi Jonathan, Just for clarification, this is fundamentally John's original patch with some changes on the way that write_reg operation returns the error. I should ask for someone else, how to correctly handle this situation since I did not have experience with this situation. Actually, when I worked on this patch, I was confused about using different authorship from the email. I got confused because of the following statement: "Make sure that the email you specify here is the same email you used to set up sending mail. The Linux kernel developers will not accept a patch where the "From" email differs from the "Signed-off-by" line, which is what will happen if these two emails do not match." [1] Anyway, I think this is not a newbie issue, and I should asked first. Thanks for the great explanation, I will not make this kind of mistake again. Thanks [1] - https://kernelnewbies.org/FirstKernelPatch > > --- > > drivers/staging/iio/meter/ade7854-i2c.c | 24 > > drivers/staging/iio/meter/ade7854.c | 10 +- > > 2 files changed, 17 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/staging/iio/meter/ade7854-i2c.c > > b/drivers/staging/iio/meter/ade7854-i2c.c > > index 317e4f0d8176..4437f1e33261 100644 > > --- a/drivers/staging/iio/meter/ade7854-i2c.c > > +++ b/drivers/staging/iio/meter/ade7854-i2c.c > > @@ -31,7 +31,7 @@ static int ade7854_i2c_write_reg_8(struct device *dev, > > ret = i2c_master_send(st->i2c, st->tx, 3); > > mutex_unlock(&st->buf_lock); > > > > - return ret; > > + return ret < 0 ? ret : 0; > > } > > > > static int ade7854_i2c_write_reg_16(struct device *dev, > > @@ -51,7 +51,7 @@ static int ade7854_i2c_write_reg_16(struct device *dev, > > ret = i2c_master_send(st->i2c, st->tx, 4); > > mutex_unlock(&st->buf_lock); > > > > - return ret; > > + return ret < 0 ? ret : 0; > > } > > > > static int ade7854_i2c_write_reg_24(struct device *dev, > > @@ -72,7 +72,7 @@ static int ade7854_i2c_write_reg_24(struct device *dev, > > ret = i2c_master_send(st->i2c, st->tx, 5); > > mutex_unlock(&st->buf_lock); > > > > - return ret; > > + return ret < 0 ? ret : 0; > > } > > > > static int ade7854_i2c_write_reg_32(struct device *dev, > > @@ -94,7 +94,7 @@ static int ade7854_i2c_write_reg_32(struct device *dev, > > ret = i2c_master_send(st->i2c, st->tx, 6); > > mutex_unlock(&st->buf_lock); > > > > - return ret; > > + return ret < 0 ? ret : 0; > > } > So for write cases you are flattening to 0 for good and < 0 for bad. > good. > > > > static int ade7854_i2c_read_reg_8(struct device *dev, > > @@ -110,11 +110,11 @@ static int ade7854_i2c_read_reg_8(struct device *dev, > > st->tx[1] = reg_address & 0xFF; > > > > ret = i2c_master_send(st->i2c, st->tx, 2); > > - if (ret) > > + if (ret < 0) > > goto out; > > > > ret = i2c_master_recv(st->i2c, st->rx, 1); > > - if (ret) > > + if (ret < 0) > > goto out; > > > > *val = st->rx[0]; > But in read cases you are returning the number of bytes read... > Given these functions can know the 'right' answer to that why not check > it here and do the same as for writes in return 0 for good and < 0 for > bad? > > @@ -136,11 +136,11 @@ static int ade7854_i2c_read_reg_16(struct device *dev, > > st->tx[1] = reg_address & 0xFF; > > > > ret = i2c_master_send(st->i2c, st->tx, 2); > > - if (ret) > > + if (ret < 0) > > goto out; > > > > ret = i2c_master_recv(st->i2c, st->rx, 2); > > - if (ret) > > + if (ret < 0) > > goto out; > > > > *val = (st->rx[0] << 8) | st->rx[1]; > > @@ -162,11 +162,11 @@ static int ade7854_i2c_read_reg_24(s
Re: [staging] 184ecc5ceb: BUG:unable_to_handle_kernel
On Mon, Mar 19 2018, kernel test robot wrote: > FYI, we noticed the following commit (built with gcc-6): > > commit: 184ecc5ceb379b43b29fc373f497fca88c73ad38 ("staging: lustre: allow > monolithic builds") > https://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git > staging-testing > > in testcase: trinity > with following parameters: > > runtime: 300s > > test-description: Trinity is a linux system call fuzz tester. > test-url: http://codemonkey.org.uk/projects/trinity/ > > > on test machine: qemu-system-i386 -enable-kvm -smp 2 -m 320M > > caused below changes (please refer to attached dmesg/kmsg for entire > log/backtrace): > > > +--+++ > | | 26f7a294e5 | 184ecc5ceb | > +--+++ > | boot_successes | 14 | 4 | > | boot_failures| 0 | 12 | > | BUG:unable_to_handle_kernel | 0 | 12 | > | Oops:#[##] | 0 | 12 | > | EIP:cfs_cpt_spread_node | 0 | 12 | > | Kernel_panic-not_syncing:Fatal_exception | 0 | 12 | > +--+++ > > > > [ 123.548190] BUG: unable to handle kernel NULL pointer dereference at > 0008 > [ 123.551023] IP: cfs_cpt_spread_node+0x31/0x100 Thanks. This happens when both USERIO and LNET are built into the kernel drivers/staging/lustre/TODO says LNET_MINOR conflicts with USERIO_MINOR I'll add a Kconfig rule preventing them from being built together until this is resolved properly. I'll also lok into making the module init routine more robust. Thanks, NeilBrown signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
ABOUT TO TRANSFER YOUR ATM NOTIFICATION
Dear Sir/Madam I remain convinced that you will be surprised by this mail, but this remains the utmost truth. I know that it is against my official duty to release confidential issue like this to an outsider.There is nothing I can do to help you beside this; I am taking this bold step because of my belief as a Christian. Attn ATM CARD OWNER. We the management of (First City Monument Bank Ltd) wishes to inform you that your request to convert your fund into an ATM Visa Card has been completed. Our ATM processing Unit finally processed the ATM Card and your total fund valued at $13,500,000.00 Million United States Dollars has been loaded into your ATM Card account with us here in First City Monument Bank Ltd. For the immediate delivery of your ATM Visa Card to your home address via DHL or FedEx courier delivery company, kindly respond back to us with your current delivery address in full details,Such as. Your Full Name; Home Or Company Address;; Working Phone Number; Valid International Passport Copy or Driver license; Occupation/Gender; Age/Marital Status; The delivery arrangement will be made, immediately we receive your home delivery address as instructed, your ATM Card will be registered for delivery with one of the mentioned courier delivery company and you will receive the package after three days without any further delay. We wait your urgent respond. Best Regards Moses LinGard & Co FCMB (UK) Limited ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)
Hi Jonathan, Thank you for all your hard work. Your feedback is really helpful. I’m surprised that no one from Analog Device has offered any suggestions. Anyway, please see my comments inline below Regards, John > On Mar 18, 2018, at 5:23 AM, Jonathan Cameron wrote: > > On Sat, 17 Mar 2018 23:11:45 -0700 > John Syne wrote: > >> Hi Jonathan, > Hi John and All, > > I'd love to get some additional input on this from anyone interested. > There are a lot of weird and wonderful derived quantities in an energy > meter and it seems we need to make some fundamental changes to support > them - including potentially a userspace breaking change to the event > code description. > >> >> Here is the complete list of registers for the ADE7878 which I copied from >> the data sheet. I added a column “IIO Attribute” which I hope follows your >> IIO ABI. Please make any changes you feel are incorrect. BTW, there are >> several registers that cannot be generalized and are used purely for chip >> configuration. I think we should add a new naming convention, namely >> {register}_{}. Also, I see in the sys_bus_iio doc >> in_accel_x_peak_raw >> >> so shouldn’t the phase be represented as follows: >> >> in_current_A_scale > I'm still confused. What does A represent here? I assumed that was a wild > card for the channel number before but clearly not. > > Ah, you are labelling the 3 separate phases as A, B and C. Hmm. > I guess they sort of look like axis, and sort of like independent channels. > So could be indexed or done via modifiers depending on how you look at it. In metering terminology, the three phases are commonly referred to as RED, GREEN, BLUE or PhaseA, PhaseB, PhaseC and Neutral. Analog Devices uses the ABCN nomenclature so anyone using this driver will probably have referenced the Analog Devices datasheet so this terminology should be familiar. > > Hmm. With neutral in there as well I guess we need to make them > modifiers (but might change my mind later ;) > > Particularly as we are using the the modifier for RMS under the previous > plan... It appears we should treat that instead like we did for peak > and do it as an additional info mask element. The problem with doing that > on a continuous measurement is that we can't treat it as a channel to > be output through the buffered interface. I’ve changed the layout of the spreadsheet to breakout the Direction, Type, Index, Modifier and Info Mask to make sure there is no miss-understanding and will help identify all the items we need to add. The ADE7878 channels that will use the buffer are IAWV, VAWV, IBWV, VBWV, ICWV, VCWV, INWV, AVA, BVA, CVA, AWATT, BWATT, CWATT, AVAR, BVAR, and CVAR. So I guess we have to add an index Address RegisterIIO Attribute Dir TypeIndex ModifierInfo Mask R/W Bit Bit Length TypeDefault Description Length During Comm Value 0xE50C IAWVin_current0_phaseA_instantaneousin current 0 phaseA instantaneous R 24 32 SE S N/A Instantaneous value of Phase A current. 0xE50D IBWVin_current1_phaseB_instantaneousin current 1 phaseB instantaneous R 24 32 SE S N/A Instantaneous value of Phase B current. 0xE50E ICWVin_current2_phaseC_instantaneousin current 2 phaseC instantaneous R 24 32 SE S N/A Instantaneous value of Phase C current. 0xE50F INWVin_current3_phaseN_instantaneousin current 3 neutral instantaneous R 24 32 SE S N/A Instantaneous value of neutral current (ADE7868 and ADE7878 only). 0xE510 VAWVin_voltage4_phaseA_instantaneousin voltage 4 phaseA instantaneous R 24 32 SE S N/A Instantaneous value of Phase A voltage. 0xE511 VBWVin_voltage5_phaseB_instantaneousin voltage 5 phaseB instantaneous R 24 32 SE S N/A Instantaneous value of Phase B voltage. 0xE512 VCWVin_voltage6_phaseC_instantaneousin voltage 6 phaseC instantaneous R 24 32 SE S N/A Instantaneous value of Phase C voltage. 0xE513 AWATT in_power7_phaseA_instantaneous
[PATCH 1/6] staging: ks7010: Factor out repeated code.
Some of the code for reading IEs is replicated multiple times in the switch statement for get_ap_information(). Factor that code out into read_ie(). Signed-off-by: Quytelda Kahja --- drivers/staging/ks7010/ks_hostif.c | 48 +- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 05f7be4638fe..a946ce76f899 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -223,6 +223,21 @@ int get_current_ap(struct ks_wlan_private *priv, struct link_ap_info_t *ap_info) return 0; } +static u8 read_ie(unsigned char *bp, u8 max, u8 *body, char *name) +{ + u8 size; + + if (*(bp + 1) <= max) { + size = *(bp + 1); + } else { + DPRINTK(1, "size over :: %s size=%d\n", name, *(bp + 1)); + size = max; + } + + memcpy(body, bp + 2, size); + return size; +} + static int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, struct local_ap_t *ap) @@ -253,14 +268,8 @@ int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, while (bsize > offset) { switch (*bp) { /* Information Element ID */ case WLAN_EID_SSID: - if (*(bp + 1) <= IEEE80211_MAX_SSID_LEN) { - ap->ssid.size = *(bp + 1); - } else { - DPRINTK(1, "size over :: ssid size=%d\n", - *(bp + 1)); - ap->ssid.size = IEEE80211_MAX_SSID_LEN; - } - memcpy(ap->ssid.body, bp + 2, ap->ssid.size); + ap->ssid.size = read_ie(bp, IEEE80211_MAX_SSID_LEN, + ap->ssid.body, "ssid"); break; case WLAN_EID_SUPP_RATES: case WLAN_EID_EXT_SUPP_RATES: @@ -283,28 +292,15 @@ int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, break; case WLAN_EID_RSN: ap->rsn_ie.id = *bp; - if (*(bp + 1) <= RSN_IE_BODY_MAX) { - ap->rsn_ie.size = *(bp + 1); - } else { - DPRINTK(1, "size over :: rsn size=%d\n", - *(bp + 1)); - ap->rsn_ie.size = RSN_IE_BODY_MAX; - } - memcpy(ap->rsn_ie.body, bp + 2, ap->rsn_ie.size); + ap->rsn_ie.size = read_ie(bp, RSN_IE_BODY_MAX, + ap->rsn_ie.body, "rsn"); break; case WLAN_EID_VENDOR_SPECIFIC: /* WPA */ if (memcmp(bp + 2, CIPHER_ID_WPA_WEP40, 4) == 0) { /* WPA OUI check */ ap->wpa_ie.id = *bp; - if (*(bp + 1) <= RSN_IE_BODY_MAX) { - ap->wpa_ie.size = *(bp + 1); - } else { - DPRINTK(1, - "size over :: wpa size=%d\n", - *(bp + 1)); - ap->wpa_ie.size = RSN_IE_BODY_MAX; - } - memcpy(ap->wpa_ie.body, bp + 2, - ap->wpa_ie.size); + ap->wpa_ie.size = read_ie(bp, RSN_IE_BODY_MAX, + ap->wpa_ie.body, + "wpa"); } break; -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/6] staging: ks7010: Fix line over 80 characters.
There is no reason for comment describing the BSSID check for loop to be spaced so far to the right. Move it above the for loop. Signed-off-by: Quytelda Kahja --- drivers/staging/ks7010/ks_hostif.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 2de4dbbcd9de..6fc2c3647908 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -844,7 +844,8 @@ void hostif_scan_indication(struct ks_wlan_private *priv) ap_info = (struct ap_info_t *)(priv->rxp); if (priv->scan_ind_count) { - for (i = 0; i < priv->aplist.size; i++) { /* bssid check */ + /* bssid check */ + for (i = 0; i < priv->aplist.size; i++) { if (memcmp(ap_info->bssid, priv->aplist.ap[i].bssid, ETH_ALEN) != 0) continue; -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/6] staging: ks7010: Remove unnecessary parentheses.
Remove unnecessary parentheses highlighted by checkpatch. Signed-off-by: Quytelda Kahja --- drivers/staging/ks7010/ks_hostif.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 948d45280d18..00b97e8e9b4f 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -45,7 +45,7 @@ inline u8 get_BYTE(struct ks_wlan_private *priv) { u8 data; - data = *(priv->rxp)++; + data = *priv->rxp++; /* length check in advance ! */ --(priv->rx_size); return data; @@ -860,7 +860,7 @@ void hostif_scan_indication(struct ks_wlan_private *priv) DPRINTK(4, " scan_ind_count=%d :: aplist.size=%d\n", priv->scan_ind_count, priv->aplist.size); get_ap_information(priv, (struct ap_info_t *)(priv->rxp), - &(priv->aplist.ap[priv->scan_ind_count - 1])); + &priv->aplist.ap[priv->scan_ind_count - 1]); priv->aplist.size = priv->scan_ind_count; } else { DPRINTK(4, " count over :: scan_ind_count=%d\n", -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/6] staging: ks7010: Remove unnecessary braces.
Braces aren't required for a single line if statement. Signed-off-by: Quytelda Kahja --- drivers/staging/ks7010/ks_hostif.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 00b97e8e9b4f..2de4dbbcd9de 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -1387,9 +1387,8 @@ static __le16 ks_wlan_cap(struct ks_wlan_private *priv) { u16 capability = 0x; - if (priv->reg.preamble == SHORT_PREAMBLE) { + if (priv->reg.preamble == SHORT_PREAMBLE) capability |= WLAN_CAPABILITY_SHORT_PREAMBLE; - } capability &= ~(WLAN_CAPABILITY_PBCC); /* pbcc not support */ -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/6] staging: ks7010: Factor out repeated request initialization code.
The code to initialize various different types of request structs is repeated multiple times. Factor this code out into a macro called INIT_REQUEST. Signed-off-by: Quytelda Kahja --- drivers/staging/ks7010/ks_hostif.c | 55 +++--- 1 file changed, 16 insertions(+), 39 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 6fc2c3647908..3e5016aad029 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -40,6 +40,17 @@ static inline unsigned int cnt_smeqbody(struct ks_wlan_private *priv) #define KS_WLAN_MEM_FLAG (GFP_ATOMIC) +#define INIT_REQUEST(pp, priv) +{( + pp->phy_type = cpu_to_le16((uint16_t)(priv->reg.phy_type)); + pp->cts_mode = cpu_to_le16((uint16_t)(priv->reg.cts_mode)); + pp->scan_type = cpu_to_le16((uint16_t)(priv->reg.scan_type)); + pp->rate_set.size = priv->reg.rate_set.size; + pp->capability = ks_wlan_cap(priv); + memcpy(&pp->rate_set.body[0], &priv->reg.rate_set.body[0], + priv->reg.rate_set.size); +)} + static inline u8 get_BYTE(struct ks_wlan_private *priv) { @@ -1412,14 +1423,7 @@ void hostif_ps_adhoc_set_request(struct ks_wlan_private *priv) if (!pp) return; - pp->phy_type = cpu_to_le16((uint16_t)(priv->reg.phy_type)); - pp->cts_mode = cpu_to_le16((uint16_t)(priv->reg.cts_mode)); - pp->scan_type = cpu_to_le16((uint16_t)(priv->reg.scan_type)); - pp->channel = cpu_to_le16((uint16_t)(priv->reg.channel)); - pp->rate_set.size = priv->reg.rate_set.size; - pp->capability = ks_wlan_cap(priv); - memcpy(&pp->rate_set.body[0], &priv->reg.rate_set.body[0], - priv->reg.rate_set.size); + INIT_REQUEST(pp, priv); /* send to device request */ ps_confirm_wait_inc(priv); @@ -1437,16 +1441,9 @@ void hostif_infrastructure_set_request(struct ks_wlan_private *priv) if (!pp) return; - pp->phy_type = cpu_to_le16((uint16_t)(priv->reg.phy_type)); - pp->cts_mode = cpu_to_le16((uint16_t)(priv->reg.cts_mode)); - pp->scan_type = cpu_to_le16((uint16_t)(priv->reg.scan_type)); - - pp->rate_set.size = priv->reg.rate_set.size; - memcpy(&pp->rate_set.body[0], &priv->reg.rate_set.body[0], - priv->reg.rate_set.size); + INIT_REQUEST(pp, priv); pp->ssid.size = priv->reg.ssid.size; memcpy(&pp->ssid.body[0], &priv->reg.ssid.body[0], priv->reg.ssid.size); - pp->capability = ks_wlan_cap(priv); pp->beacon_lost_count = cpu_to_le16((uint16_t)(priv->reg.beacon_lost_count)); pp->auth_type = cpu_to_le16((uint16_t)(priv->reg.authenticate_type)); @@ -1486,16 +1483,9 @@ static void hostif_infrastructure_set2_request(struct ks_wlan_private *priv) if (!pp) return; - pp->phy_type = cpu_to_le16((uint16_t)(priv->reg.phy_type)); - pp->cts_mode = cpu_to_le16((uint16_t)(priv->reg.cts_mode)); - pp->scan_type = cpu_to_le16((uint16_t)(priv->reg.scan_type)); - - pp->rate_set.size = priv->reg.rate_set.size; - memcpy(&pp->rate_set.body[0], &priv->reg.rate_set.body[0], - priv->reg.rate_set.size); + INIT_REQUEST(pp, priv); pp->ssid.size = priv->reg.ssid.size; memcpy(&pp->ssid.body[0], &priv->reg.ssid.body[0], priv->reg.ssid.size); - pp->capability = ks_wlan_cap(priv); pp->beacon_lost_count = cpu_to_le16((uint16_t)(priv->reg.beacon_lost_count)); pp->auth_type = cpu_to_le16((uint16_t)(priv->reg.authenticate_type)); @@ -1538,16 +1528,9 @@ void hostif_adhoc_set_request(struct ks_wlan_private *priv) if (!pp) return; - pp->phy_type = cpu_to_le16((uint16_t)(priv->reg.phy_type)); - pp->cts_mode = cpu_to_le16((uint16_t)(priv->reg.cts_mode)); - pp->scan_type = cpu_to_le16((uint16_t)(priv->reg.scan_type)); - pp->channel = cpu_to_le16((uint16_t)(priv->reg.channel)); - pp->rate_set.size = priv->reg.rate_set.size; - memcpy(&pp->rate_set.body[0], &priv->reg.rate_set.body[0], - priv->reg.rate_set.size); + INIT_REQUEST(pp, priv); pp->ssid.size = priv->reg.ssid.size; memcpy(&pp->ssid.body[0], &priv->reg.ssid.body[0], priv->reg.ssid.size); - pp->capability = ks_wlan_cap(priv); /* send to device request */ ps_confirm_wait_inc(priv); @@ -1565,15 +1548,9 @@ void hostif_adhoc_set2_request(struct ks_wlan_private *priv) if (!pp) return; - pp->phy_type = cpu_to_le16((uint16_t)(priv->reg.phy_type)); - pp->cts_mode = cpu_to_le16((uint16_t)(priv->reg.cts_mode)); - pp->scan_type = cpu_to_le16((uint16_t)(priv->reg.scan_type)); - pp->rate_set.size = priv->reg.rate_set.size; - memcpy(&pp->rate_set.body[0], &priv->reg.rate_set.body[0], - priv->reg.rate_set.size); +
[PATCH 2/6] staging: ks7010: Factor out code into helper methods.
Some cases in the switch statement in get_ap_information() are indented as much as five levels, which makes the code difficult to read because of all the wrapping. Factor them out into helper methods. Signed-off-by: Quytelda Kahja --- drivers/staging/ks7010/ks_hostif.c | 46 +- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index a946ce76f899..948d45280d18 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -238,6 +238,30 @@ static u8 read_ie(unsigned char *bp, u8 max, u8 *body, char *name) return size; } +static void read_ie_ext_supp_rates(struct local_ap_t *ap, unsigned char *bp) +{ + if ((*(bp + 1) + ap->rate_set.size) <= RATE_SET_MAX_SIZE) { + memcpy(&ap->rate_set.body[ap->rate_set.size], + bp + 2, *(bp + 1)); + ap->rate_set.size += *(bp + 1); + } else { + DPRINTK(1, "size over :: rate size=%d\n", + (*(bp + 1) + ap->rate_set.size)); + memcpy(&ap->rate_set.body[ap->rate_set.size], bp + 2, + RATE_SET_MAX_SIZE - ap->rate_set.size); + ap->rate_set.size += (RATE_SET_MAX_SIZE - ap->rate_set.size); + } +} + +static void read_ie_wpa(struct local_ap_t *ap, unsigned char *bp) +{ + if (memcmp(bp + 2, CIPHER_ID_WPA_WEP40, 4) == 0) { /* WPA OUI check */ + ap->wpa_ie.id = *bp; + ap->wpa_ie.size = read_ie(bp, RSN_IE_BODY_MAX, + ap->wpa_ie.body, "wpa"); + } +} + static int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, struct local_ap_t *ap) @@ -273,20 +297,7 @@ int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, break; case WLAN_EID_SUPP_RATES: case WLAN_EID_EXT_SUPP_RATES: - if ((*(bp + 1) + ap->rate_set.size) <= - RATE_SET_MAX_SIZE) { - memcpy(&ap->rate_set.body[ap->rate_set.size], - bp + 2, *(bp + 1)); - ap->rate_set.size += *(bp + 1); - } else { - DPRINTK(1, "size over :: rate size=%d\n", - (*(bp + 1) + ap->rate_set.size)); - memcpy(&ap->rate_set.body[ap->rate_set.size], - bp + 2, - RATE_SET_MAX_SIZE - ap->rate_set.size); - ap->rate_set.size += - (RATE_SET_MAX_SIZE - ap->rate_set.size); - } + read_ie_ext_supp_rates(ap, bp); break; case WLAN_EID_DS_PARAMS: break; @@ -296,12 +307,7 @@ int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, ap->rsn_ie.body, "rsn"); break; case WLAN_EID_VENDOR_SPECIFIC: /* WPA */ - if (memcmp(bp + 2, CIPHER_ID_WPA_WEP40, 4) == 0) { /* WPA OUI check */ - ap->wpa_ie.id = *bp; - ap->wpa_ie.size = read_ie(bp, RSN_IE_BODY_MAX, - ap->wpa_ie.body, - "wpa"); - } + read_ie_wpa(ap, bp); break; case WLAN_EID_FH_PARAMS: -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/7] staging: wilc1000: replace switch statement by simple if condition
On Mon, Mar 19, 2018 at 07:45:46PM +0100, Greg KH wrote: > On Wed, Mar 14, 2018 at 06:15:03PM +0530, hariprasath.ela...@gmail.com wrote: > > From: HariPrasath Elango > > > > In this case,there is only a single switch case statement.So replacing > > by a simple if condition. > > > > Signed-off-by: HariPrasath Elango > > --- > > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 8 +--- > > 1 file changed, 1 insertion(+), 7 deletions(-) > > Does not apply to my tree :( Hi Greg, Sorry about that. Shall I sent a v2 after rebasing my repo ? Will that be fine ? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)
Hi Jonathan, I broke out the {Direction}_{Type}_{Index}_{Modifier}_{Info_Mask} into separate columns to make sure I understand your instructions. Good way to check the results. Probably easier to copy and paste this table into a spreadsheet. Let me know if there is anything I got wrong. Thank you again for all your help. Address RegisterIIO Attribute Device Tree or Code Direction TypeIndex ModifierInfo Mask R/W Bit Length Bit Length During CommunicationsTypeDefault Value Description 0x4380 AIGAIN in_current0_phaseA_scalein current 0 phaseA scale R/W 24 32 ZPSE S 0x00Phase A current gain adjust. 0x4381 AVGAIN in_voltage0_phaseA_scalein voltage 0 phaseA scale R/W 24 32 ZPSE S 0x00Phase A voltage gain adjust. 0x4382 BIGAIN in_current0_phaseB_scalein current 0 phaseB scale R/W 24 32 ZPSE S 0x00Phase B current gain adjust. 0x4383 BVGAIN in_voltage0_phaseB_scalein voltage 0 phaseB scale R/W 24 32 ZPSE S 0x00Phase B voltage gain adjust. 0x4384 CIGAIN in_current0_phaseC_scalein current 0 phaseC scale R/W 24 32 ZPSE S 0x00Phase C current gain adjust. 0x4385 CVGAIN in_voltage0_phaseC_scalein voltage 0 phaseC scale R/W 24 32 ZPSE S 0x00Phase C voltage gain adjust. 0x4386 NIGAIN in_current0_neutral_scale in current 0 neutral scale R/W 24 32 ZPSE S 0x00Neutral current gain adjust (ADE7868 and ADE7878 only). 0x4387 AIRMSOS in_current0_phaseA_rms_offset in current 0 phaseA offset R/W 24 32 ZPSE S 0x00Phase A current rms offset. 0x4388 AVRMSOS in_voltage0_phaseA_rms_offset in voltage 0 phaseA offset R/W 24 32 ZPSE S 0x00Phase A voltage rms offset. 0x4389 BIRMSOS in_current0_phaseB_rms_offset in current 0 phaseB offset R/W 24 32 ZPSE S 0x00Phase B current rms offset. 0x438A BVRMSOS in_voltage0_phaseB_rms_offset in voltage 0 phaseB offset R/W 24 32 ZPSE S 0x00Phase B voltage rms offset. 0x438B CIRMSOS in_current0_phaseC_rms_offset in current 0 phaseC offset R/W 24 32 ZPSE S 0x00Phase C current rms offset. 0x438C CVRMSOS in_voltage0_phaseC_rms_offset in voltage 0 phaseC offset R/W 24 32 ZPSE S 0x00Phase C voltage rms offset. 0x438D NIRMSOS in_current0_neutral_rms_offset in current 0 neutral offset R/W 24 32 ZPSE S 0x00Neutral current rms offset (ADE7868 and ADE7878 only). 0x438E AVAGAIN in_powerapparent0_phaseA_scale in powerapparent 0 phaseA scale R/W 24 32 ZPSE S 0x00Phase A apparent power gain adjust. 0x438F BVAGAIN in_powerapparent0_phaseB_scale in powerapparent 0 phaseB scale R/W 24 32 ZPSE S 0x00Phase B apparent power gain adjust. 0x4390 CVAGAIN in_powerapparent0_phaseC_scale in powerapparent 0 phaseC scale R/W 24 32 ZPSE S 0x00Phase C apparent power gain adjust. 0x4391 AWGAIN in_power0_phaseA_scale in power 0 phaseA scale R/W 24 32 ZPSE S 0x00Phase A total active power gain adjust. 0x4392 AWATTOS in_power0_phaseA_offset in power 0 phaseA offset R/W 24 32 ZPSE S 0x00Phase A total active power offset adjust. 0x4393 BWGAIN in_power0_phaseB_scale in power 0 phaseB scale R/W 24 32 ZPSE S 0x00Phase B total active power gain adjust. 0x4394 BWATTOS in_power0_phaseB_offset in power 0 phaseB offset R/W 24 32 ZPSE S 0x00Phase B total active power offset adjust. 0x4395 CWGAIN in_power0_PhaseC_scale in power 0 phaseC scale R/W 24 32 ZPSE S 0x00Phase C total active power gain adjust. 0x4396 CWATTOS in_power0_phaseC_offset in power 0 phaseC offset R/W 24 32 ZPSE S 0x00Phase C total active power offset adjust. 0x4397 AVARGAINin_powerreactive0_phaseA_scale in powerreactive 0 phaseA scale R/W 24 32 ZPSE S 0x00Phase A total reactive power gain adjust (ADE7858, ADE7868, and ADE7878). 0x4398 AVAROS in_powerreactive0_phaseA_offset in
Re: [PATCH 5/6] staging: ks7010: Fix line over 80 characters.
On Mon, 2018-03-19 at 22:58 -0700, Quytelda Kahja wrote: > There is no reason for comment describing the BSSID check for loop > to be spaced so far to the right. Move it above the for loop. [] > diff --git a/drivers/staging/ks7010/ks_hostif.c > b/drivers/staging/ks7010/ks_hostif.c [] > @@ -844,7 +844,8 @@ void hostif_scan_indication(struct ks_wlan_private *priv) > ap_info = (struct ap_info_t *)(priv->rxp); > > if (priv->scan_ind_count) { > - for (i = 0; i < priv->aplist.size; i++) { /* bssid check > */ > + /* bssid check */ > + for (i = 0; i < priv->aplist.size; i++) { > if (memcmp(ap_info->bssid, > priv->aplist.ap[i].bssid, ETH_ALEN) != 0) this could also use ether_addr_equal ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel