Re: [PATCH] staging:lustre: remove irq.h from socklnd.h
On Thu, Jun 25, 2015 at 10:59 PM, James Simmons wrote: > The header socklnd.h includes irq.h which is not need > and doesn't exist in the OpenSFS lustre branch. Having > irq.h in socklnd.h does break the build on the m68k > platform. So we can safely remove it. > > Signed-off-by: James Simmons Thanks, works fine on next-20150625! Tested-by: by: Geert Uytterhoeven Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: gdm72xx: Split Kconfig descriptions into more lines
Heh. No. That's not useful. :P Checkpatch.pl warnings are only good if they make the code better for humans. Otherwise ignore them. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: gdm72xx: Split Kconfig descriptions into more lines
On Thu, Jun 25, 2015 at 03:36:46PM -0700, Daniel Grimshaw wrote: > I am a high school student trying to become familiar with linux kernel > development. This is my first patch. > > checkpatch.pl throws a warning that config WIMAX_GDM72XX_QOS, config > WIMAX_GDM72XX_K_MODE, and config WIMAX_GDM72XX_USB do not have enough > of a description. By splitting the current description into multiple > lines, the warning is removed. > > The module still worked with these changes when tested. > > Signed-off-by: Daniel Grimshaw > Hello Daniel, Nice to see people submitting their first patch. Welcome :) Besides what Dan Carpenter said, I wanted to add a comment about how to send patches. Normally maintainers don't accept patches as attachments. You want to send the patch inline. I recommend these two resources to learn how: http://kernelnewbies.org/FirstKernelPatch Greg's talk "Write and Submit your first Linux kernel Patch" - https://www.youtube.com/watch?v=LLBrBBImJt4 There is a bit of a learning curve. It is normal to not get it right the first time. Feel free to email me directly with any further questions about this. Happy hacking, Luis ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: host_interface: add spaces around '='
On Mon, Jun 22, 2015 at 07:23:01PM +0530, Sunil Shahu wrote: > Fix coding style error by placing spaces around '=' as suggested by > checkpatch.pl script. > > Signed-off-by: Sunil Shahu > --- > drivers/staging/wilc1000/host_interface.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/wilc1000/host_interface.c > b/drivers/staging/wilc1000/host_interface.c > index 6b10bbb..d1fe73d 100644 > --- a/drivers/staging/wilc1000/host_interface.c > +++ b/drivers/staging/wilc1000/host_interface.c > @@ -7945,8 +7945,8 @@ s32 host_int_get_ipaddress(WILC_WFIDrvHandle hWFIDrv, > u8 *u16ipadd, u8 idx) > strHostIFmsg.u16MsgId = HOST_IF_MSG_GET_IPADDRESS; > > strHostIFmsg.uniHostIFmsgBody.strHostIfSetIP.au8IPAddr = u16ipadd; > - strHostIFmsg.drvHandler=hWFIDrv; > - strHostIFmsg.uniHostIFmsgBody.strHostIfSetIP.idx= idx; > + strHostIFmsg.drvHandler = hWFIDrv; > + strHostIFmsg.uniHostIFmsgBody.strHostIfSetIP.idx = idx; > > s32Error = WILC_MsgQueueSend(&gMsgQHostIF, &strHostIFmsg, > sizeof(tstrHostIFmsg), NULL); > if (s32Error) { > -- > 1.9.1 > Appliess cleanly on staging-testing. Removes all checkpatch ERRORS related to spaces placing in that file. Reviewed-by: Luis de Bethencourt ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] drivers: staging: rtl8712: remove unnecessary else statement
Hi Dan, On Mon, 2015-06-22 at 20:34 +0300, Dan Carpenter wrote: > > + u32 c = 0x1234; > ^^ > In another follow on patch you can get rid of this. It is nonsense > and > it isn't used. By "follow on patch", did you mean that I should send v3 revision of my patch or send a new patch after this patch gets applied to the tree? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
rename_rev.pl: review script for whitespace changes
I've sent my review script out a few times before but we have some new reviewers in staging who maybe haven't tried them. rename_rev.pl strips out whitespace changes. We recently had someone send a re-indent patch that deleted a line of code by mistake. The diff looked like: 18 files changed, 906 insertions(+), 927 deletions(-) It would be difficult to spot the bug manually but when you cat it through rename_rev.pl then it stands out immediately. If the patch changes // comments to /* */ then `rename_rev.pl -nc` strips out most of the comments. If the patch re-indents macros then the -ns removes slashes from the ends of lines. Sometimes people pull out some code into a separate function. The -pull option is supposed to help for that. It sometimes does... The other thing that we see a lot in staging is when people change curly braces around. The -nb option removes curly brace changes. Another thing is we had a change which did this: -#define HOST_IF_MSG_SCAN((u16)0) -(40 lines of similar code) +#define HOST_IF_MSG_SCAN0 +(40 lines of similar code) I used rename_rev.pl -e 's/\(\(u16\)(.*)\)/$1/' to make sure nothing else changed. Or if you are making hundreds of functions "static", then I just remove all the statics by doing rename_rev.pl -ea 's/static//'. The -ea option stands for Execute on All. Oh. And I am also going to include my move_rev.pl, script. That is for if we move functions from one file to another. cat patch | move_rev.pl | rename_rev.pl The rename_rev.pl script is sort of crappy, but often it removes a lot of stuff. It doesn't have to be perfect to be better, I guess. What I wish is that there were an -auto option which would find which variables were renamed. Oh! Oh! I have left out the most important feature. Say you are renaming variables from SomeName to some_name then cat patch | rename_rev.pl SomeName some_name TwoName two_name Foo foo regards, dan carpenter #!/usr/bin/perl # This is a tool to help review variable rename patches. The goal is # to strip out the automatic sed renames and the white space changes # and leaves the interesting code changes. # # Example 1: A patch renames openInfo to open_info: # cat diff | rename_review.pl openInfo open_info # # Example 2: A patch swaps the first two arguments to some_func(): # cat diff | rename_review.pl \ #-e 's/some_func\((.*?),(.*?),/some_func\($2, $1,/' # # Example 3: A patch removes the xkcd_ prefix from some but not all the # variables. Instead of trying to figure out which variables were renamed # just remove the prefix from them all: # cat diff | rename_review.pl -ea 's/xkcd_//g' # # Example 4: A patch renames 20 CamelCase variables. To review this let's # just ignore all case changes and all '_' chars. # cat diff | rename_review -ea 'tr/[A-Z]/[a-z]/' -ea 's/_//g' # # The other arguments are: # -nc removes comments # -ns removes '\' chars if they are at the end of the line. use strict; use File::Temp qw/ :mktemp /; sub usage() { print "usage: cat diff | $0 old new old new old new...\n"; print " or: cat diff | $0 -e 's/old/new/g'\n"; print " -e : execute on old lines\n"; print " -ea: execute on all lines\n"; print " -nc: no comments\n"; print " -nb: no unneeded braces\n"; print " -ns: no slashes at the end of a line\n"; print " -pull: for function pull. deletes context.\n"; exit(1); } my @subs; my @cmds; my $strip_comments; my $strip_braces; my $strip_slashes; my $pull_context; sub filter($) { my $_ = shift(); my $old = 0; if ($_ =~ /^-/) { $old = 1; } # remove the first char s/^[ +-]//; if ($strip_comments) { s/\/\*.*?\*\///g; s/\/\/.*//; } foreach my $cmd (@cmds) { if ($old || $cmd->[0] =~ /^-ea$/) { eval $cmd->[1]; } } foreach my $sub (@subs) { if ($old) { s/$sub->[0]/$sub->[1]/g; } } # remove the newline so we can move curly braces here if we want. s/\n//; return $_; } while (my $param1 = shift()) { if ($param1 =~ /^-nc$/) { $strip_comments = 1; next; } if ($param1 =~ /^-nb$/) { $strip_braces = 1; next; } if ($param1 =~ /^-ns$/) { $strip_slashes = 1; next; } if ($param1 =~ /^-pull$/) { $pull_context = 1; next; } my $param2 = shift(); if ($param2 =~ /^$/) { usage(); } if ($param1 =~ /^-e(a|)$/) { push @cmds, [$param1, $param2]; next; } push @subs, [$param1, $param2]; } my ($oldfh, $oldfile) = mkstemp("/tmp/oldX"); my ($newfh, $newfile) = mkstemp("/tmp/newX"); my $inside = 0; my $output; #recreate an old file and a new file while (<>) { if ($pull_context && !($_ =~ /^[+-@]/)) { next; } if ($_ =~ /^(---|\+\+\+)/) { next; } if ($_ =~ /^@/) { $
Re: [PATCH v2] drivers: staging: rtl8712: remove unnecessary else statement
On Fri, Jun 26, 2015 at 06:35:11PM +0530, Sunil Shahu wrote: > Hi Dan, > > On Mon, 2015-06-22 at 20:34 +0300, Dan Carpenter wrote: > > > + u32 c = 0x1234; > > ^^ > > In another follow on patch you can get rid of this. It is nonsense > > and > > it isn't used. > > By "follow on patch", did you mean that I should send v3 revision of my > patch or send a new patch after this patch gets applied to the tree? Send a new patch. You don't have to wait, you can just assume it will be merged and build on top of it. It will take a few weeks before your patch actually gets merged. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v6 1/3] cpu-hotplug: export cpu_hotplug_enable/cpu_hotplug_disable
Hyper-V module needs to disable cpu hotplug (offlining) as there is no support from hypervisor side to reassing already opened event channels to a different CPU. Currently it is been done by altering smp_ops.cpu_disable but it is hackish. Signed-off-by: Vitaly Kuznetsov --- kernel/cpu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/cpu.c b/kernel/cpu.c index 94bbe46..dc005e7 100644 --- a/kernel/cpu.c +++ b/kernel/cpu.c @@ -193,6 +193,7 @@ void cpu_hotplug_disable(void) cpu_hotplug_disabled = 1; cpu_maps_update_done(); } +EXPORT_SYMBOL_GPL(cpu_hotplug_disable); void cpu_hotplug_enable(void) { @@ -200,7 +201,7 @@ void cpu_hotplug_enable(void) cpu_hotplug_disabled = 0; cpu_maps_update_done(); } - +EXPORT_SYMBOL_GPL(cpu_hotplug_enable); #endif /* CONFIG_HOTPLUG_CPU */ /* Need to know about CPUs going up/down? */ -- 2.4.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v6 3/3] cpu-hotplug: convert cpu_hotplug_disabled to a counter
As cpu_hotplug_enable/cpu_hotplug_disable functions are now available to modules we need to convert cpu_hotplug_disabled to a counter to properly support disable -> disable -> enable call sequences. E.g. after Hyper-V vmbus module did cpu_hotplug_disable() hibernate path calls disable_nonboot_cpus() and if we hit an error in _cpu_down() enable_nonboot_cpus() will be called on the failure path (thus making cpu_hotplug_disabled = 0 and leaving cpu hotplug in 'enabled' state). Same problem is possible if more than 1 module use cpu_hotplug_disable/ cpu_hotplug_enable on their load/unload paths. When one of these modules is been unloaded it is logical to leave cpu hotplug in 'disabled' state. To support the change we need to increse cpu_hotplug_disabled counter in disable_nonboot_cpus() unconditionally as all users of disable_nonboot_cpus() are supposed to do enable_nonboot_cpus() in case an error was returned. Signed-off-by: Vitaly Kuznetsov --- Documentation/power/suspend-and-cpuhotplug.txt | 6 +++--- kernel/cpu.c | 21 + 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/Documentation/power/suspend-and-cpuhotplug.txt b/Documentation/power/suspend-and-cpuhotplug.txt index 2850df3..2fc9095 100644 --- a/Documentation/power/suspend-and-cpuhotplug.txt +++ b/Documentation/power/suspend-and-cpuhotplug.txt @@ -72,7 +72,7 @@ More details follow: | v Disable regular cpu hotplug -by setting cpu_hotplug_disabled=1 +by increasing cpu_hotplug_disabled | v Release cpu_add_remove_lock @@ -89,7 +89,7 @@ Resuming back is likewise, with the counterparts being (in the order of execution during resume): * enable_nonboot_cpus() which involves: | Acquire cpu_add_remove_lock - | Reset cpu_hotplug_disabled to 0, thereby enabling regular cpu hotplug + | Decrease cpu_hotplug_disabled, thereby enabling regular cpu hotplug | Call _cpu_up() [for all those cpus in the frozen_cpus mask, in a loop] | Release cpu_add_remove_lock v @@ -120,7 +120,7 @@ after the entire cycle is complete (i.e., suspend + resume). Acquire cpu_add_remove_lock | v - If cpu_hotplug_disabled is 1 + If cpu_hotplug_disabled > 0 return gracefully | | diff --git a/kernel/cpu.c b/kernel/cpu.c index dc005e7..cab3c92 100644 --- a/kernel/cpu.c +++ b/kernel/cpu.c @@ -190,7 +190,7 @@ void cpu_hotplug_done(void) void cpu_hotplug_disable(void) { cpu_maps_update_begin(); - cpu_hotplug_disabled = 1; + cpu_hotplug_disabled++; cpu_maps_update_done(); } EXPORT_SYMBOL_GPL(cpu_hotplug_disable); @@ -198,7 +198,7 @@ EXPORT_SYMBOL_GPL(cpu_hotplug_disable); void cpu_hotplug_enable(void) { cpu_maps_update_begin(); - cpu_hotplug_disabled = 0; + WARN_ON(--cpu_hotplug_disabled < 0); cpu_maps_update_done(); } EXPORT_SYMBOL_GPL(cpu_hotplug_enable); @@ -598,13 +598,18 @@ int disable_nonboot_cpus(void) } } - if (!error) { + if (!error) BUG_ON(num_online_cpus() > 1); - /* Make sure the CPUs won't be enabled by someone else */ - cpu_hotplug_disabled = 1; - } else { + else pr_err("Non-boot CPUs are not disabled\n"); - } + + /* +* Make sure the CPUs won't be enabled by someone else. We need to do +* this even in case of failure as all disable_nonboot_cpus() users are +* supposed to do enable_nonboot_cpus() on the failure path. +*/ + cpu_hotplug_disabled++; + cpu_maps_update_done(); return error; } @@ -623,7 +628,7 @@ void __ref enable_nonboot_cpus(void) /* Allow everyone to use the CPU hotplug again */ cpu_maps_update_begin(); - cpu_hotplug_disabled = 0; + WARN_ON(--cpu_hotplug_disabled < 0); if (cpumask_empty(frozen_cpus)) goto out; -- 2.4.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v6 2/3] Drivers: hv: vmbus: use cpu_hotplug_enable/disable
Commit e513229b4c38 ("Drivers: hv: vmbus: prevent cpu offlining on newer hypervisors") was altering smp_ops.cpu_disable to prevent CPU offlining. We can bo better by using cpu_hotplug_enable/disable functions instead of such hard-coding. Reported-by: Radim Krčmář Signed-off-by: Vitaly Kuznetsov Signed-off-by: K. Y. Srinivasan --- drivers/hv/vmbus_drv.c | 38 -- 1 file changed, 4 insertions(+), 34 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index cf20400..6de65fb 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -763,38 +763,6 @@ static void vmbus_isr(void) } } -#ifdef CONFIG_HOTPLUG_CPU -static int hyperv_cpu_disable(void) -{ - return -ENOSYS; -} - -static void hv_cpu_hotplug_quirk(bool vmbus_loaded) -{ - static void *previous_cpu_disable; - - /* -* Offlining a CPU when running on newer hypervisors (WS2012R2, Win8, -* ...) is not supported at this moment as channel interrupts are -* distributed across all of them. -*/ - - if ((vmbus_proto_version == VERSION_WS2008) || - (vmbus_proto_version == VERSION_WIN7)) - return; - - if (vmbus_loaded) { - previous_cpu_disable = smp_ops.cpu_disable; - smp_ops.cpu_disable = hyperv_cpu_disable; - pr_notice("CPU offlining is not supported by hypervisor\n"); - } else if (previous_cpu_disable) - smp_ops.cpu_disable = previous_cpu_disable; -} -#else -static void hv_cpu_hotplug_quirk(bool vmbus_loaded) -{ -} -#endif /* * vmbus_bus_init -Main vmbus driver initialization routine. @@ -836,7 +804,8 @@ static int vmbus_bus_init(int irq) if (ret) goto err_alloc; - hv_cpu_hotplug_quirk(true); + if (vmbus_proto_version > VERSION_WIN7) + cpu_hotplug_disable(); /* * Only register if the crash MSRs are available @@ -1121,7 +1090,8 @@ static void __exit vmbus_exit(void) smp_call_function_single(cpu, hv_synic_cleanup, NULL, 1); } acpi_bus_unregister_driver(&vmbus_acpi_driver); - hv_cpu_hotplug_quirk(false); + if (vmbus_proto_version > VERSION_WIN7) + cpu_hotplug_enable(); } -- 2.4.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v6 0/3] Drivers: hv: vmbus: use cpu_hotplug_enable/disable for CPU offlining prevention
Changes since v5: - Split PATCH 1 into two (PATCH 1/3 and 3/3), rewrite changelogs. [Thomas Gleixner] Changes since v4: - In disable_nonboot_cpus() do cpu_hotplug_disabled++ unconditionally as its users are doing enable_nonboot_cpus() on their failure paths. Changes since v3: - add WARN_ON when decreasing cpu_hotplug_disabled [Peter Zijlstra] Changes since v2: - Rebase on top of current Greg's char-misc-next tree [K. Y. Srinivasan] Changes since v1: - Make cpu_hotplug_disabled a counter [Radim Krčmář] Export cpu_hotplug_enable/cpu_hotplug_disable functions from cpu.c and use them instead of altering smp_ops.cpu_disable in Hyper-V vmbus module. Vitaly Kuznetsov (3): cpu-hotplug: export cpu_hotplug_enable/cpu_hotplug_disable Drivers: hv: vmbus: use cpu_hotplug_enable/disable cpu-hotplug: convert cpu_hotplug_disabled to a counter Documentation/power/suspend-and-cpuhotplug.txt | 6 ++-- drivers/hv/vmbus_drv.c | 38 +++--- kernel/cpu.c | 24 ++-- 3 files changed, 22 insertions(+), 46 deletions(-) -- 2.4.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8712: rtl871x_security.c: remove unnecessary variable initialization
Variable "u32 c" always re-initialize in for loop. Initialized value of "u32 c" is not used in function and is redundant, hence removed. Suggested-by: Dan Carpenter Signed-off-by: Sunil Shahu --- drivers/staging/rtl8712/rtl871x_security.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8712/rtl871x_security.c b/drivers/staging/rtl8712/rtl871x_security.c index 21465c9..8627928 100644 --- a/drivers/staging/rtl8712/rtl871x_security.c +++ b/drivers/staging/rtl8712/rtl871x_security.c @@ -125,7 +125,7 @@ static u8 crc32_reverseBit(u8 data) static void crc32_init(void) { sint i, j; - u32 c = 0x1234; + u32 c; u8 *p = (u8 *)&c, *p1; u8 k; -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] staging: wilc1000: cover letter
Patches to be applied on top of https://patchwork.kernel.org/patch/6655831/ Thanks! Luis ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] staging: wilc1000: remove unnecessary braces
Removing all checkpatch.pl warnings: WARNING: braces {} are not necessary for single statement blocks Signed-off-by: Luis de Bethencourt --- drivers/staging/wilc1000/host_interface.c | 276 ++ 1 file changed, 92 insertions(+), 184 deletions(-) diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c index d1fe73d..bf183b2 100644 --- a/drivers/staging/wilc1000/host_interface.c +++ b/drivers/staging/wilc1000/host_interface.c @@ -659,9 +659,8 @@ static s32 Handle_SetWfiDrvHandler(tstrHostIfSetDrvHandler *pstrHostIfSetDrvHand s32Error = SendConfigPkt(SET_CFG, &strWID, 1, true, (u32)pstrWFIDrv); - if ((pstrHostIfSetDrvHandler->u32Address) == (u32)NULL) { + if ((pstrHostIfSetDrvHandler->u32Address) == (u32)NULL) up(&hSemDeinitDrvHandle); - } if (s32Error) { @@ -705,9 +704,8 @@ static s32 Handle_SetOperationMode(void *drvHandler, tstrHostIfSetOperationMode s32Error = SendConfigPkt(SET_CFG, &strWID, 1, true, (u32)pstrWFIDrv); - if ((pstrHostIfSetOperationMode->u32Mode) == (u32)NULL) { + if ((pstrHostIfSetOperationMode->u32Mode) == (u32)NULL) up(&hSemDeinitDrvHandle); - } if (s32Error) { @@ -1204,10 +1202,9 @@ static s32 Handle_CfgParam(void *drvHandler, tstrHostIFCfgParamAttr *strHostIFCf } s32Error = SendConfigPkt(SET_CFG, strWIDList, u8WidCnt, false, (u32)pstrWFIDrv); - if (s32Error) { + if (s32Error) PRINT_ER("Error in setting CFG params\n"); - } WILC_CATCH(s32Error) { } @@ -1284,9 +1281,8 @@ static s32 Handle_Scan(void *drvHandler, tstrHostIFscanAttr *pstrHostIFscanAttr) strWIDList[u32WidsCount].u16WIDid = (u16)WID_SSID_PROBE_REQ; strWIDList[u32WidsCount].enuWIDtype = WID_STR; - for (i = 0; i < pstrHostIFscanAttr->strHiddenNetwork.u8ssidnum; i++) { + for (i = 0; i < pstrHostIFscanAttr->strHiddenNetwork.u8ssidnum; i++) valuesize += ((pstrHostIFscanAttr->strHiddenNetwork.pstrHiddenNetworkInfo[i].u8ssidlen) + 1); - } pu8HdnNtwrksWidVal = WILC_MALLOC(valuesize + 1); strWIDList[u32WidsCount].ps8WidVal = pu8HdnNtwrksWidVal; if (strWIDList[u32WidsCount].ps8WidVal != NULL) { @@ -1336,9 +1332,8 @@ static s32 Handle_Scan(void *drvHandler, tstrHostIFscanAttr *pstrHostIFscanAttr) int i; for (i = 0; i < pstrHostIFscanAttr->u8ChnlListLen; i++) { - if (pstrHostIFscanAttr->pu8ChnlFreqList[i] > 0) { + if (pstrHostIFscanAttr->pu8ChnlFreqList[i] > 0) pstrHostIFscanAttr->pu8ChnlFreqList[i] = pstrHostIFscanAttr->pu8ChnlFreqList[i] - 1; - } } } @@ -1400,9 +1395,8 @@ static s32 Handle_Scan(void *drvHandler, tstrHostIFscanAttr *pstrHostIFscanAttr) pstrHostIFscanAttr->pu8ChnlFreqList = NULL; } - if (pu8HdnNtwrksWidVal != NULL) { + if (pu8HdnNtwrksWidVal != NULL) WILC_FREE(pu8HdnNtwrksWidVal); - } return s32Error; } @@ -1778,9 +1772,8 @@ static s32 Handle_Connect(void *drvHandler, tstrHostIFconnectAttr *pstrHostIFcon strWIDList[u32WidsCount].s32ValueSize = MAX_SSID_LEN + 7; strWIDList[u32WidsCount].ps8WidVal = WILC_MALLOC(strWIDList[u32WidsCount].s32ValueSize); - if (strWIDList[u32WidsCount].ps8WidVal == NULL) { + if (strWIDList[u32WidsCount].ps8WidVal == NULL) WILC_ERRORREPORT(s32Error, WILC_NO_MEM); - } pu8CurrByte = strWIDList[u32WidsCount].ps8WidVal; @@ -1795,9 +1788,8 @@ static s32 Handle_Connect(void *drvHandler, tstrHostIFconnectAttr *pstrHostIFcon PRINT_ER("Channel out of range\n"); *(pu8CurrByte++) = 0xFF; } - if (pstrHostIFconnectAttr->pu8bssid != NULL) { + if (pstrHostIFconnectAttr->pu8bssid != NULL) WILC_memcpy(pu8CurrByte, pstrHostIFconnectAttr->pu8bssid, 6); - } pu8CurrByte += 6; /* keep the buffer at the start of the allocated pointer to use it with the free*/ @@ -1817,9 +1809,8 @@ static s32 Handle_Connect(void *drvHandler, tstrHostIFconnectAttr *pstrHostIFcon gu32FlushedJoinReqSize = strWIDList[u32WidsCount].s32ValueSize; gu8FlushedJoinReq = WILC_MALLOC(gu32FlushedJoinReqSize); } - if (strWIDList[u32WidsCount].ps8WidVal == NULL) { + if (strWIDList[u32WidsCount].ps8WidVal == NULL) WILC_ERRORREPORT(s32Error, WILC_NO_MEM); - } pu8CurrByte = strWIDList[u32WidsCount].ps8WidVal; @@ -1845,15 +1836,13 @@ static s32 Handle_Connect(void *drvHandler, tstrHostIFconnectAttr *pstrHostIFcon PRINT_D(HOSTINF_DBG, "* Cap Info %0x*\n", (*(pu8CurrByte - 2) | ((*(pu8CurrByte - 1)) << 8))); /* sa*/ -
[PATCH 2/4] staging: wilc1000: add blank lines after declarations
Fix all checkpatch.pl warnings: WARNING: Missing a blank line after declarations Signed-off-by: Luis de Bethencourt --- drivers/staging/wilc1000/host_interface.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c index bf183b2..e13074f 100644 --- a/drivers/staging/wilc1000/host_interface.c +++ b/drivers/staging/wilc1000/host_interface.c @@ -841,6 +841,7 @@ static s32 Handle_SetMacAddress(void *drvHandler, tstrHostIfSetMacAddress *pstrH tstrWID strWID; tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)drvHandler; u8 *mac_buf = (u8 *)WILC_MALLOC(ETH_ALEN); + if (mac_buf == NULL) { PRINT_ER("No buffer to send mac address\n"); return WILC_FAIL; @@ -1225,6 +1226,7 @@ static s32 Handle_CfgParam(void *drvHandler, tstrHostIFCfgParamAttr *strHostIFCf static s32 Handle_wait_msg_q_empty(void) { s32 s32Error = WILC_SUCCESS; + g_wilc_initialized = 0; up(&hWaitResponse); return s32Error; @@ -2357,6 +2359,7 @@ static s32 Handle_RcvdGnrlAsyncInfo(void *drvHandler, tstrRcvdGnrlAsyncInfo *pst tstrDisconnectNotifInfo strDisconnectNotifInfo; s32 s32Err = WILC_SUCCESS; tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *) drvHandler; + if (pstrWFIDrv == NULL) PRINT_ER("Driver handler is NULL\n"); PRINT_D(GENERIC_DBG, "Current State = %d,Received state = %d\n", pstrWFIDrv->enuHostIFstate, @@ -3208,6 +3211,7 @@ static s32 Handle_GetChnl(void *drvHandler) tstrWID strWID; /* tstrWILC_WFIDrv * pstrWFIDrv = (tstrWILC_WFIDrv *)hWFIDrv; */ tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)drvHandler; + strWID.u16WIDid = (u16)WID_CURRENT_CHANNEL; strWID.enuWIDtype = WID_CHAR; strWID.ps8WidVal = (s8 *)&gu8Chnl; @@ -3441,6 +3445,7 @@ static void Handle_AddBeacon(void *drvHandler, tstrHostIFSetBeacon *pstrSetBeaco tstrWID strWID; u8 *pu8CurrByte; tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)drvHandler; + PRINT_D(HOSTINF_DBG, "Adding BEACON\n"); strWID.u16WIDid = (u16)WID_ADD_BEACON; @@ -3512,6 +3517,7 @@ static void Handle_DelBeacon(void *drvHandler, tstrHostIFDelBeacon *pstrDelBeaco tstrWID strWID; u8 *pu8CurrByte; tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)drvHandler; + strWID.u16WIDid = (u16)WID_DEL_BEACON; strWID.enuWIDtype = WID_CHAR; strWID.s32ValueSize = sizeof(char); @@ -3608,6 +3614,7 @@ static void Handle_AddStation(void *drvHandler, tstrWILC_AddStaParam *pstrStatio tstrWID strWID; u8 *pu8CurrByte; tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)drvHandler; + PRINT_D(HOSTINF_DBG, "Handling add station\n"); strWID.u16WIDid = (u16)WID_ADD_STA; strWID.enuWIDtype = WID_BIN; @@ -3647,11 +3654,13 @@ static void Handle_AddStation(void *drvHandler, tstrWILC_AddStaParam *pstrStatio static void Handle_DelAllSta(void *drvHandler, tstrHostIFDelAllSta *pstrDelAllStaParam) { s32 s32Error = WILC_SUCCESS; + tstrWID strWID; u8 *pu8CurrByte; tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)drvHandler; u8 i; u8 au8Zero_Buff[6] = {0}; + strWID.u16WIDid = (u16)WID_DEL_ALL_STA; strWID.enuWIDtype = WID_STR; strWID.s32ValueSize = (pstrDelAllStaParam->u8Num_AssocSta * ETH_ALEN) + 1; @@ -4019,6 +4028,7 @@ static void Handle_PowerManagement(void *drvHandler, tstrHostIfPowerMgmtParam *s tstrWID strWID; s8 s8PowerMode; tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)drvHandler; + strWID.u16WIDid = (u16)WID_POWER_MANAGEMENT; if (strPowerMgmtParam->bIsEnabled == true) { @@ -4864,6 +4874,7 @@ s32 host_int_add_ptk(WILC_WFIDrvHandle hWFIDrv, const u8 *pu8Ptk, u8 u8PtkKeylen tstrHostIFmsg strHostIFmsg; u8 u8KeyLen = u8PtkKeylen; u32 i; + if (pstrWFIDrv == NULL) WILC_ERRORREPORT(s32Error, WILC_INVALID_ARGUMENT); if (pu8RxMic != NULL) @@ -6421,6 +6432,7 @@ void host_int_send_join_leave_info_to_host void GetPeriodicRSSI(void *pvArg) { tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *)pvArg; + if (pstrWFIDrv == NULL) { PRINT_ER("Driver handler is NULL\n"); return; @@ -6900,6 +6912,7 @@ void host_int_ScanCompleteReceived(u8 *pu8Buffer, u32 u32Length) tstrHostIFmsg strHostIFmsg; u32 drvHandler; tstrWILC_WFIDrv *pstrWFIDrv = NULL; + drvHandler = ((pu8Buffer[u32Length - 4]) | (pu8Buffer[u32Length - 3] << 8) | (pu8Buffer[u32Length - 2] << 16) | (pu8Buffer[u32Length - 1] << 24)); pstrWFIDrv = (tstrWILC_WFIDrv *)drvHandler; @@ -7216,6 +7229,7 @@ s32 host_int_add_station(WILC_WFIDrvHandle hWFIDrv, tstrWILC_AddStaParam *pstrSt WILC_memcpy(pstrA
[PATCH 3/4] staging: wilc1000: remove whitespaces before quoted newlines
Fix all checkpatch.pl warnings: WARNING: unnecessary whitespace before a quoted newline Signed-off-by: Luis de Bethencourt --- drivers/staging/wilc1000/host_interface.c | 84 +++ 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c index e13074f..79b7c2a 100644 --- a/drivers/staging/wilc1000/host_interface.c +++ b/drivers/staging/wilc1000/host_interface.c @@ -699,7 +699,7 @@ static s32 Handle_SetOperationMode(void *drvHandler, tstrHostIfSetOperationMode strWID.s32ValueSize = sizeof(u32); /*Sending Cfg*/ - PRINT_INFO(HOSTINF_DBG, "pstrWFIDrv= %p \n", pstrWFIDrv); + PRINT_INFO(HOSTINF_DBG, "pstrWFIDrv= %p\n", pstrWFIDrv); s32Error = SendConfigPkt(SET_CFG, &strWID, 1, true, (u32)pstrWFIDrv); @@ -740,7 +740,7 @@ s32 Handle_set_IPAddress(void *drvHandler, u8 *pu8IPAddr, u8 idx) if (pu8IPAddr[0] < 192) pu8IPAddr[0] = 0; - PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %d.%d.%d.%d \n", idx, pu8IPAddr[0], pu8IPAddr[1], pu8IPAddr[2], pu8IPAddr[3]); + PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %d.%d.%d.%d\n", idx, pu8IPAddr[0], pu8IPAddr[1], pu8IPAddr[2], pu8IPAddr[3]); WILC_memcpy(gs8SetIP[idx], pu8IPAddr, IP_ALEN); @@ -810,7 +810,7 @@ s32 Handle_get_IPAddress(void *drvHandler, u8 *pu8IPAddr, u8 idx) PRINT_ER("Failed to get IP address\n"); WILC_ERRORREPORT(s32Error, WILC_INVALID_STATE); } else { - PRINT_INFO(HOSTINF_DBG, "IP address retrieved:: u8IfIdx = %d \n", idx); + PRINT_INFO(HOSTINF_DBG, "IP address retrieved:: u8IfIdx = %d\n", idx); PRINT_INFO(HOSTINF_DBG, "%d.%d.%d.%d\n", gs8GetIP[idx][0], gs8GetIP[idx][1], gs8GetIP[idx][2], gs8GetIP[idx][3]); PRINT_INFO(HOSTINF_DBG, "\n"); } @@ -1253,7 +1253,7 @@ static s32 Handle_Scan(void *drvHandler, tstrHostIFscanAttr *pstrHostIFscanAttr) tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *) drvHandler; PRINT_D(HOSTINF_DBG, "Setting SCAN params\n"); - PRINT_D(HOSTINF_DBG, "Scanning: In [%d] state \n", pstrWFIDrv->enuHostIFstate); + PRINT_D(HOSTINF_DBG, "Scanning: In [%d] state\n", pstrWFIDrv->enuHostIFstate); pstrWFIDrv->strWILC_UsrScanReq.pfUserScanResult = pstrHostIFscanAttr->pfScanResult; pstrWFIDrv->strWILC_UsrScanReq.u32UserScanPvoid = pstrHostIFscanAttr->pvUserArg; @@ -1524,7 +1524,7 @@ static s32 Handle_Connect(void *drvHandler, tstrHostIFconnectAttr *pstrHostIFcon DeallocateSurveyResults(pstrSurveyResults); } else { WILC_ERRORREPORT(s32Error, WILC_FAIL); - PRINT_ER("ParseSurveyResults() Error(%d) \n", s32Err); + PRINT_ER("ParseSurveyResults() Error(%d)\n", s32Err); } @@ -1555,7 +1555,7 @@ static s32 Handle_Connect(void *drvHandler, tstrHostIFconnectAttr *pstrHostIFcon if (i < pstrWFIDrv->u32SurveyResultsCount) { u8bssDscListIndex = i; - PRINT_INFO(HOSTINF_DBG, "Connecting to network of Bss Idx %d and SSID %s and channel %d \n", + PRINT_INFO(HOSTINF_DBG, "Connecting to network of Bss Idx%d and SSID %s and channel%d\n", u8bssDscListIndex, pstrWFIDrv->astrSurveyResults[u8bssDscListIndex].SSID, pstrWFIDrv->astrSurveyResults[u8bssDscListIndex].Channel); @@ -2006,7 +2006,7 @@ static s32 Handle_Connect(void *drvHandler, tstrHostIFconnectAttr *pstrHostIFcon } } else { - PRINT_ER("Connect callback function pointer is NULL \n"); + PRINT_ER("Connect callback function pointer is NULL\n"); } } @@ -2162,7 +2162,7 @@ static s32 Handle_ConnectTimeout(void *drvHandler) strConnectInfo.pu8ReqIEs = NULL; } } else { - PRINT_ER("Connect callback function pointer is NULL \n"); + PRINT_ER("Connect callback function pointer is NULL\n"); } /* Here we will notify our firmware also with the Connection failure {through sending to it Cfg packet carrying @@ -2301,7 +2301,7 @@ static s32 Handle_RcvdNtwrkInfo(void *drvHandler, tstrRcvdNetworkInfo *pstrRcvdN } } else { - PRINT_WRN(HOSTINF_DBG, "Discovered networks exceeded max. limit \n"); + PRINT_WRN(HOSTINF_DBG, "Discovered networks exceeded max. limit\n"); } } else { pstrNetworkInfo->bNewNetwork = false; @@ -2423,7 +2423,7 @@ static s32 Handle_RcvdGnrlAsyncInfo(void *drvHandler, tstrRcvdGnrlAsyncInfo *pst s32Err = ParseAsso
[PATCH 4/4] staging: wilc1000: fix typos in PRINT_ERR()
Fix typo "packe" to "packet". Signed-off-by: Luis de Bethencourt --- drivers/staging/wilc1000/host_interface.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c index 79b7c2a..1915fc6 100644 --- a/drivers/staging/wilc1000/host_interface.c +++ b/drivers/staging/wilc1000/host_interface.c @@ -3688,7 +3688,7 @@ static void Handle_DelAllSta(void *drvHandler, tstrHostIFDelAllSta *pstrDelAllSt s32Error = SendConfigPkt(SET_CFG, &strWID, 1, true, (u32)pstrWFIDrv); if (s32Error) { - PRINT_ER("Failed to send add station config packe\n"); + PRINT_ER("Failed to send add station config packet\n"); WILC_ERRORREPORT(s32Error, WILC_FAIL); } @@ -3735,7 +3735,7 @@ static void Handle_DelStation(void *drvHandler, tstrHostIFDelSta *pstrDelStaPara s32Error = SendConfigPkt(SET_CFG, &strWID, 1, false, (u32)pstrWFIDrv); if (s32Error) { - PRINT_ER("Failed to send add station config packe\n"); + PRINT_ER("Failed to send add station config packet\n"); WILC_ERRORREPORT(s32Error, WILC_FAIL); } -- 2.1.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] staging: wilc1000: fix typos in PRINT_ERR()
Fix typo "packe" to "packet". Signed-off-by: Luis de Bethencourt --- drivers/staging/wilc1000/host_interface.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c index 79b7c2a..1915fc6 100644 --- a/drivers/staging/wilc1000/host_interface.c +++ b/drivers/staging/wilc1000/host_interface.c @@ -3688,7 +3688,7 @@ static void Handle_DelAllSta(void *drvHandler, tstrHostIFDelAllSta *pstrDelAllSt s32Error = SendConfigPkt(SET_CFG, &strWID, 1, true, (u32)pstrWFIDrv); if (s32Error) { - PRINT_ER("Failed to send add station config packe\n"); + PRINT_ER("Failed to send add station config packet\n"); WILC_ERRORREPORT(s32Error, WILC_FAIL); } @@ -3735,7 +3735,7 @@ static void Handle_DelStation(void *drvHandler, tstrHostIFDelSta *pstrDelStaPara s32Error = SendConfigPkt(SET_CFG, &strWID, 1, false, (u32)pstrWFIDrv); if (s32Error) { - PRINT_ER("Failed to send add station config packe\n"); + PRINT_ER("Failed to send add station config packet\n"); WILC_ERRORREPORT(s32Error, WILC_FAIL); } -- 2.1.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] staging: wilc1000: remove whitespaces before quoted newlines
On Fri, Jun 26, 2015 at 07:49:32AM -0700, Joe Perches wrote: > On Fri, 2015-06-26 at 16:45 +0200, Luis de Bethencourt wrote: > > Fix all checkpatch.pl warnings: > > WARNING: unnecessary whitespace before a quoted newline > > Unassociated but: > > > diff --git a/drivers/staging/wilc1000/host_interface.c > > b/drivers/staging/wilc1000/host_interface.c > [] > > @@ -740,7 +740,7 @@ s32 Handle_set_IPAddress(void *drvHandler, u8 > > *pu8IPAddr, u8 idx) > > if (pu8IPAddr[0] < 192) > > pu8IPAddr[0] = 0; > > > > - PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %d.%d.%d.%d \n", > > idx, pu8IPAddr[0], pu8IPAddr[1], pu8IPAddr[2], pu8IPAddr[3]); > > + PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %d.%d.%d.%d\n", > > idx, pu8IPAddr[0], pu8IPAddr[1], pu8IPAddr[2], pu8IPAddr[3]); > > These printks with ip addresses could use vsprintf extension %pI4 > > PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %pI4\n", > idx, pu8IPAddr); > > Hi Joe, I could send this as a 5th patch in the series if you'd like, and set you as author. - PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %d.%d.%d.%d\n", idx, pu8IPAddr[0], pu8IPAddr[1], pu8IPAddr[2], pu8IPAddr[3]); + PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %pI4\n", idx, pu8IPAddr); Thanks, Luis ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] staging: wilc1000: remove whitespaces before quoted newlines
On Fri, 2015-06-26 at 17:11 +0200, Luis de Bethencourt wrote: > On Fri, Jun 26, 2015 at 07:49:32AM -0700, Joe Perches wrote: > > On Fri, 2015-06-26 at 16:45 +0200, Luis de Bethencourt wrote: > > > Fix all checkpatch.pl warnings: > > > WARNING: unnecessary whitespace before a quoted newline > > > > Unassociated but: > > > > > diff --git a/drivers/staging/wilc1000/host_interface.c > > > b/drivers/staging/wilc1000/host_interface.c > > [] > > > @@ -740,7 +740,7 @@ s32 Handle_set_IPAddress(void *drvHandler, u8 > > > *pu8IPAddr, u8 idx) > > > if (pu8IPAddr[0] < 192) > > > pu8IPAddr[0] = 0; > > > > > > - PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %d.%d.%d.%d \n", > > > idx, pu8IPAddr[0], pu8IPAddr[1], pu8IPAddr[2], pu8IPAddr[3]); > > > + PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %d.%d.%d.%d\n", > > > idx, pu8IPAddr[0], pu8IPAddr[1], pu8IPAddr[2], pu8IPAddr[3]); > > > > These printks with ip addresses could use vsprintf extension %pI4 > > > > PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %pI4\n", > >idx, pu8IPAddr); > > > > > > Hi Joe, Hello Luis. > I could send this as a 5th patch in the series if you'd like, and set you as > author. Sure, but if you're doing it, you are the author. You could use a "Suggested-by" tag, but I don't care. > - PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %d.%d.%d.%d\n", > idx, pu8IPAddr[0], pu8IPAddr[1], pu8IPAddr[2], pu8IPAddr[3]); > + PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %pI4\n", idx, > pu8IPAddr); There are 3 sites in host_interface that could be changed. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/4] staging: wilc1000: switch printks to vsprintf IPv4 extension
Switch printks with IP addresses to use vsprintf extension %pI4. Suggested-by: Joe Perches Signed-off-by: Luis de Bethencourt --- drivers/staging/wilc1000/host_interface.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c index 1915fc6..483926f 100644 --- a/drivers/staging/wilc1000/host_interface.c +++ b/drivers/staging/wilc1000/host_interface.c @@ -740,7 +740,7 @@ s32 Handle_set_IPAddress(void *drvHandler, u8 *pu8IPAddr, u8 idx) if (pu8IPAddr[0] < 192) pu8IPAddr[0] = 0; - PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %d.%d.%d.%d\n", idx, pu8IPAddr[0], pu8IPAddr[1], pu8IPAddr[2], pu8IPAddr[3]); + PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %pI4\n", idx, pu8IPAddr); WILC_memcpy(gs8SetIP[idx], pu8IPAddr, IP_ALEN); @@ -796,7 +796,7 @@ s32 Handle_get_IPAddress(void *drvHandler, u8 *pu8IPAddr, u8 idx) s32Error = SendConfigPkt(GET_CFG, &strWID, 1, true, (u32)pstrWFIDrv); - PRINT_INFO(HOSTINF_DBG, "%d.%d.%d.%d\n", (u8)(strWID.ps8WidVal[0]), (u8)(strWID.ps8WidVal[1]), (u8)(strWID.ps8WidVal[2]), (u8)(strWID.ps8WidVal[3])); + PRINT_INFO(HOSTINF_DBG, "%pI4\n", strWID.ps8WidVal); WILC_memcpy(gs8GetIP[idx], strWID.ps8WidVal, IP_ALEN); @@ -811,7 +811,7 @@ s32 Handle_get_IPAddress(void *drvHandler, u8 *pu8IPAddr, u8 idx) WILC_ERRORREPORT(s32Error, WILC_INVALID_STATE); } else { PRINT_INFO(HOSTINF_DBG, "IP address retrieved:: u8IfIdx = %d\n", idx); - PRINT_INFO(HOSTINF_DBG, "%d.%d.%d.%d\n", gs8GetIP[idx][0], gs8GetIP[idx][1], gs8GetIP[idx][2], gs8GetIP[idx][3]); + PRINT_INFO(HOSTINF_DBG, "%pI4\n", gs8GetIP[idx]); PRINT_INFO(HOSTINF_DBG, "\n"); } -- 2.1.4 This patch is 5/4 because it is a suggestion made by Joe Perches. Based on top of the previous 4 wilc1000 patches. https://lkml.org/lkml/2015/6/26/392 Thank you Joe :) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8712: rtl871x_security.c: remove unnecessary variable initialization
On Fri, Jun 26, 2015 at 07:42:24PM +0530, Sunil Shahu wrote: > Variable "u32 c" always re-initialize in for loop. > Initialized value of "u32 c" is not used in function > and is redundant, hence removed. > > Suggested-by: Dan Carpenter > Signed-off-by: Sunil Shahu > --- > drivers/staging/rtl8712/rtl871x_security.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/rtl8712/rtl871x_security.c > b/drivers/staging/rtl8712/rtl871x_security.c > index 21465c9..8627928 100644 > --- a/drivers/staging/rtl8712/rtl871x_security.c > +++ b/drivers/staging/rtl8712/rtl871x_security.c > @@ -125,7 +125,7 @@ static u8 crc32_reverseBit(u8 data) > static void crc32_init(void) > { > sint i, j; > - u32 c = 0x1234; > + u32 c; > u8 *p = (u8 *)&c, *p1; > u8 k; > > -- > 1.9.1 The patch is good, the value assigned to c is immediately overwritten. Unfortunately the patch fails to apply in staging-testing and linux-next. Could you please rebase? Thanks, Luis ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8712: rtl871x_security.c: remove unnecessary variable initialization
On Fri, Jun 26, 2015 at 05:50:27PM +0200, Luis de Bethencourt wrote: > On Fri, Jun 26, 2015 at 07:42:24PM +0530, Sunil Shahu wrote: > > Variable "u32 c" always re-initialize in for loop. > > Initialized value of "u32 c" is not used in function > > and is redundant, hence removed. > > > > Suggested-by: Dan Carpenter > > Signed-off-by: Sunil Shahu > > --- > > drivers/staging/rtl8712/rtl871x_security.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/rtl8712/rtl871x_security.c > > b/drivers/staging/rtl8712/rtl871x_security.c > > index 21465c9..8627928 100644 > > --- a/drivers/staging/rtl8712/rtl871x_security.c > > +++ b/drivers/staging/rtl8712/rtl871x_security.c > > @@ -125,7 +125,7 @@ static u8 crc32_reverseBit(u8 data) > > static void crc32_init(void) > > { > > sint i, j; > > - u32 c = 0x1234; > > + u32 c; > > u8 *p = (u8 *)&c, *p1; > > u8 k; > > > > -- > > 1.9.1 > > The patch is good, the value assigned to c is immediately overwritten. > > Unfortunately the patch fails to apply in staging-testing and linux-next. > Could > you please rebase? It's based on earlier patches on the list. Normally only Greg tests that patches apply and it will for him because he applies them as the order they arrive. But we should probably start putting notes on the patches which earlier patches are required... regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8712: rtl871x_security.c: remove unnecessary variable initialization
On Fri, Jun 26, 2015 at 06:51:33PM +0300, Dan Carpenter wrote: > On Fri, Jun 26, 2015 at 05:50:27PM +0200, Luis de Bethencourt wrote: > > On Fri, Jun 26, 2015 at 07:42:24PM +0530, Sunil Shahu wrote: > > > Variable "u32 c" always re-initialize in for loop. > > > Initialized value of "u32 c" is not used in function > > > and is redundant, hence removed. > > > > > > Suggested-by: Dan Carpenter > > > Signed-off-by: Sunil Shahu > > > --- > > > drivers/staging/rtl8712/rtl871x_security.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/rtl8712/rtl871x_security.c > > > b/drivers/staging/rtl8712/rtl871x_security.c > > > index 21465c9..8627928 100644 > > > --- a/drivers/staging/rtl8712/rtl871x_security.c > > > +++ b/drivers/staging/rtl8712/rtl871x_security.c > > > @@ -125,7 +125,7 @@ static u8 crc32_reverseBit(u8 data) > > > static void crc32_init(void) > > > { > > > sint i, j; > > > - u32 c = 0x1234; > > > + u32 c; > > > u8 *p = (u8 *)&c, *p1; > > > u8 k; > > > > > > -- > > > 1.9.1 > > > > The patch is good, the value assigned to c is immediately overwritten. > > > > Unfortunately the patch fails to apply in staging-testing and linux-next. > > Could > > you please rebase? > > It's based on earlier patches on the list. > > Normally only Greg tests that patches apply and it will for him because > he applies them as the order they arrive. But we should probably start > putting notes on the patches which earlier patches are required... > > regards, > dan carpenter > I see. Thanks for the explanation. Then the patch is good, but it doesn't need a Reviewed-by line since it you pre-approved it. Regards, Luis ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192u: ieee80211_rx: Fix incorrect type in assignments
On Thu, Jun 25, 2015 at 02:06:44PM +0200, Arnd Bergmann wrote: > On Wednesday 24 June 2015 13:34:58 Gaston Gonzalez wrote: > > On Tue, Jun 23, 2015 at 12:13:47PM +0200, Arnd Bergmann wrote: > > > On Sunday 21 June 2015 19:12:09 Gaston Gonzalez wrote: > > > > /* WMM spec P.11: The minimum value for AIFSN shall be > > > > 2 */ > > > > qos_param->aifs[aci] = (qos_param->aifs[aci] < 2) ? > > > > 2:qos_param->aifs[aci]; > > > > > > > > - qos_param->cw_min[aci] = ac_params->ecw_min_max & 0x0F; > > > > + qos_param->cw_min[aci] = > > > > + cpu_to_le16(ac_params->ecw_min_max & 0x0F); > > > > > > > > - qos_param->cw_max[aci] = (ac_params->ecw_min_max & > > > > 0xF0) >> 4; > > > > + qos_param->cw_max[aci] = > > > > + cpu_to_le16((ac_params->ecw_min_max & 0xF0) >> > > > > 4); > > > > > > > > qos_param->flag[aci] = > > > > (ac_params->aci_aifsn & 0x10) ? 0x01 : 0x00; > > > > - qos_param->tx_op_limit[aci] = > > > > le16_to_cpu(ac_params->tx_op_limit); > > > > + qos_param->tx_op_limit[aci] = ac_params->tx_op_limit; > > > > } > > > > return 0; > > > > > > This certainly needs a more thorough description of how you determined > > > that > > > the byte swaps that you add are in fact required. Did you test it on > > > a big-endian machine? > > > > > > Arnd > > > > Hello Arnd, > > > > Thank you for reviewing this. > > After your email and reviwing this again I'm getting a bit suspicious > > myself, but this is what I saw: > > > > -- First warning: > > > > qos_param->cw_min[aci] is defined as __le16() in ieee80211.h > > (ieee80211_qos_parameters structure) > > > > ac_params-> ecw_min_max is defined as u8 in ieee80211.h > > (ieee80211_qos_ac_parameter structure) > > > > So the assignment is: __le16 = u8 & 0x0F; > > > > -- Second warning: > > > > qos_param->cw_max[aci] is __le16() > > ac_params-> ecw_min_max is u8 > > > > The assignment is: __le16 = (u8 & 0xF0) >> 4; > > > > Thus, for the warning 1 and 2, I understand that the result won't be the > > same if the machine is big-endian or little-endian, and that's why we > > need a cpu_to_le16. Am I missing something? > > I think your analysis is right, as long as the __le16 annotation is > actually correct. It usually helps to look at the git history to > see what the intent of the patch was that introduced the assignment > and the patch that introduced the __le16 type. Presumably one of them > was incorrect, and it would be good to figure out where it went wrong, > and to add a 'Fixes:' tag in your patch description that pinpoints > the exact mistake. > Ok, will do. > > -- Third warning: > > > > In this case both sides of the assignment are already defined as __le16: > > > > qos_param->tx_op_limit[aci] (ieee80211_qos_parameters structure defined > > in ieee80211.h)) > > > > ac_params->tx_op_limit (ieee80211_qos_ac_parameter structure defined in > > ieee80211.h) > > > > So the assignment is: __le16() = le16_to_cpu(__le16) > > > > Im getting suspicious now, but it sounded wrong to me. > > In the case the right part is correct, I guess the left part should be > > u16 type? > > Again, your logic sounds good: there is clearly something wrong here, but > it's not obvious to conclude whether it is an incorrect annotation or > an extraneous byte swap. Besides looking at the git history, it also > helps to look at all other uses of the two sides of the assignment: > > See if qos_param->tx_op_limit is in fact used as a little-endian > value (e.g. by copying to memory or a register), and if the value that > got written to ac_params->tx_op_limit was byte-swapped already at > the time of assignment. > Ok, I'll do it too. > > Regarding the test: I tested it on my machine, but is of course little- > > endian :( I could built a qemu virtual machine to test it on a > > big-endian emulated platform. Should that work? > > Yes, that would work: you can assign the USB device to the qemu machine > and run a kernel in there. The easiest emulation to try is probably > a PowerPC PAPR machine with a file system from > https://people.debian.org/~aurel32/qemu/powerpc/. > MIPS should work as well. > Ok, thanks a lot for all the pointers. Gaston > Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: rename_rev.pl: review script for whitespace changes
On Fri, Jun 26, 2015 at 04:15:24PM +0300, Dan Carpenter wrote: > I've sent my review script out a few times before but we have some new > reviewers in staging who maybe haven't tried them. > > rename_rev.pl strips out whitespace changes. We recently had someone > send a re-indent patch that deleted a line of code by mistake. The diff > looked like: > > 18 files changed, 906 insertions(+), 927 deletions(-) > > It would be difficult to spot the bug manually but when you cat it > through rename_rev.pl then it stands out immediately. > > If the patch changes // comments to /* */ then `rename_rev.pl -nc` > strips out most of the comments. > > If the patch re-indents macros then the -ns removes slashes from the > ends of lines. > > Sometimes people pull out some code into a separate function. The -pull > option is supposed to help for that. It sometimes does... > > The other thing that we see a lot in staging is when people change curly > braces around. The -nb option removes curly brace changes. > > Another thing is we had a change which did this: > > -#define HOST_IF_MSG_SCAN((u16)0) > -(40 lines of similar code) > +#define HOST_IF_MSG_SCAN0 > +(40 lines of similar code) > > I used rename_rev.pl -e 's/\(\(u16\)(.*)\)/$1/' to make sure nothing > else changed. > > Or if you are making hundreds of functions "static", then I just remove > all the statics by doing rename_rev.pl -ea 's/static//'. The -ea option > stands for Execute on All. > > Oh. And I am also going to include my move_rev.pl, script. That is for > if we move functions from one file to another. > > cat patch | move_rev.pl | rename_rev.pl > > The rename_rev.pl script is sort of crappy, but often it removes a lot > of stuff. It doesn't have to be perfect to be better, I guess. > > What I wish is that there were an -auto option which would find which > variables were renamed. Oh! Oh! I have left out the most important > feature. Say you are renaming variables from SomeName to some_name then > cat patch | rename_rev.pl SomeName some_name TwoName two_name Foo foo > > regards, > dan carpenter Thanks a lot for these, much appreciated, I'll work to add them to my test scripts for patches. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net-next] hv_netvsc: Add support to set MTU reservation from guest side
When packet encapsulation is in use, the MTU needs to be reduced for headroom reservation. The existing code takes the updated MTU value only from the host side. But vSwitch extensions, such as Open vSwitch, require the flexibility to change the MTU to different values from within a guest during the lifecycle of a vNIC, when the encapsulation protocol is changed. The patch supports this kind of MTU changes. Signed-off-by: Haiyang Zhang Reviewed-by: K. Y. Srinivasan --- drivers/net/hyperv/netvsc_drv.c |3 +-- drivers/net/hyperv/rndis_filter.c |2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 358475e..68e7ece 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -743,8 +743,7 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu) if (nvdev->nvsp_version >= NVSP_PROTOCOL_VERSION_2) limit = NETVSC_MTU - ETH_HLEN; - /* Hyper-V hosts don't support MTU < ETH_DATA_LEN (1500) */ - if (mtu < ETH_DATA_LEN || mtu > limit) + if (mtu < 68 || mtu > limit) return -EINVAL; nvdev->start_remove = true; diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c index 006c1b8..172824e 100644 --- a/drivers/net/hyperv/rndis_filter.c +++ b/drivers/net/hyperv/rndis_filter.c @@ -1053,7 +1053,7 @@ int rndis_filter_device_add(struct hv_device *dev, ret = rndis_filter_query_device(rndis_device, RNDIS_OID_GEN_MAXIMUM_FRAME_SIZE, &mtu, &size); - if (ret == 0 && size == sizeof(u32)) + if (ret == 0 && size == sizeof(u32) && mtu < net_device->ndev->mtu) net_device->ndev->mtu = mtu; /* Get the mac address */ -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: xgifb: Delete an unnecessary check before the function call "XGIfb_search_crt2type"
From: Markus Elfring Date: Fri, 26 Jun 2015 21:50:41 +0200 The XGIfb_search_crt2type() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/staging/xgifb/XGI_main_26.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/xgifb/XGI_main_26.c b/drivers/staging/xgifb/XGI_main_26.c index 943d463..ee0906e 100644 --- a/drivers/staging/xgifb/XGI_main_26.c +++ b/drivers/staging/xgifb/XGI_main_26.c @@ -2090,8 +2090,7 @@ static int __init xgifb_init(void) { char *option = NULL; - if (forcecrt2type != NULL) - XGIfb_search_crt2type(forcecrt2type); + XGIfb_search_crt2type(forcecrt2type); if (fb_get_options("xgifb", &option)) return -ENODEV; XGIfb_setup(option); -- 2.4.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging:slicoss:slicoss.h remove volatile variables
I am a high school student trying to become familiar with the opensource process and linux kernel. This is my first submission to the mailing list. I fixed the slicoss sub-system. The TODO file asks to remove volatile variables - also, checkpatch.pl warnings included volatile variables. I removed "volatile" from the variables /isr /and /linkstatus/ in the header file, because they are not needed. The two variables are used in the slicoss.c file, where /isr/ is used as function parameters, string outputs, pointers, logic, and one assignment, while /linkstatus /is used as pointers, logic, and one assignment. All but the assignments will not change these variables, and the assignment does not warrant a volatile qualifier. To make sure the changes were correct, I ran the files with checkpatch.pl again, test built it, and rebooted it. Signed-off-by: Vikul Gupta diff --git a/drivers/staging/slicoss/slic.h b/drivers/staging/slicoss/slic.h index 3a5aa88..f19f86a 100644 --- a/drivers/staging/slicoss/slic.h +++ b/drivers/staging/slicoss/slic.h @@ -357,8 +357,8 @@ struct base_driver { }; struct slic_shmem { -volatile u32 isr; -volatile u32 linkstatus; +u32 isr; +u32 linkstatus; volatile struct slic_stats inicstats; }; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging:slicoss:slicoss.h remove volatile variables
On Fri, Jun 26, 2015 at 12:57:32PM -0700, Vikul Gupta wrote: > I am a high school student trying to become familiar with the opensource > process and linux kernel. This is my first submission to the mailing list. Great, but this paragraph doesn't belong in here, as this whole thing will end up in the changelog body. You can put any comments / discussion below the --- line and it will not show up in the changelog when I apply the patch. > I fixed the slicoss sub-system. The whole subsystem, nice? :) Seriously, this sentance is not needed. > The TODO file asks to remove volatile > variables - also, checkpatch.pl warnings included volatile variables. > > I removed "volatile" from the variables /isr /and /linkstatus/ in the header what's with the '/' usage? > file, because they are not needed. The two variables are used in the > slicoss.c file, where /isr/ is used as function parameters, string outputs, > pointers, logic, and one assignment, while /linkstatus /is used as pointers, > logic, and one assignment. All but the assignments will not change these > variables, and the assignment does not warrant a volatile qualifier. > > To make sure the changes were correct, I ran the files with checkpatch.pl > again, test built it, and rebooted it. can you test this code? rebooting and not loading the driver isn't much proof of much other than the code still builds (which is a good thing to test.) Also, this paragraph is not needed in the changelog either. Care to fix this all up and resend? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 7/9] staging: vme_user: remove unused variable
Signed-off-by: Dmitry Kalinkin --- drivers/staging/vme/devices/vme_user.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c index ef876a4..947a38e 100644 --- a/drivers/staging/vme/devices/vme_user.c +++ b/drivers/staging/vme/devices/vme_user.c @@ -143,18 +143,14 @@ static ssize_t resource_to_user(int minor, char __user *buf, size_t count, static ssize_t resource_from_user(unsigned int minor, const char __user *buf, size_t count, loff_t *ppos) { - ssize_t copied = 0; - if (count > image[minor].size_buf) count = image[minor].size_buf; if (__copy_from_user(image[minor].kern_buf, buf, (unsigned long)count)) return -EFAULT; - copied = vme_master_write(image[minor].resource, image[minor].kern_buf, - count, *ppos); - - return copied; + return vme_master_write(image[minor].resource, image[minor].kern_buf, + count, *ppos); } static ssize_t buffer_to_user(unsigned int minor, char __user *buf, -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 3/9] staging: vme_user: fix NULL comparison style
Signed-off-by: Dmitry Kalinkin --- drivers/staging/vme/devices/vme_user.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c index 494655a..2ff15f0 100644 --- a/drivers/staging/vme/devices/vme_user.c +++ b/drivers/staging/vme/devices/vme_user.c @@ -527,7 +527,7 @@ static int vme_user_master_mmap(unsigned int minor, struct vm_area_struct *vma) } vma_priv = kmalloc(sizeof(struct vme_user_vma_priv), GFP_KERNEL); - if (vma_priv == NULL) { + if (!vma_priv) { mutex_unlock(&image[minor].mutex); return -ENOMEM; } @@ -588,7 +588,7 @@ static int vme_user_probe(struct vme_dev *vdev) char *name; /* Save pointer to the bridge device */ - if (vme_user_bridge != NULL) { + if (vme_user_bridge) { dev_err(&vdev->dev, "Driver can only be loaded for 1 device\n"); err = -EINVAL; goto err_dev; @@ -636,7 +636,7 @@ static int vme_user_probe(struct vme_dev *vdev) */ image[i].resource = vme_slave_request(vme_user_bridge, VME_A24, VME_SCT); - if (image[i].resource == NULL) { + if (!image[i].resource) { dev_warn(&vdev->dev, "Unable to allocate slave resource\n"); err = -ENOMEM; @@ -645,7 +645,7 @@ static int vme_user_probe(struct vme_dev *vdev) image[i].size_buf = PCI_BUF_SIZE; image[i].kern_buf = vme_alloc_consistent(image[i].resource, image[i].size_buf, &image[i].pci_buf); - if (image[i].kern_buf == NULL) { + if (!image[i].kern_buf) { dev_warn(&vdev->dev, "Unable to allocate memory for buffer\n"); image[i].pci_buf = 0; @@ -663,7 +663,7 @@ static int vme_user_probe(struct vme_dev *vdev) /* XXX Need to properly request attributes */ image[i].resource = vme_master_request(vme_user_bridge, VME_A32, VME_SCT, VME_D32); - if (image[i].resource == NULL) { + if (!image[i].resource) { dev_warn(&vdev->dev, "Unable to allocate master resource\n"); err = -ENOMEM; @@ -671,7 +671,7 @@ static int vme_user_probe(struct vme_dev *vdev) } image[i].size_buf = PCI_BUF_SIZE; image[i].kern_buf = kmalloc(image[i].size_buf, GFP_KERNEL); - if (image[i].kern_buf == NULL) { + if (!image[i].kern_buf) { err = -ENOMEM; vme_master_free(image[i].resource); goto err_master; -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 0/9] vme_user checkpatch fixes and read()/write() rework
First four patches are fixes for various checpatch warnings. Next there is a change to drop large read()/write() stub followed by a change to rework user copy error codes. Last three changes are refactorings. v2 fixes ("vme_user: return -EFAULT on __copy_*_user errors") that had EINVAL instead of EFAULT in a couple of places. v3 fixes ("allow large read()/write()") to also remove the comment right above resource_to_user() v3 also renames ("vme_user: return -EFAULT on __copy_*_user errors") to ("switch to returning -EFAULT on __copy_*_user errors") Dmitry Kalinkin (9): staging: vme_user: fix code alignment staging: vme_user: fix blank lines staging: vme_user: fix NULL comparison style staging: vme_user: fix kmalloc style staging: vme_user: allow large read()/write() staging: vme_user: switch to returning -EFAULT on __copy_*_user errors staging: vme_user: remove unused variable staging: vme_user: remove distracting comment staging: vme_user: remove okcount variable drivers/staging/vme/devices/vme_user.c | 164 ++--- 1 file changed, 51 insertions(+), 113 deletions(-) -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 9/9] staging: vme_user: remove okcount variable
Signed-off-by: Dmitry Kalinkin --- drivers/staging/vme/devices/vme_user.c | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c index 7ca943c..b3e3c2d 100644 --- a/drivers/staging/vme/devices/vme_user.c +++ b/drivers/staging/vme/devices/vme_user.c @@ -182,7 +182,6 @@ static ssize_t vme_user_read(struct file *file, char __user *buf, size_t count, unsigned int minor = MINOR(file_inode(file)->i_rdev); ssize_t retval; size_t image_size; - size_t okcount; if (minor == CONTROL_MINOR) return 0; @@ -200,16 +199,14 @@ static ssize_t vme_user_read(struct file *file, char __user *buf, size_t count, /* Ensure not reading past end of the image */ if (*ppos + count > image_size) - okcount = image_size - *ppos; - else - okcount = count; + count = image_size - *ppos; switch (type[minor]) { case MASTER_MINOR: - retval = resource_to_user(minor, buf, okcount, ppos); + retval = resource_to_user(minor, buf, count, ppos); break; case SLAVE_MINOR: - retval = buffer_to_user(minor, buf, okcount, ppos); + retval = buffer_to_user(minor, buf, count, ppos); break; default: retval = -EINVAL; @@ -228,7 +225,6 @@ static ssize_t vme_user_write(struct file *file, const char __user *buf, unsigned int minor = MINOR(file_inode(file)->i_rdev); ssize_t retval; size_t image_size; - size_t okcount; if (minor == CONTROL_MINOR) return 0; @@ -245,16 +241,14 @@ static ssize_t vme_user_write(struct file *file, const char __user *buf, /* Ensure not reading past end of the image */ if (*ppos + count > image_size) - okcount = image_size - *ppos; - else - okcount = count; + count = image_size - *ppos; switch (type[minor]) { case MASTER_MINOR: - retval = resource_from_user(minor, buf, okcount, ppos); + retval = resource_from_user(minor, buf, count, ppos); break; case SLAVE_MINOR: - retval = buffer_from_user(minor, buf, okcount, ppos); + retval = buffer_from_user(minor, buf, count, ppos); break; default: retval = -EINVAL; -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 1/9] staging: vme_user: fix code alignment
Signed-off-by: Dmitry Kalinkin --- drivers/staging/vme/devices/vme_user.c | 33 + 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c index 9cca97a..ccf9602 100644 --- a/drivers/staging/vme/devices/vme_user.c +++ b/drivers/staging/vme/devices/vme_user.c @@ -128,7 +128,7 @@ struct vme_user_vma_priv { * transfer the data directly into the user space buffers. */ static ssize_t resource_to_user(int minor, char __user *buf, size_t count, - loff_t *ppos) + loff_t *ppos) { ssize_t retval; ssize_t copied = 0; @@ -167,7 +167,7 @@ static ssize_t resource_to_user(int minor, char __user *buf, size_t count, * transfer the data directly from the user space buffers out to VME. */ static ssize_t resource_from_user(unsigned int minor, const char __user *buf, - size_t count, loff_t *ppos) + size_t count, loff_t *ppos) { ssize_t retval; ssize_t copied = 0; @@ -195,7 +195,7 @@ static ssize_t resource_from_user(unsigned int minor, const char __user *buf, } static ssize_t buffer_to_user(unsigned int minor, char __user *buf, - size_t count, loff_t *ppos) + size_t count, loff_t *ppos) { void *image_ptr; ssize_t retval; @@ -214,7 +214,7 @@ static ssize_t buffer_to_user(unsigned int minor, char __user *buf, } static ssize_t buffer_from_user(unsigned int minor, const char __user *buf, - size_t count, loff_t *ppos) + size_t count, loff_t *ppos) { void *image_ptr; size_t retval; @@ -233,7 +233,7 @@ static ssize_t buffer_from_user(unsigned int minor, const char __user *buf, } static ssize_t vme_user_read(struct file *file, char __user *buf, size_t count, - loff_t *ppos) +loff_t *ppos) { unsigned int minor = MINOR(file_inode(file)->i_rdev); ssize_t retval; @@ -279,7 +279,7 @@ static ssize_t vme_user_read(struct file *file, char __user *buf, size_t count, } static ssize_t vme_user_write(struct file *file, const char __user *buf, - size_t count, loff_t *ppos) + size_t count, loff_t *ppos) { unsigned int minor = MINOR(file_inode(file)->i_rdev); ssize_t retval; @@ -354,7 +354,7 @@ static loff_t vme_user_llseek(struct file *file, loff_t off, int whence) * already been defined. */ static int vme_user_ioctl(struct inode *inode, struct file *file, - unsigned int cmd, unsigned long arg) + unsigned int cmd, unsigned long arg) { struct vme_master master; struct vme_slave slave; @@ -390,12 +390,13 @@ static int vme_user_ioctl(struct inode *inode, struct file *file, * to userspace as they are */ retval = vme_master_get(image[minor].resource, - &master.enable, &master.vme_addr, - &master.size, &master.aspace, - &master.cycle, &master.dwidth); + &master.enable, + &master.vme_addr, + &master.size, &master.aspace, + &master.cycle, &master.dwidth); copied = copy_to_user(argp, &master, - sizeof(struct vme_master)); + sizeof(struct vme_master)); if (copied != 0) { pr_warn("Partial copy to userspace\n"); return -EFAULT; @@ -435,12 +436,12 @@ static int vme_user_ioctl(struct inode *inode, struct file *file, * to userspace as they are */ retval = vme_slave_get(image[minor].resource, - &slave.enable, &slave.vme_addr, - &slave.size, &pci_addr, &slave.aspace, - &slave.cycle); + &slave.enable, &slave.vme_addr, + &slave.size, &pci_addr, + &slave.aspace, &slave.cycle); copied = copy_to_user(argp, &slave, - sizeof(struct vme_slave)); + sizeof(struct vme_slave)); if (copied != 0) { pr_warn("Partial copy to userspace\n"); return -EFAULT; @@ -606,7 +607,7 @@ static int vme_user_probe(struct vme_dev *vdev)
[PATCHv3 5/9] staging: vme_user: allow large read()/write()
This changes large master transfers to do shorter read/write rather than return -EINVAL. User space will now be able to optimistically request a large transfer and get at least some data. This also removes comments suggesting on how to implement large transfers. Current vme_master_* read and write implementations use CPU copies that don't produce burst PCI accesses and subsequently no block transfer on VME bus. In the end overall performance is quiet low and it can't be fixed by doing direct copy to user space. Much easier solution would be to just reuse kernel buffer. Signed-off-by: Dmitry Kalinkin --- drivers/staging/vme/devices/vme_user.c | 73 +++--- 1 file changed, 24 insertions(+), 49 deletions(-) diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c index 3467cde..a2345db 100644 --- a/drivers/staging/vme/devices/vme_user.c +++ b/drivers/staging/vme/devices/vme_user.c @@ -120,75 +120,50 @@ struct vme_user_vma_priv { atomic_t refcnt; }; -/* - * We are going ot alloc a page during init per window for small transfers. - * Small transfers will go VME -> buffer -> user space. Larger (more than a - * page) transfers will lock the user space buffer into memory and then - * transfer the data directly into the user space buffers. - */ static ssize_t resource_to_user(int minor, char __user *buf, size_t count, loff_t *ppos) { ssize_t retval; ssize_t copied = 0; - if (count <= image[minor].size_buf) { - /* We copy to kernel buffer */ - copied = vme_master_read(image[minor].resource, - image[minor].kern_buf, count, *ppos); - if (copied < 0) - return (int)copied; - - retval = __copy_to_user(buf, image[minor].kern_buf, - (unsigned long)copied); - if (retval != 0) { - copied = (copied - retval); - pr_info("User copy failed\n"); - return -EINVAL; - } + if (count > image[minor].size_buf) + count = image[minor].size_buf; - } else { - /* XXX Need to write this */ - pr_info("Currently don't support large transfers\n"); - /* Map in pages from userspace */ + /* We copy to kernel buffer */ + copied = vme_master_read(image[minor].resource, image[minor].kern_buf, +count, *ppos); + if (copied < 0) + return (int)copied; - /* Call vme_master_read to do the transfer */ + retval = __copy_to_user(buf, image[minor].kern_buf, + (unsigned long)copied); + if (retval != 0) { + copied = (copied - retval); + pr_info("User copy failed\n"); return -EINVAL; } return copied; } -/* - * We are going to alloc a page during init per window for small transfers. - * Small transfers will go user space -> buffer -> VME. Larger (more than a - * page) transfers will lock the user space buffer into memory and then - * transfer the data directly from the user space buffers out to VME. - */ static ssize_t resource_from_user(unsigned int minor, const char __user *buf, size_t count, loff_t *ppos) { ssize_t retval; ssize_t copied = 0; - if (count <= image[minor].size_buf) { - retval = __copy_from_user(image[minor].kern_buf, buf, - (unsigned long)count); - if (retval != 0) - copied = (copied - retval); - else - copied = count; - - copied = vme_master_write(image[minor].resource, - image[minor].kern_buf, copied, *ppos); - } else { - /* XXX Need to write this */ - pr_info("Currently don't support large transfers\n"); - /* Map in pages from userspace */ - - /* Call vme_master_write to do the transfer */ - return -EINVAL; - } + if (count > image[minor].size_buf) + count = image[minor].size_buf; + + retval = __copy_from_user(image[minor].kern_buf, buf, + (unsigned long)count); + if (retval != 0) + copied = (copied - retval); + else + copied = count; + + copied = vme_master_write(image[minor].resource, image[minor].kern_buf, + copied, *ppos); return copied; } -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 8/9] staging: vme_user: remove distracting comment
Signed-off-by: Dmitry Kalinkin --- drivers/staging/vme/devices/vme_user.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c index 947a38e..7ca943c 100644 --- a/drivers/staging/vme/devices/vme_user.c +++ b/drivers/staging/vme/devices/vme_user.c @@ -128,7 +128,6 @@ static ssize_t resource_to_user(int minor, char __user *buf, size_t count, if (count > image[minor].size_buf) count = image[minor].size_buf; - /* We copy to kernel buffer */ copied = vme_master_read(image[minor].resource, image[minor].kern_buf, count, *ppos); if (copied < 0) -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 4/9] staging: vme_user: fix kmalloc style
Signed-off-by: Dmitry Kalinkin --- drivers/staging/vme/devices/vme_user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c index 2ff15f0..3467cde 100644 --- a/drivers/staging/vme/devices/vme_user.c +++ b/drivers/staging/vme/devices/vme_user.c @@ -526,7 +526,7 @@ static int vme_user_master_mmap(unsigned int minor, struct vm_area_struct *vma) return err; } - vma_priv = kmalloc(sizeof(struct vme_user_vma_priv), GFP_KERNEL); + vma_priv = kmalloc(sizeof(*vma_priv), GFP_KERNEL); if (!vma_priv) { mutex_unlock(&image[minor].mutex); return -ENOMEM; -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 2/9] staging: vme_user: fix blank lines
Signed-off-by: Dmitry Kalinkin --- drivers/staging/vme/devices/vme_user.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c index ccf9602..494655a 100644 --- a/drivers/staging/vme/devices/vme_user.c +++ b/drivers/staging/vme/devices/vme_user.c @@ -101,13 +101,13 @@ struct image_desc { struct vme_resource *resource; /* VME resource */ int mmap_count; /* Number of current mmap's */ }; + static struct image_desc image[VME_DEVS]; static struct cdev *vme_user_cdev; /* Character device */ static struct class *vme_user_sysfs_class; /* Sysfs class */ static struct vme_dev *vme_user_bridge;/* Pointer to user device */ - static const int type[VME_DEVS] = {MASTER_MINOR, MASTER_MINOR, MASTER_MINOR, MASTER_MINOR, SLAVE_MINOR,SLAVE_MINOR, @@ -120,7 +120,6 @@ struct vme_user_vma_priv { atomic_t refcnt; }; - /* * We are going ot alloc a page during init per window for small transfers. * Small transfers will go VME -> buffer -> user space. Larger (more than a @@ -836,7 +835,6 @@ static void __exit vme_user_exit(void) vme_unregister_driver(&vme_user_driver); } - MODULE_PARM_DESC(bus, "Enumeration of VMEbus to which the driver is connected"); module_param_array(bus, int, &bus_num, 0); -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 6/9] staging: vme_user: switch to returning -EFAULT on __copy_*_user errors
Signed-off-by: Dmitry Kalinkin --- drivers/staging/vme/devices/vme_user.c | 47 -- 1 file changed, 11 insertions(+), 36 deletions(-) diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c index a2345db..ef876a4 100644 --- a/drivers/staging/vme/devices/vme_user.c +++ b/drivers/staging/vme/devices/vme_user.c @@ -123,7 +123,6 @@ struct vme_user_vma_priv { static ssize_t resource_to_user(int minor, char __user *buf, size_t count, loff_t *ppos) { - ssize_t retval; ssize_t copied = 0; if (count > image[minor].size_buf) @@ -135,13 +134,8 @@ static ssize_t resource_to_user(int minor, char __user *buf, size_t count, if (copied < 0) return (int)copied; - retval = __copy_to_user(buf, image[minor].kern_buf, - (unsigned long)copied); - if (retval != 0) { - copied = (copied - retval); - pr_info("User copy failed\n"); - return -EINVAL; - } + if (__copy_to_user(buf, image[minor].kern_buf, (unsigned long)copied)) + return -EFAULT; return copied; } @@ -149,21 +143,16 @@ static ssize_t resource_to_user(int minor, char __user *buf, size_t count, static ssize_t resource_from_user(unsigned int minor, const char __user *buf, size_t count, loff_t *ppos) { - ssize_t retval; ssize_t copied = 0; if (count > image[minor].size_buf) count = image[minor].size_buf; - retval = __copy_from_user(image[minor].kern_buf, buf, - (unsigned long)count); - if (retval != 0) - copied = (copied - retval); - else - copied = count; + if (__copy_from_user(image[minor].kern_buf, buf, (unsigned long)count)) + return -EFAULT; copied = vme_master_write(image[minor].resource, image[minor].kern_buf, - copied, *ppos); + count, *ppos); return copied; } @@ -172,38 +161,24 @@ static ssize_t buffer_to_user(unsigned int minor, char __user *buf, size_t count, loff_t *ppos) { void *image_ptr; - ssize_t retval; image_ptr = image[minor].kern_buf + *ppos; + if (__copy_to_user(buf, image_ptr, (unsigned long)count)) + return -EFAULT; - retval = __copy_to_user(buf, image_ptr, (unsigned long)count); - if (retval != 0) { - retval = (count - retval); - pr_warn("Partial copy to userspace\n"); - } else - retval = count; - - /* Return number of bytes successfully read */ - return retval; + return count; } static ssize_t buffer_from_user(unsigned int minor, const char __user *buf, size_t count, loff_t *ppos) { void *image_ptr; - size_t retval; image_ptr = image[minor].kern_buf + *ppos; + if (__copy_from_user(image_ptr, buf, (unsigned long)count)) + return -EFAULT; - retval = __copy_from_user(image_ptr, buf, (unsigned long)count); - if (retval != 0) { - retval = (count - retval); - pr_warn("Partial copy to userspace\n"); - } else - retval = count; - - /* Return number of bytes successfully read */ - return retval; + return count; } static ssize_t vme_user_read(struct file *file, char __user *buf, size_t count, -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] staging: wilc1000: remove whitespaces before quoted newlines
On Fri, 2015-06-26 at 16:45 +0200, Luis de Bethencourt wrote: > Fix all checkpatch.pl warnings: > WARNING: unnecessary whitespace before a quoted newline Unassociated but: > diff --git a/drivers/staging/wilc1000/host_interface.c > b/drivers/staging/wilc1000/host_interface.c [] > @@ -740,7 +740,7 @@ s32 Handle_set_IPAddress(void *drvHandler, u8 *pu8IPAddr, > u8 idx) > if (pu8IPAddr[0] < 192) > pu8IPAddr[0] = 0; > > - PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %d.%d.%d.%d \n", > idx, pu8IPAddr[0], pu8IPAddr[1], pu8IPAddr[2], pu8IPAddr[3]); > + PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %d.%d.%d.%d\n", > idx, pu8IPAddr[0], pu8IPAddr[1], pu8IPAddr[2], pu8IPAddr[3]); These printks with ip addresses could use vsprintf extension %pI4 PRINT_INFO(HOSTINF_DBG, "Indx = %d, Handling set IP = %pI4\n", idx, pu8IPAddr); ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: lustre: Deletion of unnecessary checks before three function calls
From: Markus Elfring Date: Fri, 26 Jun 2015 23:10:42 +0200 The following functions test whether their argument is NULL and then return immediately. * kfree * ll_file_data_put * ptlrpc_connection_put Thus the test around such calls is not needed. This issue was detected by using the Coccinelle software. See also a previous update suggestion: "remove unneeded null test before free" by Julia Lawall https://lkml.org/lkml/2015/5/1/498 https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg878600.html Signed-off-by: Markus Elfring --- drivers/staging/lustre/lustre/llite/file.c | 3 +-- drivers/staging/lustre/lustre/llite/llite_lib.c | 2 +- drivers/staging/lustre/lustre/ptlrpc/import.c | 6 ++ drivers/staging/lustre/lustre/ptlrpc/service.c | 4 +--- 4 files changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c index 3075db2..1a85c41 100644 --- a/drivers/staging/lustre/lustre/llite/file.c +++ b/drivers/staging/lustre/lustre/llite/file.c @@ -702,8 +702,7 @@ out_och_free: out_openerr: if (opendir_set != 0) ll_stop_statahead(inode, lli->lli_opendir_key); - if (fd != NULL) - ll_file_data_put(fd); + ll_file_data_put(fd); } else { ll_stats_ops_tally(ll_i2sbi(inode), LPROC_LL_OPEN, 1); } diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c index 2513988..ab4839c 100644 --- a/drivers/staging/lustre/lustre/llite/llite_lib.c +++ b/drivers/staging/lustre/lustre/llite/llite_lib.c @@ -1114,7 +1114,7 @@ void ll_clear_inode(struct inode *inode) if (lli->lli_mds_read_och) ll_md_real_close(inode, FMODE_READ); - if (S_ISLNK(inode->i_mode) && lli->lli_symlink_name) { + if (S_ISLNK(inode->i_mode)) { kfree(lli->lli_symlink_name); lli->lli_symlink_name = NULL; } diff --git a/drivers/staging/lustre/lustre/ptlrpc/import.c b/drivers/staging/lustre/lustre/ptlrpc/import.c index c9b8481..1eae389 100644 --- a/drivers/staging/lustre/lustre/ptlrpc/import.c +++ b/drivers/staging/lustre/lustre/ptlrpc/import.c @@ -555,14 +555,12 @@ static int import_select_connection(struct obd_import *imp) imp_conn->oic_last_attempt = cfs_time_current_64(); /* switch connection, don't mind if it's same as the current one */ - if (imp->imp_connection) - ptlrpc_connection_put(imp->imp_connection); + ptlrpc_connection_put(imp->imp_connection); imp->imp_connection = ptlrpc_connection_addref(imp_conn->oic_conn); dlmexp = class_conn2export(&imp->imp_dlm_handle); LASSERT(dlmexp != NULL); - if (dlmexp->exp_connection) - ptlrpc_connection_put(dlmexp->exp_connection); + ptlrpc_connection_put(dlmexp->exp_connection); dlmexp->exp_connection = ptlrpc_connection_addref(imp_conn->oic_conn); class_export_put(dlmexp); diff --git a/drivers/staging/lustre/lustre/ptlrpc/service.c b/drivers/staging/lustre/lustre/ptlrpc/service.c index 9117f1c..b9ae0b7 100644 --- a/drivers/staging/lustre/lustre/ptlrpc/service.c +++ b/drivers/staging/lustre/lustre/ptlrpc/service.c @@ -2826,9 +2826,7 @@ void ptlrpc_hr_fini(void) ptlrpc_stop_hr_threads(); cfs_percpt_for_each(hrp, i, ptlrpc_hr.hr_partitions) { - if (hrp->hrp_thrs != NULL) { - kfree(hrp->hrp_thrs); - } + kfree(hrp->hrp_thrs); } cfs_percpt_free(ptlrpc_hr.hr_partitions); -- 2.4.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging:slicoss:slicoss.h remove volatile variables
Hi Vikul, welcome! See my comment below... On Fri, Jun 26, 2015 at 12:57 PM, Vikul Gupta wrote: > I am a high school student trying to become familiar with the opensource > process and linux kernel. This is my first submission to the mailing list. > > I fixed the slicoss sub-system. The TODO file asks to remove volatile > variables - also, checkpatch.pl warnings included volatile variables. > > I removed "volatile" from the variables /isr /and /linkstatus/ in the header > file, because they are not needed. The two variables are used in the > slicoss.c file, where /isr/ is used as function parameters, string outputs, > pointers, logic, and one assignment, while /linkstatus /is used as pointers, > logic, and one assignment. All but the assignments will not change these > variables, and the assignment does not warrant a volatile qualifier. It is not safe to simply drop volatile from these fields. For example, slic_card_init polls on isr waiting for the device to write to it. If you drop volatile, the compiler is within its rights to pull the read out of the loop. > > To make sure the changes were correct, I ran the files with checkpatch.pl > again, test built it, and rebooted it. > > Signed-off-by: Vikul Gupta > > diff --git a/drivers/staging/slicoss/slic.h b/drivers/staging/slicoss/slic.h > index 3a5aa88..f19f86a 100644 > --- a/drivers/staging/slicoss/slic.h > +++ b/drivers/staging/slicoss/slic.h > @@ -357,8 +357,8 @@ struct base_driver { > }; > > struct slic_shmem { > -volatile u32 isr; > -volatile u32 linkstatus; > +u32 isr; > +u32 linkstatus; > volatile struct slic_stats inicstats; > }; > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [v3.16.y-ckt][v3.18.y][v3.19.y-ckt][v4.0.y][PATCH 1/1]Drivers: hv: vmbus: Add support for VMBus panic notifier handler
On Tue, Jun 16, 2015 at 04:22:12PM -0400, Joseph Salisbury wrote: > From: Nick Meier > > BugLink: http://bugs.launchpad.net/bugs/1463584 > > Hyper-V allows a guest to notify the Hyper-V host that a panic > condition occured. This notification can include up to five 64 > bit values. These 64 bit values are written into crash MSRs. > Once the data has been written into the crash MSRs, the host is > then notified by writing into a Crash Control MSR. On the Hyper-V > host, the panic notification data is captured in the Windows Event > log as a 18590 event. > > Crash MSRs are defined in appendix H of the Hypervisor Top Level > Functional Specification. At the time of this patch, v4.0 is the > current functional spec. The URL for the v4.0 document is: > > http://download.microsoft.com/download/A/B/4/AB43A34E-BDD0-4FA6-BDEF-79EEF16E880B/Hypervisor > Top Level Functional Specification v4.0.docx > > Signed-off-by: Nick Meier > Signed-off-by: K. Y. Srinivasan > Signed-off-by: Greg Kroah-Hartman > (Backported from commit 96c1d0581d00f7abe033350edb021a9d947d8d81) > Signed-off-by: Joseph Salisbury > --- > drivers/hv/hyperv_vmbus.h | 11 +++ > drivers/hv/vmbus_drv.c| 35 +++ > 2 files changed, 46 insertions(+) This is a new feature, why should it be added to -stable releases? Sorry, not going to take it, upgrade to 4.1 if you want this feature. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: board: Add dependency on CLKDEV_LOOKUP
The code depends on CLKDEV_LOOKUP since commit 225d68d852f1 ("staging: board: Add support for devices with complex dependencies"). Related build error (powerpc:allmodconfig): drivers/built-in.o: In function `.board_staging_register_clock': (.init.text+0x1d8e0): undefined reference to `.clk_add_alias' Fixes: 225d68d852f1 ("staging: board: Add support for devices with complex dependencies") Cc: Geert Uytterhoeven Signed-off-by: Guenter Roeck --- drivers/staging/board/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/board/Kconfig b/drivers/staging/board/Kconfig index b8ee81840666..4ff5a795055f 100644 --- a/drivers/staging/board/Kconfig +++ b/drivers/staging/board/Kconfig @@ -1,6 +1,6 @@ config STAGING_BOARD bool "Staging Board Support" - depends on OF_ADDRESS + depends on OF_ADDRESS && CLKDEV_LOOKUP help Select to enable per-board staging support code. -- 2.1.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging:lustre: remove irq.h from socklnd.h
On Thu, Jun 25, 2015 at 04:59:46PM -0400, James Simmons wrote: > The header socklnd.h includes irq.h which is not need > and doesn't exist in the OpenSFS lustre branch. Having > irq.h in socklnd.h does break the build on the m68k > platform. So we can safely remove it. > > Signed-off-by: James Simmons The m68k build problem is now seen in mainline, and this patch fixes it. Tested-by: Guenter Roeck Guenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: lustre: obdclass: fix macro coding style issue in uuid.c
This patch fixes the checkpatch.pl error: ERROR: Macros with complex values should be enclosed in parentheses +#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) Signed-off-by: Vasiliy Korchagin --- drivers/staging/lustre/lustre/obdclass/uuid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/obdclass/uuid.c b/drivers/staging/lustre/lustre/obdclass/uuid.c index ff0a01b..8d00882 100644 --- a/drivers/staging/lustre/lustre/obdclass/uuid.c +++ b/drivers/staging/lustre/lustre/obdclass/uuid.c @@ -55,7 +55,7 @@ static inline __u32 consume(int nob, __u8 **ptr) return value; } -#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) +#define CONSUME(val, ptr) ((val) = consume(sizeof(val), (ptr))) static void uuid_unpack(class_uuid_t in, __u16 *uu, int nr) { -- 2.4.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: obdclass: fix macro coding style issue in uuid.c
On Sat, 2015-06-27 at 05:44 +0100, Vasiliy Korchagin wrote: > This patch fixes the checkpatch.pl error: > > ERROR: Macros with complex values should be enclosed in parentheses > +#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) > > Signed-off-by: Vasiliy Korchagin > --- > drivers/staging/lustre/lustre/obdclass/uuid.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/lustre/lustre/obdclass/uuid.c > b/drivers/staging/lustre/lustre/obdclass/uuid.c > index ff0a01b..8d00882 100644 > --- a/drivers/staging/lustre/lustre/obdclass/uuid.c > +++ b/drivers/staging/lustre/lustre/obdclass/uuid.c > @@ -55,7 +55,7 @@ static inline __u32 consume(int nob, __u8 **ptr) > return value; > } > > -#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) > +#define CONSUME(val, ptr) ((val) = consume(sizeof(val), (ptr))) CONSUME is used exactly once. It'd be likely be simpler to remove it and expand it in-place instead. The static inline __u32 consume() function is also used once and might as well be expanded in-place too. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Subject: [PATCH 1/2] staging : Comedi : comedi_fops : Fixed the return error, code
>From 9ea09e194d6ccdd0b229b408df1c86b43b1fdd7d Mon Sep 17 00:00:00 2001 From: santhosh pai Date: Mon, 22 Jun 2015 23:26:33 +0530 Subject: [PATCH 2/2] staging : Comedi : comedi_fops : Fixed the return error code try_module_get fails when the reference count of the module is not allowed to be incremented ,and hence -ENXIO is returned indicating no device or address. Signed-off-by: santhosh pai --- drivers/staging/comedi/comedi_fops.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c index d6a37e9..1ab443c 100644 --- a/drivers/staging/comedi/comedi_fops.c +++ b/drivers/staging/comedi/comedi_fops.c @@ -2606,7 +2606,7 @@ static int comedi_open(struct inode *inode, struct file *file) } if (dev->attached && dev->use_count == 0) { if (!try_module_get(dev->driver->module)) { - rc = -EPERM; + rc = -ENXIO; goto out; } if (dev->open) { -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: lustre: obdclass: fix macro coding style issue in uuid.c
This patch fixes the checkpatch.pl error: ERROR: Macros with complex values should be enclosed in parentheses +#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) by expanding it as this macro is used only once. Signed-off-by: Vasiliy Korchagin --- Notes: Here is another version with macro expansion. Inline function expansion doesn't seem like a good idea to me as it would make things overcomplicated. drivers/staging/lustre/lustre/obdclass/uuid.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/staging/lustre/lustre/obdclass/uuid.c b/drivers/staging/lustre/lustre/obdclass/uuid.c index ff0a01b..d121c5c 100644 --- a/drivers/staging/lustre/lustre/obdclass/uuid.c +++ b/drivers/staging/lustre/lustre/obdclass/uuid.c @@ -55,8 +55,6 @@ static inline __u32 consume(int nob, __u8 **ptr) return value; } -#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) - static void uuid_unpack(class_uuid_t in, __u16 *uu, int nr) { __u8 *ptr = in; @@ -64,7 +62,7 @@ static void uuid_unpack(class_uuid_t in, __u16 *uu, int nr) LASSERT(nr * sizeof(*uu) == sizeof(class_uuid_t)); while (nr-- > 0) - CONSUME(uu[nr], &ptr); + uu[nr] = consume(sizeof(uu[nr]), &ptr); } void class_uuid_unparse(class_uuid_t uu, struct obd_uuid *out) -- 2.4.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: obdclass: fix macro coding style issue in uuid.c
On Sat, 2015-06-27 at 06:36 +0100, Vasiliy Korchagin wrote: > This patch fixes the checkpatch.pl error: > > ERROR: Macros with complex values should be enclosed in parentheses > +#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) > > by expanding it as this macro is used only once. [] > Notes: > Here is another version with macro expansion. Inline function expansion > doesn't > seem like a good idea to me as it would make things overcomplicated. It looks like it'd be simpler to use vsprintf extension %pU --- drivers/staging/lustre/lustre/obdclass/uuid.c | 34 +-- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/drivers/staging/lustre/lustre/obdclass/uuid.c b/drivers/staging/lustre/lustre/obdclass/uuid.c index ff0a01b..b0b0157 100644 --- a/drivers/staging/lustre/lustre/obdclass/uuid.c +++ b/drivers/staging/lustre/lustre/obdclass/uuid.c @@ -43,40 +43,8 @@ #include "../include/obd_support.h" #include "../include/obd_class.h" - -static inline __u32 consume(int nob, __u8 **ptr) -{ - __u32 value; - - LASSERT(nob <= sizeof(value)); - - for (value = 0; nob > 0; --nob) - value = (value << 8) | *((*ptr)++); - return value; -} - -#define CONSUME(val, ptr) (val) = consume(sizeof(val), (ptr)) - -static void uuid_unpack(class_uuid_t in, __u16 *uu, int nr) -{ - __u8 *ptr = in; - - LASSERT(nr * sizeof(*uu) == sizeof(class_uuid_t)); - - while (nr-- > 0) - CONSUME(uu[nr], &ptr); -} - void class_uuid_unparse(class_uuid_t uu, struct obd_uuid *out) { - /* uu as an array of __u16's */ - __u16 uuid[sizeof(class_uuid_t) / sizeof(__u16)]; - - CLASSERT(ARRAY_SIZE(uuid) == 8); - - uuid_unpack(uu, uuid, ARRAY_SIZE(uuid)); - sprintf(out->uuid, "%04x%04x-%04x-%04x-%04x-%04x%04x%04x", - uuid[0], uuid[1], uuid[2], uuid[3], - uuid[4], uuid[5], uuid[6], uuid[7]); + sprintf(out->uuid, "%pU", uu); } EXPORT_SYMBOL(class_uuid_unparse); ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel