[PATCH v4] staging: rtl8192e: Aligning the * on each line in block comments

[Arushi]

This patch fixes the issue by aligning the * on each line in block comments.
[Patch v1] is rejected as the changes done is not following the linux
coding style and [Patch v2] is rejected because forgot to mention the
cause of rejection of [Patch v1].The cause of rejection of [Patch v3] is
that the mention of the cause was not in the correct format.

Signed-off-by: Arushi 

---

In the [patch v1] to allign the * on each line in block was not correct
as it is also not following the correct coding style of kernel so it got
rejected and the details of all the other patch versions is given above.
---
 drivers/staging/speakup/fakekey.c| 10 +-
 drivers/staging/speakup/i18n.c   | 14 +++---
 drivers/staging/speakup/main.c   |  2 +-
 drivers/staging/speakup/speakup_acntsa.c |  2 +-
 drivers/staging/speakup/speakup_apollo.c |  2 +-
 drivers/staging/speakup/speakup_decext.c |  2 +-
 drivers/staging/speakup/speakup_decpc.c  |  4 ++--
 drivers/staging/speakup/speakup_dtlk.c   |  2 +-
 drivers/staging/speakup/speakup_dtlk.h   | 10 +-
 drivers/staging/speakup/speakup_ltlk.c   |  2 +-
 10 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/speakup/fakekey.c 
b/drivers/staging/speakup/fakekey.c
index 8f058b42f68d..d76da0a1382c 100644
--- a/drivers/staging/speakup/fakekey.c
+++ b/drivers/staging/speakup/fakekey.c
@@ -63,8 +63,8 @@ void speakup_remove_virtual_keyboard(void)
 }
 
 /*
-* Send a simulated down-arrow to the application.
-*/
+ * Send a simulated down-arrow to the application.
+ */
 void speakup_fake_down_arrow(void)
 {
unsigned long flags;
@@ -87,9 +87,9 @@ void speakup_fake_down_arrow(void)
 }
 
 /*
-* Are we handling a simulated keypress on the current CPU?
-* Returns a boolean.
-*/
+ * Are we handling a simulated keypress on the current CPU?
+ * Returns a boolean.
+ */
 bool speakup_fake_key_pressed(void)
 {
return this_cpu_read(reporting_keystroke);
diff --git a/drivers/staging/speakup/i18n.c b/drivers/staging/speakup/i18n.c
index 8960079e4d60..2f9b3df7f78d 100644
--- a/drivers/staging/speakup/i18n.c
+++ b/drivers/staging/speakup/i18n.c
@@ -401,7 +401,7 @@ char *spk_msg_get(enum msg_index_t index)
  * Finds the start of the next format specifier in the argument string.
  * Return value: pointer to start of format
  * specifier, or NULL if no specifier exists.
-*/
+ */
 static char *next_specifier(char *input)
 {
int found = 0;
@@ -450,7 +450,7 @@ static char *skip_width(char *input)
  * Note that this code only accepts a handful of conversion specifiers:
  * c d s x and ld.  Not accidental; these are exactly the ones used in
  * the default group of formatted messages.
-*/
+ */
 static char *skip_conversion(char *input)
 {
if ((input[0] == 'l') && (input[1] == 'd'))
@@ -463,7 +463,7 @@ static char *skip_conversion(char *input)
 /*
  * Function: find_specifier_end
  * Return a pointer to the end of the format specifier.
-*/
+ */
 static char *find_specifier_end(char *input)
 {
input++;/* Advance over %. */
@@ -478,7 +478,7 @@ static char *find_specifier_end(char *input)
  * Compare the format specifiers pointed to by *input1 and *input2.
  * Return 1 if they are the same, 0 otherwise.  Advance *input1 and *input2
  * so that they point to the character following the end of the specifier.
-*/
+ */
 static int compare_specifiers(char **input1, char **input2)
 {
int same = 0;
@@ -500,7 +500,7 @@ static int compare_specifiers(char **input1, char **input2)
  * Check that two format strings contain the same number of format specifiers,
  * and that the order of specifiers is the same in both strings.
  * Return 1 if the condition holds, 0 if it doesn't.
-*/
+ */
 static int fmt_validate(char *template, char *user)
 {
int valid = 1;
@@ -537,7 +537,7 @@ static int fmt_validate(char *template, char *user)
  * Failure conditions:
  * -EINVAL -  Invalid format specifiers in formatted message or illegal index.
  * -ENOMEM -  Unable to allocate memory.
-*/
+ */
 ssize_t spk_msg_set(enum msg_index_t index, char *text, size_t length)
 {
int rc = 0;
@@ -573,7 +573,7 @@ ssize_t spk_msg_set(enum msg_index_t index, char *text, 
size_t length)
 /*
  * Find a message group, given its name.  Return a pointer to the structure
  * if found, or NULL otherwise.
-*/
+ */
 struct msg_group_t *spk_find_msg_group(const char *group_name)
 {
struct msg_group_t *group = NULL;
diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index bf539d38..c2f70ef5b9b3 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -16,7 +16,7 @@
  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  *  GNU General Public License for more details.
-*/
+ */
 
 #include 
 #include 
diff --git 

[PATCH v4] staging: rtl8192e: Aligning the * on each line in block comments

[Arushi]

This patch fixes the issue by aligning the * on each line in block comments.
[Patch v1] is rejected as the changes done is not following the linux
coding style and [Patch v2] is rejected because forgot to mention the
cause of rejection of [Patch v1].The cause of rejection of [Patch v3] is
that the mention of the cause was not in the correct format.

Signed-off-by: Arushi 

---

In the [patch v1] to allign the * on each line in block was not correct
as it is also not following the correct coding style of kernel so it got
rejected and the details of all the other patch versions is given above.
---
 drivers/staging/speakup/fakekey.c| 10 +-
 drivers/staging/speakup/i18n.c   | 14 +++---
 drivers/staging/speakup/main.c   |  2 +-
 drivers/staging/speakup/speakup_acntsa.c |  2 +-
 drivers/staging/speakup/speakup_apollo.c |  2 +-
 drivers/staging/speakup/speakup_decext.c |  2 +-
 drivers/staging/speakup/speakup_decpc.c  |  4 ++--
 drivers/staging/speakup/speakup_dtlk.c   |  2 +-
 drivers/staging/speakup/speakup_dtlk.h   | 10 +-
 drivers/staging/speakup/speakup_ltlk.c   |  2 +-
 10 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/speakup/fakekey.c 
b/drivers/staging/speakup/fakekey.c
index 8f058b42f68d..d76da0a1382c 100644
--- a/drivers/staging/speakup/fakekey.c
+++ b/drivers/staging/speakup/fakekey.c
@@ -63,8 +63,8 @@ void speakup_remove_virtual_keyboard(void)
 }
 
 /*
-* Send a simulated down-arrow to the application.
-*/
+ * Send a simulated down-arrow to the application.
+ */
 void speakup_fake_down_arrow(void)
 {
unsigned long flags;
@@ -87,9 +87,9 @@ void speakup_fake_down_arrow(void)
 }
 
 /*
-* Are we handling a simulated keypress on the current CPU?
-* Returns a boolean.
-*/
+ * Are we handling a simulated keypress on the current CPU?
+ * Returns a boolean.
+ */
 bool speakup_fake_key_pressed(void)
 {
return this_cpu_read(reporting_keystroke);
diff --git a/drivers/staging/speakup/i18n.c b/drivers/staging/speakup/i18n.c
index 8960079e4d60..2f9b3df7f78d 100644
--- a/drivers/staging/speakup/i18n.c
+++ b/drivers/staging/speakup/i18n.c
@@ -401,7 +401,7 @@ char *spk_msg_get(enum msg_index_t index)
  * Finds the start of the next format specifier in the argument string.
  * Return value: pointer to start of format
  * specifier, or NULL if no specifier exists.
-*/
+ */
 static char *next_specifier(char *input)
 {
int found = 0;
@@ -450,7 +450,7 @@ static char *skip_width(char *input)
  * Note that this code only accepts a handful of conversion specifiers:
  * c d s x and ld.  Not accidental; these are exactly the ones used in
  * the default group of formatted messages.
-*/
+ */
 static char *skip_conversion(char *input)
 {
if ((input[0] == 'l') && (input[1] == 'd'))
@@ -463,7 +463,7 @@ static char *skip_conversion(char *input)
 /*
  * Function: find_specifier_end
  * Return a pointer to the end of the format specifier.
-*/
+ */
 static char *find_specifier_end(char *input)
 {
input++;/* Advance over %. */
@@ -478,7 +478,7 @@ static char *find_specifier_end(char *input)
  * Compare the format specifiers pointed to by *input1 and *input2.
  * Return 1 if they are the same, 0 otherwise.  Advance *input1 and *input2
  * so that they point to the character following the end of the specifier.
-*/
+ */
 static int compare_specifiers(char **input1, char **input2)
 {
int same = 0;
@@ -500,7 +500,7 @@ static int compare_specifiers(char **input1, char **input2)
  * Check that two format strings contain the same number of format specifiers,
  * and that the order of specifiers is the same in both strings.
  * Return 1 if the condition holds, 0 if it doesn't.
-*/
+ */
 static int fmt_validate(char *template, char *user)
 {
int valid = 1;
@@ -537,7 +537,7 @@ static int fmt_validate(char *template, char *user)
  * Failure conditions:
  * -EINVAL -  Invalid format specifiers in formatted message or illegal index.
  * -ENOMEM -  Unable to allocate memory.
-*/
+ */
 ssize_t spk_msg_set(enum msg_index_t index, char *text, size_t length)
 {
int rc = 0;
@@ -573,7 +573,7 @@ ssize_t spk_msg_set(enum msg_index_t index, char *text, 
size_t length)
 /*
  * Find a message group, given its name.  Return a pointer to the structure
  * if found, or NULL otherwise.
-*/
+ */
 struct msg_group_t *spk_find_msg_group(const char *group_name)
 {
struct msg_group_t *group = NULL;
diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index bf539d38..c2f70ef5b9b3 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -16,7 +16,7 @@
  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  *  GNU General Public License for more details.
-*/
+ */
 
 #include 
 #include 
diff --git 

RE: [PATCH 9/9] staging: fsl-mc: dprc: drop unused APIs

[Stuart Yoder]

> --- a/drivers/staging/fsl-mc/include/dprc.h
> +++ b/drivers/staging/fsl-mc/include/dprc.h
> @@ -42,20 +42,6 @@
> 
>  struct fsl_mc_io;
> 
> -/**
> - * Set this value as the icid value in dprc_cfg structure when creating a
> - * container, in case the ICID is not selected by the user and should be
> - * allocated by the DPRC from the pool of ICIDs.
> - */
> -#define DPRC_GET_ICID_FROM_POOL  (u16)(~(0))
> -
> -/**
> - * Set this value as the portal_id value in dprc_cfg structure when creating 
> a
> - * container, in case the portal ID is not specifically selected by the
> - * user and should be allocated by the DPRC from the pool of portal ids.
> - */
> -#define DPRC_GET_PORTAL_ID_FROM_POOL (int)(~(0))
> -
>  int dprc_open(struct fsl_mc_io *mc_io,
> u32 cmd_flags,
> int container_id,
> @@ -65,79 +51,6 @@ int dprc_close(struct fsl_mc_io *mc_io,
>  u32 cmd_flags,
>  u16 token);
> 
> -/**
> - * Container general options
> - *
> - * These options may be selected at container creation by the container 
> creator
> - * and can be retrieved using dprc_get_attributes()
> - */
> -
> -/*
> - * Spawn Policy Option allowed - Indicates that the new container is allowed
> - * to spawn and have its own child containers.
> - */
> -#define DPRC_CFG_OPT_SPAWN_ALLOWED   0x0001
> -
> -/*
> - * General Container allocation policy - Indicates that the new container is
> - * allowed to allocate requested resources from its parent container; if not
> - * set, the container is only allowed to use resources in its own pools; Note
> - * that this is a container's global policy, but the parent container may
> - * override it and set specific quota per resource type.
> - */
> -#define DPRC_CFG_OPT_ALLOC_ALLOWED   0x0002
> -
> -/*
> - * Object initialization allowed - software context associated with this
> - * container is allowed to invoke object initialization operations.
> - */
> -#define DPRC_CFG_OPT_OBJ_CREATE_ALLOWED  0x0004
> -
> -/*
> - * Topology change allowed - software context associated with this
> - * container is allowed to invoke topology operations, such as attach/detach
> - * of network objects.
> - */
> -#define DPRC_CFG_OPT_TOPOLOGY_CHANGES_ALLOWED0x0008
> -
> -/* AIOP - Indicates that container belongs to AIOP.  */
> -#define DPRC_CFG_OPT_AIOP0x0020
> -
> -/* IRQ Config - Indicates that the container allowed to configure its IRQs.  
> */
> -#define DPRC_CFG_OPT_IRQ_CFG_ALLOWED 0x0040
> -
> -/**
> - * struct dprc_cfg - Container configuration options
> - * @icid: Container's ICID; if set to 'DPRC_GET_ICID_FROM_POOL', a free
> - *   ICID value is allocated by the DPRC
> - * @portal_id: Portal ID; if set to 'DPRC_GET_PORTAL_ID_FROM_POOL', a free
> - *   portal ID is allocated by the DPRC
> - * @options: Combination of 'DPRC_CFG_OPT_' options
> - * @label: Object's label
> - */
> -struct dprc_cfg {
> - u16 icid;
> - int portal_id;
> - u64 options;
> - char label[16];
> -};
> -
> -int dprc_create_container(struct fsl_mc_io *mc_io,
> -   u32 cmd_flags,
> -   u16 token,
> -   struct dprc_cfg *cfg,
> -   int *child_container_id,
> -   u64 *child_portal_offset);
> -
> -int dprc_destroy_container(struct fsl_mc_io *mc_io,
> -u32 cmd_flags,
> -u16 token,
> -int child_container_id);
> -
> -int dprc_reset_container(struct fsl_mc_io *mc_io,
> -  u32 cmd_flags,
> -  u16 token,
> -  int child_container_id);
> 
>  /* IRQ */
> 
> @@ -233,6 +146,7 @@ int dprc_clear_irq_status(struct fsl_mc_io *mc_io,
> u8 irq_index,
> u32 status);
> 
> +

Extra, uneeded newline added above.

Stuart
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: lustre: remove CLASSERT macro

[Dilger, Andreas]

On Feb 2, 2017, at 04:26, Arnd Bergmann  wrote:
> 
> lustre uses a fake switch() statement as a compile-time assert, but 
> unfortunately
> each use of that causes a warning when building with clang:
> 
> drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c:2907:2: warning: no case 
> matching constant switch condition '42'
> drivers/staging/lustre/lnet/klnds/socklnd/../../../include/linux/libcfs/libcfs_private.h:294:36:
>  note: expanded from macro 'CLASSERT'
> #define CLASSERT(cond) do {switch (42) {case (cond): case 0: break; } } while 
> (0)
> 
> As Greg suggested, let's just kill off this macro completely instead of
> fixing it. This replaces it with BUILD_BUG_ON(), which means we have
> to negate all the conditions in the process.
> 
> Signed-off-by: Arnd Bergmann 

I appreciate that you also fixed the comments where needed.

Reviewed-by: Andreas Dilger 

Cheers, Andreas

> ---
> .../lustre/include/linux/libcfs/libcfs_private.h   |  16 --
> .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c|  16 +-
> .../staging/lustre/lnet/klnds/socklnd/socklnd.c|   4 +-
> .../staging/lustre/lnet/klnds/socklnd/socklnd_cb.c |   2 +-
> .../lustre/lnet/klnds/socklnd/socklnd_proto.c  |   2 +-
> drivers/staging/lustre/lnet/libcfs/hash.c  |   2 +-
> drivers/staging/lustre/lnet/lnet/acceptor.c|   4 +-
> drivers/staging/lustre/lnet/lnet/api-ni.c  | 134 +++---
> drivers/staging/lustre/lnet/lnet/lib-socket.c  |   2 +-
> drivers/staging/lustre/lnet/lnet/net_fault.c   |   8 +-
> drivers/staging/lustre/lnet/lnet/router_proc.c |   4 +-
> drivers/staging/lustre/lustre/include/cl_object.h  |   2 +-
> drivers/staging/lustre/lustre/include/lu_object.h  |   2 +-
> drivers/staging/lustre/lustre/include/lustre_net.h |   4 +-
> drivers/staging/lustre/lustre/ldlm/ldlm_request.c  |   2 +-
> drivers/staging/lustre/lustre/ldlm/ldlm_resource.c |   2 +-
> drivers/staging/lustre/lustre/llite/super25.c  |   2 +-
> drivers/staging/lustre/lustre/llite/vvp_dev.c  |   2 +-
> drivers/staging/lustre/lustre/lov/lov_pack.c   |   6 +-
> drivers/staging/lustre/lustre/mdc/mdc_lib.c|  12 +-
> drivers/staging/lustre/lustre/mdc/mdc_locks.c  |   2 +-
> drivers/staging/lustre/lustre/mdc/mdc_request.c|   2 +-
> drivers/staging/lustre/lustre/osc/osc_io.c |   2 +-
> drivers/staging/lustre/lustre/osc/osc_request.c|  16 +-
> drivers/staging/lustre/lustre/ptlrpc/client.c  |   4 +-
> drivers/staging/lustre/lustre/ptlrpc/import.c  |   2 +-
> .../staging/lustre/lustre/ptlrpc/pack_generic.c| 102 +--
> drivers/staging/lustre/lustre/ptlrpc/pers.c|   2 +-
> drivers/staging/lustre/lustre/ptlrpc/wiretest.c| 194 ++---
> 29 files changed, 269 insertions(+), 285 deletions(-)
> 
> diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h 
> b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> index aab15d8112a4..2dae85798ec1 100644
> --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> @@ -277,22 +277,6 @@ do { 
> \
> #define CFS_ALLOC_PTR(ptr)  LIBCFS_ALLOC(ptr, sizeof(*(ptr)))
> #define CFS_FREE_PTR(ptr)   LIBCFS_FREE(ptr, sizeof(*(ptr)))
> 
> -/** Compile-time assertion.
> - *
> - * Check an invariant described by a constant expression at compile time by
> - * forcing a compiler error if it does not hold.  \a cond must be a constant
> - * expression as defined by the ISO C Standard:
> - *
> - *   6.8.4.2  The switch statement
> - *   
> - *   [#3] The expression of each case label shall be  an  integer
> - *   constant   expression  and  no  two  of  the  case  constant
> - *   expressions in the same switch statement shall have the same
> - *   value  after  conversion...
> - *
> - */
> -#define CLASSERT(cond) do {switch (42) {case (cond): case 0: break; } } 
> while (0)
> -
> /* max value for numeric network address */
> #define MAX_NUMERIC_VALUE 0x
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c 
> b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> index 7f761b327166..b1e8508f9fc7 100644
> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> @@ -258,8 +258,8 @@ int kiblnd_unpack_msg(struct kib_msg *msg, int nob)
>   if (flip) {
>   /* leave magic unflipped as a clue to peer endianness */
>   msg->ibm_version = version;
> - CLASSERT(sizeof(msg->ibm_type) == 1);
> - CLASSERT(sizeof(msg->ibm_credits) == 1);
> + BUILD_BUG_ON(sizeof(msg->ibm_type) != 1);
> + BUILD_BUG_ON(sizeof(msg->ibm_credits) != 1);
>   msg->ibm_nob = msg_nob;
>   __swab64s(>ibm_srcnid);
>   

Re: [PATCH] staging: fbtft: change 'gamma' array to u32

[Joe Perches]

On Thu, 2017-02-02 at 15:43 +0100, Arnd Bergmann wrote:
> Having a local variable of 1024 bytes on 64-bit architectures is a bit
> too much, and I ran into this warning while trying to see what functions
> use the largest stack:
> 
> drivers/staging/fbtft/fbtft-sysfs.c: In function 'store_gamma_curve':
> drivers/staging/fbtft/fbtft-sysfs.c:132:1: warning: the frame size of 1032 
> bytes is larger than 1024 bytes [-Wframe-larger-than=]
> 
> As there is no need for 64-bit gamma values (on 32-bit architectures,
> we don't use those either), I'm changing the type from 'unsigned long'
> to 'u32' here, which cuts the required space in half everywhere.
[]
> diff --git a/drivers/staging/fbtft/fb_hx8347d.c 
> b/drivers/staging/fbtft/fb_hx8347d.c
[]
> @@ -102,7 +102,7 @@ static void set_addr_win(struct fbtft_par *par, int xs, 
> int ys, int xe, int ye)
>   *   VRN0 VRN1 VRN2 VRN3 VRN4 VRN5 PRN0 PRN1 PKN0 PKN1 PKN2 PKN3 PKN4 CGM
>   */
>  #define CURVE(num, idx)  curves[num * par->gamma.num_values + idx]
> -static int set_gamma(struct fbtft_par *par, unsigned long *curves)
> +static int set_gamma(struct fbtft_par *par, u32 *curves)
>  {
>   unsigned long mask[] = {
>   0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x7f, 0x7f, 0x1f, 0x1f,

presumably these should be static const and maybe u8 or u32 too

> diff --git a/drivers/staging/fbtft/fb_ili9163.c 
> b/drivers/staging/fbtft/fb_ili9163.c
[]
> @@ -202,7 +202,7 @@ static int set_var(struct fbtft_par *par)
>  
>  #ifdef GAMMA_ADJ
>  #define CURVE(num, idx)  curves[num * par->gamma.num_values + idx]
> -static int gamma_adj(struct fbtft_par *par, unsigned long *curves)
> +static int gamma_adj(struct fbtft_par *par, u32 *curves)
>  {
>   unsigned long mask[] = {
>   0x3F, 0x3F, 0x3F, 0x3F, 0x3F,

etc...

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 4/9] staging: fsl-mc: don't use devres api for refcounted objects

[Stuart Yoder]

> -Original Message-
> From: laurentiu.tu...@nxp.com [mailto:laurentiu.tu...@nxp.com]
> Sent: Wednesday, February 01, 2017 5:43 AM
> To: gre...@linuxfoundation.org
> Cc: de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org; ag...@suse.de; 
> a...@arndb.de; Ioana
> Ciornei ; Ruxandra Ioana Radulescu 
> ; Bharat Bhushan
> ; Stuart Yoder ; Catalin 
> Horghidan
> ; Leo Li ; Roy Pledge 
> ; Laurentiu
> Tudor 
> Subject: [PATCH 4/9] staging: fsl-mc: don't use devres api for refcounted 
> objects
> 
> From: Laurentiu Tudor 
> 
> Mixing two memory management systems, in this case
> managed device resource api and refcounted objects
> is a bad idea. Lifetime of an object is controlled
> by its refcount so allocating it with other apis
> that have their own lifetime control is not ok.
> Drop devm_*() apis in favor of plain allocations.
> 
> While at it, let's drop the slab cache for objects
> until we actually have proof that it improves
> performance. This allows for some code cleanup.

Those 2 changes (dropping devm_* apis and slab cache
changes) are 2 orthogonal things, right?  It would
be better to split those into separate patches.  Mixing
them makes it harder to review.

> Signed-off-by: Laurentiu Tudor 
> ---
>  drivers/staging/fsl-mc/bus/fsl-mc-bus.c | 43 
> +
>  1 file changed, 6 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c 
> b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> index 6601bde..c493427 100644
> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> @@ -27,8 +27,6 @@
>  #include "fsl-mc-private.h"
>  #include "dprc-cmd.h"
> 
> -static struct kmem_cache *mc_dev_cache;
> -
>  /**
>   * Default DMA mask for devices on a fsl-mc bus
>   */
> @@ -422,17 +420,12 @@ bool fsl_mc_is_root_dprc(struct device *dev)
>  static void fsl_mc_device_release(struct device *dev)
>  {
>   struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> - struct fsl_mc_bus *mc_bus = NULL;
> 
>   kfree(mc_dev->regions);
> -
> - if (strcmp(mc_dev->obj_desc.type, "dprc") == 0)
> - mc_bus = to_fsl_mc_bus(mc_dev);
> -
> - if (mc_bus)
> - devm_kfree(mc_dev->dev.parent, mc_bus);
> + if (!strcmp(mc_dev->obj_desc.type, "dprc"))
> + kfree(to_fsl_mc_bus(mc_dev));
>   else
> - kmem_cache_free(mc_dev_cache, mc_dev);
> + kfree(mc_dev);
>  }
> 
>  /**
> @@ -457,7 +450,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
>   /*
>* Allocate an MC bus device object:
>*/
> - mc_bus = devm_kzalloc(parent_dev, sizeof(*mc_bus), GFP_KERNEL);
> + mc_bus = kzalloc(sizeof(*mc_bus), GFP_KERNEL);
>   if (!mc_bus)
>   return -ENOMEM;
> 
> @@ -466,7 +459,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
>   /*
>* Allocate a regular fsl_mc_device object:
>*/
> - mc_dev = kmem_cache_zalloc(mc_dev_cache, GFP_KERNEL);
> + mc_dev = kzalloc(sizeof(*mc_dev), GFP_KERNEL);
>   if (!mc_dev)
>   return -ENOMEM;
>   }
> @@ -561,10 +554,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
> 
>  error_cleanup_dev:
>   kfree(mc_dev->regions);
> - if (mc_bus)
> - devm_kfree(parent_dev, mc_bus);
> - else
> - kmem_cache_free(mc_dev_cache, mc_dev);
> + kfree(mc_bus ? (void *)mc_bus : (void *)mc_dev);
> 
>   return error;
>  }
> @@ -578,23 +568,11 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
>   */
>  void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
>  {
> - struct fsl_mc_bus *mc_bus = NULL;
> -
> - kfree(mc_dev->regions);
> -
>   /*
>* The device-specific remove callback will get invoked by device_del()
>*/
>   device_del(_dev->dev);
>   put_device(_dev->dev);
> -
> - if (strcmp(mc_dev->obj_desc.type, "dprc") == 0)
> - mc_bus = to_fsl_mc_bus(mc_dev);
> -
> - if (mc_bus)
> - devm_kfree(mc_dev->dev.parent, mc_bus);
> - else
> - kmem_cache_free(mc_dev_cache, mc_dev);
>  }

The above changes in fsl_mc_device_remove(), I think 
actually belong in patch #3 of this series.

Stuart
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 3/9] staging: fsl-mc: add device release callback

[Stuart Yoder]

> -Original Message-
> From: upstream-release-boun...@linux.freescale.net [mailto:upstream-release-
> boun...@linux.freescale.net] On Behalf Of laurentiu.tu...@nxp.com
> Sent: Wednesday, February 01, 2017 5:43 AM
> To: gre...@linuxfoundation.org
> Cc: de...@driverdev.osuosl.org; a...@arndb.de; Ruxandra Ioana Radulescu 
> ;
> Roy Pledge ; linux-ker...@vger.kernel.org; ag...@suse.de; 
> Catalin Horghidan
> ; Leo Li ; Stuart Yoder 
> ;
> Laurentiu Tudor 
> Subject: [upstream-release] [PATCH 3/9] staging: fsl-mc: add device release 
> callback
> 
> From: Laurentiu Tudor 
> 
> When hot unplugging a mc-bus device the kernel displays
> this pertinent message, followed by a stack dump:
> "Device 'foo.N' does not have a release() function,
>  it is broken and must be fixed."
> Add the required callback to fix.
> 
> Signed-off-by: Laurentiu Tudor 
> ---
>  drivers/staging/fsl-mc/bus/fsl-mc-bus.c | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c 
> b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> index 7c6a43b..6601bde 100644
> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> @@ -419,6 +419,22 @@ bool fsl_mc_is_root_dprc(struct device *dev)
>   return dev == root_dprc_dev;
>  }
> 
> +static void fsl_mc_device_release(struct device *dev)
> +{
> + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> + struct fsl_mc_bus *mc_bus = NULL;
> +
> + kfree(mc_dev->regions);
> +
> + if (strcmp(mc_dev->obj_desc.type, "dprc") == 0)
> + mc_bus = to_fsl_mc_bus(mc_dev);
> +
> + if (mc_bus)
> + devm_kfree(mc_dev->dev.parent, mc_bus);
> + else
> + kmem_cache_free(mc_dev_cache, mc_dev);
> +}
> +
>  /**
>   * Add a newly discovered fsl-mc device to be visible in Linux
>   */
> @@ -460,6 +476,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
>   device_initialize(_dev->dev);
>   mc_dev->dev.parent = parent_dev;
>   mc_dev->dev.bus = _mc_bus_type;
> + mc_dev->dev.release = fsl_mc_device_release;
>   dev_set_name(_dev->dev, "%s.%d", obj_desc->type, obj_desc->id);
> 
>   if (strcmp(obj_desc->type, "dprc") == 0) {
> --

With this patch applied, you still have this:

void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
{
struct fsl_mc_bus *mc_bus = NULL;

kfree(mc_dev->regions);

/*
 * The device-specific remove callback will get invoked by device_del()
 */
device_del(_dev->dev);
put_device(_dev->dev);

if (strcmp(mc_dev->obj_desc.type, "dprc") == 0)
mc_bus = to_fsl_mc_bus(mc_dev);

if (mc_bus)
devm_kfree(mc_dev->dev.parent, mc_bus);
else
kmem_cache_free(mc_dev_cache, mc_dev);
}

...i.e. you are doing the same thing in 2 places.  You
need to remove the kfree/devm_kfree/ kmem_cache_free,
here, no?

Stuart
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: rtl8188eu: remove not necessary braces {} (checkpatch fix)

[Joe Perches]

On Thu, 2017-02-02 at 08:06 +0200, Martin Karamihov wrote:
> On 02/01/2017 08:11 PM, Joe Perches wrote:
> > ...and clarifying the code for a
> > human reader is much more important than making a
> > file not have any checkpatch warnings.
> 
> I agree. I respect the developers' own coding style and believe that 
> some things (>80 characters long lines, name conventions, etc) should be 
> fixed by themselves (if they want to do that). I selected several TODOs 
> with requests for code cleanups and chose a fix in attempt to accomplish 
> my task with minimal inconvenience for the maintainers. I failed in my 
> first attempts breaking some basic rules, sorry about that.

No, not at all.  You did fine for a basic checkpatch cleanup.
I just wanted to tell you to think beyond checkpatch.

cheers, Joe

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rtl8188eu: checkpatch fixes: removed not necessary braces {}

[Joe Perches]

On Sun, 2017-01-29 at 17:43 +0100, Bjørn Mork wrote:
> Greg KH  writes:
> 
> > Please take some time, and go read Documentation/SubmittingPatches.
> 
> That's quickly done nowadays:
> 
>  bjorn@miraculix:/usr/local/src/git/linux$ cat Documentation/SubmittingPatches
>  This file has moved to process/submitting-patches.rst
> 
> 
> I don't know why it was necessary to move that file, but it looks like
> you, and the gazillion links and references to this document, just have
> to adopt to a new location.
> 
> Or submit a patch to fix it :)

I did exactly that.
https://lkml.org/lkml/2017/1/10/1059

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 16/24] media: Add i.MX media core driver

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:34PM -0800, Steve Longerbeam wrote:
> +/* register an internal subdev as a platform device */
> +static struct imx_media_subdev *
> +add_internal_subdev(struct imx_media_dev *imxmd,
> + const struct internal_subdev *isd,
> + int ipu_id)
> +{
> + struct imx_media_internal_sd_platformdata pdata;
> + struct platform_device_info pdevinfo = {0};
> + struct imx_media_subdev *imxsd;
> + struct platform_device *pdev;
> +
> + switch (isd->id->grp_id) {
> + case IMX_MEDIA_GRP_ID_CAMIF0...IMX_MEDIA_GRP_ID_CAMIF1:
> + pdata.grp_id = isd->id->grp_id +
> + ((2 * ipu_id) << IMX_MEDIA_GRP_ID_CAMIF_BIT);
> + break;
> + default:
> + pdata.grp_id = isd->id->grp_id;
> + break;
> + }
> +
> + /* the id of IPU this subdev will control */
> + pdata.ipu_id = ipu_id;
> +
> + /* create subdev name */
> + imx_media_grp_id_to_sd_name(pdata.sd_name, sizeof(pdata.sd_name),
> + pdata.grp_id, ipu_id);
> +
> + pdevinfo.name = isd->id->name;
> + pdevinfo.id = ipu_id * num_isd + isd->id->index;
> + pdevinfo.parent = imxmd->dev;
> + pdevinfo.data = 
> + pdevinfo.size_data = sizeof(pdata);
> + pdevinfo.dma_mask = DMA_BIT_MASK(32);
> +
> + pdev = platform_device_register_full();
> + if (IS_ERR(pdev))
> + return ERR_CAST(pdev);
> +
> + imxsd = imx_media_add_async_subdev(imxmd, NULL, dev_name(>dev));
> + if (IS_ERR(imxsd))
> + return imxsd;
> +
> + imxsd->num_sink_pads = isd->num_sink_pads;
> + imxsd->num_src_pads = isd->num_src_pads;
> +
> + return imxsd;
> +}

You seem to create platform devices here, but I see nowhere that you
ever remove them - so if you get to the lucky point of being able to
rmmod imx-media and then try to re-insert it, you end up with a load
of kernel warnings, one for each device created this way, and
platform_device_register_full() fails:

WARNING: CPU: 0 PID: 2143 at /home/rmk/git/linux-rmk/fs/sysfs/dir.c:31 
sysfs_warn_dup+0x64/0x80
sysfs: cannot create duplicate filename 
'/devices/soc0/soc/soc:media@0/imx-ipuv3-smfc.2'
Modules linked in: imx_media(C+) rfcomm bnep bluetooth nfsd imx_camif(C) 
imx_ic(C) imx_smfc(C) caam_jr snd_soc_imx_sgtl5000 snd_soc_fsl_asoc_card 
uvcvideo snd_soc_imx_spdif imx_mipi_csi2(C) imx_media_common(C) 
snd_soc_imx_audmux imx219 snd_soc_sgtl5000 video_multiplexer imx_sdma caam 
imx2_wdt rc_cec coda v4l2_mem2mem videobuf2_v4l2 snd_soc_fsl_ssi 
snd_soc_fsl_spdif videobuf2_dma_contig imx_pcm_dma videobuf2_core 
videobuf2_vmalloc videobuf2_memops imx_thermal dw_hdmi_cec dw_hdmi_ahb_audio 
etnaviv fuse rc_pinnacle_pctv_hd [last unloaded: imx_media]
CPU: 0 PID: 2143 Comm: modprobe Tainted: G C  4.10.0-rc6+ #2103
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Backtrace:
[] (dump_backtrace) from [] (show_stack+0x18/0x1c)
 r6:6013 r5: r4: r3:
[] (show_stack) from [] (dump_stack+0xa4/0xdc)
[] (dump_stack) from [] (__warn+0xdc/0x108)
 r6:c08ad998 r5: r4:cf4e9a78 r3:ec984980
[] (__warn) from [] (warn_slowpath_fmt+0x40/0x48)
 r10:e5800010 r8:ef1fa818 r7:ef1fa810 r6:ef202300 r5:e9809000 r4:ee868000
[] (warn_slowpath_fmt) from [] (sysfs_warn_dup+0x64/0x80)
 r3:ee868000 r2:c08ad9c0
[] (sysfs_warn_dup) from [] (sysfs_create_dir_ns+0x90/0x98)
 r6:ffef r5:ef202300 r4:eb5e3418
[] (sysfs_create_dir_ns) from [] 
(kobject_add_internal+0xa4/0x2d8)
 r6:ef1fa818 r5: r4:eb5e3418
[] (kobject_add_internal) from [] (kobject_add+0x50/0x98)
 r8:bf1f9018 r7:ef1fa810 r6:ef1fa818 r5: r4:eb5e3418
[] (kobject_add) from [] (device_add+0xc8/0x538)
 r3:ef1fa834 r2:
 r6:eb5e3418 r5: r4:eb5e3410
[] (device_add) from [] (platform_device_add+0xb0/0x214)
 r10:e5800010 r9: r8:bf1f9018 r7:e5806fd4 r6:eb5e3410 r5:eb5e3400
 r4:
[] (platform_device_add) from [] 
(platform_device_register_full+0xe8/0x110)
 r7:e5806fd4 r6:0002 r5:eb5e3400 r4:cf4e9c18
[] (platform_device_register_full) from [] 
(add_ipu_internal_subdevs+0x128/0x2c8 [imx_media])
 r5:bf1f9000 r4:0918
[] (add_ipu_internal_subdevs [imx_media]) from [] 
(imx_media_add_internal_subdevs+0x2c/0x70 [imx_media])
 r10:0048 r9:f31ceef8 r8:ef7f1d94 r7:e5800230 r6:e5800200 r5:e5800010
 r4:cf4e9c90
[] (imx_media_add_internal_subdevs [imx_media]) from [] 
(imx_media_probe+0xc4/0x1c0 [imx_media])
 r5: r4:e5800010
[] (imx_media_probe [imx_media]) from [] 
(platform_drv_probe+0x58/0xb8)
 r8: r7:bf1fbd48 r6:fdfb r5:ef1fa810 r4:fffe
[] (platform_drv_probe) from [] 
(driver_probe_device+0x204/0x2c8)
 r7:bf1fbd48 r6: r5:c1419de8 r4:ef1fa810
[] (driver_probe_device) from [] (__driver_attach+0xbc/0xc0)
 r10:0124 r8:0001 r7: r6:ef1fa844 r5:bf1fbd48 r4:ef1fa810
[] (__driver_attach) from [] (bus_for_each_dev+0x5c/0x90)
 r6:c0418d54 r5:bf1fbd48 r4: 

Re: [PATCH v3 16/24] media: Add i.MX media core driver

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:34PM -0800, Steve Longerbeam wrote:
> +struct imx_media_dev {
> + struct media_device md;
> + struct v4l2_device  v4l2_dev;

This is similarly buggy.

struct v4l2_device {
struct device *dev;
#if defined(CONFIG_MEDIA_CONTROLLER)
struct media_device *mdev;
#endif
struct list_head subdevs;
spinlock_t lock;
char name[V4L2_DEVICE_NAME_SIZE];
void (*notify)(struct v4l2_subdev *sd,
unsigned int notification, void *arg);
struct v4l2_ctrl_handler *ctrl_handler;
struct v4l2_prio_state prio;
struct kref ref;
void (*release)(struct v4l2_device *v4l2_dev);
};

Notice the kref and release function.  This is the only way the
memory backing "struct v4l2_device" may be released.  If you wish to
embed this structure into another structure, then the lifetime of
that other structure is determined by this one.  IOW, when this
release function is called, only then may you kfree() the memory
backing struct imx_media_dev.

> + struct device *dev;

And... do you need all these struct device pointers?

imxmd->dev = dev;
imxmd->md.dev = dev;

As media_device already contains a pointer, can't you re-use that?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 20/24] media: imx: Add Camera Interface subdev driver

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:38PM -0800, Steve Longerbeam wrote:
> +struct camif_priv {
> + struct device *dev;
> + struct video_devicevfd;

You can't do this.

> +static struct video_device camif_videodev = {
> + .fops   = _fops,
> + .ioctl_ops  = _ioctl_ops,
> + .minor  = -1,
> + .release= video_device_release,
> + .vfl_dir= VFL_DIR_RX,
> + .tvnorms= V4L2_STD_NTSC | V4L2_STD_PAL | V4L2_STD_SECAM,
> +};

> +static int camif_probe(struct platform_device *pdev)
> +{
> + struct imx_media_internal_sd_platformdata *pdata;
> + struct camif_priv *priv;
> + struct video_device *vfd;
> + int ret;
> +
> + priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;

You kmalloc this structure, so this structure has the lifetime of
the driver being bound to the platform device.

> + vfd = >vfd;
> + *vfd = camif_videodev;

However, "*vfd" contains a struct device, and you _correctly_ set the
release function for "*vfd" to video_device_release via camif_videodev.

However, if you try to rmmod imx-media, then you end up with a kernel
warning that you're freeing memory containing a held lock, and later
chaos ensues because kmalloc has been corrupted.

The root cause of this is embedding the device structure within the
video_device into the driver's private data.  *Any* structure what so
ever that contains a kref is reference counted, and that includes
struct device, and therefore also includes struct video_device.  What
that means is that its lifetime is _not_ under _your_ control, and
you may not free it except through its release function (which is
video_device_release().)  However, that also tries to kfree (with an
offset of 4) your private data, which results in the warning and the
corrupted kmalloc free lists.

The solution is simple, make "vfd" a pointer in your private data
structure and kmalloc() it separately, letting video_device_release()
kfree() that data when it needs to.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Thu, Feb 02, 2017 at 11:12:41AM -0800, Steve Longerbeam wrote:
> Here is the current .queue_setup() op in imx-media-capture.c:
> 
> static int capture_queue_setup(struct vb2_queue *vq,
>unsigned int *nbuffers,
>unsigned int *nplanes,
>unsigned int sizes[],
>struct device *alloc_devs[])
> {
> struct capture_priv *priv = vb2_get_drv_priv(vq);
> struct v4l2_pix_format *pix = >vdev.fmt.fmt.pix;
> unsigned int count = *nbuffers;
> 
> if (vq->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> return -EINVAL;
> 
> if (*nplanes) {
> if (*nplanes != 1 || sizes[0] < pix->sizeimage)
> return -EINVAL;
> count += vq->num_buffers;
> }
> 
> while (pix->sizeimage * count > VID_MEM_LIMIT)
> count--;

That's a weird way of writing:

unsigned int max_num = VID_MEM_LIMIT / pix->sizeimage;
count = max(count, max_num);

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

On 02/02/2017 10:58 AM, Russell King - ARM Linux wrote:

On Thu, Feb 02, 2017 at 10:26:55AM -0800, Steve Longerbeam wrote:

On 02/02/2017 09:56 AM, Russell King - ARM Linux wrote:

and for whatever reason we end up falling out through free_ring.  This
is VERY bad news, because it means that the ring which SMFC took a copy
of is now freed beneath its feet.

Yes, that is bad. That was a bug, if imx_media_dma_buf_queue_from_vb()
returned error, the ring should not have been freed, it should have only
returned the error. And further bad stuff happens from that point on.

But all of this is gone in version 4.

I think there's an error in how you think the queue_setup() works.

camif_queue_setup() always returns the number of buffers between
IMX_MEDIA_MIN_RING_BUFS and IMX_MEDIA_MAX_RING_BUFS.  However, it seems
that, looking through the videobuf2-core.c code, that the value is
passed to __vb2_queue_alloc() to allocate the specified number of
_additional_ buffers over and on-top of the existing q->num_buffers:

static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory,
  unsigned int num_buffers, unsigned int num_planes,
  const unsigned plane_sizes[VB2_MAX_PLANES])
{
 for (buffer = 0; buffer < num_buffers; ++buffer) {
...
 vb->index = q->num_buffers + buffer;

and

int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
 unsigned int *count)
{
 unsigned int num_buffers, allocated_buffers, num_planes = 0;
...
 num_buffers = min_t(unsigned int, *count, VB2_MAX_FRAME);
 num_buffers = max_t(unsigned int, num_buffers, q->min_buffers_needed);
...
 /*
  * Ask the driver how many buffers and planes per buffer it requires.
  * Driver also sets the size and allocator context for each plane.
  */
 ret = call_qop(q, queue_setup, q, _buffers, _planes,
plane_sizes, q->alloc_devs);
 if (ret)
 return ret;

 /* Finally, allocate buffers and video memory */
 allocated_buffers =
 __vb2_queue_alloc(q, memory, num_buffers, num_planes, 
plane_sizes);

or:

int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
 unsigned int *count, unsigned requested_planes,
 const unsigned requested_sizes[])
{
 unsigned int num_planes = 0, num_buffers, allocated_buffers;
...
 num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers);
 if (requested_planes && requested_sizes) {
 num_planes = requested_planes;
...
 /*
  * Ask the driver, whether the requested number of buffers, planes per
  * buffer and their sizes are acceptable
  */
 ret = call_qop(q, queue_setup, q, _buffers,
_planes, plane_sizes, q->alloc_devs);
 if (ret)
 return ret;

 /* Finally, allocate buffers and video memory */
 allocated_buffers = __vb2_queue_alloc(q, memory, num_buffers,
 num_planes, plane_sizes);


It seems to me that if you don't take account of the existing queue
size, your camif_queue_setup() has the side effect that each time
either of these are called.  Hence, the vb2 queue increases by the
same amount each time, which is probably what you don't want.

The documentation on queue_setup() leaves much to be desired:

  * @queue_setup:called from VIDIOC_REQBUFS() and VIDIOC_CREATE_BUFS()
  *  handlers before memory allocation. It can be called
  *  twice: if the original number of requested buffers
  *  could not be allocated, then it will be called a
  *  second time with the actually allocated number of
  *  buffers to verify if that is OK.
  *  The driver should return the required number of buffers
  *  in \*num_buffers, the required number of planes per
  *  buffer in \*num_planes, the size of each plane should 
be
  *  set in the sizes\[\] array and optional per-plane
  *  allocator specific device in the alloc_devs\[\] array.
  *  When called from VIDIOC_REQBUFS,() \*num_planes == 0,
  *  the driver has to use the currently configured format 
to
  *  determine the plane sizes and \*num_buffers is the 
total
  *  number of buffers that are being allocated. When called
  *  from VIDIOC_CREATE_BUFS,() \*num_planes != 0 and it
  *  describes the requested number of planes and sizes\[\]
  *  contains the requested plane sizes. If either
  *  \*num_planes or the requested sizes are invalid 
callback
  *  

Re: [PATCH v3 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Thu, Feb 02, 2017 at 10:26:55AM -0800, Steve Longerbeam wrote:
> On 02/02/2017 09:56 AM, Russell King - ARM Linux wrote:
> >and for whatever reason we end up falling out through free_ring.  This
> >is VERY bad news, because it means that the ring which SMFC took a copy
> >of is now freed beneath its feet.
> 
> Yes, that is bad. That was a bug, if imx_media_dma_buf_queue_from_vb()
> returned error, the ring should not have been freed, it should have only
> returned the error. And further bad stuff happens from that point on.
> 
> But all of this is gone in version 4.

I think there's an error in how you think the queue_setup() works.

camif_queue_setup() always returns the number of buffers between
IMX_MEDIA_MIN_RING_BUFS and IMX_MEDIA_MAX_RING_BUFS.  However, it seems
that, looking through the videobuf2-core.c code, that the value is
passed to __vb2_queue_alloc() to allocate the specified number of
_additional_ buffers over and on-top of the existing q->num_buffers:

static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory,
 unsigned int num_buffers, unsigned int num_planes,
 const unsigned plane_sizes[VB2_MAX_PLANES])
{
for (buffer = 0; buffer < num_buffers; ++buffer) {
...
vb->index = q->num_buffers + buffer;

and

int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
unsigned int *count)
{
unsigned int num_buffers, allocated_buffers, num_planes = 0;
...
num_buffers = min_t(unsigned int, *count, VB2_MAX_FRAME);
num_buffers = max_t(unsigned int, num_buffers, q->min_buffers_needed);
...
/*
 * Ask the driver how many buffers and planes per buffer it requires.
 * Driver also sets the size and allocator context for each plane.
 */
ret = call_qop(q, queue_setup, q, _buffers, _planes,
   plane_sizes, q->alloc_devs);
if (ret)
return ret;

/* Finally, allocate buffers and video memory */
allocated_buffers =
__vb2_queue_alloc(q, memory, num_buffers, num_planes, 
plane_sizes);

or:

int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
unsigned int *count, unsigned requested_planes,
const unsigned requested_sizes[])
{
unsigned int num_planes = 0, num_buffers, allocated_buffers;
...
num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers);
if (requested_planes && requested_sizes) {
num_planes = requested_planes;
...
/*
 * Ask the driver, whether the requested number of buffers, planes per
 * buffer and their sizes are acceptable
 */
ret = call_qop(q, queue_setup, q, _buffers,
   _planes, plane_sizes, q->alloc_devs);
if (ret)
return ret;

/* Finally, allocate buffers and video memory */
allocated_buffers = __vb2_queue_alloc(q, memory, num_buffers,
num_planes, plane_sizes);


It seems to me that if you don't take account of the existing queue
size, your camif_queue_setup() has the side effect that each time
either of these are called.  Hence, the vb2 queue increases by the
same amount each time, which is probably what you don't want.

The documentation on queue_setup() leaves much to be desired:

 * @queue_setup:called from VIDIOC_REQBUFS() and VIDIOC_CREATE_BUFS()
 *  handlers before memory allocation. It can be called
 *  twice: if the original number of requested buffers
 *  could not be allocated, then it will be called a
 *  second time with the actually allocated number of
 *  buffers to verify if that is OK.
 *  The driver should return the required number of buffers
 *  in \*num_buffers, the required number of planes per
 *  buffer in \*num_planes, the size of each plane should be
 *  set in the sizes\[\] array and optional per-plane
 *  allocator specific device in the alloc_devs\[\] array.
 *  When called from VIDIOC_REQBUFS,() \*num_planes == 0,
 *  the driver has to use the currently configured format to
 *  determine the plane sizes and \*num_buffers is the total
 *  number of buffers that are being allocated. When called
 *  from VIDIOC_CREATE_BUFS,() \*num_planes != 0 and it
 *  describes the requested number of planes and sizes\[\]
 *  contains the requested plane sizes. If either
 *  \*num_planes or the requested sizes are invalid callback
 *  must return %-EINVAL. In this case \*num_buffers are
 *  being allocated 

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

On 02/02/2017 09:56 AM, Russell King - ARM Linux wrote:

On Thu, Feb 02, 2017 at 05:22:46PM +, Russell King - ARM Linux wrote:

I thought, maybe, it's the IPU overwriting past the end of the buffer,
but I've added checks and that doesn't seem to have fired.  I also
wondered if it was some kind of use-after-free of the ring, so I made
imx_media_free_dma_buf_ring() memset the ring to 0x5a5a5a5a before
kfree()ing it... doesn't look like it's that either.  I'm going to
continue poking to see if I can figure out what's going on.

I take that back... here's a use-after-free of that buffer, on the
very first run:

Alignment trap: not handling instruction e1921f9f at []
Unhandled fault: alignment exception (0x001) at 0x5a5a5b5e
pgd = c0004000
[5a5a5b5e] *pgd=
Internal error: : 1 [#1] SMP ARM
Modules linked in: imx_csi(C) rfcomm bnep bluetooth nfsd imx_camif(C) imx_ic(C) 
imx_smfc(C) caam_jr snd_soc_imx_spdif snd_soc_imx_sgtl5000 
snd_soc_fsl_asoc_card imx_media(C) uvcvideo imx_mipi_csi2(C) 
imx_media_common(C) imx219 snd_soc_sgtl5000 snd_soc_imx_audmux caam 
video_multiplexer imx_sdma imx2_wdt coda v4l2_mem2mem videobuf2_v4l2 
videobuf2_dma_contig videobuf2_core rc_cec snd_soc_fsl_ssi snd_soc_fsl_spdif 
videobuf2_vmalloc videobuf2_memops imx_pcm_dma imx_thermal dw_hdmi_ahb_audio 
dw_hdmi_cec etnaviv fuse rc_pinnacle_pctv_hd
CPU: 0 PID: 99 Comm: kworker/0:3 Tainted: G C  4.10.0-rc6+ #2103
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Workqueue: lru-add-drain wq_barrier_func
task: ee4e24c0 task.stack: ee6da000
PC is at __lock_acquire+0xd4/0x17b0
LR is at lock_acquire+0xd8/0x250
pc : []lr : []psr: 20070193
sp : ee6dbb60  ip : 0001  fp : ee6dbbe4
r10: e9efad60  r9 : c0a70384  r8 : 
r7 : c0a38680  r6 :   r5 : ee4e24c0  r4 : c1400408
r3 :   r2 : 5a5a5b5e  r1 :   r0 : 5a5a5a5a
Flags: nzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 3d7ec04a  DAC: 0051
Process kworker/0:3 (pid: 99, stack limit = 0xee6da210)
Stack: (0xee6dbb60 to 0xee6dc000)
bb60: c0a38680 0002 c0b9d8c4 ee4e29a8 ee6dbc04 ee6dbb80 c0089708 c0088d44
bb80: ee6dbb9c 050f c00867fc c0086728 ee6dbbf4 ee6dbba0 87eba239 c035aa2f
bba0: 0001 ee4e29a8 c00c4f84 0001 0026 0560e36b  
bbc0: 60070193 e9efad60   c0a70384  ee6dbc3c ee6dbbe8
bbe0: c008b108 c0089400 0001 0080  bf0d2a8c  
bc00: c008b108 c0089400 0001 c09e04ec  e9efad50 60070193 bf0d2a8c
bc20: 0139  c09e04ec ee6dbcec ee6dbc6c ee6dbc40 c07016f4 c008b03c
bc40: 0001  bf0d2a8c ee6dbcec ee6dbc84 e9efac00 e9efad50 ee9785c4
bc60: ee6dbc84 ee6dbc70 bf0d2a8c c07016b4 ee978410 e9efb400 ee6dbca4 ee6dbc88
bc80: bf1224b8 bf0d2a7c bf122458 ee88d4c0 e9efb400 e9efb410 ee6dbce4 ee6dbca8
bca0: c009f5dc bf122464 0001 c09e04ec  e9efb400 c009f9f8 e9efb400
bcc0: e9efb400 e9efb410  0009 f4001100 ee6dbe38 ee6dbd04 ee6dbce8
bce0: c009f984 c009f544 c0701d10  e9efb400 e9efb460 ee6dbd24 ee6dbd08
bd00: c009fa00 c009f96c c09d0418 e9efb400 e9efb460 e9efb410 ee6dbd44 ee6dbd28
bd20: c00a3174 c009f9cc c00a30c4  ee6dbd90 ee4a3010 ee6dbd54 ee6dbd48
bd40: c009ecf0 c00a30d0 ee6dbd84 ee6dbd58 c0409328 c009ecdc c09d0448 0001
bd60: 0026 ef1efc10 c09e756c ee4a3010 0026 ef008400 ee6dbdcc ee6dbd88
bd80: c0409458 c040928c 0001  0001 0002 0003 000a
bda0: 000b 000c 000d 000e ee6dbdcc c09d52d0  
bdc0: ee6dbddc ee6dbdd0 c009ecf0 c0409408 ee6dbe04 ee6dbde0 c009ee24 c009ecdc
bde0: ee6dbe38 f4000100 f400010c c09e0af0 03eb c0a38a78 ee6dbe34 ee6dbe08
be00: c00094c8 c009edd4 ee4e24c0 c0701d50 20070013  ee6dbe6c ef7be600
be20: ee6da000 c09f5dc6 ee6dbe9c ee6dbe38 c00149f0 c0009488 0001 ee4e2988
be40:  60070093 20070013 ddb9799c 20070013 ee6dbef0 ef7be600 c09e04ec
be60: c09f5dc6 ee6dbe9c 0288 ee6dbe88 c008b60c c0701d50 20070013 
be80: 0051  ddb9799c ddb97998 ee6dbebc ee6dbea0 c0083824 c0701d20
bea0: c004e9c4 ee6e6d80 ddb97978 ef7ba940 ee6dbecc ee6dbec0 c004e9d8 c00837e8
bec0: ee6dbf2c ee6dbed0 c0050958 c004e9d0 0001  c0050898 
bee0: c0701d8c ee4e24c0 000f  c0a73e7c c0bc8834  c08947f8
bf00: 0008 ee6e6d80 ee6e6d98 ee6e6d98 0008 ef7ba940 ef7ba940 c09dd900
bf20: ee6dbf44 ee6dbf30 c0050e78 c0050774 ee6e6d80 ef7ba974 ee6dbf7c ee6dbf48
bf40: c0051094 c0050e54  ee6e8ac0 ee509eb8 ee509e80  ee6e8ac0
bf60: ee509eb8 ee6e6d80 ef0c9e58 c0050e88 ee6dbfac ee6dbf80 c0057b90 c0050e94
bf80: ee6da000 ee6e8ac0 c0057a88     
bfa0:  ee6dbfb0 c000fdf0 c0057a94    
bfc0:        
bfe0:     0013  3fffd861 3fffdc61
Backtrace:
[] 

Re: [PATCH v3] staging: rtl8192e: Aligning the * on each line in block comments

[Joe Perches]

On Thu, 2017-02-02 at 23:28 +0530, Arushi wrote:
> This patch fixes the issue by aligning the * on each line in block comments.
> [Patch v1] is rejected as the changes done is not following the linux
> coding style and [Patch v2] is rejected because forgot to mention the
> cause of rejection of [Patch v1].
> 
> Signed-off-by: Arushi 
> 
> --
> 
> Changes done from the [PATCH v1] is
> 
> diff --git a/drivers/staging/speakup/speakup_decpc.c
> b/drivers/staging/speakup/speakup_decpc.c

Don't put a diff into the commit message.

Put a textual description of the changes
below the --- line.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3] staging: rtl8192e: Aligning the * on each line in block comments

[Arushi]

This patch fixes the issue by aligning the * on each line in block comments.
[Patch v1] is rejected as the changes done is not following the linux
coding style and [Patch v2] is rejected because forgot to mention the
cause of rejection of [Patch v1].

Signed-off-by: Arushi 

--

Changes done from the [PATCH v1] is

diff --git a/drivers/staging/speakup/speakup_decpc.c
b/drivers/staging/speakup/speakup_decpc.c
index ac299a399e45..6bf38e49a96d 100644
--- a/drivers/staging/speakup/speakup_decpc.c
+++ b/drivers/staging/speakup/speakup_decpc.c
@@ -85,8 +85,8 @@
 #defineCTRL_io_priority0x0c00  /*   change i/o priority
*/
 #defineCTRL_free_mem   0x0d00  /*   get free paragraphs
on module */
 #defineCTRL_get_lang   0x0e00  /* return bit mask of
loaded
-* languages
-*/
+* languages
+*/
 #defineCMD_test0x2000  /*
self-test request */
 #defineTEST_mask   0x0F00  /* isolate test field */
 #defineTEST_null   0x  /* no test requested */
diff --git a/drivers/staging/speakup/speakup_dtlk.h
b/drivers/staging/speakup/speakup_dtlk.h
index 46d885fcfb20..b3b3cfc3db07 100644
--- a/drivers/staging/speakup/speakup_dtlk.h
+++ b/drivers/staging/speakup/speakup_dtlk.h
@@ -24,11 +24,11 @@
 * usec later.
 */
 #define TTS_ALMOST_FULL0x08/* mask for AF bit: When set to
1,
-* indicates that less than 300
 bytes
-* are available in the TTS
 input
-* buffer. AF is always 0 in the
 PCM,
-* TGN and CVSD modes.
-*/
+* indicates that less than 300
bytes
+* are available in the TTS
input
+* buffer. AF is always 0 in the
PCM,
+* TGN and CVSD modes.
+*/
 #define TTS_ALMOST_EMPTY 0x04  /* mask for AE bit: When set to 1,
 * indicates that less than 300 bytes
 * are remaining in DoubleTalk's input
---
 drivers/staging/speakup/fakekey.c| 10 +-
 drivers/staging/speakup/i18n.c   | 14 +++---
 drivers/staging/speakup/main.c   |  2 +-
 drivers/staging/speakup/speakup_acntsa.c |  2 +-
 drivers/staging/speakup/speakup_apollo.c |  2 +-
 drivers/staging/speakup/speakup_decext.c |  2 +-
 drivers/staging/speakup/speakup_decpc.c  |  4 ++--
 drivers/staging/speakup/speakup_dtlk.c   |  2 +-
 drivers/staging/speakup/speakup_dtlk.h   | 10 +-
 drivers/staging/speakup/speakup_ltlk.c   |  2 +-
 10 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/speakup/fakekey.c 
b/drivers/staging/speakup/fakekey.c
index 8f058b42f68d..d76da0a1382c 100644
--- a/drivers/staging/speakup/fakekey.c
+++ b/drivers/staging/speakup/fakekey.c
@@ -63,8 +63,8 @@ void speakup_remove_virtual_keyboard(void)
 }
 
 /*
-* Send a simulated down-arrow to the application.
-*/
+ * Send a simulated down-arrow to the application.
+ */
 void speakup_fake_down_arrow(void)
 {
unsigned long flags;
@@ -87,9 +87,9 @@ void speakup_fake_down_arrow(void)
 }
 
 /*
-* Are we handling a simulated keypress on the current CPU?
-* Returns a boolean.
-*/
+ * Are we handling a simulated keypress on the current CPU?
+ * Returns a boolean.
+ */
 bool speakup_fake_key_pressed(void)
 {
return this_cpu_read(reporting_keystroke);
diff --git a/drivers/staging/speakup/i18n.c b/drivers/staging/speakup/i18n.c
index 8960079e4d60..2f9b3df7f78d 100644
--- a/drivers/staging/speakup/i18n.c
+++ b/drivers/staging/speakup/i18n.c
@@ -401,7 +401,7 @@ char *spk_msg_get(enum msg_index_t index)
  * Finds the start of the next format specifier in the argument string.
  * Return value: pointer to start of format
  * specifier, or NULL if no specifier exists.
-*/
+ */
 static char *next_specifier(char *input)
 {
int found = 0;
@@ -450,7 +450,7 @@ static char *skip_width(char *input)
  * Note that this code only accepts a handful of conversion specifiers:
  * c d s x and ld.  Not accidental; these are exactly the ones used in
  * the default group of formatted messages.
-*/
+ */
 static char *skip_conversion(char *input)
 {
if ((input[0] == 'l') && (input[1] == 'd'))
@@ -463,7 +463,7 @@ static char 

Re: [PATCH v3 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Thu, Feb 02, 2017 at 05:22:46PM +, Russell King - ARM Linux wrote:
> I thought, maybe, it's the IPU overwriting past the end of the buffer,
> but I've added checks and that doesn't seem to have fired.  I also
> wondered if it was some kind of use-after-free of the ring, so I made
> imx_media_free_dma_buf_ring() memset the ring to 0x5a5a5a5a before
> kfree()ing it... doesn't look like it's that either.  I'm going to
> continue poking to see if I can figure out what's going on.

I take that back... here's a use-after-free of that buffer, on the
very first run:

Alignment trap: not handling instruction e1921f9f at []
Unhandled fault: alignment exception (0x001) at 0x5a5a5b5e
pgd = c0004000
[5a5a5b5e] *pgd=
Internal error: : 1 [#1] SMP ARM
Modules linked in: imx_csi(C) rfcomm bnep bluetooth nfsd imx_camif(C) imx_ic(C) 
imx_smfc(C) caam_jr snd_soc_imx_spdif snd_soc_imx_sgtl5000 
snd_soc_fsl_asoc_card imx_media(C) uvcvideo imx_mipi_csi2(C) 
imx_media_common(C) imx219 snd_soc_sgtl5000 snd_soc_imx_audmux caam 
video_multiplexer imx_sdma imx2_wdt coda v4l2_mem2mem videobuf2_v4l2 
videobuf2_dma_contig videobuf2_core rc_cec snd_soc_fsl_ssi snd_soc_fsl_spdif 
videobuf2_vmalloc videobuf2_memops imx_pcm_dma imx_thermal dw_hdmi_ahb_audio 
dw_hdmi_cec etnaviv fuse rc_pinnacle_pctv_hd
CPU: 0 PID: 99 Comm: kworker/0:3 Tainted: G C  4.10.0-rc6+ #2103
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Workqueue: lru-add-drain wq_barrier_func
task: ee4e24c0 task.stack: ee6da000
PC is at __lock_acquire+0xd4/0x17b0
LR is at lock_acquire+0xd8/0x250
pc : []lr : []psr: 20070193
sp : ee6dbb60  ip : 0001  fp : ee6dbbe4
r10: e9efad60  r9 : c0a70384  r8 : 
r7 : c0a38680  r6 :   r5 : ee4e24c0  r4 : c1400408
r3 :   r2 : 5a5a5b5e  r1 :   r0 : 5a5a5a5a
Flags: nzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 3d7ec04a  DAC: 0051
Process kworker/0:3 (pid: 99, stack limit = 0xee6da210)
Stack: (0xee6dbb60 to 0xee6dc000)
bb60: c0a38680 0002 c0b9d8c4 ee4e29a8 ee6dbc04 ee6dbb80 c0089708 c0088d44
bb80: ee6dbb9c 050f c00867fc c0086728 ee6dbbf4 ee6dbba0 87eba239 c035aa2f
bba0: 0001 ee4e29a8 c00c4f84 0001 0026 0560e36b  
bbc0: 60070193 e9efad60   c0a70384  ee6dbc3c ee6dbbe8
bbe0: c008b108 c0089400 0001 0080  bf0d2a8c  
bc00: c008b108 c0089400 0001 c09e04ec  e9efad50 60070193 bf0d2a8c
bc20: 0139  c09e04ec ee6dbcec ee6dbc6c ee6dbc40 c07016f4 c008b03c
bc40: 0001  bf0d2a8c ee6dbcec ee6dbc84 e9efac00 e9efad50 ee9785c4
bc60: ee6dbc84 ee6dbc70 bf0d2a8c c07016b4 ee978410 e9efb400 ee6dbca4 ee6dbc88
bc80: bf1224b8 bf0d2a7c bf122458 ee88d4c0 e9efb400 e9efb410 ee6dbce4 ee6dbca8
bca0: c009f5dc bf122464 0001 c09e04ec  e9efb400 c009f9f8 e9efb400
bcc0: e9efb400 e9efb410  0009 f4001100 ee6dbe38 ee6dbd04 ee6dbce8
bce0: c009f984 c009f544 c0701d10  e9efb400 e9efb460 ee6dbd24 ee6dbd08
bd00: c009fa00 c009f96c c09d0418 e9efb400 e9efb460 e9efb410 ee6dbd44 ee6dbd28
bd20: c00a3174 c009f9cc c00a30c4  ee6dbd90 ee4a3010 ee6dbd54 ee6dbd48
bd40: c009ecf0 c00a30d0 ee6dbd84 ee6dbd58 c0409328 c009ecdc c09d0448 0001
bd60: 0026 ef1efc10 c09e756c ee4a3010 0026 ef008400 ee6dbdcc ee6dbd88
bd80: c0409458 c040928c 0001  0001 0002 0003 000a
bda0: 000b 000c 000d 000e ee6dbdcc c09d52d0  
bdc0: ee6dbddc ee6dbdd0 c009ecf0 c0409408 ee6dbe04 ee6dbde0 c009ee24 c009ecdc
bde0: ee6dbe38 f4000100 f400010c c09e0af0 03eb c0a38a78 ee6dbe34 ee6dbe08
be00: c00094c8 c009edd4 ee4e24c0 c0701d50 20070013  ee6dbe6c ef7be600
be20: ee6da000 c09f5dc6 ee6dbe9c ee6dbe38 c00149f0 c0009488 0001 ee4e2988
be40:  60070093 20070013 ddb9799c 20070013 ee6dbef0 ef7be600 c09e04ec
be60: c09f5dc6 ee6dbe9c 0288 ee6dbe88 c008b60c c0701d50 20070013 
be80: 0051  ddb9799c ddb97998 ee6dbebc ee6dbea0 c0083824 c0701d20
bea0: c004e9c4 ee6e6d80 ddb97978 ef7ba940 ee6dbecc ee6dbec0 c004e9d8 c00837e8
bec0: ee6dbf2c ee6dbed0 c0050958 c004e9d0 0001  c0050898 
bee0: c0701d8c ee4e24c0 000f  c0a73e7c c0bc8834  c08947f8
bf00: 0008 ee6e6d80 ee6e6d98 ee6e6d98 0008 ef7ba940 ef7ba940 c09dd900
bf20: ee6dbf44 ee6dbf30 c0050e78 c0050774 ee6e6d80 ef7ba974 ee6dbf7c ee6dbf48
bf40: c0051094 c0050e54  ee6e8ac0 ee509eb8 ee509e80  ee6e8ac0
bf60: ee509eb8 ee6e6d80 ef0c9e58 c0050e88 ee6dbfac ee6dbf80 c0057b90 c0050e94
bf80: ee6da000 ee6e8ac0 c0057a88     
bfa0:  ee6dbfb0 c000fdf0 c0057a94    
bfc0:        
bfe0:     0013  3fffd861 3fffdc61
Backtrace:
[] (__lock_acquire) from [] (lock_acquire+0xd8/0x250)
 

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

Hi Russell,

I don't recommend spending too much time debugging this
OOPS. The dma buffer ring has been removed completely
in version 4 (which I'm trying to get ready to post hopefully
by end of this week).

Steve


On 02/02/2017 09:22 AM, Russell King - ARM Linux wrote:

I seem to be getting some sort of memory corruption with this driver.

I've had two instances now of uninitialised spinlocks in
imx_media_dma_buf_get_active() which show that the spinlock being
taken in this function is all-zeros.

That very quickly leads to an oops, where I've seen buf->ring is
NULL in imx_media_dma_buf_set_active().

Not quite sure what's going on, but the trigger (at least for me) is
to change my gstreamer pipeline from:

DISPLAY=:0 gst-launch-1.0 -v v4l2src device=/dev/video3 ! bayer2rgbneon ! 
xvimagesink

to

DISPLAY=:0 gst-launch-1.0 -v v4l2src device=/dev/video3 ! queue ! bayer2rgbneon 
! xvimagesink

and it seems to take as little as two or three attempts to provoke the
kernel to totally die.

I've just tried a third time.  I can run the first gstreamer command
five times.  The I ran the second command and immediately got this:

INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 0 PID: 1008 Comm: Xorg Tainted: G C  4.10.0-rc6+ #2103
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Backtrace:
[] (dump_backtrace) from [] (show_stack+0x18/0x1c)
  r6:600f0193 r5: r4: r3:
[] (show_stack) from [] (dump_stack+0xa4/0xdc)
[] (dump_stack) from [] (register_lock_class+0x1d4/0x554)
  r6:c1400408 r5: r4: r3:ee47a4c0
[] (register_lock_class) from [] 
(__lock_acquire+0x80/0x17b0)
  r10:d995f760 r9:c0a70384 r8: r7:c0a38680 r6: r5:ee47a4c0
  r4:c1400408
[] (__lock_acquire) from [] (lock_acquire+0xd8/0x250)
  r10: r9:c0a70384 r8: r7: r6:d995f760 r5:600f0193
  r4:
[] (lock_acquire) from [] (_raw_spin_lock_irqsave+0x4c/0x60)
  r10:ed501e64 r9:c09e04ec r8: r7:0139 r6:bf0d7a8c r5:600f0193
  r4:d995f750
[] (_raw_spin_lock_irqsave) from [] 
(imx_media_dma_buf_get_active+0x1c/0x94 [imx_media_common])
  r6:e98b2c10 r5:d995f750 r4:d995f600
[] (imx_media_dma_buf_get_active [imx_media_common]) from 
[] (imx_smfc_eof_interrupt+0x60/0x124 [imx_smfc])
  r5:ee935dc4 r4:ee935c10
[] (imx_smfc_eof_interrupt [imx_smfc]) from [] 
(__handle_irq_event_percpu+0xa4/0x428)
  r6:e98b2c10 r5:e98b2c00 r4:ebfb6d40 r3:bf12c458
[] (__handle_irq_event_percpu) from [] 
(handle_irq_event_percpu+0x24/0x60)
  r10:ed501fb0 r9:f4001100 r8:0009 r7: r6:e98b2c10 r5:e98b2c00
  r4:e98b2c00
[] (handle_irq_event_percpu) from [] 
(handle_irq_event+0x40/0x64)
  r5:e98b2c60 r4:e98b2c00
[] (handle_irq_event) from [] (handle_level_irq+0xb0/0x138)
  r6:e98b2c10 r5:e98b2c60 r4:e98b2c00 r3:c09d0418
[] (handle_level_irq) from [] (generic_handle_irq+0x20/0x30)
  r6:ee4a3010 r5:ed501f08 r4: r3:c00a30c4
[] (generic_handle_irq) from [] (ipu_irq_handle+0xa8/0xd8)
[] (ipu_irq_handle) from [] (ipu_irq_handler+0x5c/0xb4)
  r8:ef008400 r7:0026 r6:ee4a3010 r5:c09e756c r4:ef1efc10
[] (ipu_irq_handler) from [] (generic_handle_irq+0x20/0x30)
  r6: r5: r4:c09d52d0
[] (generic_handle_irq) from [] 
(__handle_domain_irq+0x5c/0xb8)
[] (__handle_domain_irq) from [] (gic_handle_irq+0x4c/0x9c)
  r8:c0a38a78 r7:03eb r6:c09e0af0 r5:f400010c r4:f4000100 r3:ed501fb0
[] (gic_handle_irq) from [] (__irq_usr+0x58/0x80)
Exception stack(0xed501fb0 to 0xed501ff8)
1fa0: b698b4e0  0042c000 b698c708
1fc0: 0010 81231b10 81231b18 80e89670 b698b4e0 8114957c 7f79b000 81149438
1fe0: 7f79b248 bee08b98 7f708609 b6904220 600f0030 
  r10:7f79b000 r9:8114957c r8:10c5387d r7:10c5387d r6: r5:600f0030
  r4:b6904220 r3:ee47a4c0
[ cut here ]
WARNING: CPU: 0 PID: 1008 at 
/home/rmk/git/linux-rmk/drivers/staging/media/imx/imx-smfc.c:159 
imx_smfc_eof_interrupt+0x118/0x124 [imx_smfc]
Modules linked in: imx_csi(C) rfcomm bnep bluetooth nfsd imx_camif(C) imx_ic(C) 
imx_smfc(C) caam_jr snd_soc_imx_sgtl5000 uvcvideo snd_soc_fsl_asoc_card 
snd_soc_imx_spdif imx_media(C) imx_mipi_csi2(C) imx_media_common(C) 
snd_soc_imx_audmux imx219 snd_soc_sgtl5000 caam video_multiplexer imx_sdma 
imx2_wdt rc_cec snd_soc_fsl_ssi coda v4l2_mem2mem videobuf2_v4l2 
videobuf2_dma_contig videobuf2_core snd_soc_fsl_spdif imx_pcm_dma 
videobuf2_vmalloc dw_hdmi_ahb_audio dw_hdmi_cec videobuf2_memops imx_thermal 
etnaviv fuse rc_pinnacle_pctv_hd
CPU: 0 PID: 1008 Comm: Xorg Tainted: G C  4.10.0-rc6+ #2103
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Backtrace:
[] (dump_backtrace) from [] (show_stack+0x18/0x1c)
  r6:600f0193 r5: r4: r3:
[] (show_stack) from [] (dump_stack+0xa4/0xdc)
[] (dump_stack) from [] (__warn+0xdc/0x108)
  r6:bf12d004 r5: r4: r3:ee47a4c0
[] 

Re: [PATCH v2] staging: rtl8192e: Aligning the * on each line in block comments

[Greg KH]

On Thu, Feb 02, 2017 at 10:26:02PM +0530, Arushi wrote:
> This patch fixes the issue by aligning the * on each line in block comments.
> 
> Signed-off-by: Arushi 
> ---
>  drivers/staging/speakup/fakekey.c| 10 +-
>  drivers/staging/speakup/i18n.c   | 14 +++---
>  drivers/staging/speakup/main.c   |  2 +-
>  drivers/staging/speakup/speakup_acntsa.c |  2 +-
>  drivers/staging/speakup/speakup_apollo.c |  2 +-
>  drivers/staging/speakup/speakup_decext.c |  2 +-
>  drivers/staging/speakup/speakup_decpc.c  |  4 ++--
>  drivers/staging/speakup/speakup_dtlk.c   |  2 +-
>  drivers/staging/speakup/speakup_dtlk.h   | 10 +-
>  drivers/staging/speakup/speakup_ltlk.c   |  2 +-
>  10 files changed, 25 insertions(+), 25 deletions(-)

What changed from v1?  Always put that information below the --- line.
Please resend v3 with that information.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 00/24] i.MX Media Driver

[Russell King - ARM Linux]

I seem to be getting some sort of memory corruption with this driver.

I've had two instances now of uninitialised spinlocks in
imx_media_dma_buf_get_active() which show that the spinlock being
taken in this function is all-zeros.

That very quickly leads to an oops, where I've seen buf->ring is
NULL in imx_media_dma_buf_set_active().

Not quite sure what's going on, but the trigger (at least for me) is
to change my gstreamer pipeline from:

DISPLAY=:0 gst-launch-1.0 -v v4l2src device=/dev/video3 ! bayer2rgbneon ! 
xvimagesink

to

DISPLAY=:0 gst-launch-1.0 -v v4l2src device=/dev/video3 ! queue ! bayer2rgbneon 
! xvimagesink

and it seems to take as little as two or three attempts to provoke the
kernel to totally die.

I've just tried a third time.  I can run the first gstreamer command
five times.  The I ran the second command and immediately got this:

INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 0 PID: 1008 Comm: Xorg Tainted: G C  4.10.0-rc6+ #2103
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Backtrace:
[] (dump_backtrace) from [] (show_stack+0x18/0x1c)
 r6:600f0193 r5: r4: r3:
[] (show_stack) from [] (dump_stack+0xa4/0xdc)
[] (dump_stack) from [] (register_lock_class+0x1d4/0x554)
 r6:c1400408 r5: r4: r3:ee47a4c0
[] (register_lock_class) from [] 
(__lock_acquire+0x80/0x17b0)
 r10:d995f760 r9:c0a70384 r8: r7:c0a38680 r6: r5:ee47a4c0
 r4:c1400408
[] (__lock_acquire) from [] (lock_acquire+0xd8/0x250)
 r10: r9:c0a70384 r8: r7: r6:d995f760 r5:600f0193
 r4:
[] (lock_acquire) from [] (_raw_spin_lock_irqsave+0x4c/0x60)
 r10:ed501e64 r9:c09e04ec r8: r7:0139 r6:bf0d7a8c r5:600f0193
 r4:d995f750
[] (_raw_spin_lock_irqsave) from [] 
(imx_media_dma_buf_get_active+0x1c/0x94 [imx_media_common])
 r6:e98b2c10 r5:d995f750 r4:d995f600
[] (imx_media_dma_buf_get_active [imx_media_common]) from 
[] (imx_smfc_eof_interrupt+0x60/0x124 [imx_smfc])
 r5:ee935dc4 r4:ee935c10
[] (imx_smfc_eof_interrupt [imx_smfc]) from [] 
(__handle_irq_event_percpu+0xa4/0x428)
 r6:e98b2c10 r5:e98b2c00 r4:ebfb6d40 r3:bf12c458
[] (__handle_irq_event_percpu) from [] 
(handle_irq_event_percpu+0x24/0x60)
 r10:ed501fb0 r9:f4001100 r8:0009 r7: r6:e98b2c10 r5:e98b2c00
 r4:e98b2c00
[] (handle_irq_event_percpu) from [] 
(handle_irq_event+0x40/0x64)
 r5:e98b2c60 r4:e98b2c00
[] (handle_irq_event) from [] (handle_level_irq+0xb0/0x138)
 r6:e98b2c10 r5:e98b2c60 r4:e98b2c00 r3:c09d0418
[] (handle_level_irq) from [] (generic_handle_irq+0x20/0x30)
 r6:ee4a3010 r5:ed501f08 r4: r3:c00a30c4
[] (generic_handle_irq) from [] (ipu_irq_handle+0xa8/0xd8)
[] (ipu_irq_handle) from [] (ipu_irq_handler+0x5c/0xb4)
 r8:ef008400 r7:0026 r6:ee4a3010 r5:c09e756c r4:ef1efc10
[] (ipu_irq_handler) from [] (generic_handle_irq+0x20/0x30)
 r6: r5: r4:c09d52d0
[] (generic_handle_irq) from [] 
(__handle_domain_irq+0x5c/0xb8)
[] (__handle_domain_irq) from [] (gic_handle_irq+0x4c/0x9c)
 r8:c0a38a78 r7:03eb r6:c09e0af0 r5:f400010c r4:f4000100 r3:ed501fb0
[] (gic_handle_irq) from [] (__irq_usr+0x58/0x80)
Exception stack(0xed501fb0 to 0xed501ff8)
1fa0: b698b4e0  0042c000 b698c708
1fc0: 0010 81231b10 81231b18 80e89670 b698b4e0 8114957c 7f79b000 81149438
1fe0: 7f79b248 bee08b98 7f708609 b6904220 600f0030 
 r10:7f79b000 r9:8114957c r8:10c5387d r7:10c5387d r6: r5:600f0030
 r4:b6904220 r3:ee47a4c0
[ cut here ]
WARNING: CPU: 0 PID: 1008 at 
/home/rmk/git/linux-rmk/drivers/staging/media/imx/imx-smfc.c:159 
imx_smfc_eof_interrupt+0x118/0x124 [imx_smfc]
Modules linked in: imx_csi(C) rfcomm bnep bluetooth nfsd imx_camif(C) imx_ic(C) 
imx_smfc(C) caam_jr snd_soc_imx_sgtl5000 uvcvideo snd_soc_fsl_asoc_card 
snd_soc_imx_spdif imx_media(C) imx_mipi_csi2(C) imx_media_common(C) 
snd_soc_imx_audmux imx219 snd_soc_sgtl5000 caam video_multiplexer imx_sdma 
imx2_wdt rc_cec snd_soc_fsl_ssi coda v4l2_mem2mem videobuf2_v4l2 
videobuf2_dma_contig videobuf2_core snd_soc_fsl_spdif imx_pcm_dma 
videobuf2_vmalloc dw_hdmi_ahb_audio dw_hdmi_cec videobuf2_memops imx_thermal 
etnaviv fuse rc_pinnacle_pctv_hd
CPU: 0 PID: 1008 Comm: Xorg Tainted: G C  4.10.0-rc6+ #2103
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Backtrace:
[] (dump_backtrace) from [] (show_stack+0x18/0x1c)
 r6:600f0193 r5: r4: r3:
[] (show_stack) from [] (dump_stack+0xa4/0xdc)
[] (dump_stack) from [] (__warn+0xdc/0x108)
 r6:bf12d004 r5: r4: r3:ee47a4c0
[] (__warn) from [] (warn_slowpath_null+0x28/0x30)
 r10:ed501e64 r8: r7:0139 r6:e98b2c10 r5:ee935dc4 r4:ee935c10
[] (warn_slowpath_null) from [] 
(imx_smfc_eof_interrupt+0x118/0x124 [imx_smfc])
[] (imx_smfc_eof_interrupt [imx_smfc]) from [] 
(__handle_irq_event_percpu+0xa4/0x428)
 

[PATCH v2] staging: rtl8192e: Aligning the * on each line in block comments

[Arushi]

This patch fixes the issue by aligning the * on each line in block comments.

Signed-off-by: Arushi 
---
 drivers/staging/speakup/fakekey.c| 10 +-
 drivers/staging/speakup/i18n.c   | 14 +++---
 drivers/staging/speakup/main.c   |  2 +-
 drivers/staging/speakup/speakup_acntsa.c |  2 +-
 drivers/staging/speakup/speakup_apollo.c |  2 +-
 drivers/staging/speakup/speakup_decext.c |  2 +-
 drivers/staging/speakup/speakup_decpc.c  |  4 ++--
 drivers/staging/speakup/speakup_dtlk.c   |  2 +-
 drivers/staging/speakup/speakup_dtlk.h   | 10 +-
 drivers/staging/speakup/speakup_ltlk.c   |  2 +-
 10 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/speakup/fakekey.c 
b/drivers/staging/speakup/fakekey.c
index 8f058b42f68d..d76da0a1382c 100644
--- a/drivers/staging/speakup/fakekey.c
+++ b/drivers/staging/speakup/fakekey.c
@@ -63,8 +63,8 @@ void speakup_remove_virtual_keyboard(void)
 }
 
 /*
-* Send a simulated down-arrow to the application.
-*/
+ * Send a simulated down-arrow to the application.
+ */
 void speakup_fake_down_arrow(void)
 {
unsigned long flags;
@@ -87,9 +87,9 @@ void speakup_fake_down_arrow(void)
 }
 
 /*
-* Are we handling a simulated keypress on the current CPU?
-* Returns a boolean.
-*/
+ * Are we handling a simulated keypress on the current CPU?
+ * Returns a boolean.
+ */
 bool speakup_fake_key_pressed(void)
 {
return this_cpu_read(reporting_keystroke);
diff --git a/drivers/staging/speakup/i18n.c b/drivers/staging/speakup/i18n.c
index 8960079e4d60..2f9b3df7f78d 100644
--- a/drivers/staging/speakup/i18n.c
+++ b/drivers/staging/speakup/i18n.c
@@ -401,7 +401,7 @@ char *spk_msg_get(enum msg_index_t index)
  * Finds the start of the next format specifier in the argument string.
  * Return value: pointer to start of format
  * specifier, or NULL if no specifier exists.
-*/
+ */
 static char *next_specifier(char *input)
 {
int found = 0;
@@ -450,7 +450,7 @@ static char *skip_width(char *input)
  * Note that this code only accepts a handful of conversion specifiers:
  * c d s x and ld.  Not accidental; these are exactly the ones used in
  * the default group of formatted messages.
-*/
+ */
 static char *skip_conversion(char *input)
 {
if ((input[0] == 'l') && (input[1] == 'd'))
@@ -463,7 +463,7 @@ static char *skip_conversion(char *input)
 /*
  * Function: find_specifier_end
  * Return a pointer to the end of the format specifier.
-*/
+ */
 static char *find_specifier_end(char *input)
 {
input++;/* Advance over %. */
@@ -478,7 +478,7 @@ static char *find_specifier_end(char *input)
  * Compare the format specifiers pointed to by *input1 and *input2.
  * Return 1 if they are the same, 0 otherwise.  Advance *input1 and *input2
  * so that they point to the character following the end of the specifier.
-*/
+ */
 static int compare_specifiers(char **input1, char **input2)
 {
int same = 0;
@@ -500,7 +500,7 @@ static int compare_specifiers(char **input1, char **input2)
  * Check that two format strings contain the same number of format specifiers,
  * and that the order of specifiers is the same in both strings.
  * Return 1 if the condition holds, 0 if it doesn't.
-*/
+ */
 static int fmt_validate(char *template, char *user)
 {
int valid = 1;
@@ -537,7 +537,7 @@ static int fmt_validate(char *template, char *user)
  * Failure conditions:
  * -EINVAL -  Invalid format specifiers in formatted message or illegal index.
  * -ENOMEM -  Unable to allocate memory.
-*/
+ */
 ssize_t spk_msg_set(enum msg_index_t index, char *text, size_t length)
 {
int rc = 0;
@@ -573,7 +573,7 @@ ssize_t spk_msg_set(enum msg_index_t index, char *text, 
size_t length)
 /*
  * Find a message group, given its name.  Return a pointer to the structure
  * if found, or NULL otherwise.
-*/
+ */
 struct msg_group_t *spk_find_msg_group(const char *group_name)
 {
struct msg_group_t *group = NULL;
diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index bf539d38..c2f70ef5b9b3 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -16,7 +16,7 @@
  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  *  GNU General Public License for more details.
-*/
+ */
 
 #include 
 #include 
diff --git a/drivers/staging/speakup/speakup_acntsa.c 
b/drivers/staging/speakup/speakup_acntsa.c
index b4e49cdd2014..b2e352712766 100644
--- a/drivers/staging/speakup/speakup_acntsa.c
+++ b/drivers/staging/speakup/speakup_acntsa.c
@@ -1,6 +1,6 @@
 /*
  * originally written by: Kirk Reiser 
-* this version considerably modified by David Borowski, david...@rogers.com
+ * this version considerably modified by David Borowski, david...@rogers.com
  *
  * Copyright (C) 1998-99  Kirk Reiser.
  

[PATCH] staging: lustre: fix coding style issue in vvp_page.c

[Zhengyi Shen]

This is a patch to fix "WARNING: line over 80 characters" found by
checkpatch.pl in vvp_page.c.

Signed-off-by: Zhengyi Shen 
---
 drivers/staging/lustre/lustre/llite/vvp_page.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/vvp_page.c 
b/drivers/staging/lustre/lustre/llite/vvp_page.c
index 23d6630..687c0c7 100644
--- a/drivers/staging/lustre/lustre/llite/vvp_page.c
+++ b/drivers/staging/lustre/lustre/llite/vvp_page.c
@@ -227,7 +227,8 @@ static int vvp_page_prep_write(const struct lu_env *env,
  * This takes inode as a separate argument, because inode on which error is to
  * be set can be different from \a vmpage inode in case of direct-io.
  */
-static void vvp_vmpage_error(struct inode *inode, struct page *vmpage, int 
ioret)
+static void vvp_vmpage_error(struct inode *inode, struct page *vmpage,
+int ioret)
 {
struct vvp_object *obj = cl_inode2vvp(inode);
 
-- 
2.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: fbtft: change 'gamma' array to u32

[Arnd Bergmann]

Having a local variable of 1024 bytes on 64-bit architectures is a bit
too much, and I ran into this warning while trying to see what functions
use the largest stack:

drivers/staging/fbtft/fbtft-sysfs.c: In function 'store_gamma_curve':
drivers/staging/fbtft/fbtft-sysfs.c:132:1: warning: the frame size of 1032 
bytes is larger than 1024 bytes [-Wframe-larger-than=]

As there is no need for 64-bit gamma values (on 32-bit architectures,
we don't use those either), I'm changing the type from 'unsigned long'
to 'u32' here, which cuts the required space in half everywhere.

Signed-off-by: Arnd Bergmann 
---
 drivers/staging/fbtft/fb_hx8340bn.c | 2 +-
 drivers/staging/fbtft/fb_hx8347d.c  | 2 +-
 drivers/staging/fbtft/fb_hx8353d.c  | 2 +-
 drivers/staging/fbtft/fb_ili9163.c  | 2 +-
 drivers/staging/fbtft/fb_ili9320.c  | 2 +-
 drivers/staging/fbtft/fb_ili9325.c  | 2 +-
 drivers/staging/fbtft/fb_ili9341.c  | 2 +-
 drivers/staging/fbtft/fb_pcd8544.c  | 2 +-
 drivers/staging/fbtft/fb_s6d1121.c  | 2 +-
 drivers/staging/fbtft/fb_ssd1289.c  | 2 +-
 drivers/staging/fbtft/fb_ssd1305.c  | 2 +-
 drivers/staging/fbtft/fb_ssd1306.c  | 2 +-
 drivers/staging/fbtft/fb_ssd1325.c  | 2 +-
 drivers/staging/fbtft/fb_ssd1331.c  | 2 +-
 drivers/staging/fbtft/fb_ssd1351.c  | 2 +-
 drivers/staging/fbtft/fb_st7735r.c  | 2 +-
 drivers/staging/fbtft/fb_st7789v.c  | 2 +-
 drivers/staging/fbtft/fb_tls8204.c  | 2 +-
 drivers/staging/fbtft/fbtft-core.c  | 2 +-
 drivers/staging/fbtft/fbtft-sysfs.c | 8 
 drivers/staging/fbtft/fbtft.h   | 4 ++--
 drivers/staging/fbtft/internal.h| 2 +-
 22 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/fbtft/fb_hx8340bn.c 
b/drivers/staging/fbtft/fb_hx8340bn.c
index c24331cc179c..1ca1fcd353d6 100644
--- a/drivers/staging/fbtft/fb_hx8340bn.c
+++ b/drivers/staging/fbtft/fb_hx8340bn.c
@@ -158,7 +158,7 @@ static int set_var(struct fbtft_par *par)
  *   ON0 ON1 CN0 CN1 CN2 CN3 CN4 MN0 MN1 MN2 MN3 MN4 MN5   GC
  */
 #define CURVE(num, idx)  curves[num * par->gamma.num_values + idx]
-static int set_gamma(struct fbtft_par *par, unsigned long *curves)
+static int set_gamma(struct fbtft_par *par, u32 *curves)
 {
unsigned long mask[] = {
0x0f, 0x0f, 0x1f, 0x0f, 0x0f, 0x0f, 0x1f, 0x07, 0x07, 0x07,
diff --git a/drivers/staging/fbtft/fb_hx8347d.c 
b/drivers/staging/fbtft/fb_hx8347d.c
index 450a61e3f99c..bbf78f8644a8 100644
--- a/drivers/staging/fbtft/fb_hx8347d.c
+++ b/drivers/staging/fbtft/fb_hx8347d.c
@@ -102,7 +102,7 @@ static void set_addr_win(struct fbtft_par *par, int xs, int 
ys, int xe, int ye)
  *   VRN0 VRN1 VRN2 VRN3 VRN4 VRN5 PRN0 PRN1 PKN0 PKN1 PKN2 PKN3 PKN4 CGM
  */
 #define CURVE(num, idx)  curves[num * par->gamma.num_values + idx]
-static int set_gamma(struct fbtft_par *par, unsigned long *curves)
+static int set_gamma(struct fbtft_par *par, u32 *curves)
 {
unsigned long mask[] = {
0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x7f, 0x7f, 0x1f, 0x1f,
diff --git a/drivers/staging/fbtft/fb_hx8353d.c 
b/drivers/staging/fbtft/fb_hx8353d.c
index 72e4ff8c5553..2c18051a44b3 100644
--- a/drivers/staging/fbtft/fb_hx8353d.c
+++ b/drivers/staging/fbtft/fb_hx8353d.c
@@ -118,7 +118,7 @@ static int set_var(struct fbtft_par *par)
 }
 
 /* gamma string format: */
-static int set_gamma(struct fbtft_par *par, unsigned long *curves)
+static int set_gamma(struct fbtft_par *par, u32 *curves)
 {
write_reg(par, 0xE0,
  curves[0], curves[1], curves[2], curves[3],
diff --git a/drivers/staging/fbtft/fb_ili9163.c 
b/drivers/staging/fbtft/fb_ili9163.c
index 6b8f8b17e9a3..579e17734612 100644
--- a/drivers/staging/fbtft/fb_ili9163.c
+++ b/drivers/staging/fbtft/fb_ili9163.c
@@ -202,7 +202,7 @@ static int set_var(struct fbtft_par *par)
 
 #ifdef GAMMA_ADJ
 #define CURVE(num, idx)  curves[num * par->gamma.num_values + idx]
-static int gamma_adj(struct fbtft_par *par, unsigned long *curves)
+static int gamma_adj(struct fbtft_par *par, u32 *curves)
 {
unsigned long mask[] = {
0x3F, 0x3F, 0x3F, 0x3F, 0x3F,
diff --git a/drivers/staging/fbtft/fb_ili9320.c 
b/drivers/staging/fbtft/fb_ili9320.c
index 278e4c7e95e5..20ba86da028b 100644
--- a/drivers/staging/fbtft/fb_ili9320.c
+++ b/drivers/staging/fbtft/fb_ili9320.c
@@ -221,7 +221,7 @@ static int set_var(struct fbtft_par *par)
  *  VRN0 VRN1 RN0 RN1 KN0 KN1 KN2 KN3 KN4 KN5
  */
 #define CURVE(num, idx)  curves[num * par->gamma.num_values + idx]
-static int set_gamma(struct fbtft_par *par, unsigned long *curves)
+static int set_gamma(struct fbtft_par *par, u32 *curves)
 {
unsigned long mask[] = {
0x1f, 0x1f, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07,
diff --git a/drivers/staging/fbtft/fb_ili9325.c 
b/drivers/staging/fbtft/fb_ili9325.c
index 19e33bab9cac..7189de5ae4b3 100644
--- a/drivers/staging/fbtft/fb_ili9325.c
+++ b/drivers/staging/fbtft/fb_ili9325.c
@@ -215,7 +215,7 @@ static int set_var(struct fbtft_par *par)
  *  

[PATCH] staging: rtl8192u: move stats_IndicateArray off stack

[Arnd Bergmann]

Putting 128 pointers on the stack is rather wasteful, in particular
on 64-bit architectures:

drivers/staging/rtl8192u/ieee80211/rtl819x_TSProc.c: In function 
'RxPktPendingTimeout':
drivers/staging/rtl8192u/ieee80211/rtl819x_TSProc.c:92:1: warning: the frame 
size of 1072 bytes is larger than 1024 bytes [-Wframe-larger-than=]

The rtl8192e driver has the exact same function, except that stores the
array in its 'ieee' structure. Let's do it the same way here for consistency.

Signed-off-by: Arnd Bergmann 
---
 drivers/staging/rtl8192u/ieee80211/ieee80211.h  | 1 +
 drivers/staging/rtl8192u/ieee80211/rtl819x_TSProc.c | 5 ++---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h 
b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
index 17e985a1babf..10bb23739748 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
@@ -1894,6 +1894,7 @@ struct ieee80211_device {
//u32 STA_EDCA_PARAM[4];
//CHANNEL_ACCESS_SETTING ChannelAccessSetting;
 
+   struct ieee80211_rxb *stats_IndicateArray[REORDER_WIN_SIZE];
 
/* Callback functions */
void (*set_security)(struct net_device *dev,
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_TSProc.c 
b/drivers/staging/rtl8192u/ieee80211/rtl819x_TSProc.c
index 6033502eff3d..b9ff8fec2edf 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_TSProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_TSProc.c
@@ -31,7 +31,6 @@ static void RxPktPendingTimeout(unsigned long data)
 
//u32 flags = 0;
unsigned long flags = 0;
-   struct ieee80211_rxb *stats_IndicateArray[REORDER_WIN_SIZE];
u8 index = 0;
bool bPktInBuf = false;
 
@@ -55,7 +54,7 @@ static void RxPktPendingTimeout(unsigned long data)
pRxTs->RxIndicateSeq = 
(pRxTs->RxIndicateSeq + 1) % 4096;
 

IEEE80211_DEBUG(IEEE80211_DL_REORDER,"RxPktPendingTimeout(): IndicateSeq: 
%d\n", pReorderEntry->SeqNum);
-   stats_IndicateArray[index] = 
pReorderEntry->prxb;
+   ieee->stats_IndicateArray[index] = 
pReorderEntry->prxb;
index++;
 
list_add_tail(>List, 
>RxReorder_Unused_List);
@@ -79,7 +78,7 @@ static void RxPktPendingTimeout(unsigned long data)
spin_unlock_irqrestore(&(ieee->reorder_spinlock), 
flags);
return;
}
-   ieee80211_indicate_packets(ieee, stats_IndicateArray, index);
+   ieee80211_indicate_packets(ieee, ieee->stats_IndicateArray, 
index);
}
 
if(bPktInBuf && (pRxTs->RxTimeoutIndicateSeq==0x))
-- 
2.9.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging/emxx_udc: Fix styling issues

[Zhengyi Shen]

Fix line over 80 characters.
Statements longer than 80 columns were broken into sensible chunks.
Descendants in four different palces were in the same uniform style.

Signed-off-by: Zhengyi Shen 
---
 drivers/staging/emxx_udc/emxx_udc.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c 
b/drivers/staging/emxx_udc/emxx_udc.c
index 77b242e..e771aee 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -902,7 +902,8 @@ static int _nbu2ss_epn_out_pio(
/* Copy of every four bytes */
for (i = 0; i < iWordLength; i++) {
pBuf32->dw =
-   _nbu2ss_readl(>EP_REGS[ep->epnum - 1].EP_READ);
+   _nbu2ss_readl(
+   >EP_REGS[ep->epnum - 1].EP_READ);
pBuf32++;
}
result = iWordLength * sizeof(u32);
@@ -912,7 +913,8 @@ static int _nbu2ss_epn_out_pio(
if (data > 0) {
/*-*/
/* Copy of fraction byte */
-   Temp32.dw = _nbu2ss_readl(>EP_REGS[ep->epnum - 
1].EP_READ);
+   Temp32.dw =
+   _nbu2ss_readl(>EP_REGS[ep->epnum - 1].EP_READ);
for (i = 0 ; i < data ; i++)
pBuf32->byte.DATA[i] = Temp32.byte.DATA[i];
result += data;
@@ -977,8 +979,8 @@ static int _nbu2ss_epn_out_transfer(
 
/*-*/
/* Receive Length */
-   iRecvLength
-   = _nbu2ss_readl(>EP_REGS[num].EP_LEN_DCNT) & EPn_LDATA;
+   iRecvLength =
+   _nbu2ss_readl(>EP_REGS[num].EP_LEN_DCNT) & EPn_LDATA;
 
if (iRecvLength != 0) {
result = _nbu2ss_epn_out_data(udc, ep, req, iRecvLength);
@@ -2651,7 +2653,8 @@ static int nbu2ss_ep_queue(
}
 
req = container_of(_req, struct nbu2ss_req, req);
-   if (unlikely(!_req->complete || !_req->buf || 
!list_empty(>queue))) {
+   if (unlikely(!_req->complete || !_req->buf
+   || !list_empty(>queue))) {
if (!_req->complete)
pr_err("udc: %s --- !_req->complete\n", __func__);
 
-- 
2.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 05/10] staging: unisys: visorbus: Clarify reason for pointer check in bus_destroy()

[Sell, Timothy C]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Thursday, February 02, 2017 7:14 AM
> To: Kershner, David A 
> Cc: driverdev-devel@linuxdriverproject.org; *S-Par-Maintainer
> ; jes.soren...@gmail.com; Binder, David
> Anthony 
> Subject: Re: [PATCH 05/10] staging: unisys: visorbus: Clarify reason for 
> pointer
> check in bus_destroy()
> 
> On Wed, Feb 01, 2017 at 05:38:57PM -0500, David Kershner wrote:
> > From: David Binder 
> >
> > Clarifies why the pointer returned from visorbus_get_device_by_id() in
> > bus_destroy() is validated. The check is performed in order to be extra
> > careful, for the sake of added security, that the s-Par backend is
> > providing us with a valid bus/device pair.
> >
> > Signed-off-by: David Binder 
> > Signed-off-by: David Kershner 
> > Reported-by: Greg Kroah-Hartman 
> > ---
> >  drivers/staging/unisys/visorbus/visorchipset.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/staging/unisys/visorbus/visorchipset.c
> b/drivers/staging/unisys/visorbus/visorchipset.c
> > index c90ea6a..2d1b226 100644
> > --- a/drivers/staging/unisys/visorbus/visorchipset.c
> > +++ b/drivers/staging/unisys/visorbus/visorchipset.c
> > @@ -755,6 +755,7 @@ bus_destroy(struct controlvm_message *inmsg)
> > int err;
> >
> > bus_info = visorbus_get_device_by_id(bus_no, BUS_ROOT_DEVICE,
> NULL);
> > +   /* Validate that s-Par backend gave a good bus */
> 
> I don't remember what I said in my review, but this comment is pretty
> useless.
> 
> I guess my point is, how could BUS_ROOT_DEVICE ever NOT be a valid
> device on the bus?  What would have made it go away?

We are essentially validating "bus_no" here, whose value we just received
from the s-Par backend via a message.  If bus_no is invalid, i.e., we have
no record of a bus with that number ever being created, then
visorbus_get_device_by_id() will return NULL.
 
> 
> Comments like this don't make much sense, maybe my review didn't either
> :)
> 
> thanks,
> 
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] [media] staging: bcm2835: mark all symbols as 'static'

[Arnd Bergmann]

On Thu, Feb 2, 2017 at 1:22 PM, Greg Kroah-Hartman
 wrote:
> On Thu, Feb 02, 2017 at 01:11:36PM +0100, Arnd Bergmann wrote:
>> On Thu, Feb 2, 2017 at 1:04 PM, Arnd Bergmann  wrote:
>> > On Thu, Feb 2, 2017 at 12:34 PM, Arnd Bergmann  wrote:
>> >> I got a link error in allyesconfig:
>> >> Fixes: 7b3ad5abf027 ("staging: Import the BCM2835 MMAL-based V4L2 camera 
>> >> driver.")
>> >> Signed-off-by: Arnd Bergmann 
>> >
>> > Please disregard this patch version, it's broken.
>>
>> Too late, I see it's already applied, I'll send a follow-up to revert
>> the first hunk.
>
> Ah, I could have just dropped your patch (it's a testing branch that I
> can rebase), but I took your newer patch that fixed it up, so all is
> good.
>
> That's what I get for applying patches too quickly :)

I should really have been more careful about testing. I had the first
version in my
working tree while doing randconfig tests. None of the new randconfig builds
ran into the issue (the driver gets rarely enabled because of its dependencies),
and the original failure had already been marked as fixed in my build system
after an earlier patch only changed one of the prototypes.

Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] [media] staging: bcm2835: mark all symbols as 'static'

[Greg Kroah-Hartman]

On Thu, Feb 02, 2017 at 01:11:36PM +0100, Arnd Bergmann wrote:
> On Thu, Feb 2, 2017 at 1:04 PM, Arnd Bergmann  wrote:
> > On Thu, Feb 2, 2017 at 12:34 PM, Arnd Bergmann  wrote:
> >> I got a link error in allyesconfig:
> >> Fixes: 7b3ad5abf027 ("staging: Import the BCM2835 MMAL-based V4L2 camera 
> >> driver.")
> >> Signed-off-by: Arnd Bergmann 
> >
> > Please disregard this patch version, it's broken.
> 
> Too late, I see it's already applied, I'll send a follow-up to revert
> the first hunk.

Ah, I could have just dropped your patch (it's a testing branch that I
can rebase), but I took your newer patch that fixed it up, so all is
good.

That's what I get for applying patches too quickly :)

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: bcm2835: don't mark 'bcm2835_v4l2_debug' as static

[Arnd Bergmann]

This one unfortunately slipped through my own build testing, my patch
caused a new build error:

bcm2835-camera.c:53:12: error: static declaration of 'bcm2835_v4l2_debug' 
follows non-static declaration

We want the symbol to be global as it is indeed used in more than one
file and declared 'extern' in a header.

Fixes: 757b9bd074 ("staging: bcm2835: mark all symbols as 'static'")
Signed-off-by: Arnd Bergmann 
---
 drivers/staging/media/platform/bcm2835/bcm2835-camera.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c 
b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
index ced8eb5de0f0..ca15a698e018 100644
--- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
+++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
@@ -50,7 +50,7 @@ MODULE_AUTHOR("Vincent Sanders");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(BM2835_MMAL_VERSION);
 
-static int bcm2835_v4l2_debug;
+int bcm2835_v4l2_debug;
 module_param_named(debug, bcm2835_v4l2_debug, int, 0644);
 MODULE_PARM_DESC(bcm2835_v4l2_debug, "Debug level 0-2");
 
-- 
2.9.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 00/10] staging: unisys: additional error handling and channel cleanups.

[Greg KH]

On Wed, Feb 01, 2017 at 05:38:52PM -0500, David Kershner wrote:
> This series fixes a problem where we were using a Linux specific data
> structure in our s-Par channel. These channels are shared across multiple
> OS types and should not use OS specific structures unless absolutely
> needed. It also adds some additional error handling and other cleanup.
> 
> This patchset also introduces some changes being made to the functions that
> invoke the uevents that occur on chipset_ready/selftest/notready messages
> from the s-Par firmware. This simplifies the functions so that we can
> tackle more challenging changes in later patches.

I've applied most of these.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/2] staging: rts5208: remove redundant retval status check

[Colin King]

From: Colin Ian King 

The retval status checks in the proceeding do loop return out
of function ms_read_attritbute_info if there is an error
condition,  thus we never reach the end of the loop with
retval failed status.  Therefore, the retval status check
at end of the do loop is redundant and can be removed.

Detected with CoverityScan, CID#143000 ("Logically dead code")

Signed-off-by: Colin Ian King 
---
 drivers/staging/rts5208/ms.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/staging/rts5208/ms.c b/drivers/staging/rts5208/ms.c
index 28d56c5..806c121 100644
--- a/drivers/staging/rts5208/ms.c
+++ b/drivers/staging/rts5208/ms.c
@@ -1108,12 +1108,6 @@ static int ms_read_attribute_info(struct rtsx_chip *chip)
i++;
} while (i < 1024);
 
-   if (retval != STATUS_SUCCESS) {
-   kfree(buf);
-   rtsx_trace(chip);
-   return STATUS_FAIL;
-   }
-
if ((buf[0] != 0xa5) && (buf[1] != 0xc3)) {
/* Signature code is wrong */
kfree(buf);
-- 
2.10.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 10/10] staging: unisys: visorbus: Clarify reason for device pointer checks

[Greg KH]

On Wed, Feb 01, 2017 at 05:39:02PM -0500, David Kershner wrote:
> From: David Binder 
> 
> Clarifies why the pointers returned from visorbus_get_device_by_id() in
> visorbus are validated. The check is performed in order to be extra
> careful, for the sake of added security, that the s-Par backend is
> providing us with a valid device.
> 
> Signed-off-by: David Binder 
> Signed-off-by: David Kershner 
> ---
>  drivers/staging/unisys/visorbus/visorchipset.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/unisys/visorbus/visorchipset.c 
> b/drivers/staging/unisys/visorbus/visorchipset.c
> index 188e224..07f8d05 100644
> --- a/drivers/staging/unisys/visorbus/visorchipset.c
> +++ b/drivers/staging/unisys/visorbus/visorchipset.c
> @@ -897,6 +897,7 @@ my_device_changestate(struct controlvm_message *inmsg)
>   int err;
>  
>   dev_info = visorbus_get_device_by_id(bus_no, dev_no, NULL);
> + /* Validate that s-Par backend gave a good device */

Same comment here as the previous review, this comment really isn't
needed, right?

>   if (!dev_info) {
>   POSTCODE_LINUX(DEVICE_CHANGESTATE_FAILURE_PC, dev_no, bus_no,
>  DIAG_SEVERITY_ERR);
> @@ -957,6 +958,7 @@ my_device_destroy(struct controlvm_message *inmsg)
>   int err;
>  
>   dev_info = visorbus_get_device_by_id(bus_no, dev_no, NULL);
> + /* Validate that s-Par backend gave a good device */

Or here.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 09/10] staging: unisys: visorbus: Clarify reason for bus pointer checks

[Greg KH]

On Wed, Feb 01, 2017 at 05:39:01PM -0500, David Kershner wrote:
> From: David Binder 
> 
> Clarifies why the pointers returned from visorbus_get_device_by_id() in
> visorbus are validated. The check is performed in order to be extra
> careful, for the sake of added security, that the s-Par backend is
> providing us with a valid bus.
> 
> Signed-off-by: David Binder 
> Reported-by: Greg Kroah-Hartman 
> Signed-off-by: David Kershner 
> ---
>  drivers/staging/unisys/visorbus/visorbus_main.c | 1 +
>  drivers/staging/unisys/visorbus/visorchipset.c  | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c 
> b/drivers/staging/unisys/visorbus/visorbus_main.c
> index aea1aa2..4a11c89 100644
> --- a/drivers/staging/unisys/visorbus/visorbus_main.c
> +++ b/drivers/staging/unisys/visorbus/visorbus_main.c
> @@ -800,6 +800,7 @@ fix_vbus_dev_info(struct visor_device *visordev)
>   return;
>  
>   bdev = visorbus_get_device_by_id(bus_no, BUS_ROOT_DEVICE, NULL);
> + /* Validate that s-Par backend gave a good bus */

Same as the other reviews...

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 05/10] staging: unisys: visorbus: Clarify reason for pointer check in bus_destroy()

[Greg KH]

On Wed, Feb 01, 2017 at 05:38:57PM -0500, David Kershner wrote:
> From: David Binder 
> 
> Clarifies why the pointer returned from visorbus_get_device_by_id() in
> bus_destroy() is validated. The check is performed in order to be extra
> careful, for the sake of added security, that the s-Par backend is
> providing us with a valid bus/device pair.
> 
> Signed-off-by: David Binder 
> Signed-off-by: David Kershner 
> Reported-by: Greg Kroah-Hartman 
> ---
>  drivers/staging/unisys/visorbus/visorchipset.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/unisys/visorbus/visorchipset.c 
> b/drivers/staging/unisys/visorbus/visorchipset.c
> index c90ea6a..2d1b226 100644
> --- a/drivers/staging/unisys/visorbus/visorchipset.c
> +++ b/drivers/staging/unisys/visorbus/visorchipset.c
> @@ -755,6 +755,7 @@ bus_destroy(struct controlvm_message *inmsg)
>   int err;
>  
>   bus_info = visorbus_get_device_by_id(bus_no, BUS_ROOT_DEVICE, NULL);
> + /* Validate that s-Par backend gave a good bus */

I don't remember what I said in my review, but this comment is pretty
useless.

I guess my point is, how could BUS_ROOT_DEVICE ever NOT be a valid
device on the bus?  What would have made it go away?

Comments like this don't make much sense, maybe my review didn't either
:)

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/2] staging: rts5208: remove unncessary result set and check, just return SUCCESS

[Colin King]

From: Colin Ian King 

Minor clean up, there is no need to assign result to zero, then
check if it is less than zero. Just return SUCCESS.

Signed-off-by: Colin Ian King 
---
 drivers/staging/rts5208/rtsx.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rts5208/rtsx.c b/drivers/staging/rts5208/rtsx.c
index 68d75d0..b8177f5 100644
--- a/drivers/staging/rts5208/rtsx.c
+++ b/drivers/staging/rts5208/rtsx.c
@@ -198,23 +198,21 @@ static int command_abort(struct scsi_cmnd *srb)
  */
 static int device_reset(struct scsi_cmnd *srb)
 {
-   int result = 0;
struct rtsx_dev *dev = host_to_rtsx(srb->device->host);
 
dev_info(>pci->dev, "%s called\n", __func__);
 
-   return result < 0 ? FAILED : SUCCESS;
+   return SUCCESS;
 }
 
 /* Simulate a SCSI bus reset by resetting the device's USB port. */
 static int bus_reset(struct scsi_cmnd *srb)
 {
-   int result = 0;
struct rtsx_dev *dev = host_to_rtsx(srb->device->host);
 
dev_info(>pci->dev, "%s called\n", __func__);
 
-   return result < 0 ? FAILED : SUCCESS;
+   return SUCCESS;
 }
 
 /*
-- 
2.10.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] [media] staging: bcm2835: mark all symbols as 'static'

[Arnd Bergmann]

On Thu, Feb 2, 2017 at 1:04 PM, Arnd Bergmann  wrote:
> On Thu, Feb 2, 2017 at 12:34 PM, Arnd Bergmann  wrote:
>> I got a link error in allyesconfig:
>> Fixes: 7b3ad5abf027 ("staging: Import the BCM2835 MMAL-based V4L2 camera 
>> driver.")
>> Signed-off-by: Arnd Bergmann 
>
> Please disregard this patch version, it's broken.

Too late, I see it's already applied, I'll send a follow-up to revert
the first hunk.

Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/10] staging: unisys: include: fix improper use of dma_data_direction

[Greg KH]

On Wed, Feb 01, 2017 at 05:38:53PM -0500, David Kershner wrote:
> From: Steven Matthews 
> 
> Replace use of standard Linux dma_data_direction with a Unisys-
> specific uis_dma_data_direction and provide a function to convert
> from the latter to the former.  This is necessary because Unisys
> s-Par depends on the exact format of this field in multiple OSs
> and languages, and so using the standard version creates an
> unnecessary dependency between the kernel and s-Par.
> 
> Signed-off-by: Steven Matthews 
> Signed-off-by: David Kershner 
> ---
>  drivers/staging/unisys/include/iochannel.h  | 11 +++--
>  drivers/staging/unisys/visorhba/visorhba_main.c | 22 +-
>  2 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/unisys/include/iochannel.h 
> b/drivers/staging/unisys/include/iochannel.h
> index 54f4900..6e80462 100644
> --- a/drivers/staging/unisys/include/iochannel.h
> +++ b/drivers/staging/unisys/include/iochannel.h
> @@ -31,7 +31,6 @@
>  
>  #include 
>  
> -#include 
>  #include "channel.h"
>  
>  #define ULTRA_VHBA_CHANNEL_PROTOCOL_SIGNATURE 
> ULTRA_CHANNEL_PROTOCOL_SIGNATURE
> @@ -80,6 +79,14 @@
>  /* Size of cdb - i.e., SCSI cmnd */
>  #define MAX_CMND_SIZE 16
>  
> +/* Unisys-specific DMA direction values */
> +enum uis_dma_data_direction {
> + UIS_DMA_BIDIRECTIONAL = 0,
> + UIS_DMA_TO_DEVICE,
> + UIS_DMA_FROM_DEVICE,
> + UIS_DMA_NONE

An enumerated type that is shared across platforms yet you don't
specifically set what each type is?  That's just begging for nasty bugs
in the future.

Please set all of them so you _know_ what the values are, never trust a
C compiler you haven't written yourself :)

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rtl8192u: Adding a blank line after declarations

[Greg KH]

On Thu, Feb 02, 2017 at 01:21:13AM +0530, simran singhal wrote:
> This patch fixes the checkpatch warning by adding a blank line after
> declarations.
> WARNING: Missing a blank line after declarations
> 
> Signed-off-by: simran singhal 
> ---
>  drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 9 +
>  1 file changed, 9 insertions(+)

You sent me two different patches, with the same subject line, for the
same file.  Which one is which?  I can't take these, please resend both
of them, with unique subjects, as a patch series.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] [media] staging: bcm2835: mark all symbols as 'static'

[Arnd Bergmann]

On Thu, Feb 2, 2017 at 12:34 PM, Arnd Bergmann  wrote:
> I got a link error in allyesconfig:
>
> drivers/staging/media/platform/bcm2835/bcm2835-camera.o: In function 
> `vidioc_enum_framesizes':
> bcm2835-camera.c:(.text.vidioc_enum_framesizes+0x0): multiple definition of 
> `vidioc_enum_framesizes'
> drivers/media/platform/vivid/vivid-vid-cap.o:vivid-vid-cap.c:(.text.vidioc_enum_framesizes+0x0):
>  first defined here
>
> While both drivers are equally at fault for this problem, the bcm2835 one was
> just added and is easier to fix, as it is only one file, and none of its 
> symbols
> need to be globally visible. This marks the three global symbols as static.
>
> Fixes: 7b3ad5abf027 ("staging: Import the BCM2835 MMAL-based V4L2 camera 
> driver.")
> Signed-off-by: Arnd Bergmann 

Please disregard this patch version, it's broken.

> @@ -50,7 +50,7 @@ MODULE_AUTHOR("Vincent Sanders");
>  MODULE_LICENSE("GPL");
>  MODULE_VERSION(BM2835_MMAL_VERSION);
>
> -int bcm2835_v4l2_debug;
> +static int bcm2835_v4l2_debug;
>  module_param_named(debug, bcm2835_v4l2_debug, int, 0644);
>  MODULE_PARM_DESC(bcm2835_v4l2_debug, "Debug level 0-2");
>

This symbol is in fact used in more than one file.

Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Philipp Zabel]

On Fri, 2017-01-06 at 18:11 -0800, Steve Longerbeam wrote:
> Adds MIPI CSI-2 Receiver subdev driver. This subdev is required
> for sensors with a MIPI CSI2 interface.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/Makefile|   1 +
>  drivers/staging/media/imx/imx-mipi-csi2.c | 501 
> ++
>  2 files changed, 502 insertions(+)
>  create mode 100644 drivers/staging/media/imx/imx-mipi-csi2.c
> 
> diff --git a/drivers/staging/media/imx/Makefile 
> b/drivers/staging/media/imx/Makefile
> index fe9e992..0decef7 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-ic.o
>  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
>  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o
>  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o
> +obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-mipi-csi2.o
> diff --git a/drivers/staging/media/imx/imx-mipi-csi2.c 
> b/drivers/staging/media/imx/imx-mipi-csi2.c
> new file mode 100644
> index 000..daa6e1d
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-mipi-csi2.c
> @@ -0,0 +1,501 @@
> +/*
> + * MIPI CSI-2 Receiver Subdev for Freescale i.MX5/6 SOC.
> + *
> + * Copyright (c) 2012-2014 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "imx-media.h"
> +
> +/*
> + * there must be 5 pads: 1 input pad from sensor, and
> + * the 4 virtual channel output pads
> + */
> +#define CSI2_NUM_SINK_PADS  1
> +#define CSI2_NUM_SRC_PADS   4
> +#define CSI2_NUM_PADS   5
> +
> +struct imxcsi2_dev {
> + struct device  *dev;
> + struct imx_media_dev   *md;
> + struct v4l2_subdev  sd;
> + struct media_pad   pad[CSI2_NUM_PADS];
> + struct v4l2_mbus_framefmt format_mbus;
> + struct v4l2_subdev *src_sd;
> + struct v4l2_subdev *sink_sd[CSI2_NUM_SRC_PADS];

I see no reason to store pointers to the remote v4l2_subdevs.

> + intinput_pad;
> + struct clk *dphy_clk;
> + struct clk *cfg_clk;
> + struct clk *pix_clk; /* what is this? */
> + void __iomem   *base;
> + int intr1;
> + int intr2;

The interrupts are not used, I'd remove them and the dead code in
_probe.

> + struct v4l2_of_bus_mipi_csi2 bus;
> + boolon;
> + boolstream_on;
> +};
> +
> +#define DEVICE_NAME "imx6-mipi-csi2"
> +
> +/* Register offsets */
> +#define CSI2_VERSION0x000
> +#define CSI2_N_LANES0x004
> +#define CSI2_PHY_SHUTDOWNZ  0x008
> +#define CSI2_DPHY_RSTZ  0x00c
> +#define CSI2_RESETN 0x010
> +#define CSI2_PHY_STATE  0x014
> +#define CSI2_DATA_IDS_1 0x018
> +#define CSI2_DATA_IDS_2 0x01c
> +#define CSI2_ERR1   0x020
> +#define CSI2_ERR2   0x024
> +#define CSI2_MSK1   0x028
> +#define CSI2_MSK2   0x02c
> +#define CSI2_PHY_TST_CTRL0  0x030
> +#define CSI2_PHY_TST_CTRL1  0x034
> +#define CSI2_SFT_RESET  0xf00
> +
> +static inline struct imxcsi2_dev *sd_to_dev(struct v4l2_subdev *sdev)
> +{
> + return container_of(sdev, struct imxcsi2_dev, sd);
> +}
> +
> +static inline u32 imxcsi2_read(struct imxcsi2_dev *csi2, unsigned int regoff)
> +{
> + return readl(csi2->base + regoff);
> +}
> +
> +static inline void imxcsi2_write(struct imxcsi2_dev *csi2, u32 val,
> +  unsigned int regoff)
> +{
> + writel(val, csi2->base + regoff);
> +}

Do those two wrappers really make the code more readable?

> +static void imxcsi2_set_lanes(struct imxcsi2_dev *csi2)
> +{
> + int lanes = csi2->bus.num_data_lanes;
> +
> + imxcsi2_write(csi2, lanes - 1, CSI2_N_LANES);
> +}
> +
> +static void imxcsi2_enable(struct imxcsi2_dev *csi2, bool enable)
> +{
> + if (enable) {
> + imxcsi2_write(csi2, 0x, CSI2_PHY_SHUTDOWNZ);
> + imxcsi2_write(csi2, 0x, CSI2_DPHY_RSTZ);
> + imxcsi2_write(csi2, 0x, CSI2_RESETN);

Given that these registers only contain a single bit, and bits 31:1 are
documented as reserved, 0, I think these should write 1 instead of
0x.

> + } else {
> + imxcsi2_write(csi2, 0x0, CSI2_PHY_SHUTDOWNZ);
> + imxcsi2_write(csi2, 0x0, CSI2_DPHY_RSTZ);
> + imxcsi2_write(csi2, 0x0, CSI2_RESETN);
> + }
> +}
> +
> +static void imxcsi2_reset(struct imxcsi2_dev *csi2)
> +{
> + 

[PATCH] [media] staging: bcm2835: mark all symbols as 'static'

[Arnd Bergmann]

I got a link error in allyesconfig:

drivers/staging/media/platform/bcm2835/bcm2835-camera.o: In function 
`vidioc_enum_framesizes':
bcm2835-camera.c:(.text.vidioc_enum_framesizes+0x0): multiple definition of 
`vidioc_enum_framesizes'
drivers/media/platform/vivid/vivid-vid-cap.o:vivid-vid-cap.c:(.text.vidioc_enum_framesizes+0x0):
 first defined here

While both drivers are equally at fault for this problem, the bcm2835 one was
just added and is easier to fix, as it is only one file, and none of its symbols
need to be globally visible. This marks the three global symbols as static.

Fixes: 7b3ad5abf027 ("staging: Import the BCM2835 MMAL-based V4L2 camera 
driver.")
Signed-off-by: Arnd Bergmann 
---
 drivers/staging/media/platform/bcm2835/bcm2835-camera.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c 
b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
index 105d88102cd9..ced8eb5de0f0 100644
--- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
+++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
@@ -50,7 +50,7 @@ MODULE_AUTHOR("Vincent Sanders");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(BM2835_MMAL_VERSION);
 
-int bcm2835_v4l2_debug;
+static int bcm2835_v4l2_debug;
 module_param_named(debug, bcm2835_v4l2_debug, int, 0644);
 MODULE_PARM_DESC(bcm2835_v4l2_debug, "Debug level 0-2");
 
@@ -1312,7 +1312,7 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void 
*priv,
return ret;
 }
 
-int vidioc_enum_framesizes(struct file *file, void *fh,
+static int vidioc_enum_framesizes(struct file *file, void *fh,
   struct v4l2_frmsizeenum *fsize)
 {
struct bm2835_mmal_dev *dev = video_drvdata(file);
@@ -1842,7 +1842,7 @@ static int __init bm2835_mmal_init_device(struct 
bm2835_mmal_dev *dev,
return 0;
 }
 
-void bcm2835_cleanup_instance(struct bm2835_mmal_dev *dev)
+static void bcm2835_cleanup_instance(struct bm2835_mmal_dev *dev)
 {
if (!dev)
return;
-- 
2.9.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: lustre: remove CLASSERT macro

[Arnd Bergmann]

lustre uses a fake switch() statement as a compile-time assert, but 
unfortunately
each use of that causes a warning when building with clang:

drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c:2907:2: warning: no case 
matching constant switch condition '42'
drivers/staging/lustre/lnet/klnds/socklnd/../../../include/linux/libcfs/libcfs_private.h:294:36:
 note: expanded from macro 'CLASSERT'
 #define CLASSERT(cond) do {switch (42) {case (cond): case 0: break; } } while 
(0)

As Greg suggested, let's just kill off this macro completely instead of
fixing it. This replaces it with BUILD_BUG_ON(), which means we have
to negate all the conditions in the process.

Signed-off-by: Arnd Bergmann 
---
 .../lustre/include/linux/libcfs/libcfs_private.h   |  16 --
 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c|  16 +-
 .../staging/lustre/lnet/klnds/socklnd/socklnd.c|   4 +-
 .../staging/lustre/lnet/klnds/socklnd/socklnd_cb.c |   2 +-
 .../lustre/lnet/klnds/socklnd/socklnd_proto.c  |   2 +-
 drivers/staging/lustre/lnet/libcfs/hash.c  |   2 +-
 drivers/staging/lustre/lnet/lnet/acceptor.c|   4 +-
 drivers/staging/lustre/lnet/lnet/api-ni.c  | 134 +++---
 drivers/staging/lustre/lnet/lnet/lib-socket.c  |   2 +-
 drivers/staging/lustre/lnet/lnet/net_fault.c   |   8 +-
 drivers/staging/lustre/lnet/lnet/router_proc.c |   4 +-
 drivers/staging/lustre/lustre/include/cl_object.h  |   2 +-
 drivers/staging/lustre/lustre/include/lu_object.h  |   2 +-
 drivers/staging/lustre/lustre/include/lustre_net.h |   4 +-
 drivers/staging/lustre/lustre/ldlm/ldlm_request.c  |   2 +-
 drivers/staging/lustre/lustre/ldlm/ldlm_resource.c |   2 +-
 drivers/staging/lustre/lustre/llite/super25.c  |   2 +-
 drivers/staging/lustre/lustre/llite/vvp_dev.c  |   2 +-
 drivers/staging/lustre/lustre/lov/lov_pack.c   |   6 +-
 drivers/staging/lustre/lustre/mdc/mdc_lib.c|  12 +-
 drivers/staging/lustre/lustre/mdc/mdc_locks.c  |   2 +-
 drivers/staging/lustre/lustre/mdc/mdc_request.c|   2 +-
 drivers/staging/lustre/lustre/osc/osc_io.c |   2 +-
 drivers/staging/lustre/lustre/osc/osc_request.c|  16 +-
 drivers/staging/lustre/lustre/ptlrpc/client.c  |   4 +-
 drivers/staging/lustre/lustre/ptlrpc/import.c  |   2 +-
 .../staging/lustre/lustre/ptlrpc/pack_generic.c| 102 +--
 drivers/staging/lustre/lustre/ptlrpc/pers.c|   2 +-
 drivers/staging/lustre/lustre/ptlrpc/wiretest.c| 194 ++---
 29 files changed, 269 insertions(+), 285 deletions(-)

diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h 
b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
index aab15d8112a4..2dae85798ec1 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
@@ -277,22 +277,6 @@ do {   
\
 #define CFS_ALLOC_PTR(ptr)  LIBCFS_ALLOC(ptr, sizeof(*(ptr)))
 #define CFS_FREE_PTR(ptr)   LIBCFS_FREE(ptr, sizeof(*(ptr)))
 
-/** Compile-time assertion.
- *
- * Check an invariant described by a constant expression at compile time by
- * forcing a compiler error if it does not hold.  \a cond must be a constant
- * expression as defined by the ISO C Standard:
- *
- *   6.8.4.2  The switch statement
- *   
- *   [#3] The expression of each case label shall be  an  integer
- *   constant   expression  and  no  two  of  the  case  constant
- *   expressions in the same switch statement shall have the same
- *   value  after  conversion...
- *
- */
-#define CLASSERT(cond) do {switch (42) {case (cond): case 0: break; } } while 
(0)
-
 /* max value for numeric network address */
 #define MAX_NUMERIC_VALUE 0x
 
diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c 
b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
index 7f761b327166..b1e8508f9fc7 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
@@ -258,8 +258,8 @@ int kiblnd_unpack_msg(struct kib_msg *msg, int nob)
if (flip) {
/* leave magic unflipped as a clue to peer endianness */
msg->ibm_version = version;
-   CLASSERT(sizeof(msg->ibm_type) == 1);
-   CLASSERT(sizeof(msg->ibm_credits) == 1);
+   BUILD_BUG_ON(sizeof(msg->ibm_type) != 1);
+   BUILD_BUG_ON(sizeof(msg->ibm_credits) != 1);
msg->ibm_nob = msg_nob;
__swab64s(>ibm_srcnid);
__swab64s(>ibm_srcstamp);
@@ -1247,10 +1247,10 @@ static void kiblnd_map_tx_pool(struct kib_tx_pool *tpo)
dev = net->ibn_dev;
 
/* pre-mapped messages are not bigger than 1 page */
-   CLASSERT(IBLND_MSG_SIZE <= PAGE_SIZE);
+   BUILD_BUG_ON(IBLND_MSG_SIZE > PAGE_SIZE);
 
/* No fancy arithmetic when we do 

Re: [PATCH] staging: lustre: shut up clang warnings on CLASSERT()

[Arnd Bergmann]

On Feb 2, 2017 11:43 AM, "Greg Kroah-Hartman"
 wrote:
> On Thu, Feb 02, 2017 at 11:40:32AM +0100, Arnd Bergmann wrote:
> > I've done a semi-automated patch to replace CLASSERT() with the respective
> > BUILD_BUG_ON() now, but that patch is quite large.
>
> Should be easy to script, I missed that this was a build-time check.
> Heck, I'll take a script to do this, or I can just do it in my end

My patch has been through a couple of randconfig and allmodconfig builds now,
should be good. If it's wrong, it will at least blow up at compile time.

Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 22/24] media: imx: Add MIPI CSI-2 OV5640 sensor subdev driver

[Laurent Pinchart]

Hi Steve,

Thank you for the patch. Many of the comments below apply to the ov5642 driver 
too, please take them into account when reworking patch 23/24.

On Friday 06 Jan 2017 18:11:40 Steve Longerbeam wrote:
> This driver is based on ov5640_mipi.c from Freescale imx_3.10.17_1.0.0_beta
> branch, modified heavily to bring forward to latest interfaces and code
> cleanup.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/Kconfig   |8 +
>  drivers/staging/media/imx/Makefile  |2 +
>  drivers/staging/media/imx/ov5640-mipi.c | 2348 

You're missing DT bindings.

The driver should go to drivers/media/i2c/ as it should not be specific to the 
i.MX6, and you can just call it ov5640.c.

>  3 files changed, 2358 insertions(+)
>  create mode 100644 drivers/staging/media/imx/ov5640-mipi.c
> 
> diff --git a/drivers/staging/media/imx/Kconfig
> b/drivers/staging/media/imx/Kconfig index ce2d2c8..09f373d 100644
> --- a/drivers/staging/media/imx/Kconfig
> +++ b/drivers/staging/media/imx/Kconfig
> @@ -17,5 +17,13 @@ config VIDEO_IMX_CAMERA
>   ---help---
> A video4linux camera capture driver for i.MX5/6.
> 
> +config IMX_OV5640_MIPI
> +   tristate "OmniVision OV5640 MIPI CSI-2 camera support"
> +   depends on GPIOLIB && VIDEO_IMX_CAMERA

The sensor driver is generic, it shouldn't depend on IMX. It should however 
depend on at least I2C and OF by the look of it.

> +   select IMX_MIPI_CSI2
> +   default y
> +   ---help---
> + MIPI CSI-2 OV5640 Camera support.
> +
>  endmenu
>  endif

[snip]

> diff --git a/drivers/staging/media/imx/ov5640-mipi.c
> b/drivers/staging/media/imx/ov5640-mipi.c new file mode 100644
> index 000..54647a7
> --- /dev/null
> +++ b/drivers/staging/media/imx/ov5640-mipi.c
> @@ -0,0 +1,2348 @@
> +/*
> + * Copyright (c) 2014 Mentor Graphics Inc.
> + * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights
> Reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Pet peeve of mine, please sort the headers alphabetically. It makes it easier 
to locate duplicated.

> +
> +#define OV5640_VOLTAGE_ANALOG   280
> +#define OV5640_VOLTAGE_DIGITAL_CORE 150
> +#define OV5640_VOLTAGE_DIGITAL_IO   180
> +
> +#define MIN_FPS 15
> +#define MAX_FPS 30
> +#define DEFAULT_FPS 30
> +
> +/* min/typical/max system clock (xclk) frequencies */
> +#define OV5640_XCLK_MIN  600
> +#define OV5640_XCLK_TYP 2400
> +#define OV5640_XCLK_MAX 5400
> +
> +/* min/typical/max pixel clock (mclk) frequencies */
> +#define OV5640_MCLK_MIN 4800
> +#define OV5640_MCLK_TYP 4800
> +#define OV5640_MCLK_MAX 9600
> +
> +#define OV5640_CHIP_ID  0x300A
> +#define OV5640_SLAVE_ID 0x3100
> +#define OV5640_DEFAULT_SLAVE_ID 0x3c

You're mixing lower-case and upper-case hex constants. Let's pick one. Kernel 
code usually favours lower-case.

Please define macros for all the other numerical constants you use in the 
driver (register addresses and values). The large registers tables can be an 
exception if you don't have access to the information, but for registers 
written manually, hardcoding numerical values isn't good.

> +
> +#define OV5640_MAX_CONTROLS 64
> +
> +enum ov5640_mode {
> + ov5640_mode_MIN = 0,
> + ov5640_mode_QCIF_176_144 = 0,
> + ov5640_mode_QVGA_320_240,
> + ov5640_mode_VGA_640_480,
> + ov5640_mode_NTSC_720_480,
> + ov5640_mode_PAL_720_576,
> + ov5640_mode_XGA_1024_768,
> + ov5640_mode_720P_1280_720,
> + ov5640_mode_1080P_1920_1080,
> + ov5640_mode_QSXGA_2592_1944,
> + ov5640_num_modes,
> + ov5640_mode_INIT = 0xff, /*only for sensor init*/

Please add spaces after /* and before */.

Enumerated values should be all upper-case.

> +};
> +
> +enum ov5640_frame_rate {
> + ov5640_15_fps,
> + ov5640_30_fps
> +};
> +
> +static int ov5640_framerates[] = {
> + [ov5640_15_fps] = 15,
> + [ov5640_30_fps] = 30,
> +};
> +#define ov5640_num_framerates ARRAY_SIZE(ov5640_framerates)
> +
> +/* image size under 1280 * 960 are SUBSAMPLING
> + * image size upper 1280 * 960 are SCALING
> + */

The kernel multi-line comment style is

/*
 * text
 * text
 */

> +enum ov5640_downsize_mode {
> + SUBSAMPLING,
> + SCALING,
> +};
> +
> +struct reg_value {
> + u16 reg_addr;
> + u8 val;
> + u8 mask;
> + u32 delay_ms;
> +};
> +
> +struct ov5640_mode_info {
> + enum ov5640_mode mode;
> + enum 

Re: [PATCH] staging: lustre: shut up clang warnings on CLASSERT()

[Greg Kroah-Hartman]

On Thu, Feb 02, 2017 at 11:40:32AM +0100, Arnd Bergmann wrote:
> On Thu, Feb 2, 2017 at 10:54 AM, Greg Kroah-Hartman
>  wrote:
> b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> >> > index aab15d8112a4..2d5435029185 100644
> >> > --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> >> > +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> >> > @@ -291,7 +291,7 @@ do { 
> >> >\
> >> >  *   value  after  conversion...
> >> >  *
> >> >  */
> >> > -#define CLASSERT(cond) do {switch (42) {case (cond): case 0: break; } } 
> >> > while (0)
> >> > +#define CLASSERT(cond) do {switch (42) {case (cond): case 0: default: 
> >> > break; } } while (0)
> >
> > Ugh, why not just use the in-kernel ASSERT macro instead?
> 
> We don't have one ;-)

Oh nice!

> I've done a semi-automated patch to replace CLASSERT() with the respective
> BUILD_BUG_ON() now, but that patch is quite large.

Should be easy to script, I missed that this was a build-time check.
Heck, I'll take a script to do this, or I can just do it in my end.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: lustre: shut up clang warnings on CLASSERT()

[Arnd Bergmann]

On Thu, Feb 2, 2017 at 10:54 AM, Greg Kroah-Hartman
 wrote:
b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
>> > index aab15d8112a4..2d5435029185 100644
>> > --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
>> > +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
>> > @@ -291,7 +291,7 @@ do {   
>> >  \
>> >  *   value  after  conversion...
>> >  *
>> >  */
>> > -#define CLASSERT(cond) do {switch (42) {case (cond): case 0: break; } } 
>> > while (0)
>> > +#define CLASSERT(cond) do {switch (42) {case (cond): case 0: default: 
>> > break; } } while (0)
>
> Ugh, why not just use the in-kernel ASSERT macro instead?

We don't have one ;-)

I've done a semi-automated patch to replace CLASSERT() with the respective
BUILD_BUG_ON() now, but that patch is quite large.

Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: lustre: shut up clang warnings on CLASSERT()

[Greg Kroah-Hartman]

On Thu, Feb 02, 2017 at 07:50:24AM +, Dilger, Andreas wrote:
> On Feb 1, 2017, at 09:52, Arnd Bergmann  wrote:
> > 
> > lustre uses a fake switch() statement as a compile-time assert, but 
> > unfortunately
> > each use of that causes a warning when building with clang:
> > 
> > drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c:2907:2: warning: no 
> > case matching constant switch condition '42'
> > drivers/staging/lustre/lnet/klnds/socklnd/../../../include/linux/libcfs/libcfs_private.h:294:36:
> >  note: expanded from macro 'CLASSERT'
> > #define CLASSERT(cond) do {switch (42) {case (cond): case 0: break; } } 
> > while (0)
> > 
> > Adding a 'default:' label in there shuts up the warning.
> > 
> > Signed-off-by: Arnd Bergmann 
> 
> Reviewed-by: Andreas Dilger 
> 
> > ---
> > drivers/staging/lustre/include/linux/libcfs/libcfs_private.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h 
> > b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> > index aab15d8112a4..2d5435029185 100644
> > --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> > +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> > @@ -291,7 +291,7 @@ do {
> > \
> >  *   value  after  conversion...
> >  *
> >  */
> > -#define CLASSERT(cond) do {switch (42) {case (cond): case 0: break; } } 
> > while (0)
> > +#define CLASSERT(cond) do {switch (42) {case (cond): case 0: default: 
> > break; } } while (0)

Ugh, why not just use the in-kernel ASSERT macro instead?

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

..Reply

[Mrs.Fatima R.M.]

Dear Comrade, 

I hope that I not offend you. Anyway,i found your email through your country 
email data base because of how important is it to me, reaching out to some one 
like for something serious,  let me introduce myself. My name is Fatima Rukaya 
Muhammad, I'm from Tartous Syria and living here in Tartous, Syria. I hope that 
you hear about this country? My country located Middle East, Mediterranean 
coast of Syria.
 
  I search for serious Tie up Mogul in business as my investment partner so 
that it will be possible to transfer the my capital deposited in southeast Asia 
that is over due now to its custody, so that i can Commiserate with the person 
to help my chaos situation to exit, nothing is working out here, everything is 
dead and we live in fear everyday, the situation is getting worst evryday.

 So, i want to relocate to your country at all cost, i will creat a direct 
communcation with the Bank where the money is deposited.

  whatever you may agree on or you want to give me advice i will appreciate it 
because is not possible to move to your country without someone from your 
country and that is the reason of this mail to you.
 
Because US has imposed economic sanctions on Syria in 2004, later tightening 
them to include some Syrian investors with ties to the regime. (Reuters)

So is has to be personal thing between me, please try to get back to me for as 
soon as possible.


thanks

Mrs. Rukaya 


http://www.lexicool.com/
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel