[PATCH v3] Revert "staging: radio-bcm2048: fixed bare use of unsigned int"
This reverts previous changes to checkpatch warning: WARNING: Prefer 'unsigned int' to bare use of 'unsigned' --- Changes in v2: - Added changelog Changes in v3: - Revert changes to using bare unsigned drivers/staging/media/bcm2048/radio-bcm2048.c | 44 +-- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c b/drivers/staging/media/bcm2048/radio-bcm2048.c index 7d33bce..d605c41 100644 --- a/drivers/staging/media/bcm2048/radio-bcm2048.c +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c @@ -2020,27 +2020,27 @@ static irqreturn_t bcm2048_handler(int irq, void *dev) return count; \ } -DEFINE_SYSFS_PROPERTY(power_state, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(mute, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(audio_route, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(dac_output, unsigned int, int, "%u", 0) - -DEFINE_SYSFS_PROPERTY(fm_hi_lo_injection, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(fm_frequency, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(fm_af_frequency, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(fm_deemphasis, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(fm_rds_mask, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(fm_best_tune_mode, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(fm_search_rssi_threshold, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(fm_search_mode_direction, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(fm_search_tune_mode, unsigned int, int, "%u", value > 3) - -DEFINE_SYSFS_PROPERTY(rds, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(rds_b_block_mask, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(rds_b_block_match, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(rds_pi_mask, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(rds_pi_match, unsigned int, int, "%u", 0) -DEFINE_SYSFS_PROPERTY(rds_wline, unsigned int, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(power_state, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(mute, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(audio_route, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(dac_output, unsigned, int, "%u", 0) + +DEFINE_SYSFS_PROPERTY(fm_hi_lo_injection, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(fm_frequency, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(fm_af_frequency, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(fm_deemphasis, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(fm_rds_mask, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(fm_best_tune_mode, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(fm_search_rssi_threshold, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(fm_search_mode_direction, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(fm_search_tune_mode, unsigned, int, "%u", value > 3) + +DEFINE_SYSFS_PROPERTY(rds, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(rds_b_block_mask, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(rds_b_block_match, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(rds_pi_mask, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(rds_pi_match, unsigned, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(rds_wline, unsigned, int, "%u", 0) property_read(rds_pi, unsigned int, "%x") property_str_read(rds_rt, (BCM2048_MAX_RDS_RT + 1)) property_str_read(rds_ps, (BCM2048_MAX_RDS_PS + 1)) @@ -2052,7 +2052,7 @@ static irqreturn_t bcm2048_handler(int irq, void *dev) property_read(region_top_frequency, unsigned int, "%u") property_signed_read(fm_carrier_error, int, "%d") property_signed_read(fm_rssi, int, "%d") -DEFINE_SYSFS_PROPERTY(region, unsigned int, int, "%u", 0) +DEFINE_SYSFS_PROPERTY(region, unsigned, int, "%u", 0) static struct device_attribute attrs[] = { __ATTR(power_state, 0644, bcm2048_power_state_read, -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: radio-bcm2048: fixed bare use of unsigned int
On Thu, 23 Mar 2017 14:31:53 +0100 Greg Kroah-Hartman wrote: > On Wed, Mar 22, 2017 at 01:33:39PM +1100, Eddie Youseph wrote: > > Fixed checkpatch WARNING: Prefer 'unsigned int' to bare use of 'unsigned' > > > > Signed-off-by: Eddie Youseph > > --- > > Changes in v2: > > - Added changelog > > Did you actually build this change? > > Please do so... > > thanks, > > greg k-h I recompiled and was faced with many errors. I will need to revert the changes. I "wasn't getting errors" the first time around because I forgot I did a "make oldconfig" before, and bcm2048 wasn't being included in the build. regards, eddie youseph ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
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: https://github.com/0day-ci/linux/commits/Arushi-Singhal/staging-media-atomisp-compress-return-logic/20170327-030611 config: x86_64-allmodconfig (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 All errors (new ones prefixed by >>): drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/bufq/src/bufq.c: In function 'ia_css_bufq_dequeue_isys_event': >> drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/bufq/src/bufq.c:467:18: >> error: unused variable 'return_err' [-Werror=unused-variable] enum ia_css_err return_err; ^~ cc1: all warnings being treated as errors vim +/return_err +467 drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/bufq/src/bufq.c a49d2536 Alan Cox 2017-02-17 461 } a49d2536 Alan Cox 2017-02-17 462 a49d2536 Alan Cox 2017-02-17 463 enum ia_css_err ia_css_bufq_dequeue_isys_event( a49d2536 Alan Cox 2017-02-17 464 uint8_t item[BUFQ_EVENT_SIZE]) a49d2536 Alan Cox 2017-02-17 465 { a49d2536 Alan Cox 2017-02-17 466 #if !defined(HAS_NO_INPUT_SYSTEM) a49d2536 Alan Cox 2017-02-17 @467 enum ia_css_err return_err; a49d2536 Alan Cox 2017-02-17 468 int error = 0; a49d2536 Alan Cox 2017-02-17 469 ia_css_queue_t *q; a49d2536 Alan Cox 2017-02-17 470 :: The code at line 467 was first introduced by commit :: a49d25364dfb9f8a64037488a39ab1f56c5fa419 staging/atomisp: Add support for the Intel IPU v2 :: TO: Alan Cox :: CC: Greg Kroah-Hartman --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 even though doing so introduces two new checkpatch line over 80 warnings, resulting code is cleaner. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks7010_sdio.c | 20 +++- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index bbee02c..ab00e77 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -569,10 +569,8 @@ static void ks_sdio_interrupt(struct sdio_func *func) } /* DPRINTK(1, "GCR_B=%02X\n", rw_data); */ if (rw_data == GCR_B_ACTIVE) { - if (atomic_read(&priv->psstatus.status) == - PS_SNOOZE) { - atomic_set(&priv->psstatus.status, - PS_WAKEUP); + if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) { + atomic_set(&priv->psstatus.status, PS_WAKEUP); priv->wakeup_count = 0; } complete(&priv->psstatus.wakeup_wait); @@ -590,19 +588,15 @@ static void ks_sdio_interrupt(struct sdio_func *func) } DPRINTK(4, "WSTATUS_RSIZE=%02X\n", rw_data); rsize = rw_data & RSIZE_MASK; - if (rsize != 0) { /* Read schedule */ - ks_wlan_hw_rx((void *)priv, - (uint16_t)(rsize << 4)); - } + if (rsize != 0) /* Read schedule */ + ks_wlan_hw_rx((void *)priv, (uint16_t)(rsize << 4)); + if (rw_data & WSTATUS_MASK) { if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) { if (cnt_txqbody(priv)) { ks_wlan_hw_wakeup_request(priv); - queue_delayed_work - (priv->ks_wlan_hw. - ks7010sdio_wq, - &priv->ks_wlan_hw. - rw_wq, 1); + queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq, + &priv->ks_wlan_hw.rw_wq, 1); return; } } else { -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 subsequent code by one level. Patch 03 fixes white space issues, clearing a couple of checkpatch warnings and trivial style issues. Code is build-tested only (x86_64 and PowerPC). Tobin C. Harding (3): staging: ks7010: rename identifier retval to ret staging: ks7010: invert conditional, reduce indentation staging: ks7010: fix whitespace issues drivers/staging/ks7010/ks7010_sdio.c | 118 --- 1 file changed, 55 insertions(+), 63 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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. Harding --- drivers/staging/ks7010/ks7010_sdio.c | 117 ++- 1 file changed, 59 insertions(+), 58 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index 08e89b3..bbee02c 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -543,72 +543,73 @@ static void ks_sdio_interrupt(struct sdio_func *func) priv = card->priv; DPRINTK(4, "\n"); - if (priv->dev_state >= DEVICE_STATE_BOOT) { - ret = ks7010_sdio_read(priv, INT_PENDING, &status, - sizeof(status)); + if (priv->dev_state < DEVICE_STATE_BOOT) + goto queue_delayed_work; + + ret = ks7010_sdio_read(priv, INT_PENDING, &status, + sizeof(status)); + if (ret) { + DPRINTK(1, "read INT_PENDING Failed!!(%d)\n", ret); + goto queue_delayed_work; + } + DPRINTK(4, "INT_PENDING=%02X\n", rw_data); + + /* schedule task for interrupt status */ + /* bit7 -> Write General Communication B register */ + /* read (General Communication B register) */ + /* bit5 -> Write Status Idle */ + /* bit2 -> Read Status Busy */ + if (status & INT_GCR_B || + atomic_read(&priv->psstatus.status) == PS_SNOOZE) { + ret = ks7010_sdio_read(priv, GCR_B, &rw_data, + sizeof(rw_data)); if (ret) { - DPRINTK(1, "read INT_PENDING Failed!!(%d)\n", ret); + DPRINTK(1, " error : GCR_B=%02X\n", rw_data); goto queue_delayed_work; } - DPRINTK(4, "INT_PENDING=%02X\n", rw_data); - - /* schedule task for interrupt status */ - /* bit7 -> Write General Communication B register */ - /* read (General Communication B register) */ - /* bit5 -> Write Status Idle */ - /* bit2 -> Read Status Busy */ - if (status & INT_GCR_B || - atomic_read(&priv->psstatus.status) == PS_SNOOZE) { - ret = ks7010_sdio_read(priv, GCR_B, &rw_data, - sizeof(rw_data)); - if (ret) { - DPRINTK(1, " error : GCR_B=%02X\n", rw_data); - goto queue_delayed_work; - } - /* DPRINTK(1, "GCR_B=%02X\n", rw_data); */ - if (rw_data == GCR_B_ACTIVE) { - if (atomic_read(&priv->psstatus.status) == - PS_SNOOZE) { - atomic_set(&priv->psstatus.status, - PS_WAKEUP); - priv->wakeup_count = 0; - } - complete(&priv->psstatus.wakeup_wait); + /* DPRINTK(1, "GCR_B=%02X\n", rw_data); */ + if (rw_data == GCR_B_ACTIVE) { + if (atomic_read(&priv->psstatus.status) == + PS_SNOOZE) { + atomic_set(&priv->psstatus.status, + PS_WAKEUP); + priv->wakeup_count = 0; } + complete(&priv->psstatus.wakeup_wait); } + } - do { - /* read (WriteStatus/ReadDataSize FN1:00_0014) */ - ret = ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data, - sizeof(rw_data)); - if (ret) { - DPRINTK(1, " error : WSTATUS_RSIZE=%02X\n", - rw_data); - goto queue_delayed_work; - } - DPRINTK(4, "WSTATUS_RSIZE=%02X\n", rw_data); - rsize = rw_data & RSIZE_MASK; - if (rsize != 0) { /* Read schedule */ - ks_wlan_hw_rx((void *)priv, - (uint16_t)(rsize << 4)); - } - if (rw_data & WSTATUS_MASK) { - if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) { - if (cnt_txqbody(priv)) { - ks_wlan_
[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 +++-- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index b16618b..08e89b3 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -534,7 +534,7 @@ static void ks7010_rw_function(struct work_struct *work) static void ks_sdio_interrupt(struct sdio_func *func) { - int retval; + int ret; struct ks_sdio_card *card; struct ks_wlan_private *priv; unsigned char status, rsize, rw_data; @@ -544,11 +544,10 @@ static void ks_sdio_interrupt(struct sdio_func *func) DPRINTK(4, "\n"); if (priv->dev_state >= DEVICE_STATE_BOOT) { - retval = - ks7010_sdio_read(priv, INT_PENDING, &status, -sizeof(status)); - if (retval) { - DPRINTK(1, "read INT_PENDING Failed!!(%d)\n", retval); + ret = ks7010_sdio_read(priv, INT_PENDING, &status, + sizeof(status)); + if (ret) { + DPRINTK(1, "read INT_PENDING Failed!!(%d)\n", ret); goto queue_delayed_work; } DPRINTK(4, "INT_PENDING=%02X\n", rw_data); @@ -560,10 +559,9 @@ static void ks_sdio_interrupt(struct sdio_func *func) /* bit2 -> Read Status Busy */ if (status & INT_GCR_B || atomic_read(&priv->psstatus.status) == PS_SNOOZE) { - retval = - ks7010_sdio_read(priv, GCR_B, &rw_data, -sizeof(rw_data)); - if (retval) { + ret = ks7010_sdio_read(priv, GCR_B, &rw_data, + sizeof(rw_data)); + if (ret) { DPRINTK(1, " error : GCR_B=%02X\n", rw_data); goto queue_delayed_work; } @@ -581,10 +579,9 @@ static void ks_sdio_interrupt(struct sdio_func *func) do { /* read (WriteStatus/ReadDataSize FN1:00_0014) */ - retval = - ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data, -sizeof(rw_data)); - if (retval) { + ret = ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data, + sizeof(rw_data)); + if (ret) { DPRINTK(1, " error : WSTATUS_RSIZE=%02X\n", rw_data); goto queue_delayed_work; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
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; net...@vger.kernel.org > Cc: kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org > Subject: [PATCH] netvsc: fix dereference before null check errors > > From: Colin Ian King > > ndev is being checked to see if it is a null pointer however before > the null check ndev is being dereferenced; hence there is a potential > null pointer dereference bug that needs fixing. Fix this by only > dereferencing ndev after the null check. > > Detected by CoverityScan, CID#1420760, CID#140761 ("Dereference > before null check") > > Signed-off-by: Colin Ian King Reviewed-by: Haiyang Zhang Thank you! ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
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; net...@vger.kernel.org > Cc: kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org > Subject: [PATCH] netvsc: fix unititialized return value in variable ret > > From: Colin Ian King > > It is possible for an uninitialized value of ret to be returned > so fix this by initializing ret to zero. > > Detected by CoverityScan, CID#1420762 ("Uninitialized scalar variable") > > Fixes: 163891d7d429 ("netvsc: handle offline mtu and channel change") > Signed-off-by: Colin Ian King > --- > drivers/net/hyperv/netvsc_drv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/hyperv/netvsc_drv.c > b/drivers/net/hyperv/netvsc_drv.c > index eb7ae79d47bb..f830bbbd8ad4 100644 > --- a/drivers/net/hyperv/netvsc_drv.c > +++ b/drivers/net/hyperv/netvsc_drv.c > @@ -855,7 +855,7 @@ static int netvsc_change_mtu(struct net_device *ndev, > int mtu) > struct hv_device *hdev = ndevctx->device_ctx; > struct netvsc_device_info device_info; > bool was_running; > - int ret; > + int ret = 0; This was just fix by commit: 386f57622cc3bbb39b9c00fc8762bc9ab28e0f8d Thanks. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
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 bridge between Greybus kernel and userspace. The real driver must be implemented in userspace. This give the ability to support any kind of transport layer (such as Bluetooth, WiFi, Ethernet). The controller uses Generic Netlink. My original intent was to implement a TCP/IP controller but it was very slow, unstable. In addition, some features such as SVC or module discovery were to add to implement in kernel side. With the generic netlink controller, we can easily add support of new controller. It also helps to deal with all the project ARA legacy such as SVC. I have started to implement an application which is able to emulate the SVC, a module (like gbsim but only support control and loopback protcols), TCP/IP, Bluetooth and UART. It is still under development but it was stable enough to test this series. The application can found here: https://github.com/anobli/gbridge I'm not going to comment on the actual patch itself, but I do want to say that I'm very happy to see progress on making Gerybus generic to any transport. -- Karim Yaghmour CEO - Opersys inc. / www.opersys.com http://twitter.com/karimyaghmour ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 complex function call with simplified call using newly defined variable. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks_hostif.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index d8c2abe..dcc04d0 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -1127,6 +1127,7 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) struct ethhdr *eth; int ret; int no_key; + size_t size; if (((priv->connect_status & CONNECT_STATUS_MASK) == DISCONNECT_STATUS) || (priv->connect_status & FORCE_DISCONNECT) || @@ -1154,9 +1155,8 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) } DPRINTK(4, "skb_buff length=%d\n", skb_len); - pp = kmalloc(hif_align_size(sizeof(*pp) + ETH_ALEN + skb_len + MICHAEL_MIC_LEN), -KS_WLAN_MEM_FLAG); - + size = sizeof(*pp) + ETH_ALEN + skb_len + MICHAEL_MIC_LEN; + pp = kmalloc(hif_align_size(size), KS_WLAN_MEM_FLAG); if (!pp) { DPRINTK(3, "allocate memory failed..\n"); ret = -ENOMEM; @@ -1255,9 +1255,8 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) } /* header value set */ - pp->header.size = - cpu_to_le16((uint16_t) - (sizeof(*pp) - sizeof(pp->header.size) + skb_len)); + size = sizeof(*pp) - sizeof(pp->header.size) + skb_len; + pp->header.size = cpu_to_le16((uint16_t)size); pp->header.event = cpu_to_le16((uint16_t)HIF_DATA_REQ); /* tx request */ -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index f1ce049..d8c2abe 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -1147,8 +1147,8 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) goto err_kfree_skb; } - /* for PowerSave */ - if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) { /* power save wakeup */ + /* power save wakeup */ + if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) { if (!netif_queue_stopped(priv->net_dev)) netif_stop_queue(priv->net_dev); } @@ -1178,15 +1178,14 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) } /* MAC address copy */ - memcpy(p, buffer, ETH_ALEN * 2);/* DST/SRC MAC address */ + memcpy(p, buffer, ETH_ALEN * 2); p += ETH_ALEN * 2; buffer += ETH_ALEN * 2; length -= ETH_ALEN * 2; + /* EtherType/Length check */ if (*(buffer + 1) + (*buffer << MICHAEL_MIC_LEN) > 1500) { - /* ProtocolEAP = *(buffer+1) + (*buffer << 8); */ - /* DPRINTK(2, "Send [SNAP]Type %x\n",ProtocolEAP); */ - /* SAP/CTL/OUI(6 byte) add */ + /* SAP/CTL/OUI (6 bytes) add */ *p++ = 0xAA;/* DSAP */ *p++ = 0xAA;/* SSAP */ *p++ = 0x03;/* CTL */ @@ -1196,7 +1195,6 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) skb_len += 6; } else { DPRINTK(4, "DIX\n"); - /* Length(2 byte) delete */ buffer += 2; length -= 2; skb_len -= 2; @@ -1227,7 +1225,7 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) if (priv->wpa.rsn_enabled && priv->wpa.key[0].key_len) { if (eth_proto == ETHER_PROTOCOL_TYPE_EAP && no_key) { - pp->auth_type = cpu_to_le16((uint16_t)TYPE_AUTH); /* no encryption */ + pp->auth_type = cpu_to_le16((uint16_t)TYPE_AUTH); } else { if (priv->wpa.pairwise_suite == IW_AUTH_CIPHER_TKIP) { MichaelMICFunction(&michael_mic, -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 */ Replace magic number 6 with ETH_ALEN. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks_hostif.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 0455cda..18c006d 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -1154,7 +1154,7 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) } DPRINTK(4, "skb_buff length=%d\n", skb_len); - pp = kmalloc(hif_align_size(sizeof(*pp) + 6 + skb_len + MICHAEL_MIC_LEN), + pp = kmalloc(hif_align_size(sizeof(*pp) + ETH_ALEN + skb_len + MICHAEL_MIC_LEN), KS_WLAN_MEM_FLAG); if (!pp) { -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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. include/uapi/linux/if_ether.h define ETH_ALEN 6 /* Octets in one ethernet addr */ Replace magic number 12 with ETH_ALEN * 2. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks_hostif.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 18c006d..f1ce049 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -1178,10 +1178,10 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) } /* MAC address copy */ - memcpy(p, buffer, 12); /* DST/SRC MAC address */ - p += 12; - buffer += 12; - length -= 12; + memcpy(p, buffer, ETH_ALEN * 2);/* DST/SRC MAC address */ + p += ETH_ALEN * 2; + buffer += ETH_ALEN * 2; + length -= ETH_ALEN * 2; /* EtherType/Length check */ if (*(buffer + 1) + (*buffer << MICHAEL_MIC_LEN) > 1500) { /* ProtocolEAP = *(buffer+1) + (*buffer << 8); */ -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index d9f3a0d..690fa01 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -312,7 +312,7 @@ int hostif_data_indication_wpa(struct ks_wlan_private *priv, { struct ether_hdr *eth_hdr; unsigned short eth_proto; - unsigned char RecvMIC[8]; + unsigned char recv_mic[8]; char buf[128]; unsigned long now; struct mic_failure_t *mic_failure; @@ -345,7 +345,7 @@ int hostif_data_indication_wpa(struct ks_wlan_private *priv, DPRINTK(4, "TKIP: protocol=%04X: size=%u\n", eth_proto, priv->rx_size); /* MIC save */ - memcpy(&RecvMIC[0], (priv->rxp) + ((priv->rx_size) - 8), 8); + memcpy(&recv_mic[0], (priv->rxp) + ((priv->rx_size) - 8), 8); priv->rx_size = priv->rx_size - 8; if (auth_type > 0 && auth_type < 4) { /* auth_type check */ MichaelMICFunction(&michael_mic, @@ -355,7 +355,7 @@ int hostif_data_indication_wpa(struct ks_wlan_private *priv, (uint8_t)0, /* priority */ (uint8_t *)michael_mic.Result); } - if (memcmp(michael_mic.Result, RecvMIC, 8) != 0) { + if (memcmp(michael_mic.Result, recv_mic, 8) != 0) { now = jiffies; mic_failure = &priv->wpa.mic_failure; /* MIC FAILURE */ -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 implementation of the Miachale MIC algorithm. The kernel already has an implementation, eventually we should be using the kernel implementation. As a first step, let's clean up the code around the calls to Michael MIC so that it is easier to switch implementations later on. Patch 01 simplifies a complex logical statement. Patch 02 removes one [unnecessary] instance of camel case without touching the Michael MIC implementation. Patch 03 adds an item to the TODO file. Patch 04 renames an identifier to be more typical of the kernel. The following patches clean up hostif_data_request(). Patch 05 fixes logical continuation warnings. Patch 06 fixes multiple line dereference warnings. Patch 07 fixes a Smatch warning, null check after dereference. Patches 08 through 10 replace magic numbers. Patch 11 cleans up comments. Patch 12 adds an intermediate variable to replace arithmetic expressions as function argument. Code is build tested only (x86_64 and PowerPC). Tobin C. Harding (12): staging: ks7010: simplify complex logical statment staging: ks7010: rename RecvMIC to recv_mic staging: ks7010: add item to TODO file staging: ks7010: rename identifier packet to skb staging: ks7010: fix checkpatch LOGICAL_CONTINUATIONS staging: ks7010: fix checkpatch MULTILINE_DEREFERENCE staging: ks7010: move null check before dereference staging: ks7010: replace magic number 8 staging: ks7010: replace magic number 6 staging: ks7010: replace magic number 12 staging: ks7010: clean up comments staging: ks7010: add intermediate variable drivers/staging/ks7010/TODO | 2 + drivers/staging/ks7010/ks_hostif.c | 142 ++- drivers/staging/ks7010/ks_hostif.h | 2 +- drivers/staging/ks7010/michael_mic.h | 2 + 4 files changed, 78 insertions(+), 70 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 29c4928..2917dda 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -1230,9 +1230,12 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) pp->auth_type = cpu_to_le16((uint16_t)TYPE_AUTH); /* no encryption */ } else { if (priv->wpa.pairwise_suite == IW_AUTH_CIPHER_TKIP) { - MichaelMICFunction(&michael_mic, (uint8_t *)priv->wpa.key[0].tx_mic_key, (uint8_t *)&pp->data[0], (int)skb_len, (uint8_t)0, /* priority */ - (uint8_t *)michael_mic. - Result); + MichaelMICFunction(&michael_mic, + (uint8_t *)priv->wpa.key[0].tx_mic_key, + (uint8_t *)&pp->data[0], + (int)skb_len, + (uint8_t)0, /* priority */ + (uint8_t *)michael_mic.Result); memcpy(p, michael_mic.Result, 8); length += 8; skb_len += 8; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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. Move null check code to be before dereference. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks_hostif.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 2917dda..6fc8360 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -1128,13 +1128,6 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) int ret; int no_key; - skb_len = skb->len; - if (skb_len > ETH_FRAME_LEN) { - DPRINTK(1, "bad length skb_len=%d\n", skb_len); - ret = -EOVERFLOW; - goto err_kfree_skb; - } - if (((priv->connect_status & CONNECT_STATUS_MASK) == DISCONNECT_STATUS) || (priv->connect_status & FORCE_DISCONNECT) || priv->wpa.mic_failure.stop) { @@ -1147,6 +1140,13 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) return 0; } + skb_len = skb->len; + if (skb_len > ETH_FRAME_LEN) { + DPRINTK(1, "bad length skb_len=%d\n", skb_len); + ret = -EOVERFLOW; + goto err_kfree_skb; + } + /* for PowerSave */ if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) { /* power save wakeup */ if (!netif_queue_stopped(priv->net_dev)) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 provides custom wrappers that may also need to be implemented in ks7010 and also orinoco uses crypto_shash_*() kernel cryptography functions. Some knowledge of cryptography and wireless networking is required. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/TODO | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/ks7010/TODO b/drivers/staging/ks7010/TODO index 87a6dac..c8a42c2 100644 --- a/drivers/staging/ks7010/TODO +++ b/drivers/staging/ks7010/TODO @@ -27,6 +27,8 @@ Now the TODOs: - fix the 'card removal' event when card is inserted when booting - check what other upstream wireless mechanisms can be used instead of the custom ones here +- replace custom MichaelMIC implementation with kernel + implementation net/mac80211/michael.* (should we be using crypto_shash_*()?) Please send any patches to: Greg Kroah-Hartman -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 into a variable assignment. Move logical continuations to the end of the previous line. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks_hostif.c | 28 +++- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index dd899ad..29c4928 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -1126,6 +1126,7 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) struct wpa_eapol_key *eap_key; struct ethhdr *eth; int ret; + int no_key; skb_len = skb->len; if (skb_len > ETH_FRAME_LEN) { @@ -1134,9 +1135,9 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) goto err_kfree_skb; } - if (((priv->connect_status & CONNECT_STATUS_MASK) == DISCONNECT_STATUS) - || (priv->connect_status & FORCE_DISCONNECT) - || priv->wpa.mic_failure.stop) { + if (((priv->connect_status & CONNECT_STATUS_MASK) == DISCONNECT_STATUS) || + (priv->connect_status & FORCE_DISCONNECT) || + priv->wpa.mic_failure.stop) { DPRINTK(3, " DISCONNECT\n"); if (netif_queue_stopped(priv->net_dev)) netif_wake_queue(priv->net_dev); @@ -1211,8 +1212,8 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) eth_proto = ntohs(eth_hdr->h_proto); /* for MIC FAILURE REPORT check */ - if (eth_proto == ETHER_PROTOCOL_TYPE_EAP - && priv->wpa.mic_failure.failure > 0) { + if (eth_proto == ETHER_PROTOCOL_TYPE_EAP && + priv->wpa.mic_failure.failure > 0) { aa1x_hdr = (struct ieee802_1x_hdr *)(eth_hdr + 1); if (aa1x_hdr->type == IEEE802_1X_TYPE_EAPOL_KEY) { eap_key = (struct wpa_eapol_key *)(aa1x_hdr + 1); @@ -1220,11 +1221,12 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) } } + no_key = priv->wpa.key[1].key_len == 0 && + priv->wpa.key[2].key_len == 0 && + priv->wpa.key[3].key_len == 0; + if (priv->wpa.rsn_enabled && priv->wpa.key[0].key_len) { - if (eth_proto == ETHER_PROTOCOL_TYPE_EAP - && !(priv->wpa.key[1].key_len) - && !(priv->wpa.key[2].key_len) - && !(priv->wpa.key[3].key_len)) { + if (eth_proto == ETHER_PROTOCOL_TYPE_EAP && no_key) { pp->auth_type = cpu_to_le16((uint16_t)TYPE_AUTH); /* no encryption */ } else { if (priv->wpa.pairwise_suite == IW_AUTH_CIPHER_TKIP) { @@ -1264,10 +1266,10 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) (void *)skb); /* MIC FAILURE REPORT check */ - if (eth_proto == ETHER_PROTOCOL_TYPE_EAP - && priv->wpa.mic_failure.failure > 0) { - if (keyinfo & WPA_KEY_INFO_ERROR - && keyinfo & WPA_KEY_INFO_REQUEST) { + if (eth_proto == ETHER_PROTOCOL_TYPE_EAP && + priv->wpa.mic_failure.failure > 0) { + if (keyinfo & WPA_KEY_INFO_ERROR && + keyinfo & WPA_KEY_INFO_REQUEST) { DPRINTK(3, " MIC ERROR Report SET : %04X\n", keyinfo); hostif_sme_enqueue(priv, SME_MIC_FAILURE_REQUEST); } -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 constant. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks_hostif.c | 12 ++-- drivers/staging/ks7010/michael_mic.h | 2 ++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 6fc8360..0455cda 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -1154,7 +1154,7 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) } DPRINTK(4, "skb_buff length=%d\n", skb_len); - pp = kmalloc(hif_align_size(sizeof(*pp) + 6 + skb_len + 8), + pp = kmalloc(hif_align_size(sizeof(*pp) + 6 + skb_len + MICHAEL_MIC_LEN), KS_WLAN_MEM_FLAG); if (!pp) { @@ -1183,7 +1183,7 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) buffer += 12; length -= 12; /* EtherType/Length check */ - if (*(buffer + 1) + (*buffer << 8) > 1500) { + if (*(buffer + 1) + (*buffer << MICHAEL_MIC_LEN) > 1500) { /* ProtocolEAP = *(buffer+1) + (*buffer << 8); */ /* DPRINTK(2, "Send [SNAP]Type %x\n",ProtocolEAP); */ /* SAP/CTL/OUI(6 byte) add */ @@ -1236,10 +1236,10 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) (int)skb_len, (uint8_t)0, /* priority */ (uint8_t *)michael_mic.Result); - memcpy(p, michael_mic.Result, 8); - length += 8; - skb_len += 8; - p += 8; + memcpy(p, michael_mic.Result, MICHAEL_MIC_LEN); + length += MICHAEL_MIC_LEN; + skb_len += MICHAEL_MIC_LEN; + p += MICHAEL_MIC_LEN; pp->auth_type = cpu_to_le16((uint16_t)TYPE_DATA); diff --git a/drivers/staging/ks7010/michael_mic.h b/drivers/staging/ks7010/michael_mic.h index 758e429..16971ab 100644 --- a/drivers/staging/ks7010/michael_mic.h +++ b/drivers/staging/ks7010/michael_mic.h @@ -9,6 +9,8 @@ * published by the Free Software Foundation. */ +#define MICHAEL_MIC_LEN 8 + /* MichaelMIC routine define */ struct michael_mic_t { u32 K0; // Key -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 evaluates to false. Declare variable 'do_mic', assign truth value to 'do_mic' as it appears in original conditional statement. AND in the key length, as in original. If variable 'do_mic' is false, return from function. Do not change program logic. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks_hostif.c | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index e17ce22..d9f3a0d 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -320,6 +320,7 @@ int hostif_data_indication_wpa(struct ks_wlan_private *priv, union iwreq_data wrqu; unsigned int key_index = auth_type - 1; struct wpa_key_t *key = &priv->wpa.key[key_index]; + int do_mic; eth_hdr = (struct ether_hdr *)(priv->rxp); eth_proto = ntohs(eth_hdr->h_proto); @@ -333,13 +334,14 @@ int hostif_data_indication_wpa(struct ks_wlan_private *priv, priv->nstats.rx_errors++; return -EINVAL; } - if (((auth_type == TYPE_PMK1 && - priv->wpa.pairwise_suite == IW_AUTH_CIPHER_TKIP) || -(auth_type == TYPE_GMK1 && - priv->wpa.group_suite == IW_AUTH_CIPHER_TKIP) || -(auth_type == TYPE_GMK2 && - priv->wpa.group_suite == IW_AUTH_CIPHER_TKIP)) && - key->key_len) { + + do_mic = priv->wpa.pairwise_suite == IW_AUTH_CIPHER_TKIP && key->key_len; + if (!do_mic) + return 0; + + if (auth_type == TYPE_PMK1 || + auth_type == TYPE_GMK1 || + auth_type == TYPE_GMK2) { DPRINTK(4, "TKIP: protocol=%04X: size=%u\n", eth_proto, priv->rx_size); /* MIC save */ -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 --- drivers/staging/ks7010/ks_hostif.c | 42 +++--- drivers/staging/ks7010/ks_hostif.h | 2 +- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 690fa01..dd899ad 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -1109,9 +1109,9 @@ void hostif_event_check(struct ks_wlan_private *priv) #define CHECK_ALINE(size) (size % 4 ? (size + (4 - (size % 4))) : size) -int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *packet) +int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb) { - unsigned int packet_len = 0; + unsigned int skb_len = 0; unsigned char *buffer = NULL; unsigned int length = 0; @@ -1127,9 +1127,9 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *packet) struct ethhdr *eth; int ret; - packet_len = packet->len; - if (packet_len > ETH_FRAME_LEN) { - DPRINTK(1, "bad length packet_len=%d\n", packet_len); + skb_len = skb->len; + if (skb_len > ETH_FRAME_LEN) { + DPRINTK(1, "bad length skb_len=%d\n", skb_len); ret = -EOVERFLOW; goto err_kfree_skb; } @@ -1140,8 +1140,8 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *packet) DPRINTK(3, " DISCONNECT\n"); if (netif_queue_stopped(priv->net_dev)) netif_wake_queue(priv->net_dev); - if (packet) - dev_kfree_skb(packet); + if (skb) + dev_kfree_skb(skb); return 0; } @@ -1152,8 +1152,8 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *packet) netif_stop_queue(priv->net_dev); } - DPRINTK(4, "skb_buff length=%d\n", packet_len); - pp = kmalloc(hif_align_size(sizeof(*pp) + 6 + packet_len + 8), + DPRINTK(4, "skb_buff length=%d\n", skb_len); + pp = kmalloc(hif_align_size(sizeof(*pp) + 6 + skb_len + 8), KS_WLAN_MEM_FLAG); if (!pp) { @@ -1164,11 +1164,11 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *packet) p = (unsigned char *)pp->data; - buffer = packet->data; - length = packet->len; + buffer = skb->data; + length = skb->len; - /* packet check */ - eth = (struct ethhdr *)packet->data; + /* skb check */ + eth = (struct ethhdr *)skb->data; if (memcmp(&priv->eth_addr[0], eth->h_source, ETH_ALEN) != 0) { DPRINTK(1, "invalid mac address !!\n"); DPRINTK(1, "ethernet->h_source=%pM\n", eth->h_source); @@ -1192,13 +1192,13 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *packet) *p++ = 0x00;/* OUI ("00") */ *p++ = 0x00;/* OUI ("00") */ *p++ = 0x00;/* OUI ("00") */ - packet_len += 6; + skb_len += 6; } else { DPRINTK(4, "DIX\n"); /* Length(2 byte) delete */ buffer += 2; length -= 2; - packet_len -= 2; + skb_len -= 2; } /* pp->data copy */ @@ -1228,12 +1228,12 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *packet) pp->auth_type = cpu_to_le16((uint16_t)TYPE_AUTH); /* no encryption */ } else { if (priv->wpa.pairwise_suite == IW_AUTH_CIPHER_TKIP) { - MichaelMICFunction(&michael_mic, (uint8_t *)priv->wpa.key[0].tx_mic_key, (uint8_t *)&pp->data[0], (int)packet_len, (uint8_t)0, /* priority */ + MichaelMICFunction(&michael_mic, (uint8_t *)priv->wpa.key[0].tx_mic_key, (uint8_t *)&pp->data[0], (int)skb_len, (uint8_t)0, /* priority */ (uint8_t *)michael_mic. Result); memcpy(p, michael_mic.Result, 8); length += 8; - packet_len += 8; + skb_len += 8; p += 8; pp->auth_type = cpu_to_le16((uint16_t)TYPE_DATA); @@ -1254,14 +1254,14 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *packet) /* heade
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 coccinelle: > > @@ > > constant c; > > @@ > > > > -1 << c > > +BIT(c) > > > > Signed-off-by: Arushi Singhal > > Acked-by: Laurent Pinchart > > Greg, as this is a staging driver, do you want to merge the patch through > your > tree ? As this is at staging/media, better to merge via my tree. Regards, Mauro > > > --- > > changes in v2 > > -Remove unnecessary parenthesis > > > > drivers/staging/media/omap4iss/iss_csi2.c| 2 +- > > drivers/staging/media/omap4iss/iss_ipipe.c | 2 +- > > drivers/staging/media/omap4iss/iss_ipipeif.c | 2 +- > > drivers/staging/media/omap4iss/iss_resizer.c | 2 +- > > 4 files changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/staging/media/omap4iss/iss_csi2.c > > b/drivers/staging/media/omap4iss/iss_csi2.c index > > f71d5f2f179f..f6acc541e8a2 100644 > > --- a/drivers/staging/media/omap4iss/iss_csi2.c > > +++ b/drivers/staging/media/omap4iss/iss_csi2.c > > @@ -1268,7 +1268,7 @@ static int csi2_init_entities(struct iss_csi2_device > > *csi2, const char *subname) snprintf(name, sizeof(name), "CSI2%s", > > subname); > > snprintf(sd->name, sizeof(sd->name), "OMAP4 ISS %s", name); > > > > - sd->grp_id = 1 << 16; /* group ID for iss subdevs */ > > + sd->grp_id = BIT(16); /* group ID for iss subdevs */ > > v4l2_set_subdevdata(sd, csi2); > > sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > > > > diff --git a/drivers/staging/media/omap4iss/iss_ipipe.c > > b/drivers/staging/media/omap4iss/iss_ipipe.c index > > d38782e8e84c..d86ef8a031f2 100644 > > --- a/drivers/staging/media/omap4iss/iss_ipipe.c > > +++ b/drivers/staging/media/omap4iss/iss_ipipe.c > > @@ -508,7 +508,7 @@ static int ipipe_init_entities(struct iss_ipipe_device > > *ipipe) v4l2_subdev_init(sd, &ipipe_v4l2_ops); > > sd->internal_ops = &ipipe_v4l2_internal_ops; > > strlcpy(sd->name, "OMAP4 ISS ISP IPIPE", sizeof(sd->name)); > > - sd->grp_id = 1 << 16; /* group ID for iss subdevs */ > > + sd->grp_id = BIT(16); /* group ID for iss subdevs */ > > v4l2_set_subdevdata(sd, ipipe); > > sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > > > > diff --git a/drivers/staging/media/omap4iss/iss_ipipeif.c > > b/drivers/staging/media/omap4iss/iss_ipipeif.c index > > 23de8330731d..cb88b2bd0d82 100644 > > --- a/drivers/staging/media/omap4iss/iss_ipipeif.c > > +++ b/drivers/staging/media/omap4iss/iss_ipipeif.c > > @@ -739,7 +739,7 @@ static int ipipeif_init_entities(struct > > iss_ipipeif_device *ipipeif) v4l2_subdev_init(sd, &ipipeif_v4l2_ops); > > sd->internal_ops = &ipipeif_v4l2_internal_ops; > > strlcpy(sd->name, "OMAP4 ISS ISP IPIPEIF", sizeof(sd->name)); > > - sd->grp_id = 1 << 16; /* group ID for iss subdevs */ > > + sd->grp_id = BIT(16); /* group ID for iss subdevs */ > > v4l2_set_subdevdata(sd, ipipeif); > > sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > > > > diff --git a/drivers/staging/media/omap4iss/iss_resizer.c > > b/drivers/staging/media/omap4iss/iss_resizer.c index > > f1d352c711d5..4bbfa20b3c38 100644 > > --- a/drivers/staging/media/omap4iss/iss_resizer.c > > +++ b/drivers/staging/media/omap4iss/iss_resizer.c > > @@ -782,7 +782,7 @@ static int resizer_init_entities(struct > > iss_resizer_device *resizer) v4l2_subdev_init(sd, &resizer_v4l2_ops); > > sd->internal_ops = &resizer_v4l2_internal_ops; > > strlcpy(sd->name, "OMAP4 ISS ISP resizer", sizeof(sd->name)); > > - sd->grp_id = 1 << 16; /* group ID for iss subdevs */ > > + sd->grp_id = BIT(16); /* group ID for iss subdevs */ > > v4l2_set_subdevdata(sd, resizer); > > sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > Thanks, Mauro ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
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 Acked-by: Laurent Pinchart Greg, as this is a staging driver, do you want to merge the patch through your tree ? > --- > changes in v2 > -Remove unnecessary parenthesis > > drivers/staging/media/omap4iss/iss_csi2.c| 2 +- > drivers/staging/media/omap4iss/iss_ipipe.c | 2 +- > drivers/staging/media/omap4iss/iss_ipipeif.c | 2 +- > drivers/staging/media/omap4iss/iss_resizer.c | 2 +- > 4 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/media/omap4iss/iss_csi2.c > b/drivers/staging/media/omap4iss/iss_csi2.c index > f71d5f2f179f..f6acc541e8a2 100644 > --- a/drivers/staging/media/omap4iss/iss_csi2.c > +++ b/drivers/staging/media/omap4iss/iss_csi2.c > @@ -1268,7 +1268,7 @@ static int csi2_init_entities(struct iss_csi2_device > *csi2, const char *subname) snprintf(name, sizeof(name), "CSI2%s", > subname); > snprintf(sd->name, sizeof(sd->name), "OMAP4 ISS %s", name); > > - sd->grp_id = 1 << 16; /* group ID for iss subdevs */ > + sd->grp_id = BIT(16); /* group ID for iss subdevs */ > v4l2_set_subdevdata(sd, csi2); > sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > > diff --git a/drivers/staging/media/omap4iss/iss_ipipe.c > b/drivers/staging/media/omap4iss/iss_ipipe.c index > d38782e8e84c..d86ef8a031f2 100644 > --- a/drivers/staging/media/omap4iss/iss_ipipe.c > +++ b/drivers/staging/media/omap4iss/iss_ipipe.c > @@ -508,7 +508,7 @@ static int ipipe_init_entities(struct iss_ipipe_device > *ipipe) v4l2_subdev_init(sd, &ipipe_v4l2_ops); > sd->internal_ops = &ipipe_v4l2_internal_ops; > strlcpy(sd->name, "OMAP4 ISS ISP IPIPE", sizeof(sd->name)); > - sd->grp_id = 1 << 16; /* group ID for iss subdevs */ > + sd->grp_id = BIT(16); /* group ID for iss subdevs */ > v4l2_set_subdevdata(sd, ipipe); > sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > > diff --git a/drivers/staging/media/omap4iss/iss_ipipeif.c > b/drivers/staging/media/omap4iss/iss_ipipeif.c index > 23de8330731d..cb88b2bd0d82 100644 > --- a/drivers/staging/media/omap4iss/iss_ipipeif.c > +++ b/drivers/staging/media/omap4iss/iss_ipipeif.c > @@ -739,7 +739,7 @@ static int ipipeif_init_entities(struct > iss_ipipeif_device *ipipeif) v4l2_subdev_init(sd, &ipipeif_v4l2_ops); > sd->internal_ops = &ipipeif_v4l2_internal_ops; > strlcpy(sd->name, "OMAP4 ISS ISP IPIPEIF", sizeof(sd->name)); > - sd->grp_id = 1 << 16; /* group ID for iss subdevs */ > + sd->grp_id = BIT(16); /* group ID for iss subdevs */ > v4l2_set_subdevdata(sd, ipipeif); > sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > > diff --git a/drivers/staging/media/omap4iss/iss_resizer.c > b/drivers/staging/media/omap4iss/iss_resizer.c index > f1d352c711d5..4bbfa20b3c38 100644 > --- a/drivers/staging/media/omap4iss/iss_resizer.c > +++ b/drivers/staging/media/omap4iss/iss_resizer.c > @@ -782,7 +782,7 @@ static int resizer_init_entities(struct > iss_resizer_device *resizer) v4l2_subdev_init(sd, &resizer_v4l2_ops); > sd->internal_ops = &resizer_v4l2_internal_ops; > strlcpy(sd->name, "OMAP4 ISS ISP resizer", sizeof(sd->name)); > - sd->grp_id = 1 << 16; /* group ID for iss subdevs */ > + sd->grp_id = BIT(16); /* group ID for iss subdevs */ > v4l2_set_subdevdata(sd, resizer); > sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; -- Regards, Laurent Pinchart ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 driver must be implemented in userspace. This give the ability to support any kind of transport layer (such as Bluetooth, WiFi, Ethernet). The controller uses Generic Netlink. My original intent was to implement a TCP/IP controller but it was very slow, unstable. In addition, some features such as SVC or module discovery were to add to implement in kernel side. With the generic netlink controller, we can easily add support of new controller. It also helps to deal with all the project ARA legacy such as SVC. I have started to implement an application which is able to emulate the SVC, a module (like gbsim but only support control and loopback protcols), TCP/IP, Bluetooth and UART. It is still under development but it was stable enough to test this series. The application can found here: https://github.com/anobli/gbridge Alexandre Bailon (3): staging greybus: make cport_quiesce() method optional staging: greybus: Add Greybus netlink driver staging: greybus: netlink: Add a way to "hot remove" SVC drivers/staging/greybus/Kconfig | 9 ++ drivers/staging/greybus/Makefile | 2 + drivers/staging/greybus/connection.c | 3 + drivers/staging/greybus/gb_netlink.h | 38 ++ drivers/staging/greybus/netlink.c| 256 +++ 5 files changed, 308 insertions(+) create mode 100644 drivers/staging/greybus/gb_netlink.h create mode 100644 drivers/staging/greybus/netlink.c -- 2.10.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 before to use it. Signed-off-by: Alexandre Bailon --- drivers/staging/greybus/connection.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/greybus/connection.c b/drivers/staging/greybus/connection.c index 1bf0ee4..2cf6464 100644 --- a/drivers/staging/greybus/connection.c +++ b/drivers/staging/greybus/connection.c @@ -366,6 +366,9 @@ static int gb_connection_hd_cport_quiesce(struct gb_connection *connection) if (connection->mode_switch) peer_space += sizeof(struct gb_operation_msg_hdr); + if (!hd->driver->cport_quiesce) + return 0; + ret = hd->driver->cport_quiesce(hd, connection->hd_cport_id, peer_space, GB_CONNECTION_CPORT_QUIESCE_TIMEOUT); -- 2.10.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 be reinitialized by Greybus, the SVC won't do its job and Greybus won't work. Add a netlink operation to remove the SVC, which will remove the hd driver, and then register it again. Signed-off-by: Alexandre Bailon --- drivers/staging/greybus/gb_netlink.h | 1 + drivers/staging/greybus/netlink.c| 63 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/drivers/staging/greybus/gb_netlink.h b/drivers/staging/greybus/gb_netlink.h index 4af6fe5..3a6335b 100644 --- a/drivers/staging/greybus/gb_netlink.h +++ b/drivers/staging/greybus/gb_netlink.h @@ -29,6 +29,7 @@ enum { enum { GB_NL_C_UNSPEC, GB_NL_C_MSG, + GB_NL_C_HD_RESET, __GB_NL_C_MAX, }; diff --git a/drivers/staging/greybus/netlink.c b/drivers/staging/greybus/netlink.c index 84f3018..ddf2f98 100644 --- a/drivers/staging/greybus/netlink.c +++ b/drivers/staging/greybus/netlink.c @@ -25,6 +25,9 @@ static struct gb_host_device *gb_nl_hd; #define DEVICE_NAME"gb_netlink" #define CLASS_NAME "gb_netlink" +static int _gb_netlink_init(struct device *dev); +static void _gb_netlink_exit(void); + static int gb_netlink_msg(struct sk_buff *skb, struct genl_info *info) { struct nlattr *na; @@ -67,6 +70,18 @@ static int gb_netlink_msg(struct sk_buff *skb, struct genl_info *info) return 0; } +static int gb_netlink_hd_reset(struct sk_buff *skb, struct genl_info *info) +{ + struct device *dev; + struct gb_host_device *hd = gb_nl_hd; + + dev = hd->dev.parent; + _gb_netlink_exit(); + _gb_netlink_init(dev); + + return 0; +} + static struct nla_policy gb_nl_policy[GB_NL_A_MAX + 1] = { [GB_NL_A_DATA] = { .type = NLA_BINARY, .len = GB_NETLINK_MTU }, [GB_NL_A_CPORT] = { .type = NLA_U16}, @@ -78,6 +93,10 @@ static struct genl_ops gb_nl_ops[] = { .policy = gb_nl_policy, .doit = gb_netlink_msg, }, + { + .cmd = GB_NL_C_HD_RESET, + .doit = gb_netlink_hd_reset, + }, }; static struct genl_family gb_nl_family = { @@ -143,7 +162,7 @@ static struct gb_hd_driver tcpip_driver = { .message_cancel = message_cancel, }; -static void __exit gb_netlink_exit(void) +static void _gb_netlink_exit(void) { if (!gb_nl_hd) return; @@ -152,6 +171,11 @@ static void __exit gb_netlink_exit(void) gb_hd_put(gb_nl_hd); gb_nl_hd = NULL; +} + +static void __exit gb_netlink_exit(void) +{ + _gb_netlink_exit(); unregister_chrdev_region(major_dev, 1); device_destroy(gb_nl_class, major_dev); @@ -160,11 +184,32 @@ static void __exit gb_netlink_exit(void) genl_unregister_family(&gb_nl_family); } +static int _gb_netlink_init(struct device *dev) +{ + int retval; + + gb_nl_hd = gb_hd_create(&tcpip_driver, dev, GB_NETLINK_MTU, + GB_NETLINK_NUM_CPORT); + if (IS_ERR(gb_nl_hd)) + return PTR_ERR(gb_nl_hd); + + retval = gb_hd_add(gb_nl_hd); + if (retval) + goto err_gb_hd_del; + + return 0; + +err_gb_hd_del: + gb_hd_del(gb_nl_hd); + gb_hd_put(gb_nl_hd); + + return retval; +} + static int __init gb_netlink_init(void) { int retval; struct device *dev; - struct gb_host_device *gb_nl_hd; retval = genl_register_family(&gb_nl_family); if (retval) @@ -186,22 +231,12 @@ static int __init gb_netlink_init(void) goto err_class_destroy; } - gb_nl_hd = gb_hd_create(&tcpip_driver, dev, GB_NETLINK_MTU, - GB_NETLINK_NUM_CPORT); - if (IS_ERR(gb_nl_hd)) { - retval = PTR_ERR(gb_nl_hd); - goto err_device_destroy; - } - - retval = gb_hd_add(gb_nl_hd); + retval = _gb_netlink_init(dev); if (retval) - goto err_gb_hd_del; + goto err_device_destroy; return 0; -err_gb_hd_del: - gb_hd_del(gb_nl_hd); - gb_hd_put(gb_nl_hd); err_device_destroy: device_destroy(gb_nl_class, major_dev); err_chrdev_unregister: -- 2.10.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 bridge between Greybus and any kind of bus supported by the platform (e.g. Bluetooth). In addition, this add an easy way to implement some component such as SVC which is required by Greybus though it may not be available on every platforms. Signed-off-by: Alexandre Bailon --- drivers/staging/greybus/Kconfig | 9 ++ drivers/staging/greybus/Makefile | 2 + drivers/staging/greybus/gb_netlink.h | 37 ++ drivers/staging/greybus/netlink.c| 221 +++ 4 files changed, 269 insertions(+) create mode 100644 drivers/staging/greybus/gb_netlink.h create mode 100644 drivers/staging/greybus/netlink.c diff --git a/drivers/staging/greybus/Kconfig b/drivers/staging/greybus/Kconfig index 50de2d7..f9f3526 100644 --- a/drivers/staging/greybus/Kconfig +++ b/drivers/staging/greybus/Kconfig @@ -27,6 +27,15 @@ config GREYBUS_ES2 To compile this code as a module, chose M here: the module will be called gb-es2.ko +config GREYBUS_NETLINK + tristate "Greybus netlink host controller" + ---help--- + Select this option if you want to implement a Greybus + "host controller" in userspace. + + To compile this code as a module, chose M here: the module + will be called gb-netlink.ko + config GREYBUS_AUDIO tristate "Greybus Audio Class driver" depends on SOUND diff --git a/drivers/staging/greybus/Makefile b/drivers/staging/greybus/Makefile index b26b9a3..d057f1d 100644 --- a/drivers/staging/greybus/Makefile +++ b/drivers/staging/greybus/Makefile @@ -20,8 +20,10 @@ ccflags-y += -I$(src) # Greybus Host controller drivers gb-es2-y := es2.o +gb-netlink-y := netlink.o obj-$(CONFIG_GREYBUS_ES2) += gb-es2.o +obj-$(CONFIG_GREYBUS_NETLINK) += gb-netlink.o # Greybus class drivers gb-bootrom-y := bootrom.o diff --git a/drivers/staging/greybus/gb_netlink.h b/drivers/staging/greybus/gb_netlink.h new file mode 100644 index 000..4af6fe5 --- /dev/null +++ b/drivers/staging/greybus/gb_netlink.h @@ -0,0 +1,37 @@ +/* + * Greybus Netlink driver for userspace controller + * + * Copyright (c) 2017 BayLibre SAS + * + * Released under the GPLv2 only. + */ + +#ifndef __GB_NETLINK_H +#define __GB_NETLINK_H + +/* Maximum packet size */ +#define GB_NETLINK_MTU 2048 +/* Maximum number of Cports */ +#define GB_NETLINK_NUM_CPORT 32 + +#define GB_NL_NAME "GREYBUS" +#define GB_NL_PID 1 + +enum { + GB_NL_A_UNSPEC, + GB_NL_A_DATA, + GB_NL_A_CPORT, + __GB_NL_A_MAX, +}; + +#define GB_NL_A_MAX (__GB_NL_A_MAX - 1) + +enum { + GB_NL_C_UNSPEC, + GB_NL_C_MSG, + __GB_NL_C_MAX, +}; + +#define GB_NL_C_MAX (__GB_NL_C_MAX - 1) + +#endif /* __GB_NETLINK_H */ diff --git a/drivers/staging/greybus/netlink.c b/drivers/staging/greybus/netlink.c new file mode 100644 index 000..84f3018 --- /dev/null +++ b/drivers/staging/greybus/netlink.c @@ -0,0 +1,221 @@ +/* + * Greybus Netlink driver for userspace controller + * + * Copyright (c) 2017 BayLibre SAS + * + * Released under the GPLv2 only. + */ + +#include +#include +#include +#include +#include + +#include "greybus.h" +#include "gb_netlink.h" + +static dev_t major_dev; +static struct class *gb_nl_class; +static struct genl_family gb_nl_family; +static struct gb_host_device *gb_nl_hd; + +#define VERSION_NR 1 + +#define DEVICE_NAME"gb_netlink" +#define CLASS_NAME "gb_netlink" + +static int gb_netlink_msg(struct sk_buff *skb, struct genl_info *info) +{ + struct nlattr *na; + u16 cport_id; + void *data; + + if (!info) + return -EPROTO; + + na = info->attrs[GB_NL_A_CPORT]; + if (!na) { + dev_err(&gb_nl_hd->dev, + "Received message without cport id attribute\n"); + return -EPROTO; + } + + cport_id = nla_get_u32(na); + if (!cport_id_valid(gb_nl_hd, cport_id)) { + dev_err(&gb_nl_hd->dev, "invalid cport id %u received", + cport_id); + return -EINVAL; + } + + na = info->attrs[GB_NL_A_DATA]; + if (!na) { + dev_err(&gb_nl_hd->dev, + "Received message without data attribute\n"); + return -EPROTO; + } + + data = nla_data(na); + if (!data) { + dev_err(&gb_nl_hd->dev, + "Received message without data\n"); + return -EINVAL; + } + + greybus_data_rcvd(gb_nl_hd, cport_id, data, nla_len(na)); + + return 0; +} + +static struct nla_policy gb_nl_policy[GB_NL_A_MAX + 1] = { + [GB_NL_A_DATA] = {
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: > >> Drivers are written to support hardware, not particular use case. > > > > No, it is just the reverse: drivers and hardware are developed to > > support use cases. > > > > Btw, you should remember that the hardware is the full board, not just the > > SoC. In practice, the board do limit the use cases: several provide a > > single physical CSI connector, allowing just one sensor. > > > >>> This situation is there since 2009. If I remember well, you tried to > >>> write such generic plugin in the past, but never finished it, apparently > >>> because it is too complex. Others tried too over the years. > >> > >> I'd argue I know better what happened with that attempt than you do. I > >> had a prototype of a generic pipeline configuration library but due to > >> various reasons I haven't been able to continue working on that since > >> around 2012. > > ... > > > >>> The last trial was done by Jacek, trying to cover just the exynos4 > >>> driver. Yet, even such limited scope plugin was not good enough, as it > >>> was never merged upstream. Currently, there's no such plugins upstream. > >>> > >>> If we can't even merge a plugin that solves it for just *one* driver, > >>> I have no hope that we'll be able to do it for the generic case. > >> > >> I believe Jacek ceased to work on that plugin in his day job; other than > >> that, there are some matters left to be addressed in his latest patchset. > > > > The two above basically summaries the issue: the task of doing a generic > > plugin on userspace, even for a single driver is complex enough to > > not cover within a reasonable timeline. > > > > From 2009 to 2012, you were working on it, but didn't finish it. > > > > Apparently, nobody worked on it between 2013-2014 (but I may be wrong, as > > I didn't check when the generic plugin interface was added to libv4l). > > > > In the case of Jacek's work, the first patch I was able to find was > > > > written in Oct, 2014: > > https://patchwork.kernel.org/patch/5098111/ > > (not sure what happened with the version 1). > > > > The last e-mail about this subject was issued in Dec, 2016. > > > > In summary, you had this on your task for 3 years for an OMAP3 > > plugin (where you have a good expertise), and Jacek for 2 years, > > for Exynos 4, where he should also have a good knowledge. > > > > Yet, with all that efforts, no concrete results were achieved, as none > > of the plugins got merged. > > > > Even if they were merged, if we keep the same mean time to develop a > > libv4l plugin, that would mean that a plugin for i.MX6 could take 2-3 > > years to be developed. > > > > There's a clear message on it: > > - we shouldn't keep pushing for a solution via libv4l. > > Or: > - userspace plugin development had a very a low priority and > never got the attention it needed. > > I know that's *my* reason. I rarely if ever looked at it. I always assumed > Sakari and/or Laurent would look at it. If this reason is also valid for > Sakari and Laurent, then it is no wonder nothing has happened in all that > time. The reason is also valid for me. I'd really love to work on the userspace side, but I just can't find time at the moment. > We're all very driver-development-driven, and userspace gets very little > attention in general. So before just throwing in the towel we should take > a good look at the reasons why there has been little or no development: is > it because of fundamental design defects, or because nobody paid attention > to it? > > I strongly suspect it is the latter. > > In addition, I suspect end-users of these complex devices don't really care > about a plugin: they want full control and won't typically use generic > applications. If they would need support for that, we'd have seen much more > interest. The main reason for having a plugin is to simplify testing and > if this is going to be used on cheap hobbyist devkits. > > An additional complication is simply that it is hard to find fully supported > MC hardware. omap3 boards are hard to find these days, renesas boards are > not easy to get, freescale isn't the most popular either. Allwinner, > mediatek, amlogic, broadcom and qualcomm all have closed source > implementations or no implementation at all. > > I know it took me a very long time before I had a working omap3. > > So I am not at all surprised that little progress has been made. -- Regards, Laurent Pinchart ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
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 is difficult, because we either need to feed some > > library with knowledge for all kernel drivers or we need to improve > > the MC API to provide more details. > > > > For example, several drivers used to expose entities via the > > generic MEDIA_ENT_T_DEVNODE to represent entities of different > > > > types. See, for example, entities 1, 5 and 7 (and others) at: > > > https://mchehab.fedorapeople.org/mc-next-gen/igepv2_omap3isp.png > > Well... we provide enough information, so that device-specific code > does not have to be in kernel. > > There are few types of ENT_T_DEVNODE there. "ISP CCP2" does not really > provide any functionality to the user. It just has to be there, > because the pipeline needs to be connected. "ISP Preview" provides > format conversion. "ISP Resizer" provides rescaling. > > I'm not sure if it ever makes sense to use "ISP Preview > output". Normally you take data for display from "ISP Resizer > output". (Would there be some power advantage from that?) There was a bug on the preview engine to resizer hardware bus in OMAP34xx that required capturing data from the preview engine output and performing the scaling in memory-to-memory mode in some cases. You can also capture at the preview engine output if you don't need scaling, or if you want to render on the image before scaling (to draw pieces of a GUI for instance). There could be other use cases. > > A device-specific code could either be hardcoding the entity number > > or checking for the entity strings to add some logic to setup > > controls on those "unknown" entities, a generic app won't be able > > to do anything with them, as it doesn't know what function(s) such > > entity provide. > > Generic app should know if it wants RGGB10 data, then it can use "ISP > CCDC output", or if it wants "cooked" data suitable for display, then > it wants to use "ISP Resizer output". Once application knows what > output it wants, there's just one path through the system. So being > able to tell the ENT_T_DEVNODEs apart does not seem to be critical. > > OTOH, for useful camera application, different paths are needed at > different phases. -- Regards, Laurent Pinchart ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[GIT PULL] IIO driver fixes for 4.11-rc4
The following changes since commit 4495c08e84729385774601b5146d51d9e5849f81: Linux 4.11-rc2 (2017-03-12 14:47:08 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/ tags/staging-4.11-rc4 for you to fetch changes up to 43c49938bf72b2862aa12b5d66e710e15ce0f7b7: Merge tag 'iio-fixes-for-4.11c' of git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio into staging-linus (2017-03-22 12:02:01 +0100) IIO fixes for 4.11-rc4 Here are some small IIO driver fixes for 4.11-rc4 that resolve a number of tiny reported issues. All of these have been in linux-next for a while with no reported issues. Signed-off-by: Greg Kroah-Hartman Dmitry Torokhov (1): iio: magnetometer: ak8974: remove incorrect __exit markups Greg Kroah-Hartman (1): Merge tag 'iio-fixes-for-4.11c' of git://git.kernel.org/.../jic23/iio into staging-linus Lars-Peter Clausen (1): iio: sw-device: Fix config group initialization Lorenzo Bianconi (1): iio: imu: st_lsm6dsx: fix FIFO_CTRL2 overwrite during watermark configuration Michael Engl (1): iio: adc: ti_am335x_adc: fix fifo overrun recovery Song Hongyan (1): iio: hid-sensor-trigger: Change get poll value function order to avoid sensor properties losing after resume from S3 drivers/iio/adc/ti_am335x_adc.c | 13 - drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 6 +++--- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 4 ++-- drivers/iio/magnetometer/ak8974.c | 4 ++-- include/linux/iio/sw_device.h | 2 +- 5 files changed, 20 insertions(+), 9 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Please ignore this is a test] pci-hyperv: properly handle pci bus remove
Hi Long, [auto build test WARNING on pci/next] [also build test WARNING on v4.11-rc3 next-20170324] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Long-Li/pci-hyperv-properly-handle-pci-bus-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 All warnings (new ones prefixed by >>): drivers/pci/host/pci-hyperv.c: In function 'pci_devices_present_work': >> drivers/pci/host/pci-hyperv.c:1508:2: warning: enumeration value >> 'hv_pcibus_removed' not handled in switch [-Wswitch] switch(hbus->state) { ^~ >> drivers/pci/host/pci-hyperv.c:1508:2: warning: enumeration value >> 'hv_pcibus_maximum' not handled in switch [-Wswitch] vim +/hv_pcibus_removed +1508 drivers/pci/host/pci-hyperv.c 1492 put_pcichild(hpdev, hv_pcidev_ref_childlist); 1493 list_move_tail(&hpdev->list_entry, &removed); 1494 break; 1495 } 1496 } 1497 } while (found); 1498 spin_unlock_irqrestore(&hbus->device_list_lock, flags); 1499 1500 /* Delete everything that should no longer exist. */ 1501 while (!list_empty(&removed)) { 1502 hpdev = list_first_entry(&removed, struct hv_pci_dev, 1503 list_entry); 1504 list_del(&hpdev->list_entry); 1505 put_pcichild(hpdev, hv_pcidev_ref_initial); 1506 } 1507 > 1508 switch(hbus->state) { 1509 case hv_pcibus_installed: 1510 /* 1511 * Tell the core to rescan bus 1512 * because there may have been changes. 1513 */ 1514 pci_lock_rescan_remove(); 1515 pci_scan_child_bus(hbus->pci_bus); 1516 pci_unlock_rescan_remove(); --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 the pointer is dereferenced. Patch 03 removes multi-line dereference. Patch 04 fixes parenthesis alignment Patch 05 removes unnecessary 'else' statements. Code is untested. Patch set builds on x86_64 and PowerPC. Tobin C. Harding (5): staging: ks7010: remove unused execution path staging: ks7010: move null check before dereference staging: ks7010: fix parameter placement staging: ks7010: fix checkpatch PARENTHESIS_ALIGNMENT staging: ks7010: fix checkpatch UNNECESSARY_ELSE drivers/staging/ks7010/ks_wlan_net.c | 71 +++- 1 file changed, 29 insertions(+), 42 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 --- drivers/staging/ks7010/ks_wlan_net.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c index dc89c16..b467307 100644 --- a/drivers/staging/ks7010/ks_wlan_net.c +++ b/drivers/staging/ks7010/ks_wlan_net.c @@ -1771,6 +1771,8 @@ static int ks_wlan_set_encode_ext(struct net_device *dev, unsigned int commit = 0; enc = (struct iw_encode_ext *)extra; + if (!enc) + return -EINVAL; DPRINTK(2, "flags=%04X:: ext_flags=%08X\n", dwrq->flags, enc->ext_flags); @@ -1786,9 +1788,6 @@ static int ks_wlan_set_encode_ext(struct net_device *dev, if (dwrq->flags & IW_ENCODE_DISABLED) priv->wpa.key[index].key_len = 0; - if (!enc) - return -EINVAL; - priv->wpa.key[index].ext_flags = enc->ext_flags; if (enc->ext_flags & IW_ENCODE_EXT_SET_TX_KEY) { priv->wpa.txkey = index; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 unnecessary newline in preceding code. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks_wlan_net.c | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c index b467307..8315915 100644 --- a/drivers/staging/ks7010/ks_wlan_net.c +++ b/drivers/staging/ks7010/ks_wlan_net.c @@ -1830,18 +1830,17 @@ static int ks_wlan_set_encode_ext(struct net_device *dev, if (enc->key_len == 32) { memcpy(&priv->wpa.key[index].key_val[0], &enc->key[0], enc->key_len - 16); - priv->wpa.key[index].key_len = - enc->key_len - 16; + priv->wpa.key[index].key_len = enc->key_len - 16; if (priv->wpa.key_mgmt_suite == 4) {/* WPA_NONE */ - memcpy(&priv->wpa.key[index]. - tx_mic_key[0], &enc->key[16], 8); - memcpy(&priv->wpa.key[index]. - rx_mic_key[0], &enc->key[16], 8); + memcpy(&priv->wpa.key[index].tx_mic_key[0], + &enc->key[16], 8); + memcpy(&priv->wpa.key[index].rx_mic_key[0], + &enc->key[16], 8); } else { - memcpy(&priv->wpa.key[index]. - tx_mic_key[0], &enc->key[16], 8); - memcpy(&priv->wpa.key[index]. - rx_mic_key[0], &enc->key[24], 8); + memcpy(&priv->wpa.key[index].tx_mic_key[0], + &enc->key[16], 8); + memcpy(&priv->wpa.key[index].rx_mic_key[0], + &enc->key[24], 8); } commit |= (SME_WEP_VAL1 << index); } -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 b/drivers/staging/ks7010/ks_wlan_net.c index 8315915..20a97a8 100644 --- a/drivers/staging/ks7010/ks_wlan_net.c +++ b/drivers/staging/ks7010/ks_wlan_net.c @@ -1938,7 +1938,7 @@ static int ks_wlan_set_pmksa(struct net_device *dev, for (i = 0; i < PMK_LIST_MAX; i++) { pmk = &priv->pmklist.pmk[i]; if (memcmp("\x00\x00\x00\x00\x00\x00", - pmk->bssid, ETH_ALEN) == 0) + pmk->bssid, ETH_ALEN) == 0) break; /* loop */ } memcpy(pmk->bssid, pmksa->bssid.sa_data, ETH_ALEN); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 --- drivers/staging/ks7010/ks_wlan_net.c | 31 +++ 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c index 20a97a8..4ba11b5 100644 --- a/drivers/staging/ks7010/ks_wlan_net.c +++ b/drivers/staging/ks7010/ks_wlan_net.c @@ -202,6 +202,7 @@ static int ks_wlan_set_freq(struct net_device *dev, { struct ks_wlan_private *priv = (struct ks_wlan_private *)netdev_priv(dev); + int channel; if (priv->sleep_mode == SLP_SLEEP) return -EPERM; @@ -220,25 +221,23 @@ static int ks_wlan_set_freq(struct net_device *dev, fwrq->m = c + 1; } /* Setting by channel number */ - if ((fwrq->m > 1000) || (fwrq->e > 0)) { + if ((fwrq->m > 1000) || (fwrq->e > 0)) return -EOPNOTSUPP; - } else { - int channel = fwrq->m; - /* We should do a better check than that, -* based on the card capability !!! -*/ - if ((channel < 1) || (channel > 14)) { - netdev_dbg(dev, - "%s: New channel value of %d is invalid!\n", - dev->name, fwrq->m); - return -EINVAL; - } else { - /* Yes ! We can set it !!! */ - priv->reg.channel = (u8)(channel); - priv->need_commit |= SME_MODE_SET; - } + + channel = fwrq->m; + /* We should do a better check than that, +* based on the card capability !!! +*/ + if ((channel < 1) || (channel > 14)) { + netdev_dbg(dev, "%s: New channel value of %d is invalid!\n", + dev->name, fwrq->m); + return -EINVAL; } + /* Yes ! We can set it !!! */ + priv->reg.channel = (u8)(channel); + priv->need_commit |= SME_MODE_SET; + return -EINPROGRESS;/* Call commit handler */ } -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 of multi-way statement that can never be executed. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks_wlan_net.c | 14 ++ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c index 5e68699..dc89c16 100644 --- a/drivers/staging/ks7010/ks_wlan_net.c +++ b/drivers/staging/ks7010/ks_wlan_net.c @@ -1199,27 +1199,17 @@ static int ks_wlan_set_power(struct net_device *dev, { struct ks_wlan_private *priv = (struct ks_wlan_private *)netdev_priv(dev); - short enabled; if (priv->sleep_mode == SLP_SLEEP) return -EPERM; - /* for SLEEP MODE */ - enabled = vwrq->disabled ? 0 : 1; - if (enabled == 0) { /* 0 */ + if (vwrq->disabled) { priv->reg.powermgt = POWMGT_ACTIVE_MODE; - } else if (enabled) { /* 1 */ + } else { if (priv->reg.operation_mode == MODE_INFRASTRUCTURE) priv->reg.powermgt = POWMGT_SAVE1_MODE; else return -EINVAL; - } else if (enabled) { /* 2 */ - if (priv->reg.operation_mode == MODE_INFRASTRUCTURE) - priv->reg.powermgt = POWMGT_SAVE2_MODE; - else - return -EINVAL; - } else { - return -EINVAL; } hostif_sme_enqueue(priv, SME_POW_MNGMT_REQUEST); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
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 the problem. > > And my other point is, I think most people who have a need to work with > > the media framework on a particular platform will likely already be > > quite familiar with that platform. > > I disagree. The most popular platform device currently is Raspberry PI. > > I doubt that almost all owners of RPi + camera module know anything > about MC. They just use Raspberry's official driver with just provides > the V4L2 interface. > > I have a strong opinion that, for hardware like RPi, just the V4L2 > API is enough for more than 90% of the cases. Maybe V4L2 API is enough for 90% of the users. But I don't believe that means that we should provide compatibility. V4L2 API is not good enough for complex devices, and if we can make RPi people fix userspace... that's a good thing. > > The media graph for imx6 is fairly self-explanatory in my opinion. > > Yes that graph has to be generated, but just with a simple 'media-ctl > > --print-dot', I don't see how that is difficult for the user. > > Again, IMHO, the problem is not how to setup the pipeline, but, instead, > the need to forward controls to the subdevices. > > To use a camera, the user needs to set up a set of controls for the > image to make sense (bright, contrast, focus, etc). If the driver > doesn't forward those controls to the subdevs, an application like > "camorama" won't actually work for real, as the user won't be able > to adjust those parameters via GUI. I believe this can be fixed in libv4l2. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 b/drivers/staging/ks7010/ks7010_sdio.c index e25bd71..1018050 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -334,7 +334,7 @@ static void tx_device_task(void *dev) if (cnt_txqbody(priv) > 0) { queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq, - &priv->ks_wlan_hw.rw_wq, 0); + &priv->ks_wlan_hw.rw_wq, 0); } } -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 emits WARNING: Avoid multiple line dereference. Rename rc -> ret. Move dereference to single line and correctly align function arguments. Remove definition of 'ret' at declaration time. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks7010_sdio.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index b540be3..e25bd71 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -310,7 +310,7 @@ static void tx_device_task(void *dev) { struct ks_wlan_private *priv = (struct ks_wlan_private *)dev; struct tx_device_buffer *sp; - int rc = 0; + int ret; DPRINTK(4, "\n"); if (cnt_txqbody(priv) <= 0 || @@ -319,13 +319,11 @@ static void tx_device_task(void *dev) sp = &priv->tx_dev.tx_dev_buff[priv->tx_dev.qhead]; if (priv->dev_state >= DEVICE_STATE_BOOT) { - rc = write_to_device(priv, sp->sendp, sp->size); - if (rc) { - DPRINTK(1, "write_to_device error !!(%d)\n", - rc); - queue_delayed_work(priv->ks_wlan_hw. - ks7010sdio_wq, - &priv->ks_wlan_hw.rw_wq, 1); + ret = write_to_device(priv, sp->sendp, sp->size); + if (ret) { + DPRINTK(1, "write_to_device error !!(%d)\n", ret); + queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq, + &priv->ks_wlan_hw.rw_wq, 1); return; } } -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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 argument alignment, and checkpatch CHECK. Code is untested. Patch set applies and builds on x86_64 and PowerPC. Tobin C. Harding (3): staging: ks7010: invert conditional, reduce indentation staging: ks7010: rename identifier rc to ret staging: ks7010: fix checkpatch PARENTHESIS_ALIGNMENT drivers/staging/ks7010/ks7010_sdio.c | 41 ++-- 1 file changed, 20 insertions(+), 21 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[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. Reduce indentation in subsequent code. Signed-off-by: Tobin C. Harding --- drivers/staging/ks7010/ks7010_sdio.c | 43 ++-- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index b16618b..b540be3 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -313,30 +313,31 @@ static void tx_device_task(void *dev) int rc = 0; DPRINTK(4, "\n"); - if (cnt_txqbody(priv) > 0 && - atomic_read(&priv->psstatus.status) != PS_SNOOZE) { - sp = &priv->tx_dev.tx_dev_buff[priv->tx_dev.qhead]; - if (priv->dev_state >= DEVICE_STATE_BOOT) { - rc = write_to_device(priv, sp->sendp, sp->size); - if (rc) { - DPRINTK(1, "write_to_device error !!(%d)\n", - rc); - queue_delayed_work(priv->ks_wlan_hw. - ks7010sdio_wq, - &priv->ks_wlan_hw.rw_wq, 1); - return; - } - } - kfree(sp->sendp); /* allocated memory free */ - if (sp->complete_handler) /* TX Complete */ - (*sp->complete_handler) (sp->arg1, sp->arg2); - inc_txqhead(priv); + if (cnt_txqbody(priv) <= 0 || + atomic_read(&priv->psstatus.status) == PS_SNOOZE) + return; - if (cnt_txqbody(priv) > 0) { - queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq, - &priv->ks_wlan_hw.rw_wq, 0); + sp = &priv->tx_dev.tx_dev_buff[priv->tx_dev.qhead]; + if (priv->dev_state >= DEVICE_STATE_BOOT) { + rc = write_to_device(priv, sp->sendp, sp->size); + if (rc) { + DPRINTK(1, "write_to_device error !!(%d)\n", + rc); + queue_delayed_work(priv->ks_wlan_hw. + ks7010sdio_wq, + &priv->ks_wlan_hw.rw_wq, 1); + return; } } + kfree(sp->sendp); /* allocated memory free */ + if (sp->complete_handler) /* TX Complete */ + (*sp->complete_handler) (sp->arg1, sp->arg2); + inc_txqhead(priv); + + if (cnt_txqbody(priv) > 0) { + queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq, + &priv->ks_wlan_hw.rw_wq, 0); + } } int ks_wlan_hw_tx(struct ks_wlan_private *priv, void *p, unsigned long size, -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel