Re: [PATCH] staging: media: atomisp: compress return logic

Hi Arushi, [auto build test ERROR on next-20170323] [cannot apply to linuxtv-media/master v4.9-rc8 v4.9-rc7 v4.9-rc6 v4.11-rc4] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url:

[PATCH 2/3] staging: ks7010: invert conditional, reduce indentation

Bulk of function is guarded by an if statement. If we invert the conditional and return, the subsequent code can be indented one level less. Invert if statement conditional. '>=' to '<'. Jump to goto label if new conditional evaluates to true. Do not change program logic. Signed-off-by: Tobin C.

[PATCH 3/3] staging: ks7010: fix whitespace issues

Code is able to have white space refactoring after previous patch reduced the level of indentation. Doing so fixes two checkpatch multiple line dereference. Concatenate multiple line function calls when doing so will not take character count over 80 characters. Remove multiple line dereference

[PATCH 0/3] staging: ks7010: refactor ks_sdio_interrupt()

Checkpatch emits several warnings when parsing the function ks_sdio_interrupt(). Function lends itself to being refactored. Patch 01 renames identifier 'retval' to 'ret' in line with the rest of the driver. Patch 02 inverts if statement conditional and reduces the level of indentation in

[PATCH 1/3] staging: ks7010: rename identifier retval to ret

Driver predominately uses the identifier 'ret' for the error return code. Function uses 'retval' for this task. It is cleaner if we use a single name for a single task. Rename 'retval' to 'ret'. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks7010_sdio.c | 25

RE: [PATCH] netvsc: fix dereference before null check errors

> -Original Message- > From: Colin King [mailto:colin.k...@canonical.com] > Sent: Saturday, March 25, 2017 10:27 AM > To: KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger ; > de...@linuxdriverproject.org;

RE: [PATCH] netvsc: fix unititialized return value in variable ret

> -Original Message- > From: Colin King [mailto:colin.k...@canonical.com] > Sent: Saturday, March 25, 2017 10:17 AM > To: KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger ; > de...@linuxdriverproject.org;

Re: [greybus-dev] [PATCH 0/3] Add the Generic Netlink controller

On 03/26/2017 12:58 PM, Alexandre Bailon wrote: Currently, only the es2 hd controller is supported, which restrict the usage of greybus to few products. This series intents to add a support of new hd controllers. The driver doesn't support any hardware. Actually, the controller is just a

[PATCH 12/12] staging: ks7010: add intermediate variable

Function contains two sites where arithmetic expressions are used as function argument. Readability would be increased if these were factored out to a variable assignment then the variable used as function argument. Add variable and assign to it the result of arithmetic expression. Replace

[PATCH 11/12] staging: ks7010: clean up comments

Some function comments are unnecessary. There is some commented out code, it should be removed. Some comments say what the code is doing (instead of why), these comments are unnecessary. Clean up comments. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks_hostif.c |

[PATCH 09/12] staging: ks7010: replace magic number 6

Function uses magic number 6. Number refers to the number of octets in an 802.11 MAC ethernet address and as such, already has a global constant defined ETH_ALEN (header is already included). include/uapi/linux/if_ether.h define ETH_ALEN6 /* Octets in one ethernet addr */

[PATCH 10/12] staging: ks7010: replace magic number 12

Function uses magic number 12. Number is used for a block of two 802.11 MAC ethernet addresses. There is already a global constant ETH_ALEN defined to the number of octets in an 802.11 MAC ethernet address. The header file containing this definition is already included.

[PATCH 07/12] staging: ks7010: move null check before dereference

Smatch emits warn: variable dereferenced before check 'skb'. This is not strictly a bug because the null check is not for error checking but rather for a decision branch later in the code. However it is still cleaner to have the code that does the null check to be placed before the dereference.

[PATCH 03/12] staging: ks7010: add item to TODO file

Driver uses custom Michael MIC implementation. There is already an implementation within the kernel (net/mac80211). There is one other driver already using the kernel implementation (drivers/net/wireless/intersil/orinoco). The switch to the kernel implementation is, however, non-trivial. orinoco

[PATCH 02/12] staging: ks7010: rename RecvMIC to recv_mic

Identifier uses camel case, standard kernel style does not use camel case. Rename buffer RecvMIC to recv_mic. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks_hostif.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git

[PATCH 00/12] staging: ks7010: refactor hostif_data_request()

Checkpatch emits various warnings/checks when parsing ks_hostif.c. A number of these relate to the implementation of Michael MIC. This patch set cleans up code around the function calls using Michael MIC, including the whole of the function hostif_data_request(). Driver uses a custom

[PATCH 06/12] staging: ks7010: fix checkpatch MULTILINE_DEREFERENCE

Checkpatch emits WARNING: Avoid multiple line dereference. Fix up layout of function call, move dereference to single line. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks_hostif.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git

[PATCH 05/12] staging: ks7010: fix checkpatch LOGICAL_CONTINUATIONS

Checkpatch emits multiple CHECK: Logical continuations should be on the previous line. In one instance, logical continuation involves three key length checks, these can be assigned to a suitably named variable to make the code more readable. Add variable, separate similar logical continuations

[PATCH 08/12] staging: ks7010: replace magic number 8

Function uses magic number 8. Number refers to the micael mic length. It would be better to define and use a global constant. Other driver code in the kernel uses the name MICHAEL_MIC_LEN for the same task. Define global constant MICHAEL_MIC_LEN to be 8. Replace magic number 8 with newly defined

[PATCH 01/12] staging: ks7010: simplify complex logical statment

If statement conditional chains multiple logical AND's and OR's together making the code overly complex to read. The same condition appears in each and every OR'ed statement. This logic can be more easily represented by simply checking the value of this condition initially and returning if it

[PATCH 04/12] staging: ks7010: rename identifier packet to skb

Of the 8088 instances of 'struct sk_buff *' within net/ 6670 are named 'skb'. This is a strong indication that the identifier 'sbk' is better suited when naming sk_buff pointer variables than 'packet'. Rename identifier 'packet' to 'skb'. Signed-off-by: Tobin C. Harding ---

Re: [PATCH v2] staging: media: omap4iss: Replace a bit shift by a use of BIT.

Em Sun, 26 Mar 2017 20:57:02 +0300 Laurent Pinchart escreveu: > Hi Arushi, > > Thank you for the patch. > > On Friday 24 Mar 2017 21:31:45 Arushi Singhal wrote: > > This patch replaces bit shifting on 1 with the BIT(x) macro. > > This was done with

Re: [PATCH v2] staging: media: omap4iss: Replace a bit shift by a use of BIT.

Hi Arushi, Thank you for the patch. On Friday 24 Mar 2017 21:31:45 Arushi Singhal wrote: > This patch replaces bit shifting on 1 with the BIT(x) macro. > This was done with coccinelle: > @@ > constant c; > @@ > > -1 << c > +BIT(c) > > Signed-off-by: Arushi Singhal

[PATCH 0/3] Add the Generic Netlink controller

Currently, only the es2 hd controller is supported, which restrict the usage of greybus to few products. This series intents to add a support of new hd controllers. The driver doesn't support any hardware. Actually, the controller is just a bridge between Greybus kernel and userspace. The real

[greybus-dev][PATCH 1/3] staging: greybus: make cport_quiesce() method optional

The cport_quiesce() method is mandatory in the case of the es2 Greybus hd controller to shutdown the cports on the es2 controller. In order to add support of another controller which may not need to shutdown its cports, make the cport_quiesce() optional, and check if the controller implement it

[greybus-dev][PATCH 3/3] staging: greybus: netlink: Add a way to "hot remove" SVC

Currently, there is no way to remove the SVC. It was fine because the SVC was tied the es2 controller, and so removing the es2 controller device was removing the SVC. But the case of netlink, the SVC is emulated in userspace and it may be stopped or restarted. But because on the next run, it won't

[greybus-dev][PATCH 2/3] staging: greybus: Add Greybus netlink driver

Currently, the only hd controller supported by Greybus is the es2 controller which only support is mainly a bridge between USB and UniPro. In order to use Greybus on devices that do not support UniPro, add a the Greybus netlink hd controller. By using Generic Netlink, userspace can act as a

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

Hi Hans, On Tuesday 14 Mar 2017 08:55:36 Hans Verkuil wrote: > On 03/14/2017 04:45 AM, Mauro Carvalho Chehab wrote: > > Hi Sakari, > > > > I started preparing a long argument about it, but gave up in favor of a > > simpler one. > > > > Em Mon, 13 Mar 2017 14:46:22 +0200 Sakari Ailus escreveu: >

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

Hi Pavel, On Tuesday 14 Mar 2017 19:26:35 Pavel Machek wrote: > Hi! > > >> Mid-layer is difficult... there are _hundreds_ of possible > >> pipeline setups. If it should live in kernel or in userspace is a > >> question... but I don't think having it in kernel helps in any way. > > > > Mid-layer

Re: [Please ignore this is a test] pci-hyperv: properly handle pci bus remove

-remove/20170326-180444 base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next config: x86_64-randconfig-x016-201713 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=x86_64

[PATCH 0/5] staging: ks7010: refactor, fix checkpatch

File ks_wlan_net.c emits various warnings/checks when parsed with checkpatch. An inaccessible execution patch also exists within the file, this was found while preparing for code cleanup. Patch 01 removes the inaccessible execution path code. Patch 02 moves a non-null pointer check to be before

[PATCH 2/5] staging: ks7010: move null check before dereference

Pointer is dereferenced before the null check. Pointer variable is set (using a cast) to point to a function argument, the pointer is then dereferenced before it is checked for non-null. Move null check to before the dereference. Signed-off-by: Tobin C. Harding ---

[PATCH 3/5] staging: ks7010: fix parameter placement

Checkpatch emits multiple WARNING: Avoid multiple line dereference. Patch to fix these warnings also illuminates another function call with smelly code layout, we should fix this too so as to ease review. Move dereference to single line, do this 4 times, each fixing a checkpatch warning. Remove

[PATCH 4/5] staging: ks7010: fix checkpatch PARENTHESIS_ALIGNMENT

Checkpatch emits CHECK: Alignment should match open parenthesis. Align argument to open parenthesis. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks_wlan_net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/ks7010/ks_wlan_net.c

[PATCH 5/5] staging: ks7010: fix checkpatch UNNECESSARY_ELSE

Checkpatch emits WARNING: else is not generally useful after a break or return. Two warnings of this type are emitted, both are correct, 'else' statements are unnecessary. Remove unnecessary 'else' statements, reduce indentation in subsequent code. Signed-off-by: Tobin C. Harding

[PATCH 1/5] staging: ks7010: remove unused execution path

Multi-way decision contains two branches that can never be executed. Statement starts with a convoluted variable assignment ('enabled') then branches on the value of this variable. Variable can only take values 0 or 1, this variable is unnecessary. Remove unnecessary variable. Remove two branches

script to setup pipeline was Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

Hi! > > I do agree with you that MC places a lot of burden on the user to > > attain a lot of knowledge of the system's architecture. > > Setting up the pipeline is not the hard part. One could write a > script to do that. Can you try to write that script? I believe it would solve big part of

[PATCH 3/3] staging: ks7010: fix checkpatch PARENTHESIS_ALIGNMENT

Checkpatch emits CHECK: Alignment should match open parenthesis. Align function arguments correctly. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks7010_sdio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c

[PATCH 2/3] staging: ks7010: rename identifier rc to ret

Function uses identifier 'rc' for the error return code to a function call. Other places in the driver use 'ret' because it is more common within the kernel. Local variable 'ret' does not need to be defined to zero when it is declared. Function call site using 'ret' needs refactoring, checkpatch

[PATCH 0/3] staging: ks7010: refactor tx_device_task()

Function tx_device_task() is a candidate for refactoring. Checkpatch emits a number of warnings/checks for this function. Patch 01 inverts if statement conditional and reduces the level of nesting. Patch 02 renames rc -> ret, fixes function call and checkpatch WARNING. Patch 03 fixes function

[PATCH 1/3] staging: ks7010: invert conditional, reduce indentation

Code is unnecessarily nested. Function body is guarded by an if statement, we can invert the if statement conditional and return, subsequent code can have the indentation reduced. Invert conditional, change '&&' to '||' , '>' to '<=' and '!=' to '=='. Return if new conditional evaluates to true.