Re: [PATCH] net-next: treewide use is_vlan_dev() helper function.

[Joe Perches]

On Fri, 2017-02-03 at 22:26 -0600, Parav Pandit wrote:
> This patch makes use of is_vlan_dev() function instead of flag
> comparison which is exactly done by is_vlan_dev() helper function.

Thanks.

btw:  after applying this patch, there is one left

$ git grep -E -n "&\s*IFF_802_1Q_VLAN\b" -- "*.c"
drivers/net/ethernet/chelsio/cxgb4/l2t.c:435:   if (neigh->dev->priv_flags & 
IFF_802_1Q_VLAN)

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

Re: Staging: speakup - syle fix permissions to octal

[Joe Perches]

(adding Julia Lawall)

On Fri, 2017-02-03 at 20:44 -0800, Guenter Roeck wrote:
> On Sat, Jan 28, 2017 at 07:05:09PM +1300, Derek Robson wrote:
> > A style fix across whole driver.
> > changed permissions to octal style, found using checkpatch
> > 
> > Signed-off-by: Derek Robson 
> 
> FWIW, I think changes like this are best done using coccinelle.

I think checkpatch does it reasonably well.

Julia?  Can coccinelle do this?

I believe cocinelle doesn't handle the substitution
and octal addition very well when multiple flags
are used.

> That ensures that the results can be reproduced and are well defined.
> As it is, someone will have to check each line of your patches to ensure
> that the conversion is correct.
> 
> It would also ensure (hopefully) that we don't end up with constructs
> such as 
> 
> > -#define USER_R (S_IFREG|S_IRUGO)
> > -#define USER_W (S_IFREG|S_IWUGO)
> > +#define USER_R (S_IFREG|0444)
> > +#define USER_W (S_IFREG|0666)
> 
> which really defeat the purpose of the whole exercise.

Why do you think mixing file specific attributes
with octal permissions is a bad thing?

$ git log -1 f90774e1fd2700d
commit f90774e1fd2700de4a6e0d62866d34a26c544bd0
Author: Joe Perches 
Date:   Tue Oct 11 13:51:47 2016 -0700

checkpatch: look for symbolic permissions and suggest octal instead

S_ uses should be avoided where octal is more intelligible.

Linus didst say:

: It's *much* easier to parse and understand the octal numbers, while the
: symbolic macro names are just random line noise and hard as hell to
: understand.  You really have to think about it.
:
: So we should rather go the other way: convert existing bad symbolic
: permission bit macro use to just use the octal numbers.
:
: The symbolic names are good for the *other* bits (ie sticky bit, and the
: inode mode _type_ numbers etc), but for the permission bits, the symbolic
: names are just insane crap.  Nobody sane should ever use them.  Not in the
: kernel, not in user space.

(http://lkml.kernel.org/r/ca+55afw5v23t-zvdzp-mmd_eyxf8wbafwwb59934fv7g21u...@mail.gmail.com)

Link: 
http://lkml.kernel.org/r/7232ef011d05a92f4caa86a5e9830d87966a2eaf.1470180926.git@perches.com
Signed-off-by: Joe Perches 
Cc: Linus Torvalds 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] Drivers: staging: speakup: spk_priv.h - style fix

[Derek Robson]

Changed function definition argument to have identifier name.
found using checkpatch

Signed-off-by: Derek Robson 
---
Version 1 had missing ; and so broke the build


 drivers/staging/speakup/spk_priv.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/speakup/spk_priv.h 
b/drivers/staging/speakup/spk_priv.h
index 98c4b6f0344a..1952d7887b3b 100644
--- a/drivers/staging/speakup/spk_priv.h
+++ b/drivers/staging/speakup/spk_priv.h
@@ -64,8 +64,8 @@ void spk_synth_flush(struct spk_synth *synth);
 int spk_synth_is_alive_nop(struct spk_synth *synth);
 int spk_synth_is_alive_restart(struct spk_synth *synth);
 void synth_printf(const char *buf, ...);
-int synth_request_region(u_long, u_long);
-int synth_release_region(u_long, u_long);
+int synth_request_region(unsigned long start, unsigned long n);
+int synth_release_region(unsigned long start, unsigned long n);
 int synth_add(struct spk_synth *in_synth);
 void synth_remove(struct spk_synth *in_synth);
 
-- 
2.11.1

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

Re: Staging: speakup - syle fix permissions to octal

[Guenter Roeck]

On Sat, Jan 28, 2017 at 07:05:09PM +1300, Derek Robson wrote:
> A style fix across whole driver.
> changed permissions to octal style, found using checkpatch
> 
> Signed-off-by: Derek Robson 

FWIW, I think changes like this are best done using coccinelle.
That ensures that the results can be reproduced and are well defined.
As it is, someone will have to check each line of your patches to ensure
that the conversion is correct.

It would also ensure (hopefully) that we don't end up with constructs
such as 

> -#define USER_R (S_IFREG|S_IRUGO)
> -#define USER_W (S_IFREG|S_IWUGO)
> +#define USER_R (S_IFREG|0444)
> +#define USER_W (S_IFREG|0666)

which really defeat the purpose of the whole exercise.

In this case, it seems to me that USER_R and USER_W should be dropped
entirely. Otherwise you might as well define USER_IRUGO and use it
instead of S_IRUGO to make checkpatch happy.

But wait ... those definitions are not even used. Hmm.

Guenter

> ---
>  drivers/staging/speakup/main.c   |  4 ++--
>  drivers/staging/speakup/speakup.h|  4 ++--
>  drivers/staging/speakup/speakup_acntpc.c | 26 +-
>  drivers/staging/speakup/speakup_acntsa.c | 26 +-
>  drivers/staging/speakup/speakup_apollo.c | 28 ++--
>  drivers/staging/speakup/speakup_audptr.c | 28 ++--
>  drivers/staging/speakup/speakup_bns.c| 26 +-
>  drivers/staging/speakup/speakup_decext.c | 28 ++--
>  drivers/staging/speakup/speakup_decpc.c  | 26 +-
>  drivers/staging/speakup/speakup_dectlk.c | 28 ++--
>  drivers/staging/speakup/speakup_dtlk.c   | 32 
> 
>  drivers/staging/speakup/speakup_dummy.c  | 26 +-
>  drivers/staging/speakup/speakup_keypc.c  | 22 +++---
>  drivers/staging/speakup/speakup_ltlk.c   | 32 
> 
>  drivers/staging/speakup/speakup_soft.c   | 32 
> 
>  drivers/staging/speakup/speakup_spkout.c | 28 ++--
>  drivers/staging/speakup/speakup_txprt.c  | 26 +-
>  17 files changed, 211 insertions(+), 211 deletions(-)
> 
> diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
> index 5c192042eeac..f7e555b30deb 100644
> --- a/drivers/staging/speakup/main.c
> +++ b/drivers/staging/speakup/main.c
> @@ -58,8 +58,8 @@ MODULE_LICENSE("GPL");
>  MODULE_VERSION(SPEAKUP_VERSION);
>  
>  char *synth_name;
> -module_param_named(synth, synth_name, charp, S_IRUGO);
> -module_param_named(quiet, spk_quiet_boot, bool, S_IRUGO);
> +module_param_named(synth, synth_name, charp, 0444 );
> +module_param_named(quiet, spk_quiet_boot, bool, 0444 );
>  
>  MODULE_PARM_DESC(synth, "Synth to start if speakup is built in.");
>  MODULE_PARM_DESC(quiet, "Do not announce when the synthesizer is found.");
> diff --git a/drivers/staging/speakup/speakup.h 
> b/drivers/staging/speakup/speakup.h
> index df74c912da72..ee567aaf93a3 100644
> --- a/drivers/staging/speakup/speakup.h
> +++ b/drivers/staging/speakup/speakup.h
> @@ -10,8 +10,8 @@
>  #define MAX_DESC_LEN 72
>  
>  /* proc permissions */
> -#define USER_R (S_IFREG|S_IRUGO)
> -#define USER_W (S_IFREG|S_IWUGO)
> +#define USER_R (S_IFREG|0444)
> +#define USER_W (S_IFREG|0666)
>  
>  #define TOGGLE_0 .u.n = {NULL, 0, 0, 1, 0, 0, NULL }
>  #define TOGGLE_1 .u.n = {NULL, 1, 0, 1, 0, 0, NULL }
> diff --git a/drivers/staging/speakup/speakup_acntpc.c 
> b/drivers/staging/speakup/speakup_acntpc.c
> index efb791bb642b..c7fab261d860 100644
> --- a/drivers/staging/speakup/speakup_acntpc.c
> +++ b/drivers/staging/speakup/speakup_acntpc.c
> @@ -57,28 +57,28 @@ static struct var_t vars[] = {
>   * These attributes will appear in /sys/accessibility/speakup/acntpc.
>   */
>  static struct kobj_attribute caps_start_attribute =
> - __ATTR(caps_start, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
> + __ATTR(caps_start, 0644, spk_var_show, spk_var_store);
>  static struct kobj_attribute caps_stop_attribute =
> - __ATTR(caps_stop, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
> + __ATTR(caps_stop, 0644, spk_var_show, spk_var_store);
>  static struct kobj_attribute pitch_attribute =
> - __ATTR(pitch, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
> + __ATTR(pitch, 0644, spk_var_show, spk_var_store);
>  static struct kobj_attribute rate_attribute =
> - __ATTR(rate, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
> + __ATTR(rate, 0644, spk_var_show, spk_var_store);
>  static struct kobj_attribute tone_attribute =
> - __ATTR(tone, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
> + __ATTR(tone, 0644, spk_var_show, spk_var_store);
>  static struct kobj_attribute vol_attribute =
> - __ATTR(vol, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
> + __ATTR(vol, 0644, spk_var_show, 

[PATCH] staging: fbtft: fbtft-bus.c: Fix checkpatch error

[Youngdo Lee]

Fix checkpatch error:
ERROR: space prohibited before that close parenthesis ')'

Signed-off-by: Youngdo Lee 
---
 drivers/staging/fbtft/fbtft-bus.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-bus.c 
b/drivers/staging/fbtft/fbtft-bus.c
index ec45043..3ce265d 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -10,6 +10,7 @@
  *
  */
 
+#definenop_modifier(expr)  (expr)
 #define define_fbtft_write_reg(func, type, modifier)  \
 void func(struct fbtft_par *par, int len, ...)\
 { \
@@ -68,9 +69,9 @@ void func(struct fbtft_par *par, int len, ...)
\
 } \
 EXPORT_SYMBOL(func);
 
-define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, )
+define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, nop_modifier)
 define_fbtft_write_reg(fbtft_write_reg16_bus8, u16, cpu_to_be16)
-define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, )
+define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, nop_modifier)
 
 void fbtft_write_reg8_bus9(struct fbtft_par *par, int len, ...)
 {
-- 
1.9.1

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

[PATCH] net-next: treewide use is_vlan_dev() helper function.

[Parav Pandit]

This patch makes use of is_vlan_dev() function instead of flag
comparison which is exactly done by is_vlan_dev() helper function.

Signed-off-by: Parav Pandit 
Reviewed-by: Daniel Jurgens 
---
 drivers/infiniband/core/cma.c|  6 ++
 drivers/infiniband/sw/rxe/rxe_net.c  |  2 +-
 drivers/net/ethernet/broadcom/cnic.c |  2 +-
 drivers/net/ethernet/chelsio/cxgb3/l2t.c |  2 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c  |  4 ++--
 drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c |  8 
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c |  4 ++--
 drivers/net/hyperv/netvsc_drv.c  |  2 +-
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c|  6 +++---
 drivers/scsi/cxgbi/libcxgbi.c|  6 +++---
 drivers/scsi/fcoe/fcoe.c | 13 ++---
 include/rdma/ib_addr.h   |  6 ++
 net/hsr/hsr_slave.c  |  3 ++-
 13 files changed, 30 insertions(+), 34 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 3e70a9c..4eb5a80 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2467,14 +2467,12 @@ static int iboe_tos_to_sl(struct net_device *ndev, int 
tos)
struct net_device *dev;
 
prio = rt_tos2priority(tos);
-   dev = ndev->priv_flags & IFF_802_1Q_VLAN ?
-   vlan_dev_real_dev(ndev) : ndev;
-
+   dev = is_vlan_dev(ndev) ? vlan_dev_real_dev(ndev) : ndev;
if (dev->num_tc)
return netdev_get_prio_tc_map(dev, prio);
 
 #if IS_ENABLED(CONFIG_VLAN_8021Q)
-   if (ndev->priv_flags & IFF_802_1Q_VLAN)
+   if (is_vlan_dev(ndev))
return (vlan_dev_get_egress_qos_mask(ndev, prio) &
VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
 #endif
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c 
b/drivers/infiniband/sw/rxe/rxe_net.c
index 4abdeb3..d9d1556 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -118,7 +118,7 @@ static struct device *dma_device(struct rxe_dev *rxe)
 
ndev = rxe->ndev;
 
-   if (ndev->priv_flags & IFF_802_1Q_VLAN)
+   if (is_vlan_dev(ndev))
ndev = vlan_dev_real_dev(ndev);
 
return ndev->dev.parent;
diff --git a/drivers/net/ethernet/broadcom/cnic.c 
b/drivers/net/ethernet/broadcom/cnic.c
index b1d2ac8..cec94bb 100644
--- a/drivers/net/ethernet/broadcom/cnic.c
+++ b/drivers/net/ethernet/broadcom/cnic.c
@@ -3665,7 +3665,7 @@ static int cnic_cm_destroy(struct cnic_sock *csk)
 static inline u16 cnic_get_vlan(struct net_device *dev,
struct net_device **vlan_dev)
 {
-   if (dev->priv_flags & IFF_802_1Q_VLAN) {
+   if (is_vlan_dev(dev)) {
*vlan_dev = vlan_dev_real_dev(dev);
return vlan_dev_vlan_id(dev);
}
diff --git a/drivers/net/ethernet/chelsio/cxgb3/l2t.c 
b/drivers/net/ethernet/chelsio/cxgb3/l2t.c
index 5f226ed..5206358 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/l2t.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/l2t.c
@@ -351,7 +351,7 @@ struct l2t_entry *t3_l2t_get(struct t3cdev *cdev, struct 
dst_entry *dst,
e->smt_idx = smt_idx;
atomic_set(>refcnt, 1);
neigh_replace(e, neigh);
-   if (neigh->dev->priv_flags & IFF_802_1Q_VLAN)
+   if (is_vlan_dev(neigh->dev))
e->vlan = vlan_dev_vlan_id(neigh->dev);
else
e->vlan = VLAN_NONE;
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c 
b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index f4f5690..7059014 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -1805,7 +1805,7 @@ static void check_neigh_update(struct neighbour *neigh)
const struct device *parent;
const struct net_device *netdev = neigh->dev;
 
-   if (netdev->priv_flags & IFF_802_1Q_VLAN)
+   if (is_vlan_dev(netdev))
netdev = vlan_dev_real_dev(netdev);
parent = netdev->dev.parent;
if (parent && parent->driver == _driver.driver)
@@ -2111,7 +2111,7 @@ static int cxgb4_inet6addr_handler(struct notifier_block 
*this,
 #if IS_ENABLED(CONFIG_BONDING)
struct adapter *adap;
 #endif
-   if (event_dev->priv_flags & IFF_802_1Q_VLAN)
+   if (is_vlan_dev(event_dev))
event_dev = vlan_dev_real_dev(event_dev);
 #if IS_ENABLED(CONFIG_BONDING)
if (event_dev->flags & IFF_MASTER) {
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c 
b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index 0cf8a37..3b5d7cf 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ 

Re: Drivers: staging: speakup: spk_priv.h - style fix

[Guenter Roeck]

On Sat, Feb 04, 2017 at 12:56:41PM +1300, Derek Robson wrote:
> Changed function definition argument to have identifier name.
> found using checkpatch
> 
> Signed-off-by: Derek Robson 
> ---
>  drivers/staging/speakup/spk_priv.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/speakup/spk_priv.h 
> b/drivers/staging/speakup/spk_priv.h
> index 98c4b6f0344a..1952d7887b3b 100644
> --- a/drivers/staging/speakup/spk_priv.h
> +++ b/drivers/staging/speakup/spk_priv.h
> @@ -64,8 +64,8 @@ void spk_synth_flush(struct spk_synth *synth);
>  int spk_synth_is_alive_nop(struct spk_synth *synth);
>  int spk_synth_is_alive_restart(struct spk_synth *synth);
>  void synth_printf(const char *buf, ...);
> -int synth_request_region(u_long, u_long);
> -int synth_release_region(u_long, u_long);
> +int synth_request_region(unsigned long start, unsigned long n)
> +int synth_release_region(unsigned long start, unsigned long n)

Hmmm .. you didn't compile test your code, did you ? 

You asked why 0day reports an error. If you compile this code,
I am quite sure that you are going to see the same errors.

Guenter

>  int synth_add(struct spk_synth *in_synth);
>  void synth_remove(struct spk_synth *in_synth);
>  
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Drivers: staging: speakup: spk_priv.h - style fix

[Joe Perches]

On Sat, 2017-02-04 at 16:31 +1300, Derek Robson wrote:
> On Sat, Feb 04, 2017 at 10:41:20AM +0800, kbuild test robot wrote:
> > Hi Derek,
> > 
> > [auto build test ERROR on staging/staging-testing]
> > [also build test ERROR on v4.10-rc6 next-20170203]
> > [if your patch is applied to the wrong git tree, please drop us a note to 
> > help improve the system]
> > 
> > url:
> > https://github.com/0day-ci/linux/commits/Derek-Robson/Drivers-staging-speakup-spk_priv-h-style-fix/20170204-080247
> > config: i386-allmodconfig (attached as .config)
> > compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> > reproduce:
> > # save the attached .config to linux build tree
> > make ARCH=i386 
> > 
> > All error/warnings (new ones prefixed by >>):
> > 
> > #define param_check_int(name, p) __param_check(name, p, int)
> >  ^
> >include/linux/moduleparam.h:146:2: note: in expansion of macro 
> > 'param_check_int'
> >  param_check_##type(name, &(value));   \
> >  ^~~~
> >drivers/staging/speakup/speakup_acntsa.c:136:1: note: in expansion of 
> > macro 'module_param_named'
> > module_param_named(ser, synth_acntsa.ser, int, 0444);
> > ^~
> >include/linux/moduleparam.h:146:36: error: expected declaration 
> > specifiers before ';' token
> >  param_check_##type(name, &(value));   \
> >^
> >drivers/staging/speakup/speakup_acntsa.c:136:1: note: in expansion of 
> > macro 'module_param_named'
> > module_param_named(ser, synth_acntsa.ser, int, 0444);
> > ^~
> >include/linux/moduleparam.h:220:20: error: storage class specified for 
> > parameter '__param_str_ser'
> >  static const char __param_str_##name[] = prefix #name;  \
> 
> Can anyone tell me what I did wrong?

You removed semicolons from the function prototypes.

-int synth_request_region(u_long, u_long);
-int synth_release_region(u_long, u_long);
+int synth_request_region(unsigned long start, unsigned long n)
+int synth_release_region(unsigned long start, unsigned long n)

> I don't think it was my patch that broke the build.

You should make sure you build test a patch
before sending it on to the list.

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

Re: [PATCH] Drivers: staging: speakup: spk_priv.h - style fix

[Derek Robson]

On Sat, Feb 04, 2017 at 10:41:20AM +0800, kbuild test robot wrote:
> Hi Derek,
> 
> [auto build test ERROR on staging/staging-testing]
> [also build test ERROR on v4.10-rc6 next-20170203]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Derek-Robson/Drivers-staging-speakup-spk_priv-h-style-fix/20170204-080247
> config: i386-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386 
> 
> All error/warnings (new ones prefixed by >>):
> 
> #define param_check_int(name, p) __param_check(name, p, int)
>  ^
>include/linux/moduleparam.h:146:2: note: in expansion of macro 
> 'param_check_int'
>  param_check_##type(name, &(value));   \
>  ^~~~
>drivers/staging/speakup/speakup_acntsa.c:136:1: note: in expansion of 
> macro 'module_param_named'
> module_param_named(ser, synth_acntsa.ser, int, 0444);
> ^~
>include/linux/moduleparam.h:146:36: error: expected declaration specifiers 
> before ';' token
>  param_check_##type(name, &(value));   \
>^
>drivers/staging/speakup/speakup_acntsa.c:136:1: note: in expansion of 
> macro 'module_param_named'
> module_param_named(ser, synth_acntsa.ser, int, 0444);
> ^~
>include/linux/moduleparam.h:220:20: error: storage class specified for 
> parameter '__param_str_ser'
>  static const char __param_str_##name[] = prefix #name;  \

Can anyone tell me what I did wrong?
I don't think it was my patch that broke the build.

Thanks

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

Re: [PATCH] Drivers: staging: speakup: spk_priv.h - style fix

[kbuild test robot]

Hi Derek,

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.10-rc6 next-20170203]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Derek-Robson/Drivers-staging-speakup-spk_priv-h-style-fix/20170204-080247
config: x86_64-allyesdebian (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   In file included from drivers/staging/speakup/speakup_acntsa.c:22:0:
   drivers/staging/speakup/spk_priv.h: In function 'synth_request_region':
>> drivers/staging/speakup/spk_priv.h:69:1: error: expected '=', ',', ';', 
>> 'asm' or '__attribute__' before 'int'
int synth_add(struct spk_synth *in_synth);
^~~
>> drivers/staging/speakup/spk_priv.h:72:30: error: storage class specified for 
>> parameter 'speakup_info'
extern struct speakup_info_t speakup_info;
 ^~~~
>> drivers/staging/speakup/spk_priv.h:74:21: error: storage class specified for 
>> parameter 'synth_time_vars'
extern struct var_t synth_time_vars[];
^~~
   In file included from drivers/staging/speakup/speakup.h:5:0,
from drivers/staging/speakup/speakup_acntsa.c:23:
>> drivers/staging/speakup/i18n.h:5:1: warning: empty declaration
enum msg_index_t {
^~~~
   drivers/staging/speakup/i18n.h:221:1: warning: empty declaration
struct msg_group_t {
^~
   In file included from drivers/staging/speakup/speakup_acntsa.c:23:0:
>> drivers/staging/speakup/speakup.h:66:21: error: storage class specified for 
>> parameter 'spk_special_handler'
extern special_func spk_special_handler;
^~~
>> drivers/staging/speakup/speakup.h:84:24: error: storage class specified for 
>> parameter 'spk_sel_cons'
extern struct vc_data *spk_sel_cons;
   ^~~~
>> drivers/staging/speakup/speakup.h:85:23: error: storage class specified for 
>> parameter 'spk_xs'
extern unsigned short spk_xs, spk_ys, spk_xe, spk_ye; /* our region points 
*/
  ^~
>> drivers/staging/speakup/speakup.h:85:31: error: storage class specified for 
>> parameter 'spk_ys'
extern unsigned short spk_xs, spk_ys, spk_xe, spk_ye; /* our region points 
*/
  ^~
>> drivers/staging/speakup/speakup.h:85:39: error: storage class specified for 
>> parameter 'spk_xe'
extern unsigned short spk_xs, spk_ys, spk_xe, spk_ye; /* our region points 
*/
  ^~
>> drivers/staging/speakup/speakup.h:85:47: error: storage class specified for 
>> parameter 'spk_ye'
extern unsigned short spk_xs, spk_ys, spk_xe, spk_ye; /* our region points 
*/
  ^~
>> drivers/staging/speakup/speakup.h:87:26: error: storage class specified for 
>> parameter 'speakup_event'
extern wait_queue_head_t speakup_event;
 ^
>> drivers/staging/speakup/speakup.h:88:24: error: storage class specified for 
>> parameter 'speakup_kobj'
extern struct kobject *speakup_kobj;
   ^~~~
>> drivers/staging/speakup/speakup.h:89:28: error: storage class specified for 
>> parameter 'speakup_task'
extern struct task_struct *speakup_task;
   ^~~~
>> drivers/staging/speakup/speakup.h:90:21: error: storage class specified for 
>> parameter 'spk_key_defaults'
extern const u_char spk_key_defaults[];
^~~~
>> drivers/staging/speakup/speakup.h:93:21: error: storage class specified for 
>> parameter 'spk_mutex'
extern struct mutex spk_mutex;
^
>> drivers/staging/speakup/speakup.h:94:25: error: storage class specified for 
>> parameter 'speakup_console'
extern struct st_spk_t *speakup_console[];
^~~
>> drivers/staging/speakup/speakup.h:95:26: error: storage class specified for 
>> parameter 'synth'
extern struct spk_synth *synth;
 ^
>> drivers/staging/speakup/speakup.h:96:13: error: storage class specified for 
>> parameter 'spk_pitch_buff'
extern char spk_pitch_buff[];
^~
>> drivers/staging/speakup/speakup.h:97:16: error: storage class specified for 
>> parameter 'spk_our_keys'
extern u_char *spk_our_keys[];
   ^~~~
>> drivers/staging/speakup/speakup.h:98:14: error: storage class spe

Re: [PATCH] Drivers: staging: speakup: spk_priv.h - style fix

[kbuild test robot]

Hi Derek,

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.10-rc6 next-20170203]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Derek-Robson/Drivers-staging-speakup-spk_priv-h-style-fix/20170204-080247
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All error/warnings (new ones prefixed by >>):

#define param_check_int(name, p) __param_check(name, p, int)
 ^
   include/linux/moduleparam.h:146:2: note: in expansion of macro 
'param_check_int'
 param_check_##type(name, &(value));   \
 ^~~~
   drivers/staging/speakup/speakup_acntsa.c:136:1: note: in expansion of macro 
'module_param_named'
module_param_named(ser, synth_acntsa.ser, int, 0444);
^~
   include/linux/moduleparam.h:146:36: error: expected declaration specifiers 
before ';' token
 param_check_##type(name, &(value));   \
   ^
   drivers/staging/speakup/speakup_acntsa.c:136:1: note: in expansion of macro 
'module_param_named'
module_param_named(ser, synth_acntsa.ser, int, 0444);
^~
   include/linux/moduleparam.h:220:20: error: storage class specified for 
parameter '__param_str_ser'
 static const char __param_str_##name[] = prefix #name;  \
   ^
   include/linux/moduleparam.h:167:2: note: in expansion of macro 
'__module_param_call'
 __module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1, 0)
 ^~~
   include/linux/moduleparam.h:147:2: note: in expansion of macro 
'module_param_cb'
 module_param_cb(name, _ops_##type, , perm); \
 ^~~
   drivers/staging/speakup/speakup_acntsa.c:136:1: note: in expansion of macro 
'module_param_named'
module_param_named(ser, synth_acntsa.ser, int, 0444);
^~
   include/linux/moduleparam.h:146:36: error: parameter '__param_str_ser' is 
initialized
 param_check_##type(name, &(value));   \
   ^
   drivers/staging/speakup/speakup_acntsa.c:136:1: note: in expansion of macro 
'module_param_named'
module_param_named(ser, synth_acntsa.ser, int, 0444);
^~
   include/linux/moduleparam.h:221:49: error: storage class specified for 
parameter '__param_ser'
 static struct kernel_param __moduleparam_const __param_##name \
^
   include/linux/moduleparam.h:167:2: note: in expansion of macro 
'__module_param_call'
 __module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1, 0)
 ^~~
   include/linux/moduleparam.h:147:2: note: in expansion of macro 
'module_param_cb'
 module_param_cb(name, _ops_##type, , perm); \
 ^~~
   drivers/staging/speakup/speakup_acntsa.c:136:1: note: in expansion of macro 
'module_param_named'
module_param_named(ser, synth_acntsa.ser, int, 0444);
^~
   include/linux/moduleparam.h:221:16: error: parameter '__param_ser' is 
initialized
 static struct kernel_param __moduleparam_const __param_##name \
   ^
   include/linux/moduleparam.h:167:2: note: in expansion of macro 
'__module_param_call'
 __module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1, 0)
 ^~~
   include/linux/moduleparam.h:147:2: note: in expansion of macro 
'module_param_cb'
 module_param_cb(name, _ops_##type, , perm); \
 ^~~
   drivers/staging/speakup/speakup_acntsa.c:136:1: note: in expansion of macro 
'module_param_named'
module_param_named(ser, synth_acntsa.ser, int, 0444);
^~
   include/linux/moduleparam.h:221:16: warning: '__used__' attribute ignored 
[-Wattributes]
 static struct kernel_param __moduleparam_const __param_##name \
   ^
   include/linux/moduleparam.h:167:2: note: in expansion of macro 
'__module_param_call'
 __module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1, 0)
 ^~~
   include/linux/moduleparam.h:147:2: note: in expansion of macro 
'module_param_cb'
 module_param_cb(name, _ops_##type, , perm); \
 ^~~
   drivers/staging/speakup/speakup_acntsa.c:136:1: note: in expansion of macro 
'module_param_named'
module_param_named(ser, synth_acntsa.ser, int, 0444);
^~
   include/linux/moduleparam.h:221:49: error: section attribute not allowed for 
'__param_ser'
 static struct kernel_param __moduleparam_const __param_##name \
^
   include/linux/moduleparam.h:167:2: note: in expansion of macro 
'__module_param_call'
 _

Re: Is it time to move drivers/staging/netlogic/ out of staging?

[Florian Fainelli]

On 02/03/2017 12:44 PM, Greg KH wrote:
> On Fri, Feb 03, 2017 at 12:38:45PM -0800, Florian Fainelli wrote:
>> On 02/03/2017 12:36 PM, Greg KH wrote:
>>> On Fri, Feb 03, 2017 at 10:57:16AM -0800, Joe Perches wrote:
 On Fri, 2017-02-03 at 10:50 -0800, Florian Fainelli wrote:
> (with JC's other email)

 And now with Greg's proper email too

> On 02/03/2017 10:47 AM, Joe Perches wrote:
>> 64 bit stats isn't implemented, but is that really necessary?
>> Anything else?
>
> Joe, do you have such hardware that you are interested in getting
> supported, or was that just to reduce the amount of drivers in staging?
> I am really not clear about what happened to that entire product line,
> and whether there is any interest in having anything supported these 
> days...

 No hardware.  Just to reduce staging driver count.
>>>
>>> Without hardware or a "real" maintainer, it shouldn't be moved.
>>>
>>> Heck, if no one has the hardware, let's just delete the thing.
>>
>> I do have one, and other colleagues have some too, but I am not heavily
>> using it, nor do I have many cycles to spend on that... sounds like we
>> could keep it in staging for another 6 months and see what happens then?
> 
> Well, if it works for you, want to maintain it?  :)

I'd have to locate the documentation first, and you would have to reply
to my patch series about DSA ;)
-- 
Florian
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] Staging: iio: resolver: ad2s1210.c - style fix

[Derek Robson]

Changed symbolic permissions to octal permissions.
Found using checkpatch

Signed-off-by: Derek Robson 
---
 drivers/staging/iio/resolver/ad2s1210.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1210.c 
b/drivers/staging/iio/resolver/ad2s1210.c
index 6b992634f009..90b57c03609c 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -531,36 +531,36 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
return ret;
 }
 
-static IIO_DEVICE_ATTR(fclkin, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(fclkin, 0644,
   ad2s1210_show_fclkin, ad2s1210_store_fclkin, 0);
-static IIO_DEVICE_ATTR(fexcit, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(fexcit, 0644,
   ad2s1210_show_fexcit,ad2s1210_store_fexcit, 0);
-static IIO_DEVICE_ATTR(control, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(control, 0644,
   ad2s1210_show_control, ad2s1210_store_control, 0);
-static IIO_DEVICE_ATTR(bits, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(bits, 0644,
   ad2s1210_show_resolution, ad2s1210_store_resolution, 0);
-static IIO_DEVICE_ATTR(fault, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(fault, 0644,
   ad2s1210_show_fault, ad2s1210_clear_fault, 0);
 
-static IIO_DEVICE_ATTR(los_thrd, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(los_thrd, 0644,
   ad2s1210_show_reg, ad2s1210_store_reg,
   AD2S1210_REG_LOS_THRD);
-static IIO_DEVICE_ATTR(dos_ovr_thrd, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(dos_ovr_thrd, 0644,
   ad2s1210_show_reg, ad2s1210_store_reg,
   AD2S1210_REG_DOS_OVR_THRD);
-static IIO_DEVICE_ATTR(dos_mis_thrd, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(dos_mis_thrd, 0644,
   ad2s1210_show_reg, ad2s1210_store_reg,
   AD2S1210_REG_DOS_MIS_THRD);
-static IIO_DEVICE_ATTR(dos_rst_max_thrd, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(dos_rst_max_thrd, 0644,
   ad2s1210_show_reg, ad2s1210_store_reg,
   AD2S1210_REG_DOS_RST_MAX_THRD);
-static IIO_DEVICE_ATTR(dos_rst_min_thrd, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(dos_rst_min_thrd, 0644,
   ad2s1210_show_reg, ad2s1210_store_reg,
   AD2S1210_REG_DOS_RST_MIN_THRD);
-static IIO_DEVICE_ATTR(lot_high_thrd, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(lot_high_thrd, 0644,
   ad2s1210_show_reg, ad2s1210_store_reg,
   AD2S1210_REG_LOT_HIGH_THRD);
-static IIO_DEVICE_ATTR(lot_low_thrd, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(lot_low_thrd, 0644,
   ad2s1210_show_reg, ad2s1210_store_reg,
   AD2S1210_REG_LOT_LOW_THRD);
 
-- 
2.11.1

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

[PATCH] Drivers: staging: speakup: spk_priv.h - style fix

[Derek Robson]

Changed function definition argument to have identifier name.
found using checkpatch

Signed-off-by: Derek Robson 
---
 drivers/staging/speakup/spk_priv.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/speakup/spk_priv.h 
b/drivers/staging/speakup/spk_priv.h
index 98c4b6f0344a..1952d7887b3b 100644
--- a/drivers/staging/speakup/spk_priv.h
+++ b/drivers/staging/speakup/spk_priv.h
@@ -64,8 +64,8 @@ void spk_synth_flush(struct spk_synth *synth);
 int spk_synth_is_alive_nop(struct spk_synth *synth);
 int spk_synth_is_alive_restart(struct spk_synth *synth);
 void synth_printf(const char *buf, ...);
-int synth_request_region(u_long, u_long);
-int synth_release_region(u_long, u_long);
+int synth_request_region(unsigned long start, unsigned long n)
+int synth_release_region(unsigned long start, unsigned long n)
 int synth_add(struct spk_synth *in_synth);
 void synth_remove(struct spk_synth *in_synth);
 
-- 
2.11.1

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

Re: [PATCH] Fixed coding style issues of xmit_linux.c

[Greg KH]

On Fri, Feb 03, 2017 at 07:49:26PM +0530, Abhijit Naik wrote:
> This patch will increase readability of xmit_linux.c file.
> 
> Removed following type of coding style warnings generated for xmit_linux.c,
>   1. Braces {} are not necessary for single statement blocks
>   2. Block comments use a trailing */ on a separate line

When you have to list the different things you do in a kernel patch,
that's a huge flag that you need to break this up into multiple patches.

Please do that here.

Also, please fix up your subject to be better suited to this subsystem
and driver, look at the other patches that have been made to this part
of the kernel to get an idea of what you need to do.

thanks,

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

Re: Is it time to move drivers/staging/netlogic/ out of staging?

[Florian Fainelli]

On 02/03/2017 12:36 PM, Greg KH wrote:
> On Fri, Feb 03, 2017 at 10:57:16AM -0800, Joe Perches wrote:
>> On Fri, 2017-02-03 at 10:50 -0800, Florian Fainelli wrote:
>>> (with JC's other email)
>>
>> And now with Greg's proper email too
>>
>>> On 02/03/2017 10:47 AM, Joe Perches wrote:
 64 bit stats isn't implemented, but is that really necessary?
 Anything else?
>>>
>>> Joe, do you have such hardware that you are interested in getting
>>> supported, or was that just to reduce the amount of drivers in staging?
>>> I am really not clear about what happened to that entire product line,
>>> and whether there is any interest in having anything supported these days...
>>
>> No hardware.  Just to reduce staging driver count.
> 
> Without hardware or a "real" maintainer, it shouldn't be moved.
> 
> Heck, if no one has the hardware, let's just delete the thing.

I do have one, and other colleagues have some too, but I am not heavily
using it, nor do I have many cycles to spend on that... sounds like we
could keep it in staging for another 6 months and see what happens then?
-- 
Florian
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: Is it time to move drivers/staging/netlogic/ out of staging?

[Greg KH]

On Fri, Feb 03, 2017 at 12:38:45PM -0800, Florian Fainelli wrote:
> On 02/03/2017 12:36 PM, Greg KH wrote:
> > On Fri, Feb 03, 2017 at 10:57:16AM -0800, Joe Perches wrote:
> >> On Fri, 2017-02-03 at 10:50 -0800, Florian Fainelli wrote:
> >>> (with JC's other email)
> >>
> >> And now with Greg's proper email too
> >>
> >>> On 02/03/2017 10:47 AM, Joe Perches wrote:
>  64 bit stats isn't implemented, but is that really necessary?
>  Anything else?
> >>>
> >>> Joe, do you have such hardware that you are interested in getting
> >>> supported, or was that just to reduce the amount of drivers in staging?
> >>> I am really not clear about what happened to that entire product line,
> >>> and whether there is any interest in having anything supported these 
> >>> days...
> >>
> >> No hardware.  Just to reduce staging driver count.
> > 
> > Without hardware or a "real" maintainer, it shouldn't be moved.
> > 
> > Heck, if no one has the hardware, let's just delete the thing.
> 
> I do have one, and other colleagues have some too, but I am not heavily
> using it, nor do I have many cycles to spend on that... sounds like we
> could keep it in staging for another 6 months and see what happens then?

Well, if it works for you, want to maintain it?  :)

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

Re: Is it time to move drivers/staging/netlogic/ out of staging?

[Greg KH]

On Fri, Feb 03, 2017 at 10:57:16AM -0800, Joe Perches wrote:
> On Fri, 2017-02-03 at 10:50 -0800, Florian Fainelli wrote:
> > (with JC's other email)
> 
> And now with Greg's proper email too
> 
> > On 02/03/2017 10:47 AM, Joe Perches wrote:
> > > 64 bit stats isn't implemented, but is that really necessary?
> > > Anything else?
> > 
> > Joe, do you have such hardware that you are interested in getting
> > supported, or was that just to reduce the amount of drivers in staging?
> > I am really not clear about what happened to that entire product line,
> > and whether there is any interest in having anything supported these days...
> 
> No hardware.  Just to reduce staging driver count.

Without hardware or a "real" maintainer, it shouldn't be moved.

Heck, if no one has the hardware, let's just delete the thing.

thanks,

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

Re: [RFC] Resolving "false positive" error message from checkpatch.pl

[Joe Perches]

On Fri, 2017-02-03 at 13:10 +0100, Slawomir Stepien wrote:
> On Feb 03, 2017 11:27, Greg KH wrote:
> > On Fri, Feb 03, 2017 at 11:11:35AM +0100, Slawomir Stepien wrote:
> > > Hi all
> > > 
> > > There is a "false positive" error reported by checkpatch.pl:
> > > 
> > > ERROR: Use 4 digit octal (0777) not decimal permissions
> > > #272: FILE: drivers/staging/iio/meter/ade7759.c:272:
> > > +static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO,
> > > + ade7759_read_8bit,
> > > + ade7759_write_8bit,
> > > + ADE7759_CH1OS);
> > > 
> > > ERROR: Use 4 digit octal (0777) not decimal permissions
> > > #276: FILE: drivers/staging/iio/meter/ade7759.c:276:
> > > +static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO,
> > > + ade7759_read_8bit,
> > > + ade7759_write_8bit,
> > > + ADE7759_CH2OS);
> > > 
> > > The same for the file drivers/staging/iio/meter/ade7753.c.
> > > 
> > > We can see that this macro is matched by the pattern 
> > > "IIO_DEV_ATTR_[A-Z_]+" from
> > > @mode_permission_funcs in checkpatch.pl.
> > > 
> > > My question is: how this should be fixed?
> > 
> > Why do you think this is a false-positive?
> 
> Because the 1st arg of that macro is not supposed to be a permissions flags.
> 
> > > From what I can say, it's a false positive so the correct way to fix it 
> > > is to
> > > change the matching pattern: "IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+".
> > 
> > Huh?
> > 
> > Provide a patch to show what you are suggesting please.
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 8e96af53611c..0a1c6ec86caa 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -525,7 +525,7 @@ our @mode_permission_funcs = (
>   
> ["debugfs_create_(?:file|u8|u16|u32|u64|x8|x16|x32|x64|size_t|atomic_t|bool|blob|regset32|u32_array)",
>  2],
>   ["proc_create(?:_data|)", 2],
>   ["(?:CLASS|DEVICE|SENSOR|SENSOR_DEVICE|IIO_DEVICE)_ATTR", 2],
> - ["IIO_DEV_ATTR_[A-Z_]+", 1],
> + ["IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+", 1],
>   ["SENSOR_(?:DEVICE_|)ATTR_2", 2],
>   ["SENSOR_TEMPLATE(?:_2|)", 3],
>   ["__ATTR", 2],

There are many local MODE_DEV_ATTR_ functions
that don't have permission in as the first argument.

$ grep -rPoh --include=*.[ch] "\bIIO_DEV_ATTR_[A-Z_]+\b" *|sort|uniq|wc -l
123

Maybe it'd be better to list the ones that do
or to change the argument order of the defines
and uses so mode _is_ the first argument.

$ grep -rPoh --include=*.[ch] "\bIIO_DEV_ATTR_[A-Z_]+\b" *|sort|uniq| \
  while read line ; do git grep -w $line | grep -w define ; done|grep _mode | 
grep -v "(_mode"
drivers/staging/iio/meter/meter.h:#define IIO_DEV_ATTR_CH_OFF(_num, _mode, 
_show, _store, _addr)\
drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_FREQ(_channel, _num, 
_mode, _show, _store, _addr)  \
drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_FREQSYMBOL(_channel, 
_mode, _show, _store, _addr)  \
drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_OUT_ENABLE(_channel, 
_mode, _show, _store, _addr)  \
drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_PHASE(_channel, _num, 
_mode, _show, _store, _addr) \
drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_PHASESYMBOL(_channel, 
_mode, _show, _store, _addr) \
drivers/staging/iio/frequency/dds.h:#define 
IIO_DEV_ATTR_PINCONTROL_EN(_channel, _mode, _show, _store, _addr)\
drivers/staging/iio/frequency/dds.h:#define 
IIO_DEV_ATTR_PINCONTROL_FREQ_EN(_channel, _mode, _show, _store, _addr)\
drivers/staging/iio/frequency/dds.h:#define 
IIO_DEV_ATTR_PINCONTROL_PHASE_EN(_channel, _mode, _show, _store, _addr)\

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

Re: Is it time to move drivers/staging/netlogic/ out of staging?

[Joe Perches]

On Fri, 2017-02-03 at 10:50 -0800, Florian Fainelli wrote:
> (with JC's other email)

And now with Greg's proper email too

> On 02/03/2017 10:47 AM, Joe Perches wrote:
> > 64 bit stats isn't implemented, but is that really necessary?
> > Anything else?
> 
> Joe, do you have such hardware that you are interested in getting
> supported, or was that just to reduce the amount of drivers in staging?
> I am really not clear about what happened to that entire product line,
> and whether there is any interest in having anything supported these days...

No hardware.  Just to reduce staging driver count.

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

Is it time to move drivers/staging/netlogic/ out of staging?

[Joe Perches]

64 bit stats isn't implemented, but is that really necessary?
Anything else?

It seems like the driver doesn't need to be there anymore.


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

Re: [PATCH 3/6] staging: bcm2835-v4l2: Add a build system for the module.

[Mauro Carvalho Chehab]

Em Fri, 27 Jan 2017 13:55:00 -0800
Eric Anholt  escreveu:

> This is derived from the downstream tree's build system, but with just
> a single Kconfig option.
> 
> For now the driver only builds on 32-bit arm -- the aarch64 build
> breaks due to the driver using arm-specific cache flushing functions.
> 
> Signed-off-by: Eric Anholt 
> ---
>  drivers/staging/media/Kconfig   |  2 ++
>  drivers/staging/media/Makefile  |  1 +
>  drivers/staging/media/platform/bcm2835/Kconfig  | 10 ++
>  drivers/staging/media/platform/bcm2835/Makefile | 11 +++
>  4 files changed, 24 insertions(+)
>  create mode 100644 drivers/staging/media/platform/bcm2835/Kconfig
>  create mode 100644 drivers/staging/media/platform/bcm2835/Makefile
> 
> diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig
> index ffb8fa72c3da..abd0e2d57c20 100644
> --- a/drivers/staging/media/Kconfig
> +++ b/drivers/staging/media/Kconfig
> @@ -27,6 +27,8 @@ source "drivers/staging/media/davinci_vpfe/Kconfig"
>  
>  source "drivers/staging/media/omap4iss/Kconfig"
>  
> +source "drivers/staging/media/platform/bcm2835/Kconfig"
> +
>  source "drivers/staging/media/s5p-cec/Kconfig"
>  
>  # Keep LIRC at the end, as it has sub-menus
> diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile
> index a28e82cf6447..dc89325c463d 100644
> --- a/drivers/staging/media/Makefile
> +++ b/drivers/staging/media/Makefile
> @@ -2,6 +2,7 @@ obj-$(CONFIG_I2C_BCM2048) += bcm2048/
>  obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC) += s5p-cec/
>  obj-$(CONFIG_DVB_CXD2099)+= cxd2099/
>  obj-$(CONFIG_LIRC_STAGING)   += lirc/
> +obj-$(CONFIG_VIDEO_BCM2835)  += platform/bcm2835/
>  obj-$(CONFIG_VIDEO_DM365_VPFE)   += davinci_vpfe/
>  obj-$(CONFIG_VIDEO_OMAP4)+= omap4iss/
>  obj-$(CONFIG_VIDEO_STI_HDMI_CEC) += st-cec/
> diff --git a/drivers/staging/media/platform/bcm2835/Kconfig 
> b/drivers/staging/media/platform/bcm2835/Kconfig
> new file mode 100644
> index ..7c5245dc3225
> --- /dev/null
> +++ b/drivers/staging/media/platform/bcm2835/Kconfig
> @@ -0,0 +1,10 @@
> +config VIDEO_BCM2835
> + tristate "Broadcom BCM2835 camera driver"
> + depends on VIDEO_V4L2 && (ARCH_BCM2835 || COMPILE_TEST)
> + depends on BCM2835_VCHIQ
> + depends on ARM
> + select VIDEOBUF2_VMALLOC
> + help
> +   Say Y here to enable camera host interface devices for
> +   Broadcom BCM2835 SoC. This operates over the VCHIQ interface
> +   to a service running on VideoCore.
> diff --git a/drivers/staging/media/platform/bcm2835/Makefile 
> b/drivers/staging/media/platform/bcm2835/Makefile
> new file mode 100644
> index ..d7900a5951a8
> --- /dev/null
> +++ b/drivers/staging/media/platform/bcm2835/Makefile
> @@ -0,0 +1,11 @@
> +bcm2835-v4l2-$(CONFIG_VIDEO_BCM2835) := \
> + bcm2835-camera.o \
> + controls.o \
> + mmal-vchiq.o
> +
> +obj-$(CONFIG_VIDEO_BCM2835) += bcm2835-v4l2.o
> +
> +ccflags-y += \
> + -Idrivers/staging/vc04_services \
> + -Idrivers/staging/vc04_services/interface/vcos/linuxkernel \
> + -D__VCCOREVER__=0x0400

Huh! specifying the version of the videocore by a define seems
wrong! This is the type of thing that should be provided via DT.





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

Re: Is it time to move drivers/staging/netlogic/ out of staging?

[Florian Fainelli]

(with JC's other email)

On 02/03/2017 10:47 AM, Joe Perches wrote:
> 64 bit stats isn't implemented, but is that really necessary?
> Anything else?

Joe, do you have such hardware that you are interested in getting
supported, or was that just to reduce the amount of drivers in staging?
I am really not clear about what happened to that entire product line,
and whether there is any interest in having anything supported these days...
-- 
Florian
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: bcm2835-audio: remove unused macro

[Hendrik v. Raven]

The VC_AUDIO_MAX_MSG_LEN macro is not used anywhere and has coding style
violations.

Signed-off-by: Hendrik v. Raven 
---
 drivers/staging/bcm2835-audio/vc_vchi_audioserv_defs.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/bcm2835-audio/vc_vchi_audioserv_defs.h 
b/drivers/staging/bcm2835-audio/vc_vchi_audioserv_defs.h
index 09f07fd891bb..da96f1bc2516 100644
--- a/drivers/staging/bcm2835-audio/vc_vchi_audioserv_defs.h
+++ b/drivers/staging/bcm2835-audio/vc_vchi_audioserv_defs.h
@@ -21,9 +21,6 @@
 /* FourCC code used for VCHI connection */
 #define VC_AUDIO_SERVER_NAME  MAKE_FOURCC("AUDS")
 
-/* Maximum message length */
-#define VC_AUDIO_MAX_MSG_LEN  (sizeof( VC_AUDIO_MSG_T ))
-
 /*
  *  List of screens that are currently supported
  *  All message types supported for HOST->VC direction
-- 
2.11.0

___
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 02:29 PM, Russell King - ARM Linux wrote:

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);


I think you mean min() there, but yes thanks, fixed.

Steve

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

Re: [PATCH] staging: bcm2835-audio: fix spacing of macro

[Joe Perches]

On Fri, 2017-02-03 at 17:17 +0100, Hendrik v. Raven wrote:
> VC_AUDIO_MAX_MSG_LEN

This macro seems unused and might as well be removed.
___
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/03/2017 06:41 AM, Laurent Pinchart wrote:

Hello,

On Wednesday 01 Feb 2017 16:19:27 Steve Longerbeam wrote:

On 02/01/2017 01:30 AM, Philipp Zabel wrote:


media-ctl propagates the output pad format to all remote subdevices'
input pads for all enabled links:

https://git.linuxtv.org/v4l-utils.git/tree/utils/media-ctl/libv4l2subdev.c
#n693

Ah cool, I wasn't aware media-ctl did this, but it makes sense and
makes it easier on the user.

To be precise, userspace is responsible for propagating formats *between*
subdevs (source to sink, over a link) and drivers for propagating formats *in*
subdevs (sink to source, inside the subdev).


Hi Laurent, yes thanks for that clarification.

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

[PATCH] Staging: bcm2835-audio: removed spaces around parenthesis

[Michael Rupprecht]

Removed unnecessary spaces around parenthesis as reported by checkpatch.pl

Signed-off-by: Michael Rupprecht 
---
 drivers/staging/bcm2835-audio/bcm2835-vchiq.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/bcm2835-audio/bcm2835-vchiq.c 
b/drivers/staging/bcm2835-audio/bcm2835-vchiq.c
index c1a8f03..9ac1f72 100644
--- a/drivers/staging/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/bcm2835-audio/bcm2835-vchiq.c
@@ -44,15 +44,15 @@
 
 /* Logging macros (for remapping to other logging mechanisms, i.e., printf) */
 #ifdef AUDIO_DEBUG_ENABLE
-#define LOG_ERR( fmt, arg... )   pr_err( "%s:%d " fmt, __func__, __LINE__, 
##arg)
-#define LOG_WARN( fmt, arg... )  pr_info( "%s:%d " fmt, __func__, __LINE__, 
##arg)
-#define LOG_INFO( fmt, arg... )  pr_info( "%s:%d " fmt, __func__, __LINE__, 
##arg)
-#define LOG_DBG( fmt, arg... )   pr_info( "%s:%d " fmt, __func__, __LINE__, 
##arg)
+#define LOG_ERR(fmt, arg...)   pr_err("%s:%d " fmt, __func__, __LINE__, ##arg)
+#define LOG_WARN(fmt, arg...)  pr_info("%s:%d " fmt, __func__, __LINE__, ##arg)
+#define LOG_INFO(fmt, arg...)  pr_info("%s:%d " fmt, __func__, __LINE__, ##arg)
+#define LOG_DBG(fmt, arg...)   pr_info("%s:%d " fmt, __func__, __LINE__, ##arg)
 #else
-#define LOG_ERR( fmt, arg... )   pr_err( "%s:%d " fmt, __func__, __LINE__, 
##arg)
-#define LOG_WARN( fmt, arg... ) no_printk(fmt, ##arg)
-#define LOG_INFO( fmt, arg... ) no_printk(fmt, ##arg)
-#define LOG_DBG( fmt, arg... )  no_printk(fmt, ##arg)
+#define LOG_ERR(fmt, arg...)   pr_err("%s:%d " fmt, __func__, __LINE__, ##arg)
+#define LOG_WARN(fmt, arg...)   no_printk(fmt, ##arg)
+#define LOG_INFO(fmt, arg...)   no_printk(fmt, ##arg)
+#define LOG_DBG(fmt, arg...)no_printk(fmt, ##arg)
 #endif
 
 struct bcm2835_audio_instance {
-- 
2.7.4

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

[PATCH] staging: bcm2835-audio: fix spacing of macro

[Hendrik v. Raven]

removes the unwanted spaces inside of sizeof( ), removing a complaint of
checkpatch.pl. Also removes the second space behind the macro name.

Signed-off-by: Hendrik v. Raven 
---
 drivers/staging/bcm2835-audio/vc_vchi_audioserv_defs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/bcm2835-audio/vc_vchi_audioserv_defs.h 
b/drivers/staging/bcm2835-audio/vc_vchi_audioserv_defs.h
index 09f07fd891bb..dd2f162b1ccb 100644
--- a/drivers/staging/bcm2835-audio/vc_vchi_audioserv_defs.h
+++ b/drivers/staging/bcm2835-audio/vc_vchi_audioserv_defs.h
@@ -22,7 +22,7 @@
 #define VC_AUDIO_SERVER_NAME  MAKE_FOURCC("AUDS")
 
 /* Maximum message length */
-#define VC_AUDIO_MAX_MSG_LEN  (sizeof( VC_AUDIO_MSG_T ))
+#define VC_AUDIO_MAX_MSG_LEN (sizeof(VC_AUDIO_MSG_T))
 
 /*
  *  List of screens that are currently supported
-- 
2.11.0

___
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

[Laurentiu Tudor]

On 02/03/2017 02:02 AM, Stuart Yoder wrote:
>
>> -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?
>

Right, thanks for spotting. I started working on a v2
of the series.

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

[PATCH] staging:most/hdm-i2c: Replace symbolic permissions with octal permissions

[Zhengyi Shen]

WARNING: Symbolic permissions 'S_IRUGO' are not preferred.
Consider using octal permissions '0444'.
This warning was detected by checkpatch.pl for hdm_i2c.c.

Signed-off-by: Zhengyi Shen 
---
 drivers/staging/most/hdm-i2c/hdm_i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/most/hdm-i2c/hdm_i2c.c 
b/drivers/staging/most/hdm-i2c/hdm_i2c.c
index ba0263b..1d5b229 100644
--- a/drivers/staging/most/hdm-i2c/hdm_i2c.c
+++ b/drivers/staging/most/hdm-i2c/hdm_i2c.c
@@ -37,7 +37,7 @@ enum { CH_RX, CH_TX, NUM_CHANNELS };
 
 /* IRQ / Polling option */
 static bool polling_req;
-module_param(polling_req, bool, S_IRUGO);
+module_param(polling_req, bool, 0444);
 MODULE_PARM_DESC(polling_req, "Request Polling. Default = 0 (use irq)");
 
 /* Polling Rate */
-- 
2.9.3

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

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

[Laurent Pinchart]

Hello,

On Wednesday 01 Feb 2017 16:19:27 Steve Longerbeam wrote:
> On 02/01/2017 01:30 AM, Philipp Zabel wrote:
> > On Tue, 2017-01-31 at 17:26 -0800, Steve Longerbeam wrote:
> > [...]
> > 
> >>> # Set pad formats
> >>> media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY/1920x1080]"
> >>> media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/1920x1080]"
> >>> media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080]"
> >>> media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080]"
> >>> 
> >>> v4l2-ctl -d /dev/video4 -V
> >>> # This still is configured to 640x480, which is inconsistent with
> >>> # the 'ipu1_csi0':2 pad format. The pad set_fmt above should
> >>> # have set this, too.
> >> 
> >> Because you've only configured the source pads,
> >> and not the sink pads. The ipu_csi source format is
> >> dependent on the sink format - output crop window is
> >> limited by max input sensor frame, and since sink pad is
> >> still at 640x480, output is reduced to that.
> > 
> > No, it is set (see below). What happens is that capture_g_fmt_vid_cap
> > just returns the capture devices' priv->vdev.fmt, even if it is
> > incompatible with the connected csi subdevice's output pad format.
> > 
> > priv->vdev.fmt was never changed from the default set in
> > imx_media_capture_device_register, because capture_s/try_fmt_vid_cap
> > were not called yet.
> 
> Ah, yep, this is a bug. Need to modify the capture device's
> width/height at .set_fmt() in the subdev's device-node source
> pad (csi and prpenc/vf).
> 
> >> Maybe I'm missing something, is it expected behavior that
> >> a source format should be automatically propagated to
> >> the sink?
> > 
> > media-ctl propagates the output pad format to all remote subdevices'
> > input pads for all enabled links:
> > 
> > https://git.linuxtv.org/v4l-utils.git/tree/utils/media-ctl/libv4l2subdev.c
> > #n693
>
> Ah cool, I wasn't aware media-ctl did this, but it makes sense and
> makes it easier on the user.

To be precise, userspace is responsible for propagating formats *between* 
subdevs (source to sink, over a link) and drivers for propagating formats *in* 
subdevs (sink to source, inside the subdev).

> >>> v4l2-ctl --list-formats -d /dev/video4
> >>> # This lists all the RGB formats, which it shouldn't. There is
> >>> # no CSC in this pipeline, so we should be limited to YUV formats
> >>> # only.
> >> 
> >> right, need to fix that. Probably by poking the attached
> >> source subdev (csi or prpenc/vf) for its supported formats.
> > 
> > You are right, in bayer/raw mode only one specific format should be
> > listed, depending on the CSI output pad format.

-- 
Regards,

Laurent Pinchart

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

[PATCH] Fixed coding style issues of xmit_linux.c

[Abhijit Naik]

This patch will increase readability of xmit_linux.c file.

Removed following type of coding style warnings generated for xmit_linux.c,
1. Braces {} are not necessary for single statement blocks
2. Block comments use a trailing */ on a separate line

Signed-off-by: Abhijit Naik 
---
 drivers/staging/rtl8712/xmit_linux.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/rtl8712/xmit_linux.c 
b/drivers/staging/rtl8712/xmit_linux.c
index 695f9b9..1e86133 100644
--- a/drivers/staging/rtl8712/xmit_linux.c
+++ b/drivers/staging/rtl8712/xmit_linux.c
@@ -91,7 +91,8 @@ void r8712_set_qos(struct pkt_file *ppktfile, struct 
pkt_attrib *pattrib)
} else {
/* "When priority processing of data frames is supported,
 * a STA's SME should send EAPOL-Key frames at the highest
-* priority." */
+* priority."
+*/
 
if (pattrib->ether_type == 0x888e)
UserPriority = 7;
@@ -162,16 +163,16 @@ int r8712_xmit_entry(_pkt *pkt, struct  net_device 
*pnetdev)
struct _adapter *padapter = netdev_priv(pnetdev);
struct xmit_priv *pxmitpriv = &(padapter->xmitpriv);
 
-   if (!r8712_if_up(padapter)) {
+   if (!r8712_if_up(padapter))
goto _xmit_entry_drop;
-   }
+
pxmitframe = r8712_alloc_xmitframe(pxmitpriv);
-   if (!pxmitframe) {
+   if (!pxmitframe)
goto _xmit_entry_drop;
-   }
-   if ((!r8712_update_attrib(padapter, pkt, >attrib))) {
+
+   if ((!r8712_update_attrib(padapter, pkt, >attrib)))
goto _xmit_entry_drop;
-   }
+
padapter->ledpriv.LedControlHandler(padapter, LED_CTL_TX);
pxmitframe->pkt = pkt;
if (r8712_pre_xmit(padapter, pxmitframe)) {
-- 
2.10.2

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

[PATCH] staging: octeon: Fix line over 80 characters

[kamalheib1]

From: Kamal Heib 

This patch fix the line over 80 characters warning that was detected
using checkpatch.pl script.

Fixes: 6fe5efa1415c ('staging: octeon: Convert create_singlethread_workqueue()')
Cc: Bhaktipriya Shridhar 
Signed-off-by: Kamal Heib 
---
 drivers/staging/octeon/ethernet.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/octeon/ethernet.c 
b/drivers/staging/octeon/ethernet.c
index 4971aa54756a..a379734a54b1 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -889,7 +889,8 @@ static int cvm_oct_probe(struct platform_device *pdev)
fau -=
cvmx_pko_get_num_queues(priv->port) *
sizeof(u32);
-   
schedule_delayed_work(>port_periodic_work, HZ);
+   schedule_delayed_work(>port_periodic_work,
+ HZ);
}
}
}
-- 
2.9.3

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

Re: [RFC] Resolving "false positive" error message from checkpatch.pl

[Slawomir Stepien]

On Feb 03, 2017 04:39, Joe Perches wrote:
> On Fri, 2017-02-03 at 13:10 +0100, Slawomir Stepien wrote:
> > On Feb 03, 2017 11:27, Greg KH wrote:
> > > On Fri, Feb 03, 2017 at 11:11:35AM +0100, Slawomir Stepien wrote:
> > > > Hi all
> > > > 
> > > > There is a "false positive" error reported by checkpatch.pl:
> > > > 
> > > > ERROR: Use 4 digit octal (0777) not decimal permissions
> > > > #272: FILE: drivers/staging/iio/meter/ade7759.c:272:
> > > > +static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO,
> > > > +   ade7759_read_8bit,
> > > > +   ade7759_write_8bit,
> > > > +   ADE7759_CH1OS);
> > > > 
> > > > ERROR: Use 4 digit octal (0777) not decimal permissions
> > > > #276: FILE: drivers/staging/iio/meter/ade7759.c:276:
> > > > +static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO,
> > > > +   ade7759_read_8bit,
> > > > +   ade7759_write_8bit,
> > > > +   ADE7759_CH2OS);
> > > > 
> > > > The same for the file drivers/staging/iio/meter/ade7753.c.
> > > > 
> > > > We can see that this macro is matched by the pattern 
> > > > "IIO_DEV_ATTR_[A-Z_]+" from
> > > > @mode_permission_funcs in checkpatch.pl.
> > > > 
> > > > My question is: how this should be fixed?
> > > 
> > > Why do you think this is a false-positive?
> > 
> > Because the 1st arg of that macro is not supposed to be a permissions flags.
> > 
> > > > From what I can say, it's a false positive so the correct way to fix it 
> > > > is to
> > > > change the matching pattern: "IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+".
> > > 
> > > Huh?
> > > 
> > > Provide a patch to show what you are suggesting please.
> > 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 8e96af53611c..0a1c6ec86caa 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -525,7 +525,7 @@ our @mode_permission_funcs = (
> > 
> > ["debugfs_create_(?:file|u8|u16|u32|u64|x8|x16|x32|x64|size_t|atomic_t|bool|blob|regset32|u32_array)",
> >  2],
> > ["proc_create(?:_data|)", 2],
> > ["(?:CLASS|DEVICE|SENSOR|SENSOR_DEVICE|IIO_DEVICE)_ATTR", 2],
> > -   ["IIO_DEV_ATTR_[A-Z_]+", 1],
> > +   ["IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+", 1],
> > ["SENSOR_(?:DEVICE_|)ATTR_2", 2],
> > ["SENSOR_TEMPLATE(?:_2|)", 3],
> > ["__ATTR", 2],
> 
> There are many local MODE_DEV_ATTR_ functions
> that don't have permission in as the first argument.

Yeah...I just found the ones in the drivers/staging/iio/frequency that you
pointed out.

> $ grep -rPoh --include=*.[ch] "\bIIO_DEV_ATTR_[A-Z_]+\b" *|sort|uniq|wc -l
> 123
> 
> Maybe it'd be better to list the ones that do
> or to change the argument order of the defines
> and uses so mode _is_ the first argument.

That is my original question. What would be better here (especially that no we
can see that there are few more places where the error will pop)?

> $ grep -rPoh --include=*.[ch] "\bIIO_DEV_ATTR_[A-Z_]+\b" *|sort|uniq| \
>   while read line ; do git grep -w $line | grep -w define ; done|grep _mode | 
> grep -v "(_mode"
> drivers/staging/iio/meter/meter.h:#define IIO_DEV_ATTR_CH_OFF(_num, _mode, 
> _show, _store, _addr)  \
> drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_FREQ(_channel, _num, 
> _mode, _show, _store, _addr)\
> drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_FREQSYMBOL(_channel, 
> _mode, _show, _store, _addr)\
> drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_OUT_ENABLE(_channel, 
> _mode, _show, _store, _addr)\
> drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_PHASE(_channel, 
> _num, _mode, _show, _store, _addr)   \
> drivers/staging/iio/frequency/dds.h:#define 
> IIO_DEV_ATTR_PHASESYMBOL(_channel, _mode, _show, _store, _addr)   \
> drivers/staging/iio/frequency/dds.h:#define 
> IIO_DEV_ATTR_PINCONTROL_EN(_channel, _mode, _show, _store, _addr)\
> drivers/staging/iio/frequency/dds.h:#define 
> IIO_DEV_ATTR_PINCONTROL_FREQ_EN(_channel, _mode, _show, _store, _addr)\
> drivers/staging/iio/frequency/dds.h:#define 
> IIO_DEV_ATTR_PINCONTROL_PHASE_EN(_channel, _mode, _show, _store, _addr)\

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

Re: [RFC] Resolving "false positive" error message from checkpatch.pl

[Greg KH]

On Fri, Feb 03, 2017 at 01:10:20PM +0100, Slawomir Stepien wrote:
> On Feb 03, 2017 11:27, Greg KH wrote:
> > On Fri, Feb 03, 2017 at 11:11:35AM +0100, Slawomir Stepien wrote:
> > > Hi all
> > > 
> > > There is a "false positive" error reported by checkpatch.pl:
> > > 
> > > ERROR: Use 4 digit octal (0777) not decimal permissions
> > > #272: FILE: drivers/staging/iio/meter/ade7759.c:272:
> > > +static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO,
> > > + ade7759_read_8bit,
> > > + ade7759_write_8bit,
> > > + ADE7759_CH1OS);
> > > 
> > > ERROR: Use 4 digit octal (0777) not decimal permissions
> > > #276: FILE: drivers/staging/iio/meter/ade7759.c:276:
> > > +static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO,
> > > + ade7759_read_8bit,
> > > + ade7759_write_8bit,
> > > + ADE7759_CH2OS);
> > > 
> > > The same for the file drivers/staging/iio/meter/ade7753.c.
> > > 
> > > We can see that this macro is matched by the pattern 
> > > "IIO_DEV_ATTR_[A-Z_]+" from
> > > @mode_permission_funcs in checkpatch.pl.
> > > 
> > > My question is: how this should be fixed?
> > 
> > Why do you think this is a false-positive?
> 
> Because the 1st arg of that macro is not supposed to be a permissions flags.

Ah. I was thinking that the S_IWUSR needs to be replaced with an octal
value :)

thanks,

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

Re: [RFC] Resolving "false positive" error message from checkpatch.pl

[Slawomir Stepien]

On Feb 03, 2017 11:27, Greg KH wrote:
> On Fri, Feb 03, 2017 at 11:11:35AM +0100, Slawomir Stepien wrote:
> > Hi all
> > 
> > There is a "false positive" error reported by checkpatch.pl:
> > 
> > ERROR: Use 4 digit octal (0777) not decimal permissions
> > #272: FILE: drivers/staging/iio/meter/ade7759.c:272:
> > +static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO,
> > +   ade7759_read_8bit,
> > +   ade7759_write_8bit,
> > +   ADE7759_CH1OS);
> > 
> > ERROR: Use 4 digit octal (0777) not decimal permissions
> > #276: FILE: drivers/staging/iio/meter/ade7759.c:276:
> > +static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO,
> > +   ade7759_read_8bit,
> > +   ade7759_write_8bit,
> > +   ADE7759_CH2OS);
> > 
> > The same for the file drivers/staging/iio/meter/ade7753.c.
> > 
> > We can see that this macro is matched by the pattern "IIO_DEV_ATTR_[A-Z_]+" 
> > from
> > @mode_permission_funcs in checkpatch.pl.
> > 
> > My question is: how this should be fixed?
> 
> Why do you think this is a false-positive?

Because the 1st arg of that macro is not supposed to be a permissions flags.

> > From what I can say, it's a false positive so the correct way to fix it is 
> > to
> > change the matching pattern: "IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+".
> 
> Huh?
> 
> Provide a patch to show what you are suggesting please.

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8e96af53611c..0a1c6ec86caa 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -525,7 +525,7 @@ our @mode_permission_funcs = (

["debugfs_create_(?:file|u8|u16|u32|u64|x8|x16|x32|x64|size_t|atomic_t|bool|blob|regset32|u32_array)",
 2],
["proc_create(?:_data|)", 2],
["(?:CLASS|DEVICE|SENSOR|SENSOR_DEVICE|IIO_DEVICE)_ATTR", 2],
-   ["IIO_DEV_ATTR_[A-Z_]+", 1],
+   ["IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+", 1],
["SENSOR_(?:DEVICE_|)ATTR_2", 2],
["SENSOR_TEMPLATE(?:_2|)", 3],
["__ATTR", 2],
-- 
2.11.0
___
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

[Laurentiu Tudor]

On 02/03/2017 02:02 AM, Stuart Yoder wrote:
>
>
>> -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.
>

Agree. I think I end up doing a double free here.

---
Thanks & Best Regards, Laurentiu
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 00/60] staging: lustre: batches of fixes for lustre client

[Greg Kroah-Hartman]

On Sat, Jan 28, 2017 at 07:04:28PM -0500, James Simmons wrote:
> Batch of missing fixes for lustre for the upstream client.

I've applied most of these, please fix up the rest, rebase, and resend.

thanks,

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

Re: [PATCH 10/60] staging: lustre: obdclass: add more info to sysfs version string

[Greg Kroah-Hartman]

On Sat, Jan 28, 2017 at 07:04:38PM -0500, James Simmons wrote:
> From: Andreas Dilger 
> 
> Update the sysfs "version" file to print "lustre: " with
> the version number.
> 
> Signed-off-by: Andreas Dilger 
> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-5969
> Reviewed-on: http://review.whamcloud.com/16721
> Reviewed-by: James Simmons 
> Reviewed-by: Dmitry Eremin 
> Reviewed-by: Oleg Drokin 
> Signed-off-by: James Simmons 
> ---
>  drivers/staging/lustre/lustre/obdclass/linux/linux-module.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c 
> b/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
> index 9f5e829..22e6d1f 100644
> --- a/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
> +++ b/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
> @@ -208,7 +208,7 @@ struct miscdevice obd_psdev = {
>  static ssize_t version_show(struct kobject *kobj, struct attribute *attr,
>   char *buf)
>  {
> - return sprintf(buf, "%s\n", LUSTRE_VERSION_STRING);
> + return sprintf(buf, "lustre: %s\n", LUSTRE_VERSION_STRING);
>  }

Why?  You "know" this is lustre, why say it again?  Doesn't this affect
userspace tools?

thanks,

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

Re: [PATCH 2/9] staging: fsl-mc: fix device ref counting

[Greg KH]

On Fri, Feb 03, 2017 at 10:17:53AM +, Laurentiu Tudor wrote:
> Hi Greg,
> 
> Thanks for having a look. Comment below.
> 
> On 02/03/2017 11:56 AM, Greg KH wrote:
> > On Wed, Feb 01, 2017 at 05:43:22AM -0600, laurentiu.tu...@nxp.com wrote:
> >> From: Laurentiu Tudor 
> >>
> >> Drop unneeded get_device() call at device creation
> >> and, as per documentation, drop reference count
> >> after using device_find_child() return.
> >>
> >> Signed-off-by: Laurentiu Tudor 
> >> ---
> >>   drivers/staging/fsl-mc/bus/dprc-driver.c | 1 +
> >>   drivers/staging/fsl-mc/bus/fsl-mc-bus.c  | 1 -
> >>   2 files changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c 
> >> b/drivers/staging/fsl-mc/bus/dprc-driver.c
> >> index 4e416d8..e4b0341 100644
> >> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
> >> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
> >> @@ -188,6 +188,7 @@ static void dprc_add_new_devices(struct fsl_mc_device 
> >> *mc_bus_dev,
> >>child_dev = fsl_mc_device_lookup(obj_desc, mc_bus_dev);
> >>if (child_dev) {
> >>check_plugged_state_change(child_dev, obj_desc);
> >> +  put_device(_dev->dev);
> >>continue;
> >>}
> >>
> >> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c 
> >> b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> >> index cc20dc4..7c6a43b 100644
> >> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> >> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> >> @@ -537,7 +537,6 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
> >>goto error_cleanup_dev;
> >>}
> >>
> >> -  (void)get_device(_dev->dev);
> >
> > This implies that your device reference counting is totally wrong and
> > messed up.  Does this fix anything?  Break anything?  It should do
> > something different now...
> 
> It fixes the refcounting in the sense that I'm now seeing the error
> that i think you were referring to in your previous reviews,
> when we hot unplug a device:
> 
> "Device 'foo.N' does not have a release() function, it is broken and 
> must be fixed."
> 
> See next patch that adds the required callback.
> 
> Regarding this particular get_device(), i have no clue why the
> original author placed it here. I've looked over other bus 
> implementations and didn't see something similar.

Ah, that makes more sense, thanks.  I've now applied this and the next
patch and will wait for you to respin the rest.

thanks,

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

Re: [RFC] Resolving "false positive" error message from checkpatch.pl

[Greg KH]

On Fri, Feb 03, 2017 at 11:11:35AM +0100, Slawomir Stepien wrote:
> Hi all
> 
> There is a "false positive" error reported by checkpatch.pl:
> 
> ERROR: Use 4 digit octal (0777) not decimal permissions
> #272: FILE: drivers/staging/iio/meter/ade7759.c:272:
> +static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO,
> + ade7759_read_8bit,
> + ade7759_write_8bit,
> + ADE7759_CH1OS);
> 
> ERROR: Use 4 digit octal (0777) not decimal permissions
> #276: FILE: drivers/staging/iio/meter/ade7759.c:276:
> +static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO,
> + ade7759_read_8bit,
> + ade7759_write_8bit,
> + ADE7759_CH2OS);
> 
> The same for the file drivers/staging/iio/meter/ade7753.c.
> 
> We can see that this macro is matched by the pattern "IIO_DEV_ATTR_[A-Z_]+" 
> from
> @mode_permission_funcs in checkpatch.pl.
> 
> My question is: how this should be fixed?

Why do you think this is a false-positive?

> From what I can say, it's a false positive so the correct way to fix it is to
> change the matching pattern: "IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+".

Huh?

Provide a patch to show what you are suggesting please.

thanks,

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

[RFC] Resolving "false positive" error message from checkpatch.pl

[Slawomir Stepien]

Hi all

There is a "false positive" error reported by checkpatch.pl:

ERROR: Use 4 digit octal (0777) not decimal permissions
#272: FILE: drivers/staging/iio/meter/ade7759.c:272:
+static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO,
+   ade7759_read_8bit,
+   ade7759_write_8bit,
+   ADE7759_CH1OS);

ERROR: Use 4 digit octal (0777) not decimal permissions
#276: FILE: drivers/staging/iio/meter/ade7759.c:276:
+static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO,
+   ade7759_read_8bit,
+   ade7759_write_8bit,
+   ADE7759_CH2OS);

The same for the file drivers/staging/iio/meter/ade7753.c.

We can see that this macro is matched by the pattern "IIO_DEV_ATTR_[A-Z_]+" from
@mode_permission_funcs in checkpatch.pl.

My question is: how this should be fixed?

>From what I can say, it's a false positive so the correct way to fix it is to
change the matching pattern: "IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+".

Or maybe the fix should be done in iio code - argument position inside this
macro?

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

Re: [PATCH 2/9] staging: fsl-mc: fix device ref counting

[Laurentiu Tudor]

Hi Greg,

Thanks for having a look. Comment below.

On 02/03/2017 11:56 AM, Greg KH wrote:
> On Wed, Feb 01, 2017 at 05:43:22AM -0600, laurentiu.tu...@nxp.com wrote:
>> From: Laurentiu Tudor 
>>
>> Drop unneeded get_device() call at device creation
>> and, as per documentation, drop reference count
>> after using device_find_child() return.
>>
>> Signed-off-by: Laurentiu Tudor 
>> ---
>>   drivers/staging/fsl-mc/bus/dprc-driver.c | 1 +
>>   drivers/staging/fsl-mc/bus/fsl-mc-bus.c  | 1 -
>>   2 files changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c 
>> b/drivers/staging/fsl-mc/bus/dprc-driver.c
>> index 4e416d8..e4b0341 100644
>> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
>> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
>> @@ -188,6 +188,7 @@ static void dprc_add_new_devices(struct fsl_mc_device 
>> *mc_bus_dev,
>>  child_dev = fsl_mc_device_lookup(obj_desc, mc_bus_dev);
>>  if (child_dev) {
>>  check_plugged_state_change(child_dev, obj_desc);
>> +put_device(_dev->dev);
>>  continue;
>>  }
>>
>> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c 
>> b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
>> index cc20dc4..7c6a43b 100644
>> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
>> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
>> @@ -537,7 +537,6 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
>>  goto error_cleanup_dev;
>>  }
>>
>> -(void)get_device(_dev->dev);
>
> This implies that your device reference counting is totally wrong and
> messed up.  Does this fix anything?  Break anything?  It should do
> something different now...

It fixes the refcounting in the sense that I'm now seeing the error
that i think you were referring to in your previous reviews,
when we hot unplug a device:

"Device 'foo.N' does not have a release() function, it is broken and 
must be fixed."

See next patch that adds the required callback.

Regarding this particular get_device(), i have no clue why the
original author placed it here. I've looked over other bus 
implementations and didn't see something similar.

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

Re: [PATCH] staging:bcm2048 : Add parentheses around variable x

[Dan Carpenter]

On Sat, Dec 03, 2016 at 03:14:26PM +0530, Tabrez khan wrote:
> Add parentheses around variable x for the readability purpose.
> 

It's not really about readability...

regards,
dan carpenter

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

Re: [PATCH] staging:bcm2048 : Add parentheses around variable x

[Mauro Carvalho Chehab]

Em Sat,  3 Dec 2016 15:14:26 +0530
Tabrez khan  escreveu:

> Add parentheses around variable x for the readability purpose.
> 
> This warning was found using checkpatch.pl.
> 
> Signed-off-by: Tabrez khan 
> ---
>  drivers/staging/media/bcm2048/radio-bcm2048.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
> b/drivers/staging/media/bcm2048/radio-bcm2048.c
> index 4d9bd02..2f28dd0 100644
> --- a/drivers/staging/media/bcm2048/radio-bcm2048.c
> +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
> @@ -185,7 +185,7 @@
>  #define v4l2_to_dev(f)   ((f * BCM2048_FREQV4L2_MULTI) / 
> BCM2048_FREQDEV_UNIT)
>  
>  #define msb(x)  ((u8)((u16)x >> 8))
> -#define lsb(x)  ((u8)((u16)x &  0x00FF))
> +#define lsb(x)  ((u8)((u16)(x) &  0x00FF))

If you're willing to do that, you should do on all macros. Also,
plese remove the extra space before the hexa value.

>  #define compose_u16(msb, lsb)(((u16)msb << 8) | lsb)
>  
>  #define BCM2048_DEFAULT_POWERING_DELAY   20



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

Re: [PATCH 2/9] staging: fsl-mc: fix device ref counting

[Greg KH]

On Wed, Feb 01, 2017 at 05:43:22AM -0600, laurentiu.tu...@nxp.com wrote:
> From: Laurentiu Tudor 
> 
> Drop unneeded get_device() call at device creation
> and, as per documentation, drop reference count
> after using device_find_child() return.
> 
> Signed-off-by: Laurentiu Tudor 
> ---
>  drivers/staging/fsl-mc/bus/dprc-driver.c | 1 +
>  drivers/staging/fsl-mc/bus/fsl-mc-bus.c  | 1 -
>  2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c 
> b/drivers/staging/fsl-mc/bus/dprc-driver.c
> index 4e416d8..e4b0341 100644
> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
> @@ -188,6 +188,7 @@ static void dprc_add_new_devices(struct fsl_mc_device 
> *mc_bus_dev,
>   child_dev = fsl_mc_device_lookup(obj_desc, mc_bus_dev);
>   if (child_dev) {
>   check_plugged_state_change(child_dev, obj_desc);
> + put_device(_dev->dev);
>   continue;
>   }
>  
> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c 
> b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> index cc20dc4..7c6a43b 100644
> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> @@ -537,7 +537,6 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
>   goto error_cleanup_dev;
>   }
>  
> - (void)get_device(_dev->dev);

This implies that your device reference counting is totally wrong and
messed up.  Does this fix anything?  Break anything?  It should do
something different now...

thanks,

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

Re: [PATCH] staging: gdm724x: fix sparse warnings in file gdm_lte.c

[Greg KH]

On Tue, Jan 31, 2017 at 07:37:19PM +0100, Javier Rodriguez wrote:
> drivers/staging/gdm724x/gdm_lte.c:201:33: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/gdm724x/gdm_lte.c:201:33:expected unsigned int [unsigned] 
> [addressable] [usertype] ph_len
> drivers/staging/gdm724x/gdm_lte.c:201:33:got restricted __be16 [usertype] 
> payload_len
> drivers/staging/gdm724x/gdm_lte.c:311:39: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/gdm724x/gdm_lte.c:311:39:expected restricted __sum16 
> [addressable] [assigned] [usertype] icmp6_cksum
> drivers/staging/gdm724x/gdm_lte.c:311:39:got int
> 
> Signed-off-by: Javier Rodriguez 
> ---
>  drivers/staging/gdm724x/gdm_lte.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/gdm724x/gdm_lte.c 
> b/drivers/staging/gdm724x/gdm_lte.c
> index a182757..86b2b4f 100644
> --- a/drivers/staging/gdm724x/gdm_lte.c
> +++ b/drivers/staging/gdm724x/gdm_lte.c
> @@ -198,7 +198,7 @@ static int icmp6_checksum(struct ipv6hdr *ipv6, u16 *ptr, 
> int len)
>   memset(_header, 0, sizeof(pseudo_header));
>   memcpy(_header.ph.ph_src, >saddr.in6_u.u6_addr8, 16);
>   memcpy(_header.ph.ph_dst, >daddr.in6_u.u6_addr8, 16);
> - pseudo_header.ph.ph_len = ipv6->payload_len;
> + pseudo_header.ph.ph_len = be16_to_cpu(ipv6->payload_len);
>   pseudo_header.ph.ph_nxt = ipv6->nexthdr;
>  
>   w = (u16 *)_header;
> @@ -308,7 +308,7 @@ static int gdm_lte_emulate_ndp(struct sk_buff *skb_in, 
> u32 nic_type)
>   memcpy(icmp_na + sizeof(struct icmp6hdr), ,
>  sizeof(struct neighbour_advertisement));
>  
> - icmp6_out.icmp6_cksum = icmp6_checksum(_out,
> + icmp6_out.icmp6_cksum = (__force __sum16) 
> icmp6_checksum(_out,

That feels really wrong, are you sure this is correct?

thanks,

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

Re: [PATCH v2] staging: wlan-ng: This patch fixes the checkpatch.pl warning:

[Greg Kroah-Hartman]

On Tue, Jan 31, 2017 at 09:20:15AM -0500, Maksymilian Piechota wrote:
> Fix checkpatch.pl warning:

Your subject is odd, it leaves something to the imagination :(

> 
> WARNING: Statements should start on a tabstop
> 
> V2: whole if statement cleared up

Put this below the --- line.

v3?

thanks,

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

Re: [PATCH] staging/emxx_udc: Fix styling issues

[Greg Kroah-Hartman]

On Thu, Feb 02, 2017 at 08:56:54PM +0800, Zhengyi Shen wrote:
> 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);

That's horrid, and not any better than the original code :(

>   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);

Same here, please leave the original as-is.

>   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))) {

odd indentation :(

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

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

[Greg KH]

On Fri, Feb 03, 2017 at 10:56:12AM +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].The cause of rejection of [Patch v3] is
> that the mention of the cause was not in the correct format.
> 
> Signed-off-by: Arushi 

Note, I need a "full" name here, not just one name, unless this is what
you use to sign legal documents.

Also, you sent two different copies of this same patch, which one am I
supposed to take?

> 
> ---
> 
> 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.


This is v4, what changed in v2, v3, and v4?

Please list all of the changes, there is lots of good examples of how to
do this on the mailing list and in the documentation.

thanks,

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