[PATCH 1/1] codestyle issue fixed drivers/staging/vc04_services

[Mikhail Shvetsov]

From: Mike 

Signed-off-by: Mike 
---
 .../interface/vchiq_arm/vchiq_kern_lib.c   | 64 --
 1 file changed, 35 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_kern_lib.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_kern_lib.c
index 34f746db19cd..d21bb154f78c 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_kern_lib.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_kern_lib.c
@@ -65,10 +65,10 @@ vchiq_blocking_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle, 
void *data,
unsigned int size, VCHIQ_BULK_DIR_T dir);
 
 /
-*
-*   vchiq_initialise
-*
-***/
+ *
+ *   vchiq_initialise
+ *
+ ***/
 #define VCHIQ_INIT_RETRIES 10
 VCHIQ_STATUS_T vchiq_initialise(VCHIQ_INSTANCE_T *instance_out)
 {
@@ -80,7 +80,9 @@ VCHIQ_STATUS_T vchiq_initialise(VCHIQ_INSTANCE_T 
*instance_out)
vchiq_log_trace(vchiq_core_log_level, "%s called", __func__);
 
/* VideoCore may not be ready due to boot up timing.
-  It may never be ready if kernel and firmware are mismatched, so 
don't block forever. */
+* It may never be ready if kernel and firmware are mismatched, so don't
+* block forever.
+*/
for (i = 0; i < VCHIQ_INIT_RETRIES; i++) {
state = vchiq_get_state();
if (state)
@@ -93,7 +95,8 @@ VCHIQ_STATUS_T vchiq_initialise(VCHIQ_INSTANCE_T 
*instance_out)
goto failed;
} else if (i > 0) {
vchiq_log_warning(vchiq_core_log_level,
-   "%s: videocore initialized after %d retries\n", 
__func__, i);
+   "%s: videocore initialized after %d retries\n",
+   __func__, i);
}
 
instance = kzalloc(sizeof(*instance), GFP_KERNEL);
@@ -121,10 +124,10 @@ VCHIQ_STATUS_T vchiq_initialise(VCHIQ_INSTANCE_T 
*instance_out)
 EXPORT_SYMBOL(vchiq_initialise);
 
 /
-*
-*   vchiq_shutdown
-*
-***/
+ *
+ *   vchiq_shutdown
+ *
+ ***/
 
 VCHIQ_STATUS_T vchiq_shutdown(VCHIQ_INSTANCE_T instance)
 {
@@ -169,10 +172,10 @@ VCHIQ_STATUS_T vchiq_shutdown(VCHIQ_INSTANCE_T instance)
 EXPORT_SYMBOL(vchiq_shutdown);
 
 /
-*
-*   vchiq_is_connected
-*
-***/
+ *
+ *   vchiq_is_connected
+ *
+ ***/
 
 static int vchiq_is_connected(VCHIQ_INSTANCE_T instance)
 {
@@ -180,10 +183,10 @@ static int vchiq_is_connected(VCHIQ_INSTANCE_T instance)
 }
 
 /
-*
-*   vchiq_connect
-*
-***/
+ *
+ *   vchiq_connect
+ *
+ ***/
 
 VCHIQ_STATUS_T vchiq_connect(VCHIQ_INSTANCE_T instance)
 {
@@ -215,10 +218,10 @@ VCHIQ_STATUS_T vchiq_connect(VCHIQ_INSTANCE_T instance)
 EXPORT_SYMBOL(vchiq_connect);
 
 /
-*
-*   vchiq_add_service
-*
-***/
+ *
+ *   vchiq_add_service
+ *
+ ***/
 
 VCHIQ_STATUS_T vchiq_add_service(
VCHIQ_INSTANCE_T  instance,
@@ -260,10 +263,10 @@ VCHIQ_STATUS_T vchiq_add_service(
 EXPORT_SYMBOL(vchiq_add_service);
 
 /
-*
-*   vchiq_open_service
-*
-***/
+ *
+ *   vchiq_open_service
+ *
+ ***/
 
 VCHIQ_STATUS_T vchiq_open_service(
VCHIQ_INSTANCE_T  instance,
@@ -414,8 +417,9 @@ vchiq_blocking_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle, 
void *data,
if ((bulk->data != data) ||
(bulk->size != size)) {
/* This is not a retry of the previous one.
-   ** Cancel the signal when the transfer
-   ** completes. */
+* Cancel the signal when the transfer
+* completes.
+ 

[PATCH v3 3/4] drivers: lustre: ldlm: check result of register_shrinker()

[Aliaksei Karaliou]

ldlm_pools_init() does not check result of register_shrinker()
which was tagged __must_check recently, reported by sparse.

Signed-off-by: Aliaksei Karaliou 
---
 drivers/staging/lustre/lustre/ldlm/ldlm_pool.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

v3: Style fixes, as suggested by Andreas Cheers, Dan Carpenter and Greg KH.

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c 
b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
index da65d00a7811..8563bd32befa 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
@@ -1086,8 +1086,12 @@ int ldlm_pools_init(void)
int rc;
 
rc = ldlm_pools_thread_start();
-   if (rc == 0)
-   register_shrinker(_pools_cli_shrinker);
+   if (rc)
+   return rc;
+
+   rc = register_shrinker(_pools_cli_shrinker);
+   if (rc)
+   ldlm_pools_thread_stop();
 
return rc;
 }
-- 
2.11.0

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

[PATCH v3 2/4] drivers: lustre: ptlrpc: check result of register_shrinker()

[Aliaksei Karaliou]

sptlrpc_enc_pool_init() does not check result of register_shrinker()
which was tagged __must_check recently, reported by sparse.

Signed-off-by: Aliaksei Karaliou 
---
 drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

v3: Style fixes, as suggested by Andreas Cheers, Dan Carpenter and Greg KH.

diff --git a/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c 
b/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
index 77a3721beaee..134ee727e8b7 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
@@ -396,6 +396,8 @@ static struct shrinker pools_shrinker = {
 
 int sptlrpc_enc_pool_init(void)
 {
+   int rc;
+
/*
 * maximum capacity is 1/8 of total physical memory.
 * is the 1/8 a good number?
@@ -432,9 +434,11 @@ int sptlrpc_enc_pool_init(void)
if (!page_pools.epp_pools)
return -ENOMEM;
 
-   register_shrinker(_shrinker);
+   rc = register_shrinker(_shrinker);
+   if (rc)
+   enc_pools_free();
 
-   return 0;
+   return rc;
 }
 
 void sptlrpc_enc_pool_fini(void)
-- 
2.11.0

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

[PATCH v3 4/4] drivers: lustre: obdclass: check result of register_shrinker()

[Aliaksei Karaliou]

lu_global_init() does not check result of register_shrinker()
which was tagged __must_check recently, reported by sparse.
Patch also fixes missed cleanup of resources allocated prior to
register_shrinker() invocation and not freed after any failure.

Signed-off-by: Aliaksei Karaliou 
---
 drivers/staging/lustre/lustre/obdclass/lu_object.c | 27 ++
 1 file changed, 22 insertions(+), 5 deletions(-)

v3: Also address incomplate resource cleanup, noticed by Dan Carpenter.

diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c 
b/drivers/staging/lustre/lustre/obdclass/lu_object.c
index b938a3f9d50a..8e2e6b89e494 100644
--- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
+++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
@@ -1932,8 +1932,10 @@ int lu_global_init(void)
 
LU_CONTEXT_KEY_INIT(_global_key);
result = lu_context_key_register(_global_key);
-   if (result != 0)
+   if (result != 0) {
+   lu_ref_global_fini();
return result;
+   }
 
/*
 * At this level, we don't know what tags are needed, so allocate them
@@ -1943,17 +1945,31 @@ int lu_global_init(void)
down_write(_sites_guard);
result = lu_env_init(_shrink_env, LCT_SHRINKER);
up_write(_sites_guard);
-   if (result != 0)
+   if (result != 0) {
+   lu_context_key_degister(_global_key);
+   lu_ref_global_fini();
return result;
+   }
 
/*
 * seeks estimation: 3 seeks to read a record from oi, one to read
 * inode, one for ea. Unfortunately setting this high value results in
 * lu_object/inode cache consuming all the memory.
 */
-   register_shrinker(_site_shrinker);
+   result = register_shrinker(_site_shrinker);
+   if (result != 0) {
+   /* Order explained in lu_global_fini(). */
+   lu_context_key_degister(_global_key);
 
-   return result;
+   down_write(_sites_guard);
+   lu_env_fini(_shrink_env);
+   up_write(_sites_guard);
+
+   lu_ref_global_fini();
+   return result;
+   }
+
+   return 0;
 }
 
 /**
@@ -1961,7 +1977,8 @@ int lu_global_init(void)
  */
 void lu_global_fini(void)
 {
-   unregister_shrinker(_site_shrinker);
+   if (lu_site_shrinker.nr_deferred)
+   unregister_shrinker(_site_shrinker);
lu_context_key_degister(_global_key);
 
/*
-- 
2.11.0

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

[PATCH v3 1/4] drivers: lustre: osc: check result of register_shrinker()

[Aliaksei Karaliou]

osc_init() does not check result of register_shrinker()
which was tagged __must_check recently, reported by sparse.

Signed-off-by: Aliaksei Karaliou 
---
 drivers/staging/lustre/lustre/osc/osc_request.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c 
b/drivers/staging/lustre/lustre/osc/osc_request.c
index 53eda4c99142..45b1ebf33363 100644
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -2844,7 +2844,9 @@ static int __init osc_init(void)
if (rc)
goto out_kmem;
 
-   register_shrinker(_cache_shrinker);
+   rc = register_shrinker(_cache_shrinker);
+   if (rc)
+   goto out_type;
 
/* This is obviously too much memory, only prevent overflow here */
if (osc_reqpool_mem_max >= 1 << 12 || osc_reqpool_mem_max == 0) {
-- 
2.11.0

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

Re: [PATCH v3 4/4] drivers: lustre: obdclass: check result of register_shrinker()

[ak]

On 12/07/2017 12:55 AM, Aliaksei Karaliou wrote:


lu_global_init() does not check result of register_shrinker()
which was tagged __must_check recently, reported by sparse.
Patch also fixes missed cleanup of resources allocated prior to
register_shrinker() invocation and not freed after any failure.

Signed-off-by: Aliaksei Karaliou 
---
  drivers/staging/lustre/lustre/obdclass/lu_object.c | 27 ++
  1 file changed, 22 insertions(+), 5 deletions(-)

v3: Also address incomplate resource cleanup, noticed by Dan Carpenter.

diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c 
b/drivers/staging/lustre/lustre/obdclass/lu_object.c
index b938a3f9d50a..8e2e6b89e494 100644
--- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
+++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
@@ -1932,8 +1932,10 @@ int lu_global_init(void)
  
  	LU_CONTEXT_KEY_INIT(_global_key);

result = lu_context_key_register(_global_key);
-   if (result != 0)
+   if (result != 0) {
+   lu_ref_global_fini();
return result;
+   }
  
  	/*

 * At this level, we don't know what tags are needed, so allocate them
@@ -1943,17 +1945,31 @@ int lu_global_init(void)
down_write(_sites_guard);
result = lu_env_init(_shrink_env, LCT_SHRINKER);
up_write(_sites_guard);
-   if (result != 0)
+   if (result != 0) {
+   lu_context_key_degister(_global_key);
+   lu_ref_global_fini();
return result;
+   }
  
  	/*

 * seeks estimation: 3 seeks to read a record from oi, one to read
 * inode, one for ea. Unfortunately setting this high value results in
 * lu_object/inode cache consuming all the memory.
 */
-   register_shrinker(_site_shrinker);
+   result = register_shrinker(_site_shrinker);
+   if (result != 0) {
+   /* Order explained in lu_global_fini(). */
+   lu_context_key_degister(_global_key);
  
-	return result;

+   down_write(_sites_guard);
+   lu_env_fini(_shrink_env);
+   up_write(_sites_guard);
+
+   lu_ref_global_fini();
+   return result;
+   }
+
+   return 0;
  }
  
  /**

@@ -1961,7 +1977,8 @@ int lu_global_init(void)
   */
  void lu_global_fini(void)
  {
-   unregister_shrinker(_site_shrinker);
+   if (lu_site_shrinker.nr_deferred)
+   unregister_shrinker(_site_shrinker);
lu_context_key_degister(_global_key);
  
  	/*

I'm sorry, but second patch was blocked by gmail for some reason.
Just a single time, I'll resend the whole sequence.

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

Re: [PATCH 01/10] staging: ccree: remove inline qualifiers

[Gilad Ben-Yossef]

On Mon, Dec 4, 2017 at 11:36 AM, Dan Carpenter  wrote:
> On Sun, Dec 03, 2017 at 01:58:12PM +, Gilad Ben-Yossef wrote:
>> The ccree drivers was marking a lot of big functions in C file as
>> static inline for no good reason. Remove the inline qualifier from
>> any but the few truly single line functions.
>>
>
> The compiler is free to ignore inline hints...  It probably would make
> single line functions inline anyway.
>

Yes. I think of it more as a note to the reader: "don't add stuff to
this function. it is meant to be short and simple".


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ccree: Uninitialized return in ssi_ahash_import()

[Gilad Ben-Yossef]

On Tue, Dec 5, 2017 at 4:37 PM, Dan Carpenter  wrote:
> The return value isn't initialized on some success paths.
>
> Fixes: c5f39d07860c ("staging: ccree: fix leak of import() after init()")
> Signed-off-by: Dan Carpenter 
>
> diff --git a/drivers/staging/ccree/ssi_hash.c 
> b/drivers/staging/ccree/ssi_hash.c
> index a2e8a9d32e00..3855c42e61af 100644
> --- a/drivers/staging/ccree/ssi_hash.c
> +++ b/drivers/staging/ccree/ssi_hash.c
> @@ -1823,7 +1823,7 @@ static int ssi_ahash_import(struct ahash_request *req, 
> const void *in)
> struct device *dev = drvdata_to_dev(ctx->drvdata);
> struct ahash_req_ctx *state = ahash_request_ctx(req);
> u32 tmp;
> -   int rc;
> +   int rc = 0;
>
> memcpy(, in, sizeof(u32));
> if (tmp != CC_EXPORT_MAGIC) {

Acked-by: Gilad Ben-Yossef 

And thanks!
Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ccree: ssi_aead: fixed all coding style warnings.

[Gilad Ben-Yossef]

Hi Sunil,

On Thu, Dec 7, 2017 at 12:08 AM, SUNIL KALLUR RAMEGOWDA
 wrote:
> My first kernel patch, fixed warnings.
>

Congratulations and may there be many more! :-)

Please note that in addition to what Greg's patch-bot already
mentioned I'm pretty sure you are looking
at Linus's tree and not Greg's staging-next tree which already
contains fixes for many of the issues
your patch is addressing.

Thanks,
Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ccree: ssi_aead: fixed all coding style warnings.

[Greg KH]

On Wed, Dec 06, 2017 at 11:08:09PM +0100, SUNIL KALLUR RAMEGOWDA wrote:
> My first kernel patch, fixed warnings.
> 
> Signed-off-by: SUNIL KALLUR RAMEGOWDA 
> ---
>  drivers/staging/ccree/ssi_aead.c | 179 
> +++
>  1 file changed, 123 insertions(+), 56 deletions(-)

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch did many different things all at once, making it difficult
  to review.  All Linux kernel patches need to only do one thing at a
  time.  If you need to do multiple things (such as clean up all coding
  style issues in a file/driver), do it in a sequence of patches, each
  one doing only one thing.  This will make it easier to review the
  patches to ensure that they are correct, and to help alleviate any
  merge issues that larger patches can cause.

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 5/6] staging: pi433: Move enum option_on_off to pi433_if.h

[Greg KH]

On Thu, Dec 07, 2017 at 12:24:34AM +0200, Marcus Wolf wrote:
> 
> 
> Am 06.12.2017 um 22:42 schrieb Simon Sandström:
> > The enum is now only used for ioctl, so move it pi433_if.h.
> > 
> > Signed-off-by: Simon Sandström 
> > ---
> >   drivers/staging/pi433/pi433_if.h  | 5 +
> >   drivers/staging/pi433/rf69_enum.h | 5 -
> >   2 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/staging/pi433/pi433_if.h 
> > b/drivers/staging/pi433/pi433_if.h
> > index bcfe29840889..c8697d144e2b 100644
> > --- a/drivers/staging/pi433/pi433_if.h
> > +++ b/drivers/staging/pi433/pi433_if.h
> > @@ -37,6 +37,11 @@
> >   
> > /*---*/
> > +enum option_on_off {
> > +   OPTION_OFF,
> > +   OPTION_ON
> > +};
> > +
> >   /* IOCTL structs and commands */
> >   /**
> > diff --git a/drivers/staging/pi433/rf69_enum.h 
> > b/drivers/staging/pi433/rf69_enum.h
> > index b0715b4eb6ac..4e64e38ae4ff 100644
> > --- a/drivers/staging/pi433/rf69_enum.h
> > +++ b/drivers/staging/pi433/rf69_enum.h
> > @@ -18,11 +18,6 @@
> >   #ifndef RF69_ENUM_H
> >   #define RF69_ENUM_H
> > -enum option_on_off {
> > -   OPTION_OFF,
> > -   OPTION_ON
> > -};
> > -
> >   enum mode {
> > mode_sleep,
> > standby,
> > 
> 
> Hi Simon,
> 
> sorry - I have one more question.
> 
> Why do you want to get rid of the enum option_on_off in rf69_enum.h. I
> generated that file just to store the enums.
> 
> Now we have one enum at an other place...

No need to have lots of .h files.   This is just a simple driver, it
should only have 1 .h file at most (for anything needed by userspace).

So moving these all into one file is good.

thanks,

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

RE: [PATCH 2/2] staging: fsl-mc: do not print error in case of defer probe error

[Nipun Gupta]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Wednesday, December 06, 2017 16:46
> To: Nipun Gupta 
> Cc: Laurentiu Tudor ; stuyo...@gmail.com; Bharat
> Bhushan ; cakt...@gmail.com;
> bretth...@gmail.com; a...@arndb.de; de...@driverdev.osuosl.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH 2/2] staging: fsl-mc: do not print error in case of defer
> probe error
> 
> On Wed, Dec 06, 2017 at 09:48:07PM +0530, Nipun Gupta wrote:
> > Signed-off-by: Nipun Gupta 
> > ---
> 
> I can't take patches without any changelog text :(

I thought it is a small patch and you may take it without any changelog ;)
Ill resend updating this.

Thanks,
Nipun

> 
> Please fix and resend the series.
> 
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 5/6] staging: pi433: Move enum option_on_off to pi433_if.h

[Marcus Wolf]

Am 06.12.2017 um 22:42 schrieb Simon Sandström:

The enum is now only used for ioctl, so move it pi433_if.h.

Signed-off-by: Simon Sandström 
---
  drivers/staging/pi433/pi433_if.h  | 5 +
  drivers/staging/pi433/rf69_enum.h | 5 -
  2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
index bcfe29840889..c8697d144e2b 100644
--- a/drivers/staging/pi433/pi433_if.h
+++ b/drivers/staging/pi433/pi433_if.h
@@ -37,6 +37,11 @@
  
  /*---*/
  
+enum option_on_off {

+   OPTION_OFF,
+   OPTION_ON
+};
+
  /* IOCTL structs and commands */
  
  /**

diff --git a/drivers/staging/pi433/rf69_enum.h 
b/drivers/staging/pi433/rf69_enum.h
index b0715b4eb6ac..4e64e38ae4ff 100644
--- a/drivers/staging/pi433/rf69_enum.h
+++ b/drivers/staging/pi433/rf69_enum.h
@@ -18,11 +18,6 @@
  #ifndef RF69_ENUM_H
  #define RF69_ENUM_H
  
-enum option_on_off {

-   OPTION_OFF,
-   OPTION_ON
-};
-
  enum mode {
mode_sleep,
standby,



Hi Simon,

sorry - I have one more question.

Why do you want to get rid of the enum option_on_off in rf69_enum.h. I 
generated that file just to store the enums.


Now we have one enum at an other place...

Regards,
Marcus


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

Re: [PATCH v2 04/11] staging: pi433: Rename enum optionOnOff in rf69_enum.h

[Marcus Wolf]

Am 06.12.2017 um 21:57 schrieb Simon Sandström:

On Wed, Dec 06, 2017 at 01:44:14PM +0300, Dan Carpenter wrote:

On Wed, Dec 06, 2017 at 12:31:31PM +0200, Marcus Wolf wrote:



Am 06.12.2017 um 12:23 schrieb Dan Carpenter:



Wow...  This was the one patch I thought was going to sink this
patchset...


I don't get that. What do you mean?


Isn't enum optionOnOff part of the userspace headers?

I thought we weren't allowed to change that.


All enums are for user space (or inteded to be used in userspace in future).
Didn't introduce enums for internal use.


So what I'm asking is if we do this change, does it break any userspace
programs which are used *right now*.  In other words will programs stop
compiling because we've renamed an enum?

regards,
dan carpenter



Hi,

I'm not sure about this so I have to ask: wouldn't the header need to be
in include/uapi/ for userspace to use them? Or is it "allowed" for
userspace programs to use this internal header? Or are we afraid to
break userspace programs that keeps a copy of pi433_if.h?


Regards,
Simon



Hi Simon,

in principle, I think you are right. With my first patch, offering the 
driver I did it that way, but Greg asked me to migrate everything 
(including docu) into staging/pi433. I think, that's related to being in 
staging.
At the moment, I copy pi433_if.h and rf69_enum.h to my userspace 
program, in order to be able to compile it.


To be honest, I am a little bit astonished about that question. Don't 
you do a regression test after such a redesign? I would be a little bit 
afraid to offer such redesign without testing.



Regards,
Marcus



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

[PATCH] staging: ccree: ssi_aead: fixed all coding style warnings.

[SUNIL KALLUR RAMEGOWDA]

My first kernel patch, fixed warnings.

Signed-off-by: SUNIL KALLUR RAMEGOWDA 
---
 drivers/staging/ccree/ssi_aead.c | 179 +++
 1 file changed, 123 insertions(+), 56 deletions(-)

diff --git a/drivers/staging/ccree/ssi_aead.c b/drivers/staging/ccree/ssi_aead.c
index ba0954e..afb0036 100644
--- a/drivers/staging/ccree/ssi_aead.c
+++ b/drivers/staging/ccree/ssi_aead.c
@@ -100,7 +100,8 @@ static void ssi_aead_exit(struct crypto_aead *tfm)
 
/* Unmap enckey buffer */
if (ctx->enckey) {
-   dma_free_coherent(dev, AES_MAX_KEY_SIZE, ctx->enckey, 
ctx->enckey_dma_addr);
+   dma_free_coherent(dev, AES_MAX_KEY_SIZE,
+ ctx->enckey, ctx->enckey_dma_addr);
dev_dbg(dev, "Freed enckey DMA buffer enckey_dma_addr=%pad\n",
>enckey_dma_addr);
ctx->enckey_dma_addr = 0;
@@ -225,7 +226,8 @@ static int ssi_aead_init(struct crypto_aead *tfm)
return -ENOMEM;
 }
 
-static void ssi_aead_complete(struct device *dev, void *ssi_req, void __iomem 
*cc_base)
+static void ssi_aead_complete(struct device *dev, void *ssi_req,
+ void __iomem *cc_base)
 {
struct aead_request *areq = (struct aead_request *)ssi_req;
struct aead_req_ctx *areq_ctx = aead_request_ctx(areq);
@@ -258,12 +260,20 @@ static void ssi_aead_complete(struct device *dev, void 
*ssi_req, void __iomem *c
 ctx->authsize),
SSI_SG_FROM_BUF);
 
-   /* If an IV was generated, copy it back to the user provided 
buffer. */
+   /* If an IV was generated,
+* copy it back to the user provided buffer.
+*/
if (areq_ctx->backup_giv) {
if (ctx->cipher_mode == DRV_CIPHER_CTR)
-   memcpy(areq_ctx->backup_giv, areq_ctx->ctr_iv + 
CTR_RFC3686_NONCE_SIZE, CTR_RFC3686_IV_SIZE);
+   memcpy(areq_ctx->backup_giv,
+  areq_ctx->ctr_iv +
+   CTR_RFC3686_NONCE_SIZE,
+   CTR_RFC3686_IV_SIZE);
else if (ctx->cipher_mode == DRV_CIPHER_CCM)
-   memcpy(areq_ctx->backup_giv, areq_ctx->ctr_iv + 
CCM_BLOCK_IV_OFFSET, CCM_BLOCK_IV_SIZE);
+   memcpy(areq_ctx->backup_giv,
+  areq_ctx->ctr_iv +
+   CCM_BLOCK_IV_OFFSET,
+   CCM_BLOCK_IV_SIZE);
}
}
 
@@ -274,8 +284,8 @@ static int xcbc_setkey(struct cc_hw_desc *desc, struct 
ssi_aead_ctx *ctx)
 {
/* Load the AES key */
hw_desc_init([0]);
-   /* We are using for the source/user key the same buffer as for the 
output keys,
-* because after this key loading it is not needed anymore
+   /* We are using for the source/user key the same buffer as for the
+* output keys, because after this key loading it is not needed anymore
 */
set_din_type([0], DMA_DLLI,
 ctx->auth_state.xcbc.xcbc_keys_dma_addr, ctx->auth_keylen,
@@ -427,7 +437,8 @@ static int validate_keys_sizes(struct ssi_aead_ctx *ctx)
  * (copy to intenral buffer or hash in case of key longer than block
  */
 static int
-ssi_get_plain_hmac_key(struct crypto_aead *tfm, const u8 *key, unsigned int 
keylen)
+ssi_get_plain_hmac_key(struct crypto_aead *tfm, const u8 *key,
+  unsigned int keylen)
 {
dma_addr_t key_dma_addr = 0;
struct ssi_aead_ctx *ctx = crypto_aead_ctx(tfm);
@@ -458,9 +469,11 @@ ssi_get_plain_hmac_key(struct crypto_aead *tfm, const u8 
*key, unsigned int keyl
}
 
if (likely(keylen != 0)) {
-   key_dma_addr = dma_map_single(dev, (void *)key, keylen, 
DMA_TO_DEVICE);
+   key_dma_addr = dma_map_single(dev, (void *)key,
+ keylen, DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(dev, key_dma_addr))) {
-   dev_err(dev, "Mapping key va=0x%p len=%u for DMA 
failed\n",
+   dev_err(dev,
+   "Mapping key va=0x%p len=%u for DMA failed\n",
key, keylen);
return -ENOMEM;
}
@@ -586,7 +599,8 @@ ssi_aead_setkey(struct crypto_aead *tfm, const u8 *key, 
unsigned int keylen)
/* Copy nonce from last 4 bytes in CTR key to
 *  first 4 bytes in CTR IV
 */
-   memcpy(ctx->ctr_nonce, key + ctx->auth_keylen + 
ctx->enc_keylen -
+   memcpy(ctx->ctr_nonce, key +
+   ctx->auth_keylen + 

[PATCH v3 4/4] drivers: lustre: obdclass: check result of register_shrinker()

[Aliaksei Karaliou]

lu_global_init() does not check result of register_shrinker()
which was tagged __must_check recently, reported by sparse.
Patch also fixes missed cleanup of resources allocated prior to
register_shrinker() invocation and not freed after any failure.

Signed-off-by: Aliaksei Karaliou 
---
 drivers/staging/lustre/lustre/obdclass/lu_object.c | 27 ++
 1 file changed, 22 insertions(+), 5 deletions(-)

v3: Also address incomplate resource cleanup, noticed by Dan Carpenter.

diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c 
b/drivers/staging/lustre/lustre/obdclass/lu_object.c
index b938a3f9d50a..8e2e6b89e494 100644
--- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
+++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
@@ -1932,8 +1932,10 @@ int lu_global_init(void)
 
LU_CONTEXT_KEY_INIT(_global_key);
result = lu_context_key_register(_global_key);
-   if (result != 0)
+   if (result != 0) {
+   lu_ref_global_fini();
return result;
+   }
 
/*
 * At this level, we don't know what tags are needed, so allocate them
@@ -1943,17 +1945,31 @@ int lu_global_init(void)
down_write(_sites_guard);
result = lu_env_init(_shrink_env, LCT_SHRINKER);
up_write(_sites_guard);
-   if (result != 0)
+   if (result != 0) {
+   lu_context_key_degister(_global_key);
+   lu_ref_global_fini();
return result;
+   }
 
/*
 * seeks estimation: 3 seeks to read a record from oi, one to read
 * inode, one for ea. Unfortunately setting this high value results in
 * lu_object/inode cache consuming all the memory.
 */
-   register_shrinker(_site_shrinker);
+   result = register_shrinker(_site_shrinker);
+   if (result != 0) {
+   /* Order explained in lu_global_fini(). */
+   lu_context_key_degister(_global_key);
 
-   return result;
+   down_write(_sites_guard);
+   lu_env_fini(_shrink_env);
+   up_write(_sites_guard);
+
+   lu_ref_global_fini();
+   return result;
+   }
+
+   return 0;
 }
 
 /**
@@ -1961,7 +1977,8 @@ int lu_global_init(void)
  */
 void lu_global_fini(void)
 {
-   unregister_shrinker(_site_shrinker);
+   if (lu_site_shrinker.nr_deferred)
+   unregister_shrinker(_site_shrinker);
lu_context_key_degister(_global_key);
 
/*
-- 
2.11.0

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

[PATCH v3 3/4] drivers: lustre: ldlm: check result of register_shrinker()

[Aliaksei Karaliou]

ldlm_pools_init() does not check result of register_shrinker()
which was tagged __must_check recently, reported by sparse.

Signed-off-by: Aliaksei Karaliou 
---
 drivers/staging/lustre/lustre/ldlm/ldlm_pool.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

v3: Style fixes, as suggested by Andreas Cheers, Dan Carpenter and Greg KH.

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c 
b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
index da65d00a7811..8563bd32befa 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
@@ -1086,8 +1086,12 @@ int ldlm_pools_init(void)
int rc;
 
rc = ldlm_pools_thread_start();
-   if (rc == 0)
-   register_shrinker(_pools_cli_shrinker);
+   if (rc)
+   return rc;
+
+   rc = register_shrinker(_pools_cli_shrinker);
+   if (rc)
+   ldlm_pools_thread_stop();
 
return rc;
 }
-- 
2.11.0

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

[PATCH v3 1/4] drivers: lustre: osc: check result of register_shrinker()

[Aliaksei Karaliou]

osc_init() does not check result of register_shrinker()
which was tagged __must_check recently, reported by sparse.

Signed-off-by: Aliaksei Karaliou 
---
 drivers/staging/lustre/lustre/osc/osc_request.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c 
b/drivers/staging/lustre/lustre/osc/osc_request.c
index 53eda4c99142..45b1ebf33363 100644
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -2844,7 +2844,9 @@ static int __init osc_init(void)
if (rc)
goto out_kmem;
 
-   register_shrinker(_cache_shrinker);
+   rc = register_shrinker(_cache_shrinker);
+   if (rc)
+   goto out_type;
 
/* This is obviously too much memory, only prevent overflow here */
if (osc_reqpool_mem_max >= 1 << 12 || osc_reqpool_mem_max == 0) {
-- 
2.11.0

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

[PATCHv3] staging: pi433: pi433_if.c remove SET_CHECKED macro

[Nguyen Phan Quang Minh]

The macro calls its argument -a function- twice, makes the calling
function return prematurely -skipping resource cleanup code- and hurts
understandability.

Signed-off-by: Nguyen Phan Quang Minh 
---
v3: change pi433_receive abort code to always call
wake_up_interruptible(>tx_wait_queue); even if
rf69_set_mode(dev->spi, standby) fails.

 drivers/staging/pi433/pi433_if.c | 235 ---
 1 file changed, 172 insertions(+), 63 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 3404cb9722c9..9e9e838204bd 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -119,12 +119,6 @@ struct pi433_instance {
struct pi433_tx_cfg tx_cfg;
 };
 
-/*-*/
-
-/* macro for checked access of registers of radio module */
-#define SET_CHECKED(retval) \
-   if (retval < 0) \
-   return retval;
 
 /*-*/
 
@@ -183,28 +177,53 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
int payload_length;
 
/* receiver config */
-   SET_CHECKED(rf69_set_frequency  (dev->spi, rx_cfg->frequency));
-   SET_CHECKED(rf69_set_bit_rate   (dev->spi, rx_cfg->bit_rate));
-   SET_CHECKED(rf69_set_modulation (dev->spi, rx_cfg->modulation));
-   SET_CHECKED(rf69_set_antenna_impedance   (dev->spi, 
rx_cfg->antenna_impedance));
-   SET_CHECKED(rf69_set_rssi_threshold  (dev->spi, 
rx_cfg->rssi_threshold));
-   SET_CHECKED(rf69_set_ook_threshold_dec   (dev->spi, 
rx_cfg->thresholdDecrement));
-   SET_CHECKED(rf69_set_bandwidth   (dev->spi, 
rx_cfg->bw_mantisse, rx_cfg->bw_exponent));
-   SET_CHECKED(rf69_set_bandwidth_during_afc(dev->spi, 
rx_cfg->bw_mantisse, rx_cfg->bw_exponent));
-   SET_CHECKED(rf69_set_dagc(dev->spi, rx_cfg->dagc));
+   ret = rf69_set_frequency(dev->spi, rx_cfg->frequency);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_bit_rate(dev->spi, rx_cfg->bit_rate);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_modulation(dev->spi, rx_cfg->modulation);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_antenna_impedance(dev->spi, rx_cfg->antenna_impedance);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_rssi_threshold(dev->spi, rx_cfg->rssi_threshold);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_ook_threshold_dec(dev->spi, rx_cfg->thresholdDecrement);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_bandwidth(dev->spi, rx_cfg->bw_mantisse, 
rx_cfg->bw_exponent);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_bandwidth_during_afc(dev->spi, rx_cfg->bw_mantisse,
+   rx_cfg->bw_exponent);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_dagc(dev->spi, rx_cfg->dagc);
+   if (ret < 0)
+   return ret;
 
dev->rx_bytes_to_drop = rx_cfg->bytes_to_drop;
 
/* packet config */
/* enable */
-   SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
+   ret = rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync);
+   if (ret < 0)
+   return ret;
if (rx_cfg->enable_sync == optionOn)
{
-   SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, 
afterSyncInterrupt));
+   ret = rf69_set_fifo_fill_condition(dev->spi, 
afterSyncInterrupt);
+   if (ret < 0)
+   return ret;
}
else
{
-   SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
+   ret = rf69_set_fifo_fill_condition(dev->spi, always);
+   if (ret < 0)
+   return ret;
}
if (rx_cfg->enable_length_byte == optionOn) {
ret = rf69_set_packet_format(dev->spi, packetLengthVar);
@@ -215,36 +234,54 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
if (ret < 0)
return ret;
}
-   SET_CHECKED(rf69_set_adressFiltering(dev->spi, 
rx_cfg->enable_address_filtering));
-   SET_CHECKED(rf69_set_crc_enable (dev->spi, rx_cfg->enable_crc));
+   ret = rf69_set_adressFiltering(dev->spi, 
rx_cfg->enable_address_filtering);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_crc_enable(dev->spi, rx_cfg->enable_crc);
+   if (ret < 0)
+   return ret;
 
/* lengths */
-   SET_CHECKED(rf69_set_sync_size(dev->spi, rx_cfg->sync_length));
+   ret = rf69_set_sync_size(dev->spi, rx_cfg->sync_length);
+   if (ret < 0)
+   return ret;
if (rx_cfg->enable_length_byte == optionOn)
{
-   

[PATCH 6/6] staging: pi433: Remove SET_CHECKED usage from pi433_probe

[Simon Sandström]

SET_CHECKED returns from the function on failure and in pi433_probe it is
necessary to free the GPIOs and the device on failure.

Signed-off-by: Simon Sandström 
---
 drivers/staging/pi433/pi433_if.c | 28 +---
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 688d0cf00782..55ed45f45998 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -1124,13 +1124,27 @@ static int pi433_probe(struct spi_device *spi)
}
 
/* setup the radio module */
-   SET_CHECKED(rf69_set_mode   (spi, standby));
-   SET_CHECKED(rf69_set_data_mode  (spi, DATAMODUL_MODE_PACKET));
-   SET_CHECKED(rf69_enable_amplifier(spi, MASK_PALEVEL_PA0));
-   SET_CHECKED(rf69_disable_amplifier(spi, MASK_PALEVEL_PA1));
-   SET_CHECKED(rf69_disable_amplifier(spi, MASK_PALEVEL_PA2));
-   SET_CHECKED(rf69_set_output_power_level (spi, 13));
-   SET_CHECKED(rf69_set_antenna_impedance  (spi, fiftyOhm));
+   retval = rf69_set_mode(spi, standby);
+   if (retval < 0)
+   goto minor_failed;
+   retval = rf69_set_data_mode(spi, DATAMODUL_MODE_PACKET);
+   if (retval < 0)
+   goto minor_failed;
+   retval = rf69_enable_amplifier(spi, MASK_PALEVEL_PA0);
+   if (retval < 0)
+   goto minor_failed;
+   retval = rf69_disable_amplifier(spi, MASK_PALEVEL_PA1);
+   if (retval < 0)
+   goto minor_failed;
+   retval = rf69_disable_amplifier(spi, MASK_PALEVEL_PA2);
+   if (retval < 0)
+   goto minor_failed;
+   retval = rf69_set_output_power_level(spi, 13);
+   if (retval < 0)
+   goto minor_failed;
+   retval = rf69_set_antenna_impedance(spi, fiftyOhm);
+   if (retval < 0)
+   goto minor_failed;
 
/* determ minor number */
retval = pi433_get_minor(device);
-- 
2.11.0

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

[PATCH 0/6] staging: pi433: Minor cleanup and style fixes

[Simon Sandström]

These are the six remaining patches from "[PATCH v2 00/11] Fix indentation and
CamelCase issues in staging/pi433" that couldn't be applied, rebased on top of
staging-next.

- Simon

---

Simon Sandström (6):
  staging: pi433: Split rf69_set_crc_enabled into two functions
  staging: pi433: Split rf69_set_sync_enabled into two functions
  staging: pi433: Remove enum data_mode
  staging: pi433: Combine all rf69_set_amplifier_x()
  staging: pi433: Move enum option_on_off to pi433_if.h
  staging: pi433: Remove SET_CHECKED usage from pi433_probe

 drivers/staging/pi433/pi433_if.c  | 71 +++-
 drivers/staging/pi433/pi433_if.h  |  5 ++
 drivers/staging/pi433/rf69.c  | 97 ---
 drivers/staging/pi433/rf69.h  | 18 +++-
 drivers/staging/pi433/rf69_enum.h | 11 -
 5 files changed, 90 insertions(+), 112 deletions(-)

-- 
2.11.0

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

[PATCH 5/6] staging: pi433: Move enum option_on_off to pi433_if.h

[Simon Sandström]

The enum is now only used for ioctl, so move it pi433_if.h.

Signed-off-by: Simon Sandström 
---
 drivers/staging/pi433/pi433_if.h  | 5 +
 drivers/staging/pi433/rf69_enum.h | 5 -
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
index bcfe29840889..c8697d144e2b 100644
--- a/drivers/staging/pi433/pi433_if.h
+++ b/drivers/staging/pi433/pi433_if.h
@@ -37,6 +37,11 @@
 
 /*---*/
 
+enum option_on_off {
+   OPTION_OFF,
+   OPTION_ON
+};
+
 /* IOCTL structs and commands */
 
 /**
diff --git a/drivers/staging/pi433/rf69_enum.h 
b/drivers/staging/pi433/rf69_enum.h
index b0715b4eb6ac..4e64e38ae4ff 100644
--- a/drivers/staging/pi433/rf69_enum.h
+++ b/drivers/staging/pi433/rf69_enum.h
@@ -18,11 +18,6 @@
 #ifndef RF69_ENUM_H
 #define RF69_ENUM_H
 
-enum option_on_off {
-   OPTION_OFF,
-   OPTION_ON
-};
-
 enum mode {
mode_sleep,
standby,
-- 
2.11.0

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

[PATCH 2/6] staging: pi433: Split rf69_set_sync_enabled into two functions

[Simon Sandström]

Splits rf69_set_sync_enabled(dev, enabled) into
rf69_enable_sync(dev) and rf69_disable_sync(dev).

Signed-off-by: Simon Sandström 
---
 drivers/staging/pi433/pi433_if.c | 21 +++--
 drivers/staging/pi433/rf69.c | 18 ++
 drivers/staging/pi433/rf69.h |  4 ++--
 3 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 614eec7dd904..fb500d062df8 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -197,13 +197,20 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
 
/* packet config */
/* enable */
-   SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
if (rx_cfg->enable_sync == OPTION_ON)
{
+   ret = rf69_enable_sync(dev->spi);
+   if (ret < 0)
+   return ret;
+
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, 
afterSyncInterrupt));
}
else
{
+   ret = rf69_disable_sync(dev->spi);
+   if (ret < 0)
+   return ret;
+
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
}
if (rx_cfg->enable_length_byte == OPTION_ON) {
@@ -281,7 +288,17 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct 
pi433_tx_cfg *tx_cfg)
{
SET_CHECKED(rf69_set_preamble_length(dev->spi, 0));
}
-   SET_CHECKED(rf69_set_sync_enable  (dev->spi, tx_cfg->enable_sync));
+
+   if (tx_cfg->enable_sync == OPTION_ON) {
+   ret = rf69_enable_sync(dev->spi);
+   if (ret < 0)
+   return ret;
+   } else {
+   ret = rf69_disable_sync(dev->spi);
+   if (ret < 0)
+   return ret;
+   }
+
if (tx_cfg->enable_length_byte == OPTION_ON) {
ret = rf69_set_packet_format(dev->spi, packetLengthVar);
if (ret < 0)
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index 8c9c9bb91c53..12a1091d9936 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -746,20 +746,14 @@ int rf69_set_preamble_length(struct spi_device *spi, u16 
preambleLength)
return retval;
 }
 
-int rf69_set_sync_enable(struct spi_device *spi,
-enum option_on_off option_on_off)
+int rf69_enable_sync(struct spi_device *spi)
 {
-   #ifdef DEBUG
-   dev_dbg(>dev, "set: sync enable");
-   #endif
+   return rf69_set_bit(spi, REG_SYNC_CONFIG, MASK_SYNC_CONFIG_SYNC_ON);
+}
 
-   switch (option_on_off) {
-   case OPTION_ON: return rf69_set_bit(spi, REG_SYNC_CONFIG, 
MASK_SYNC_CONFIG_SYNC_ON);
-   case OPTION_OFF: return rf69_clear_bit(spi, REG_SYNC_CONFIG, 
MASK_SYNC_CONFIG_SYNC_ON);
-   default:
-   dev_dbg(>dev, "set: illegal input param");
-   return -EINVAL;
-   }
+int rf69_disable_sync(struct spi_device *spi)
+{
+   return rf69_clear_bit(spi, REG_SYNC_CONFIG, MASK_SYNC_CONFIG_SYNC_ON);
 }
 
 int rf69_set_fifo_fill_condition(struct spi_device *spi, enum 
fifoFillCondition fifoFillCondition)
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 9428dee97de7..177223451c87 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -59,8 +59,8 @@ int rf69_set_rssi_threshold(struct spi_device *spi, u8 
threshold);
 int rf69_set_rx_start_timeout(struct spi_device *spi, u8 timeout);
 int rf69_set_rssi_timeout(struct spi_device *spi, u8 timeout);
 int rf69_set_preamble_length(struct spi_device *spi, u16 preambleLength);
-int rf69_set_sync_enable(struct spi_device *spi,
-enum option_on_off option_on_off);
+int rf69_enable_sync(struct spi_device *spi);
+int rf69_disable_sync(struct spi_device *spi);
 int rf69_set_fifo_fill_condition(struct spi_device *spi, enum 
fifoFillCondition fifoFillCondition);
 int rf69_set_sync_size(struct spi_device *spi, u8 sync_size);
 int rf69_set_sync_tolerance(struct spi_device *spi, u8 syncTolerance);
-- 
2.11.0

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

[PATCH 3/6] staging: pi433: Remove enum data_mode

[Simon Sandström]

Call rf69_set_data_mode with DATAMODUL_MODE value directly.

Signed-off-by: Simon Sandström 
---
 drivers/staging/pi433/pi433_if.c  |  2 +-
 drivers/staging/pi433/rf69.c  | 15 ++-
 drivers/staging/pi433/rf69.h  |  2 +-
 drivers/staging/pi433/rf69_enum.h |  6 --
 4 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index fb500d062df8..3b4170b9ba94 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -1125,7 +1125,7 @@ static int pi433_probe(struct spi_device *spi)
 
/* setup the radio module */
SET_CHECKED(rf69_set_mode   (spi, standby));
-   SET_CHECKED(rf69_set_data_mode  (spi, packet));
+   SET_CHECKED(rf69_set_data_mode  (spi, DATAMODUL_MODE_PACKET));
SET_CHECKED(rf69_set_amplifier_0(spi, OPTION_ON));
SET_CHECKED(rf69_set_amplifier_1(spi, OPTION_OFF));
SET_CHECKED(rf69_set_amplifier_2(spi, OPTION_OFF));
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index 12a1091d9936..9f2ffb89033e 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -85,20 +85,9 @@ int rf69_set_mode(struct spi_device *spi, enum mode mode)
 
 }
 
-int rf69_set_data_mode(struct spi_device *spi, enum dataMode dataMode)
+int rf69_set_data_mode(struct spi_device *spi, u8 data_mode)
 {
-   #ifdef DEBUG
-   dev_dbg(>dev, "set: data mode");
-   #endif
-
-   switch (dataMode) {
-   case packet:return rf69_read_mod_write(spi, REG_DATAMODUL, 
MASK_DATAMODUL_MODE, DATAMODUL_MODE_PACKET);
-   case continuous:return rf69_read_mod_write(spi, REG_DATAMODUL, 
MASK_DATAMODUL_MODE, DATAMODUL_MODE_CONTINUOUS);
-   case continuousNoSync:  return rf69_read_mod_write(spi, REG_DATAMODUL, 
MASK_DATAMODUL_MODE, DATAMODUL_MODE_CONTINUOUS_NOSYNC);
-   default:
-   dev_dbg(>dev, "set: illegal input param");
-   return -EINVAL;
-   }
+   return rf69_read_mod_write(spi, REG_DATAMODUL, MASK_DATAMODUL_MODE, 
data_mode);
 }
 
 int rf69_set_modulation(struct spi_device *spi, enum modulation modulation)
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 177223451c87..18296b4502f3 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -26,7 +26,7 @@
 #define FIFO_THRESHOLD 15  /* in byte */
 
 int rf69_set_mode(struct spi_device *spi, enum mode mode);
-int rf69_set_data_mode(struct spi_device *spi, enum dataMode dataMode);
+int rf69_set_data_mode(struct spi_device *spi, u8 data_mode);
 int rf69_set_modulation(struct spi_device *spi, enum modulation modulation);
 enum modulation rf69_get_modulation(struct spi_device *spi);
 int rf69_set_modulation_shaping(struct spi_device *spi, enum mod_shaping 
mod_shaping);
diff --git a/drivers/staging/pi433/rf69_enum.h 
b/drivers/staging/pi433/rf69_enum.h
index 97f615effca3..b0715b4eb6ac 100644
--- a/drivers/staging/pi433/rf69_enum.h
+++ b/drivers/staging/pi433/rf69_enum.h
@@ -31,12 +31,6 @@ enum mode {
receive
 };
 
-enum dataMode {
-   packet,
-   continuous,
-   continuousNoSync
-};
-
 enum modulation {
OOK,
FSK
-- 
2.11.0

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

[PATCH 4/6] staging: pi433: Combine all rf69_set_amplifier_x()

[Simon Sandström]

Replaces the functions rf69_set_amplifier_1, _2, _3 with two
functions: rf69_enable_amplifier(dev, amp_mask) and
rf69_disable_amplifier(dev, amp_mask).

Signed-off-by: Simon Sandström 
---
 drivers/staging/pi433/pi433_if.c |  6 +++---
 drivers/staging/pi433/rf69.c | 46 
 drivers/staging/pi433/rf69.h |  8 ++-
 3 files changed, 9 insertions(+), 51 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 3b4170b9ba94..688d0cf00782 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -1126,9 +1126,9 @@ static int pi433_probe(struct spi_device *spi)
/* setup the radio module */
SET_CHECKED(rf69_set_mode   (spi, standby));
SET_CHECKED(rf69_set_data_mode  (spi, DATAMODUL_MODE_PACKET));
-   SET_CHECKED(rf69_set_amplifier_0(spi, OPTION_ON));
-   SET_CHECKED(rf69_set_amplifier_1(spi, OPTION_OFF));
-   SET_CHECKED(rf69_set_amplifier_2(spi, OPTION_OFF));
+   SET_CHECKED(rf69_enable_amplifier(spi, MASK_PALEVEL_PA0));
+   SET_CHECKED(rf69_disable_amplifier(spi, MASK_PALEVEL_PA1));
+   SET_CHECKED(rf69_disable_amplifier(spi, MASK_PALEVEL_PA2));
SET_CHECKED(rf69_set_output_power_level (spi, 13));
SET_CHECKED(rf69_set_antenna_impedance  (spi, fiftyOhm));
 
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index 9f2ffb89033e..7140fa2ea592 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -282,52 +282,14 @@ int rf69_set_frequency(struct spi_device *spi, u32 
frequency)
return 0;
 }
 
-int rf69_set_amplifier_0(struct spi_device *spi,
-enum option_on_off option_on_off)
+int rf69_enable_amplifier(struct spi_device *spi, u8 amplifier_mask)
 {
-   #ifdef DEBUG
-   dev_dbg(>dev, "set: amp #0");
-   #endif
-
-   switch (option_on_off) {
-   case OPTION_ON: return rf69_set_bit(spi, REG_PALEVEL, MASK_PALEVEL_PA0);
-   case OPTION_OFF: return rf69_clear_bit(spi, REG_PALEVEL, 
MASK_PALEVEL_PA0);
-   default:
-   dev_dbg(>dev, "set: illegal input param");
-   return -EINVAL;
-   }
-}
-
-int rf69_set_amplifier_1(struct spi_device *spi,
-enum option_on_off option_on_off)
-{
-   #ifdef DEBUG
-   dev_dbg(>dev, "set: amp #1");
-   #endif
-
-   switch (option_on_off) {
-   case OPTION_ON: return rf69_set_bit(spi, REG_PALEVEL, MASK_PALEVEL_PA1);
-   case OPTION_OFF: return rf69_clear_bit(spi, REG_PALEVEL, 
MASK_PALEVEL_PA1);
-   default:
-   dev_dbg(>dev, "set: illegal input param");
-   return -EINVAL;
-   }
+   return rf69_set_bit(spi, REG_PALEVEL, amplifier_mask);
 }
 
-int rf69_set_amplifier_2(struct spi_device *spi,
-enum option_on_off option_on_off)
+int rf69_disable_amplifier(struct spi_device *spi, u8 amplifier_mask)
 {
-   #ifdef DEBUG
-   dev_dbg(>dev, "set: amp #2");
-   #endif
-
-   switch (option_on_off) {
-   case OPTION_ON: return rf69_set_bit(spi, REG_PALEVEL, MASK_PALEVEL_PA2);
-   case OPTION_OFF: return rf69_clear_bit(spi, REG_PALEVEL, 
MASK_PALEVEL_PA2);
-   default:
-   dev_dbg(>dev, "set: illegal input param");
-   return -EINVAL;
-   }
+   return rf69_clear_bit(spi, REG_PALEVEL, amplifier_mask);
 }
 
 int rf69_set_output_power_level(struct spi_device *spi, u8 powerLevel)
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 18296b4502f3..ba25ab6aeae7 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -33,12 +33,8 @@ int rf69_set_modulation_shaping(struct spi_device *spi, enum 
mod_shaping mod_sha
 int rf69_set_bit_rate(struct spi_device *spi, u16 bitRate);
 int rf69_set_deviation(struct spi_device *spi, u32 deviation);
 int rf69_set_frequency(struct spi_device *spi, u32 frequency);
-int rf69_set_amplifier_0(struct spi_device *spi,
-enum option_on_off option_on_off);
-int rf69_set_amplifier_1(struct spi_device *spi,
-enum option_on_off option_on_off);
-int rf69_set_amplifier_2(struct spi_device *spi,
-enum option_on_off option_on_off);
+int rf69_enable_amplifier(struct spi_device *spi, u8 amplifier_mask);
+int rf69_disable_amplifier(struct spi_device *spi, u8 amplifier_mask);
 int rf69_set_output_power_level(struct spi_device *spi, u8 powerLevel);
 int rf69_set_pa_ramp(struct spi_device *spi, enum paRamp paRamp);
 int rf69_set_antenna_impedance(struct spi_device *spi, enum antennaImpedance 
antennaImpedance);
-- 
2.11.0

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

[PATCH 1/6] staging: pi433: Split rf69_set_crc_enabled into two functions

[Simon Sandström]

Splits rf69_set_crc_enabled(dev, enabled) into
rf69_enable_crc(dev) and rf69_disable_crc(dev).

Signed-off-by: Simon Sandström 
---
 drivers/staging/pi433/pi433_if.c | 22 --
 drivers/staging/pi433/rf69.c | 18 ++
 drivers/staging/pi433/rf69.h |  4 ++--
 3 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 2ae19ac565d1..614eec7dd904 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -216,7 +216,16 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
return ret;
}
SET_CHECKED(rf69_set_adressFiltering(dev->spi, 
rx_cfg->enable_address_filtering));
-   SET_CHECKED(rf69_set_crc_enable (dev->spi, rx_cfg->enable_crc));
+
+   if (rx_cfg->enable_crc == OPTION_ON) {
+   ret = rf69_enable_crc(dev->spi);
+   if (ret < 0)
+   return ret;
+   } else {
+   ret = rf69_disable_crc(dev->spi);
+   if (ret < 0)
+   return ret;
+   }
 
/* lengths */
SET_CHECKED(rf69_set_sync_size(dev->spi, rx_cfg->sync_length));
@@ -282,7 +291,16 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct 
pi433_tx_cfg *tx_cfg)
if (ret < 0)
return ret;
}
-   SET_CHECKED(rf69_set_crc_enable   (dev->spi, tx_cfg->enable_crc));
+
+   if (tx_cfg->enable_crc == OPTION_ON) {
+   ret = rf69_enable_crc(dev->spi);
+   if (ret < 0)
+   return ret;
+   } else {
+   ret = rf69_disable_crc(dev->spi);
+   if (ret < 0)
+   return ret;
+   }
 
/* configure sync, if enabled */
if (tx_cfg->enable_sync == OPTION_ON) {
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index c97fd8031ecb..8c9c9bb91c53 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -844,20 +844,14 @@ int rf69_set_packet_format(struct spi_device *spi, enum 
packetFormat packetForma
}
 }
 
-int rf69_set_crc_enable(struct spi_device *spi,
-   enum option_on_off option_on_off)
+int rf69_enable_crc(struct spi_device *spi)
 {
-   #ifdef DEBUG
-   dev_dbg(>dev, "set: crc enable");
-   #endif
+   return rf69_set_bit(spi, REG_PACKETCONFIG1, MASK_PACKETCONFIG1_CRC_ON);
+}
 
-   switch (option_on_off) {
-   case OPTION_ON: return rf69_set_bit(spi, REG_PACKETCONFIG1, 
MASK_PACKETCONFIG1_CRC_ON);
-   case OPTION_OFF: return rf69_clear_bit(spi, REG_PACKETCONFIG1, 
MASK_PACKETCONFIG1_CRC_ON);
-   default:
-   dev_dbg(>dev, "set: illegal input param");
-   return -EINVAL;
-   }
+int rf69_disable_crc(struct spi_device *spi)
+{
+   return rf69_clear_bit(spi, REG_PACKETCONFIG1, 
MASK_PACKETCONFIG1_CRC_ON);
 }
 
 int rf69_set_adressFiltering(struct spi_device *spi, enum addressFiltering 
addressFiltering)
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 1cb6db33d6fe..9428dee97de7 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -66,8 +66,8 @@ int rf69_set_sync_size(struct spi_device *spi, u8 sync_size);
 int rf69_set_sync_tolerance(struct spi_device *spi, u8 syncTolerance);
 int rf69_set_sync_values(struct spi_device *spi, u8 syncValues[8]);
 int rf69_set_packet_format(struct spi_device *spi, enum packetFormat 
packetFormat);
-int rf69_set_crc_enable(struct spi_device *spi,
-   enum option_on_off option_on_off);
+int rf69_enable_crc(struct spi_device *spi);
+int rf69_disable_crc(struct spi_device *spi);
 int rf69_set_adressFiltering(struct spi_device *spi, enum addressFiltering 
addressFiltering);
 int rf69_set_payload_length(struct spi_device *spi, u8 payloadLength);
 u8  rf69_get_payload_length(struct spi_device *spi);
-- 
2.11.0

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

Re: [PATCH v2 06/11] staging: pi433: Split rf69_set_crc_enabled into two functions

[Simon Sandström]

On Wed, Dec 06, 2017 at 04:16:04PM +0100, Greg KH wrote:
> 
> I had to stop here in applying this series, as the merge conflicts just
> got too much for me to resolve by hand.
> 
> Can you rebase this series on my staging-testing branch of staging.git
> and send the remaining patches please?
> 
> thanks,
> 
> greg k-h

Hi,

Yes, I'll send a new patchset with the remaining patches.


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

Re: [PATCH v2 04/11] staging: pi433: Rename enum optionOnOff in rf69_enum.h

[Simon Sandström]

On Wed, Dec 06, 2017 at 01:44:14PM +0300, Dan Carpenter wrote:
> On Wed, Dec 06, 2017 at 12:31:31PM +0200, Marcus Wolf wrote:
> > 
> > 
> > Am 06.12.2017 um 12:23 schrieb Dan Carpenter:
> > > 
> > > 
> > > Wow...  This was the one patch I thought was going to sink this
> > > patchset...
> > 
> > I don't get that. What do you mean?
> > 
> > > Isn't enum optionOnOff part of the userspace headers?
> > > 
> > > I thought we weren't allowed to change that.
> > 
> > All enums are for user space (or inteded to be used in userspace in future).
> > Didn't introduce enums for internal use.
> 
> So what I'm asking is if we do this change, does it break any userspace
> programs which are used *right now*.  In other words will programs stop
> compiling because we've renamed an enum?
> 
> regards,
> dan carpenter
> 

Hi,

I'm not sure about this so I have to ask: wouldn't the header need to be
in include/uapi/ for userspace to use them? Or is it "allowed" for
userspace programs to use this internal header? Or are we afraid to
break userspace programs that keeps a copy of pi433_if.h?


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

[PATCH 3/3] staging: lustre: llite: Remove redundant else keyword

[Luis de Bethencourt]

There is no need to use 'else' if in main branch 'return' is present.

Signed-off-by: Luis de Bethencourt 
---
 drivers/staging/lustre/lustre/llite/vvp_io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/vvp_io.c 
b/drivers/staging/lustre/lustre/llite/vvp_io.c
index bfae98e82d6f..e7a4778e02e4 100644
--- a/drivers/staging/lustre/lustre/llite/vvp_io.c
+++ b/drivers/staging/lustre/lustre/llite/vvp_io.c
@@ -699,7 +699,7 @@ static int vvp_io_read_start(const struct lu_env *env,
result = vvp_prep_size(env, obj, io, pos, tot, );
if (result != 0)
return result;
-   else if (exceed != 0)
+   if (exceed != 0)
goto out;
 
LU_OBJECT_HEADER(D_INODE, env, >co_lu,
-- 
2.15.1

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

[PATCH 1/3] staging: lustre: llite: Remove else after goto

[Luis de Bethencourt]

If an "if" branch is terminated by a "goto", there's no need to have an
"else" statement and an indented block of code.

Remove the "else" statement to simplify the code flow.

Signed-off-by: Luis de Bethencourt 
---

Hi,

The following patches remove unneeded 'else' after a 'goto' or 'return'.
They are meant to just make the code more readable and aren't functional
changes.

Thanks,
Luis

 drivers/staging/lustre/lustre/llite/dir.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/dir.c 
b/drivers/staging/lustre/lustre/llite/dir.c
index 5b2e47c246f3..f5b67a4923e3 100644
--- a/drivers/staging/lustre/lustre/llite/dir.c
+++ b/drivers/staging/lustre/lustre/llite/dir.c
@@ -1339,9 +1339,9 @@ static long ll_dir_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg)
   cmd == LL_IOC_MDC_GETINFO)) {
rc = 0;
goto skip_lmm;
-   } else {
-   goto out_req;
}
+
+   goto out_req;
}
 
if (cmd == IOC_MDC_GETFILESTRIPE ||
-- 
2.15.1

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

[PATCH 2/3] staging: lustre: llite: Remove redundant else keyword

[Luis de Bethencourt]

There is no need to use 'else' if in main branch 'goto' is present.

Signed-off-by: Luis de Bethencourt 
---
 drivers/staging/lustre/lustre/llite/llite_lib.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c 
b/drivers/staging/lustre/lustre/llite/llite_lib.c
index 8666f1e81ade..e84719662edf 100644
--- a/drivers/staging/lustre/lustre/llite/llite_lib.c
+++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
@@ -236,7 +236,9 @@ static int client_common_fill_super(struct super_block *sb, 
char *md, char *dt,
   "An MDT (md %s) is performing recovery, of 
which this client is not a part. Please wait for recovery to complete, abort, 
or time out.\n",
   md);
goto out;
-   } else if (err) {
+   }
+
+   if (err) {
CERROR("cannot connect to %s: rc = %d\n", md, err);
goto out;
}
-- 
2.15.1

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

Re: [PATCH] android: binder: Check for errors in binder_alloc_shrinker_init().

[Sherry Yang]

Hi Tetsuo,

It looks like this patch was not submitted to LKML. Perhaps you want
to send it to the mailing list and add the correct set of recipients
using scripts/get_maintainer.pl as suggested here
https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html.

-Sherry

On Wed, Nov 29, 2017 at 8:29 AM, Tetsuo Handa
 wrote:
> Both list_lru_init() and register_shrinker() might return an error.
>
> Signed-off-by: Tetsuo Handa 
> Cc: Sherry Yang 
> Cc: Greg Kroah-Hartman 
> Cc: Michal Hocko 
> ---
>  drivers/android/binder.c   |  4 +++-
>  drivers/android/binder_alloc.c | 12 +---
>  drivers/android/binder_alloc.h |  2 +-
>  3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 85b0bb4..a54a0f1 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -5569,7 +5569,9 @@ static int __init binder_init(void)
> struct binder_device *device;
> struct hlist_node *tmp;
>
> -   binder_alloc_shrinker_init();
> +   ret = binder_alloc_shrinker_init();
> +   if (ret)
> +   return ret;
>
> atomic_set(_transaction_log.cur, ~0U);
> atomic_set(_transaction_log_failed.cur, ~0U);
> diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
> index 0dba2308..fdf9d9f 100644
> --- a/drivers/android/binder_alloc.c
> +++ b/drivers/android/binder_alloc.c
> @@ -1006,8 +1006,14 @@ void binder_alloc_init(struct binder_alloc *alloc)
> INIT_LIST_HEAD(>buffers);
>  }
>
> -void binder_alloc_shrinker_init(void)
> +int binder_alloc_shrinker_init(void)
>  {
> -   list_lru_init(_alloc_lru);
> -   register_shrinker(_shrinker);
> +   int ret = list_lru_init(_alloc_lru);
> +
> +   if (ret == 0) {
> +   ret = register_shrinker(_shrinker);
> +   if (ret)
> +   list_lru_destroy(_alloc_lru);
> +   }
> +   return ret;
>  }
> diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
> index 0b14530..9ef64e5 100644
> --- a/drivers/android/binder_alloc.h
> +++ b/drivers/android/binder_alloc.h
> @@ -130,7 +130,7 @@ extern struct binder_buffer *binder_alloc_new_buf(struct 
> binder_alloc *alloc,
>   size_t extra_buffers_size,
>   int is_async);
>  extern void binder_alloc_init(struct binder_alloc *alloc);
> -void binder_alloc_shrinker_init(void);
> +extern int binder_alloc_shrinker_init(void);
>  extern void binder_alloc_vma_close(struct binder_alloc *alloc);
>  extern struct binder_buffer *
>  binder_alloc_prepare_to_free(struct binder_alloc *alloc,
> --
> 1.8.3.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 1/2] staging: lustre: check result of register_shrinker

[Greg KH]

On Wed, Dec 06, 2017 at 08:40:43PM +0300, Aliaksei Karaliou wrote:
> On 12/06/2017 11:51 AM, Greg KH wrote:
> 
> > On Mon, Dec 04, 2017 at 10:21:56PM +0300, Aliaksei Karaliou wrote:
> > > Lustre code lacks checking the result of register_shrinker()
> > > in several places. register_shrinker() was tagged __must_check
> > > recently so that sparse has started reporting it.
> > > 
> > > Signed-off-by: Aliaksei Karaliou 
> > > ---
> > >   drivers/staging/lustre/lustre/ldlm/ldlm_pool.c | 12 +---
> > >   drivers/staging/lustre/lustre/obdclass/lu_object.c |  5 +++--
> > >   drivers/staging/lustre/lustre/osc/osc_request.c|  4 +++-
> > >   drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c|  8 +++-
> > >   4 files changed, 22 insertions(+), 7 deletions(-)
> > > 
> > > v2: Style fixes, as suggested by Cheers, Andreas and Dan Carpenter.
> > >  Added one more patch to address resource cleanup, suggested by Dan 
> > > Carpenter.
> > > 
> > > diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c 
> > > b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
> > > index da65d00a7811..9fef2d52d6c2 100644
> > > --- a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
> > > +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
> > > @@ -1086,10 +1086,16 @@ int ldlm_pools_init(void)
> > >   int rc;
> > >   rc = ldlm_pools_thread_start();
> > > - if (rc == 0)
> > > - register_shrinker(_pools_cli_shrinker);
> > > + if (rc)
> > > + return rc;
> > > - return rc;
> > > + rc = register_shrinker(_pools_cli_shrinker);
> > > + if (rc) {
> > > + ldlm_pools_thread_stop();
> > > + return rc;
> > > + }
> > > +
> > > + return 0;
> > >   }
> > >   void ldlm_pools_fini(void)
> > > diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c 
> > > b/drivers/staging/lustre/lustre/obdclass/lu_object.c
> > > index b938a3f9d50a..9e0256ca2079 100644
> > > --- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
> > > +++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
> > > @@ -1951,7 +1951,7 @@ int lu_global_init(void)
> > >* inode, one for ea. Unfortunately setting this high value 
> > > results in
> > >* lu_object/inode cache consuming all the memory.
> > >*/
> > > - register_shrinker(_site_shrinker);
> > > + result = register_shrinker(_site_shrinker);
> > >   return result;
> > return register_shrinker()?
> Yes, It looks easier, thank you for your comments.
> But still what to do with the fact that this changed place actually leads to
> resources being not freed (what I actually fix in second patch) ?
> Should I just merge both commits together ?

Don't write a patch that you know is broken, and then fix it up in a
follow-on patch.  That's not good at all.

Just fix one call-site at a time, that should be easier, right?

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

Re: [PATCHv2] staging: pi433: pi433_if.c remove SET_CHECKED macro

[Dan Carpenter]

On Mon, Dec 04, 2017 at 02:41:51PM +0300, Dan Carpenter wrote:
> drivers/staging/pi433/pi433_if.c
>468  /* rx done, wait was interrupted or error occurred */
>469  abort:
>470  dev->interrupt_rx_allowed = true;
>471  SET_CHECKED(rf69_set_mode(dev->spi, standby));
>472  wake_up_interruptible(>tx_wait_queue);
>473  
>474  if (retval)
>475  return retval;
>476  else
>477  return bytes_total;
> 

I just want to clarify that what I'm suggesting here is something like
this:

/* rx done, wait was interrupted or error occurred */
abort:
dev->interrupt_rx_allowed = true;
-   SET_CHECKED(rf69_set_mode(dev->spi, standby));
+   if (rf69_set_mode(dev->spi, standby))
+   pr_err("rf69_set_mode() failed to go standby\n");
wake_up_interruptible(>tx_wait_queue);

if (retval)
return retval;
else
return bytes_total;

I'm pretty sure we want to wake_up() even if it fails.  It's not likely
to fail, or if it does then we're probably screwed no matter what.  So
this is mostly theoretical.

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

Re: [PATCH v2 1/2] staging: lustre: check result of register_shrinker

[Aliaksei Karaliou]

On 12/06/2017 11:51 AM, Greg KH wrote:


On Mon, Dec 04, 2017 at 10:21:56PM +0300, Aliaksei Karaliou wrote:

Lustre code lacks checking the result of register_shrinker()
in several places. register_shrinker() was tagged __must_check
recently so that sparse has started reporting it.

Signed-off-by: Aliaksei Karaliou 
---
  drivers/staging/lustre/lustre/ldlm/ldlm_pool.c | 12 +---
  drivers/staging/lustre/lustre/obdclass/lu_object.c |  5 +++--
  drivers/staging/lustre/lustre/osc/osc_request.c|  4 +++-
  drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c|  8 +++-
  4 files changed, 22 insertions(+), 7 deletions(-)

v2: Style fixes, as suggested by Cheers, Andreas and Dan Carpenter.
 Added one more patch to address resource cleanup, suggested by Dan 
Carpenter.

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c 
b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
index da65d00a7811..9fef2d52d6c2 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
@@ -1086,10 +1086,16 @@ int ldlm_pools_init(void)
int rc;
  
  	rc = ldlm_pools_thread_start();

-   if (rc == 0)
-   register_shrinker(_pools_cli_shrinker);
+   if (rc)
+   return rc;
  
-	return rc;

+   rc = register_shrinker(_pools_cli_shrinker);
+   if (rc) {
+   ldlm_pools_thread_stop();
+   return rc;
+   }
+
+   return 0;
  }
  
  void ldlm_pools_fini(void)

diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c 
b/drivers/staging/lustre/lustre/obdclass/lu_object.c
index b938a3f9d50a..9e0256ca2079 100644
--- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
+++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
@@ -1951,7 +1951,7 @@ int lu_global_init(void)
 * inode, one for ea. Unfortunately setting this high value results in
 * lu_object/inode cache consuming all the memory.
 */
-   register_shrinker(_site_shrinker);
+   result = register_shrinker(_site_shrinker);
  
  	return result;

return register_shrinker()?

Yes, It looks easier, thank you for your comments.
But still what to do with the fact that this changed place actually leads to
resources being not freed (what I actually fix in second patch) ?
Should I just merge both commits together ?

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

[PATCH 19/45] drivers: staging: remove duplicate includes

[Pravin Shedge]

These duplicate includes have been found with scripts/checkincludes.pl but
they have been removed manually to avoid removing false positives.

Signed-off-by: Pravin Shedge 
---
 drivers/staging/ccree/ssi_buffer_mgr.c| 1 -
 drivers/staging/ccree/ssi_driver.c| 1 -
 drivers/staging/irda/drivers/pxaficp_ir.c | 1 -
 3 files changed, 3 deletions(-)

diff --git a/drivers/staging/ccree/ssi_buffer_mgr.c 
b/drivers/staging/ccree/ssi_buffer_mgr.c
index 1f8a225..834bddd 100644
--- a/drivers/staging/ccree/ssi_buffer_mgr.c
+++ b/drivers/staging/ccree/ssi_buffer_mgr.c
@@ -23,7 +23,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/drivers/staging/ccree/ssi_driver.c 
b/drivers/staging/ccree/ssi_driver.c
index 1a3c481..2cec0e1 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -55,7 +55,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/staging/irda/drivers/pxaficp_ir.c 
b/drivers/staging/irda/drivers/pxaficp_ir.c
index 1dba16b..2ea00a6 100644
--- a/drivers/staging/irda/drivers/pxaficp_ir.c
+++ b/drivers/staging/irda/drivers/pxaficp_ir.c
@@ -12,7 +12,6 @@
  * Infra-red driver (SIR/FIR) for the PXA2xx embedded microprocessor
  *
  */
-#include 
 #include 
 #include 
 #include 
-- 
2.7.4

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

[PATCH 3/4] staging: unisys: combine visorchannel.h and visorbus.h

[David Kershner]

Combine the include files visorchannel.h and visorbus.h so that only one
include file is needed for the .c files.

Reported-by: Christoph Hellwig 
Signed-off-by: David Kershner 
Reviewed-by: Tim Sell 
---
 drivers/staging/unisys/include/iochannel.h |   2 +-
 drivers/staging/unisys/include/visorbus.h  | 166 ++-
 drivers/staging/unisys/include/visorchannel.h  | 179 -
 drivers/staging/unisys/visorbus/controlvmchannel.h |   2 +-
 drivers/staging/unisys/visorbus/vbuschannel.h  |   2 +-
 5 files changed, 168 insertions(+), 183 deletions(-)
 delete mode 100644 drivers/staging/unisys/include/visorchannel.h

diff --git a/drivers/staging/unisys/include/iochannel.h 
b/drivers/staging/unisys/include/iochannel.h
index a3c8754..9023cf5 100644
--- a/drivers/staging/unisys/include/iochannel.h
+++ b/drivers/staging/unisys/include/iochannel.h
@@ -34,7 +34,7 @@
 #include 
 #include 
 
-#include "visorchannel.h"
+#include "visorbus.h"
 
 /*
  * Must increment these whenever you insert or delete fields within this 
channel
diff --git a/drivers/staging/unisys/include/visorbus.h 
b/drivers/staging/unisys/include/visorbus.h
index fb4d3f8..0d8bd67 100644
--- a/drivers/staging/unisys/include/visorbus.h
+++ b/drivers/staging/unisys/include/visorbus.h
@@ -19,8 +19,172 @@
 
 #include 
 
-#include "visorchannel.h"
+#define VISOR_CHANNEL_SIGNATURE ('L' << 24 | 'N' << 16 | 'C' << 8 | 'E')
 
+/*
+ * enum channel_serverstate
+ * @CHANNELSRV_UNINITIALIZED: Channel is in an undefined state.
+ * @CHANNELSRV_READY:Channel has been initialized by server.
+ */
+enum channel_serverstate {
+   CHANNELSRV_UNINITIALIZED = 0,
+   CHANNELSRV_READY = 1
+};
+
+/*
+ * enum channel_clientstate
+ * @CHANNELCLI_DETACHED:
+ * @CHANNELCLI_DISABLED:  Client can see channel but is NOT allowed to use it
+ *   unless given TBD* explicit request
+ *   (should actually be < DETACHED).
+ * @CHANNELCLI_ATTACHING: Legacy EFI client request for EFI server to attach.
+ * @CHANNELCLI_ATTACHED:  Idle, but client may want to use channel any time.
+ * @CHANNELCLI_BUSY: Client either wants to use or is using channel.
+ * @CHANNELCLI_OWNED:"No worries" state - client can access channel
+ *   anytime.
+ */
+enum channel_clientstate {
+   CHANNELCLI_DETACHED = 0,
+   CHANNELCLI_DISABLED = 1,
+   CHANNELCLI_ATTACHING = 2,
+   CHANNELCLI_ATTACHED = 3,
+   CHANNELCLI_BUSY = 4,
+   CHANNELCLI_OWNED = 5
+};
+
+/*
+ * Values for VISOR_CHANNEL_PROTOCOL.Features: This define exists so that
+ * a guest can look at the FeatureFlags in the io channel, and configure the
+ * driver to use interrupts or not based on this setting. All feature bits for
+ * all channels should be defined here. The io channel feature bits are defined
+ * below.
+ */
+#define VISOR_DRIVER_ENABLES_INTS (0x1ULL << 1)
+#define VISOR_CHANNEL_IS_POLLING (0x1ULL << 3)
+#define VISOR_IOVM_OK_DRIVER_DISABLING_INTS (0x1ULL << 4)
+#define VISOR_DRIVER_DISABLES_INTS (0x1ULL << 5)
+#define VISOR_DRIVER_ENHANCED_RCVBUF_CHECKING (0x1ULL << 6)
+
+/*
+ * struct channel_header - Common Channel Header
+ * @signature:Signature.
+ * @legacy_state:  DEPRECATED - being replaced by.
+ * @header_size:   sizeof(struct channel_header).
+ * @size: Total size of this channel in bytes.
+ * @features: Flags to modify behavior.
+ * @chtype:   Channel type: data, bus, control, etc..
+ * @partition_handle:  ID of guest partition.
+ * @handle:   Device number of this channel in client.
+ * @ch_space_offset:   Offset in bytes to channel specific area.
+ * @version_id:   Struct channel_header Version ID.
+ * @partition_index:   Index of guest partition.
+ * @zone_uuid:Guid of Channel's zone.
+ * @cli_str_offset:Offset from channel header to null-terminated
+ *ClientString (0 if ClientString not present).
+ * @cli_state_boot:CHANNEL_CLIENTSTATE of pre-boot EFI client of this
+ *channel.
+ * @cmd_state_cli: CHANNEL_COMMANDSTATE (overloaded in Windows drivers, see
+ *ServerStateUp, ServerStateDown, etc).
+ * @cli_state_os:  CHANNEL_CLIENTSTATE of Guest OS client of this channel.
+ * @ch_characteristic: CHANNEL_CHARACTERISTIC_.
+ * @cmd_state_srv: CHANNEL_COMMANDSTATE (overloaded in Windows drivers, see
+ *ServerStateUp, ServerStateDown, etc).
+ * @srv_state:CHANNEL_SERVERSTATE.
+ * @cli_error_boot:Bits to indicate err states for boot clients, so err
+ *messages can be throttled.
+ * @cli_error_os:  Bits to indicate err states for OS clients, so err
+ *messages can be throttled.
+ * @filler:   Pad out to 128 byte cacheline.
+ * @recover_channel:   Please add all new single-byte values below 

[PATCH 2/4] staging: unisys: remove !UML flag

[David Kershner]

Remove the dependency that the drivers are not built during UML.

Reported-by: Greg Kroah-Hartman 
Signed-off-by: David Kershner 
Reviewed-by: Tim Sell 
---
 drivers/staging/unisys/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/unisys/Kconfig b/drivers/staging/unisys/Kconfig
index ca1bd9a..f3c92a5 100644
--- a/drivers/staging/unisys/Kconfig
+++ b/drivers/staging/unisys/Kconfig
@@ -3,7 +3,7 @@
 #
 menuconfig UNISYSSPAR
bool "Unisys SPAR driver support"
-   depends on X86_64 && !UML
+   depends on X86_64
depends on ACPI
---help---
Support for the Unisys SPAR drivers
-- 
1.9.1

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

[PATCH 1/4] staging: unisys: fix dependencies with UNISYSSPAR Kconfig flag

[David Kershner]

The Kconfig file for UNISYSSPAR uses select ACPI and select PCI instead of
depends on ACPI. This patch fixes the problem.

Reported-by: Greg Kroah-Hartman 
Signed-off-by: David Kershner 
Reviewed-by: Tim Sell 
---
 drivers/staging/unisys/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/unisys/Kconfig b/drivers/staging/unisys/Kconfig
index 4f1f5e6..ca1bd9a 100644
--- a/drivers/staging/unisys/Kconfig
+++ b/drivers/staging/unisys/Kconfig
@@ -4,8 +4,7 @@
 menuconfig UNISYSSPAR
bool "Unisys SPAR driver support"
depends on X86_64 && !UML
-   select PCI
-   select ACPI
+   depends on ACPI
---help---
Support for the Unisys SPAR drivers
 
-- 
1.9.1

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

[PATCH 0/4] staging: unisys: fix dependency and include issues

[David Kershner]

This patch series combines the global include files into a single include
and cleans up dependencies that were in the wrong location. We had code
dependencies on the UNISYSSPAR Kconfig flag instead of the VISORBUS config
flag. UNISYSSPAR is just there for organizational purposes.

David Kershner (4):
  staging: unisys: fix dependencies with UNISYSSPAR Kconfig flag
  staging: unisys: remove !UML flag
  staging: unisys: combine visorchannel.h and visorbus.h
  staging: unisys: move dependencies from UNISYSSPAR to VISORBUS

 drivers/staging/unisys/Kconfig |   3 -
 drivers/staging/unisys/include/iochannel.h |   2 +-
 drivers/staging/unisys/include/visorbus.h  | 166 ++-
 drivers/staging/unisys/include/visorchannel.h  | 179 -
 drivers/staging/unisys/visorbus/Kconfig|   1 +
 drivers/staging/unisys/visorbus/controlvmchannel.h |   2 +-
 drivers/staging/unisys/visorbus/vbuschannel.h  |   2 +-
 7 files changed, 169 insertions(+), 186 deletions(-)
 delete mode 100644 drivers/staging/unisys/include/visorchannel.h

-- 
1.9.1

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

[PATCH 4/4] staging: unisys: move dependencies from UNISYSSPAR to VISORBUS

[David Kershner]

The Kconfig flag UNISYSSPAR depended on ACPI and X86, these dependencies
really belong to visorbus, so move them there.

Signed-off-by: David Kershner 
Reviewed-by: Tim Sell 
---
 drivers/staging/unisys/Kconfig  | 2 --
 drivers/staging/unisys/visorbus/Kconfig | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/unisys/Kconfig b/drivers/staging/unisys/Kconfig
index f3c92a5..4d19038 100644
--- a/drivers/staging/unisys/Kconfig
+++ b/drivers/staging/unisys/Kconfig
@@ -3,8 +3,6 @@
 #
 menuconfig UNISYSSPAR
bool "Unisys SPAR driver support"
-   depends on X86_64
-   depends on ACPI
---help---
Support for the Unisys SPAR drivers
 
diff --git a/drivers/staging/unisys/visorbus/Kconfig 
b/drivers/staging/unisys/visorbus/Kconfig
index 5113880..3866804 100644
--- a/drivers/staging/unisys/visorbus/Kconfig
+++ b/drivers/staging/unisys/visorbus/Kconfig
@@ -5,6 +5,7 @@
 config UNISYS_VISORBUS
tristate "Unisys visorbus driver"
depends on UNISYSSPAR
+   depends on X86_64 && ACPI
---help---
The visorbus driver is a virtualized bus for the Unisys s-Par firmware.
Virtualized devices allow Linux guests on a system to share disks and
-- 
1.9.1

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

[PATCH] Staging: rtl8192u: Fix no spaces around '+'

[Akash Kumar]

Added spaces around '+'. Warning found using checkpatch.pl
Signed-off-by: Akash Kumar 
---
 drivers/staging/rtl8192u/ieee80211/dot11d.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/dot11d.c 
b/drivers/staging/rtl8192u/ieee80211/dot11d.c
index 64b13a5..ba284bf 100644
--- a/drivers/staging/rtl8192u/ieee80211/dot11d.c
+++ b/drivers/staging/rtl8192u/ieee80211/dot11d.c
@@ -11,7 +11,7 @@ void Dot11d_Init(struct ieee80211_device *ieee)
 
pDot11dInfo->State = DOT11D_STATE_NONE;
pDot11dInfo->CountryIeLen = 0;
-   memset(pDot11dInfo->channel_map, 0, MAX_CHANNEL_NUMBER+1);
+   memset(pDot11dInfo->channel_map, 0, MAX_CHANNEL_NUMBER + 1);
memset(pDot11dInfo->MaxTxPwrDbmList, 0xFF, MAX_CHANNEL_NUMBER+1);
RESET_CIE_WATCHDOG(ieee);
 
-- 
2.7.4

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

Re: [PATCHv2] staging: pi433: pi433_if.c remove SET_CHECKED macro

[Greg Kroah-Hartman]

On Sun, Dec 03, 2017 at 01:09:49AM +0100, Nguyen Phan Quang Minh wrote:
> The macro calls its argument -a function- twice, makes the calling
> function return prematurely -skipping resource cleanup code- and hurts
> understandability.
> 
> Signed-off-by: Nguyen Phan Quang Minh 
> ---
> Base on Dan's feedback, I checked and the macro return indeed skips over
> some cleanup code, hence remove it.
> The code is not pretty, any hints on how to do it better is greatly
> appreciated.

Please redo this based on Dan's comments.

thanks,

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

Re: [PATCH v2 06/11] staging: pi433: Split rf69_set_crc_enabled into two functions

[Greg KH]

On Tue, Dec 05, 2017 at 11:08:44PM +0100, Simon Sandström wrote:
> Splits rf69_set_crc_enabled(dev, enabled) into
> rf69_enable_crc(dev) and rf69_disable_crc(dev).
> 
> Signed-off-by: Simon Sandström 
> ---
>  drivers/staging/pi433/pi433_if.c | 22 --
>  drivers/staging/pi433/rf69.c | 18 ++
>  drivers/staging/pi433/rf69.h |  4 ++--
>  3 files changed, 28 insertions(+), 16 deletions(-)

I had to stop here in applying this series, as the merge conflicts just
got too much for me to resolve by hand.

Can you rebase this series on my staging-testing branch of staging.git
and send the remaining patches please?

thanks,

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

Re: [PATCH v2 06/11] staging: pi433: Split rf69_set_crc_enabled into two functions

[Greg KH]

On Wed, Dec 06, 2017 at 12:07:20PM +0200, Marcus Wolf wrote:
> 
> 
> Am 06.12.2017 um 11:37 schrieb Dan Carpenter:
> > On Wed, Dec 06, 2017 at 11:05:22AM +0200, Marcus Wolf wrote:
> > > 
> > > 
> > > Am 06.12.2017 um 00:08 schrieb Simon Sandström:
> > > > Splits rf69_set_crc_enabled(dev, enabled) into
> > > > rf69_enable_crc(dev) and rf69_disable_crc(dev).
> > > > 
> > > > Signed-off-by: Simon Sandström 
> > > > ---
> > > >drivers/staging/pi433/pi433_if.c | 22 --
> > > >drivers/staging/pi433/rf69.c | 18 ++
> > > >drivers/staging/pi433/rf69.h |  4 ++--
> > > >3 files changed, 28 insertions(+), 16 deletions(-)
> > > > 
> > > > diff --git a/drivers/staging/pi433/pi433_if.c 
> > > > b/drivers/staging/pi433/pi433_if.c
> > > > index 2ae19ac565d1..614eec7dd904 100644
> > > > --- a/drivers/staging/pi433/pi433_if.c
> > > > +++ b/drivers/staging/pi433/pi433_if.c
> > > > @@ -216,7 +216,16 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
> > > > pi433_rx_cfg *rx_cfg)
> > > > return ret;
> > > > }
> > > > SET_CHECKED(rf69_set_adressFiltering(dev->spi, 
> > > > rx_cfg->enable_address_filtering));
> > > > -   SET_CHECKED(rf69_set_crc_enable (dev->spi, 
> > > > rx_cfg->enable_crc));
> > > > +
> > > > +   if (rx_cfg->enable_crc == OPTION_ON) {
> > > > +   ret = rf69_enable_crc(dev->spi);
> > > > +   if (ret < 0)
> > > > +   return ret;
> > > > +   } else {
> > > > +   ret = rf69_disable_crc(dev->spi);
> > > > +   if (ret < 0)
> > > > +   return ret;
> > > > +   }
> > > 
> > > Why don't you use SET_CHECKED(...)?
> > > 
> > 
> > Marcus, please don't introduce new uses of SET_CHECKED().  It has a
> > hidden return in it which is against kernel style and introduces very
> > predictable and avoidable bugs.  For example, in probe().
> 
> Ah ok.
> 
> Thanks for clarifiytion!
> 
> What a pitty - another bunch of extra lines of code...
> 
> Or is there an other construction, allowing for one line per register
> change? Something like
> 
>   ret = rf69_set_xyz(...); if (ret) return ret;
>   ret = rf69_set_abc(...); if (ret) return ret;
> 
> is pretty ugly and voids the style guide...

Just spell it out:
ret = rf69_set_xyz();
if (ret)
goto unwind_xyz;

Almost never do you want to instantly return.  You should clean up from
the error first.

But if you do just want to exit, that's fine too, just return.  That's
the normal way here, don't do funny things in macros (like return from a
function), that way lies madness...

thanks,

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

Re: [PATCH v2] Staging: pi433: fix brace coding style issues in pi433_if.c

[Greg KH]

On Mon, Dec 04, 2017 at 09:40:10PM +0100, Tomas Marek wrote:
> This patch fix several brace on next line, braces not necessary, space
> around =/<, and space before/after open/close parenthesis coding style
> errors find by checkpatch in pi433_if.c.
> 
> In addition, the interrupt routine DIO0_irq_handler logic is updated:
> - use 'switch' statement instead of 'if/else if' combination for the
>   sake of readability, and
> - use dev_dbg_ratelimited instead of dev_dbg to avoid message flooding.

When you have to add "in addition" to a changelog comment, that's a huge
flag that the patch needs to be broken up into a patch series.  Please
do that here, only doing one "logical" thing at a time.  As it is, it's
hard to review this way.

thanks,

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

Re: [PATCH] staging: pi433: pi433_if.c codestyle space prohibited

[GregKH]

On Tue, Nov 28, 2017 at 06:13:16PM +0100, Oliver Graute wrote:
> This patch fixes the following checkpatch.pl error:
> 
> ERROR: space prohibited after that open parenthesis '('
> #973: FILE: pi433_if.c:973:
> +   if ( IS_ERR(device->gpiod[i]) )
> 
> 
> ERROR: space prohibited after that open parenthesis '('
> #19: FILE: drivers/staging/pi433/pi433_if.c:954:
> +   if ( IS_ERR(device->gpiod[i]))
> 
> Signed-off-by: Oliver Graute 
> ---
>  drivers/staging/pi433/pi433_if.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

You have sent 8 different patches to this driver recently, yet I have no
idea what order the patches should be applied in :(

Please redo them as a patch series, and send them all at once, properly
numbered, so that I have a chance to get it right.  Otherwise, I will
get it wrong...

I've now dropped all of the patches from you from my queue.

thanks,

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

Re: [PATCH] staging: pi433: rf69.c: Introduced define DEBUG_FUNC_ENTRY

[Greg KH]

On Sat, Dec 02, 2017 at 07:00:17PM +0200, Marcus Wolf wrote:
> Am 02.12.2017 um 17:00 schrieb Greg KH:
> > On Sat, Dec 02, 2017 at 01:45:50PM +0200, Marcus Wolf wrote:
> > > Since dev_dbg already depends on define DEBUG, there was no sense, to 
> > > enclose
> > > dev_dbg lines with #ifdef DEBUG.
> > > Instead of removing #ifdef DEBUG, I introduced define DEBUG_FUNC_ENTRY. 
> > > So now it is
> > > possible to switch of the function entry debug lines even while debug is 
> > > switched on.
> > 
> > Ick ick ick.
> > 
> > No, these lines should just all be deleted.  Use ftrace if you want to
> > see what functions are being called, that's what it is there for.  Don't
> > create driver-specific defines and functionality for when we already
> > have that functionality for the whole of the kernel.  That's really
> > redundant and messy.
> > 
> > > Signed-off-by: Marcus Wolf 
> > > ---
> > >   drivers/staging/pi433/rf69.c |  118 
> > > +-
> > >   1 file changed, 58 insertions(+), 60 deletions(-)
> > > 
> > > diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> > > index 12c9df9..0df084e 100644
> > > --- a/drivers/staging/pi433/rf69.c
> > > +++ b/drivers/staging/pi433/rf69.c
> > > @@ -15,8 +15,10 @@
> > >* GNU General Public License for more details.
> > >*/
> > > -/* enable prosa debug info */
> > > +/* generic enable/disable dev_dbg */
> > >   #undef DEBUG
> > > +/* enable print function entry */
> > > +#undef DEBUG_FUNC_ENTRY
> > >   /* enable print of values on reg access */
> > >   #undef DEBUG_VALUES
> > >   /* enable print of values on fifo access */
> > > @@ -40,7 +42,7 @@
> > >   int rf69_set_mode(struct spi_device *spi, enum mode mode)
> > >   {
> > > - #ifdef DEBUG
> > > + #ifdef DEBUG_FUNC_ENTRY
> > >   dev_dbg(>dev, "set: mode");
> > >   #endif
> > 
> > So this whole #ifdef dev_dbg #endif should all be removed.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Hi all,
> 
> just for clarification, why I introduced these dev_dbg during development of
> the driver and didn't use ftrace:
> 
> Since I wanted the driver to use a single module as sender and receiver at
> (almost) the same time, the module constantly needs to be reconfigured
> (constant switching between rx configuration and tx configuration - see my
> documentation for details on the idea).
> 
> The routine, accessing the registers is able to print the register number
> and the value, it reads / writes from / to the register. It's dam hard to
> keep the survey over the use 30...40 register numbers, in 10th of rows of
> register setting and reading, if you see just the numbers in the log.
> Especially this is / was hard, if one register was written several times,
> because it contains different settings. Then decoding of the adress wasn't
> enough, I even need to decode the bits in the value.
> 
> Therefore I finally introduced this dev_dbg lines at the enty of the setter
> (and getter): After that in the log I coud see something like this:
> Set gain: 0x43 0x81
> Set threshold: 0x51 0x30
> Get moulation: 0x22 0x7c
> instead of just numbers.
> That eased the debugging a lot.
> 
> When using ftrace I would be able to see, in which order the setter were
> called, but the link to the vlaues would be missing.
> 
> If those dev_dbg are unwanted, I am ok, if someone removes them.
> 
> Hope this helps understanding

Yes, for debugging the code to start with, put loads of messages in
there, that's fine.  But now that it all works, they are not needed, so
they can be removed, and ftrace used instead.

thanks,

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

Re: [PATCH V3] staging: pi433: rf69.c: Replace macros READ_REG and WRITE_REG with smarter functions

[Greg KH]

On Mon, Dec 04, 2017 at 11:45:16PM +0200, Marcus Wolf wrote:
> To increase the readability of the register accesses, the abstraction
> of the helpers was increased from simple read and write to set bit,
> clear bit and read modify write bit.
> 
> Annotation: This patch contains a lot of long lines and camel case
> var names. These long lines and camel case vars weren't introduced
> by this patch, but were long and camel cased before.
> 
> Signed-off-by: Marcus Wolf 
> ---
>  drivers/staging/pi433/rf69.c |  336 
> ++
>  1 file changed, 180 insertions(+), 156 deletions(-)

When doing a second or third or forth version of a patch, always include
below the --- line what changed from the previous versions.  The
in-kernel documentation for how to submit a patch should have all of the
needed information on this.

No need here, just for the future patches.

thanks,

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

Re: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC objects

[Laurentiu Tudor]

Hi Bharat,

On 12/06/2017 04:03 PM, Bharat Bhushan wrote:
> Hi Laurentiu,
>
>> -Original Message-
>> From: Laurentiu Tudor
>> Sent: Wednesday, December 06, 2017 7:00 PM
>> To: Nipun Gupta ; stuyo...@gmail.com; Bharat
>> Bhushan ; gre...@linuxfoundation.org;
>> cakt...@gmail.com; bretth...@gmail.com; a...@arndb.de
>> Cc: linux-ker...@vger.kernel.org; de...@driverdev.osuosl.org
>> Subject: Re: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC
>> objects
>>
>> Hi Nipun,
>>
>> Can you polish a bit this commit message? It doesn't seem to explain why this
>> is needed.
>>
>> On 12/06/2017 06:18 PM, Nipun Gupta wrote:
>>> When DPRC probing is deferred (such as where IOMMU is not probed
>>> before the fsl-mc bus), all the devices in the DPRC containers gets
>>> initialized one after another.
>>
>> Are you referring to dprc probing being deferred (do we ever do that?) or
>> devices inside the dprc deferring the probe?
>>
>>> As IRQ's were allocated only once the
>>> DPRC scanning is completed, the devices like DPIO which uses these
>>> IRQ's at initalization fails. This patch allocates the IRQ resources
>>
>> s/initalization/initialization
>>
>>> before scanning all the objects in the DPRC container.
>>>
>>> Signed-off-by: Nipun Gupta 
>>> ---
>>>drivers/staging/fsl-mc/bus/dprc-driver.c | 49 ++--
>> 
>>>1 file changed, 27 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> b/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> index 06df528..7265431 100644
>>> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> @@ -206,7 +206,8 @@ static void dprc_add_new_devices(struct
>> fsl_mc_device *mc_bus_dev,
>>> * dprc_scan_objects - Discover objects in a DPRC
>>> *
>>> * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC
>>> object
>>> - * @total_irq_count: total number of IRQs needed by objects in the DPRC.
>>> + * @total_irq_count: If argument is provided the function populates
>>> + the
>>> + * total number of IRQs created by objects in the DPRC.
>>
>> As a side node, after a cursory look i noticed that this total_irq_count
>> parameter is used only for some sanity checks. I'm thinking of dropping it 
>> in a
>> follow-up cleanup patch.
>
> How you will ensure that number of IRQ needed are not sufficient for devices 
> in the container?
> Do you think we need to either not enable additional devices or add irqs to 
> irq-pool ?

In the current implementation we allocate a pool of 
FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS (= 256) no mater what total_irq_count is.
I think this is enough for our current scenarios, but if in the future 
we ever hit this limit we can think of a mechanism along the lines of 
your example. Adding another chunk of irqs to the pool seems to me like 
a good idea in the future.

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

Re: [PATCH] staging: pi433: Fixes issue with bit shift in rf69_get_modulation

[Greg KH]

On Wed, Dec 06, 2017 at 12:02:13PM +0200, Marcus Wolf wrote:
> 
> 
> Am 06.12.2017 um 11:02 schrieb Greg KH:
> > On Wed, Nov 08, 2017 at 07:13:56PM +0200, Marcus Wolf wrote:
> > > Fixes issue with bit shift in rf69_get_modulation
> > 
> > What "issue"?
> > 
> > > 
> > > Signed-off-by: Marcus Wolf 
> > > ---
> > >   drivers/staging/pi433/rf69.c |2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> > > index 290b419..c945b4b 100644
> > > --- a/drivers/staging/pi433/rf69.c
> > > +++ b/drivers/staging/pi433/rf69.c
> > > @@ -102,7 +102,7 @@ enum modulation rf69_get_modulation(struct spi_device 
> > > *spi)
> > >   currentValue = READ_REG(REG_DATAMODUL);
> > > - switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE >> 3) { // TODO 
> > > improvement: change 3 to define
> > > + switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE) {
> > 
> > Doesn't this change the logic here?
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Hi Greg,
> 
> yes, it does.
> 
> This is one of the very few changes to pi433 driver, that does not modify
> the architecture or optics of the code, but really fixes a bug. This
> function wasn't working from the very beginning, and we had already several
> reports and patches (from me and otheres), announcing or trying to fix the
> bug. But so far all patches were skipped for some reason.
> 
> 
> Please take the patch.

Ok, then this should go into 4.15-final, I'll queue it up to that tree.

thanks,

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

RE: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC objects

[Bharat Bhushan]

Hi Laurentiu,

> -Original Message-
> From: Laurentiu Tudor
> Sent: Wednesday, December 06, 2017 7:00 PM
> To: Nipun Gupta ; stuyo...@gmail.com; Bharat
> Bhushan ; gre...@linuxfoundation.org;
> cakt...@gmail.com; bretth...@gmail.com; a...@arndb.de
> Cc: linux-ker...@vger.kernel.org; de...@driverdev.osuosl.org
> Subject: Re: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC
> objects
> 
> Hi Nipun,
> 
> Can you polish a bit this commit message? It doesn't seem to explain why this
> is needed.
> 
> On 12/06/2017 06:18 PM, Nipun Gupta wrote:
> > When DPRC probing is deferred (such as where IOMMU is not probed
> > before the fsl-mc bus), all the devices in the DPRC containers gets
> > initialized one after another.
> 
> Are you referring to dprc probing being deferred (do we ever do that?) or
> devices inside the dprc deferring the probe?
> 
> > As IRQ's were allocated only once the
> > DPRC scanning is completed, the devices like DPIO which uses these
> > IRQ's at initalization fails. This patch allocates the IRQ resources
> 
> s/initalization/initialization
> 
> > before scanning all the objects in the DPRC container.
> >
> > Signed-off-by: Nipun Gupta 
> > ---
> >   drivers/staging/fsl-mc/bus/dprc-driver.c | 49 ++--
> 
> >   1 file changed, 27 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c
> > b/drivers/staging/fsl-mc/bus/dprc-driver.c
> > index 06df528..7265431 100644
> > --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
> > +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
> > @@ -206,7 +206,8 @@ static void dprc_add_new_devices(struct
> fsl_mc_device *mc_bus_dev,
> >* dprc_scan_objects - Discover objects in a DPRC
> >*
> >* @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC
> > object
> > - * @total_irq_count: total number of IRQs needed by objects in the DPRC.
> > + * @total_irq_count: If argument is provided the function populates
> > + the
> > + * total number of IRQs created by objects in the DPRC.
> 
> As a side node, after a cursory look i noticed that this total_irq_count
> parameter is used only for some sanity checks. I'm thinking of dropping it in 
> a
> follow-up cleanup patch.

How you will ensure that number of IRQ needed are not sufficient for devices in 
the container?
Do you think we need to either not enable additional devices or add irqs to 
irq-pool ?

Thanks
-Bharat

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

Re: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC objects

[Laurentiu Tudor]

Hi Nipun,

Can you polish a bit this commit message? It doesn't seem to explain why 
this is needed.

On 12/06/2017 06:18 PM, Nipun Gupta wrote:
> When DPRC probing is deferred (such as where IOMMU is not probed
> before the fsl-mc bus), all the devices in the DPRC containers gets
> initialized one after another.

Are you referring to dprc probing being deferred (do we ever do that?) 
or devices inside the dprc deferring the probe?

> As IRQ's were allocated only once the
> DPRC scanning is completed, the devices like DPIO which uses these
> IRQ's at initalization fails. This patch allocates the IRQ resources

s/initalization/initialization

> before scanning all the objects in the DPRC container.
>
> Signed-off-by: Nipun Gupta 
> ---
>   drivers/staging/fsl-mc/bus/dprc-driver.c | 49 
> ++--
>   1 file changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c 
> b/drivers/staging/fsl-mc/bus/dprc-driver.c
> index 06df528..7265431 100644
> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
> @@ -206,7 +206,8 @@ static void dprc_add_new_devices(struct fsl_mc_device 
> *mc_bus_dev,
>* dprc_scan_objects - Discover objects in a DPRC
>*
>* @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
> - * @total_irq_count: total number of IRQs needed by objects in the DPRC.
> + * @total_irq_count: If argument is provided the function populates the
> + * total number of IRQs created by objects in the DPRC.

As a side node, after a cursory look i noticed that this total_irq_count 
parameter is used only for some sanity checks. I'm thinking of dropping 
it in a follow-up cleanup patch.

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

Re: [PATCH v2 08/11] staging: pi433: Remove enum data_mode

[Dan Carpenter]

On Wed, Dec 06, 2017 at 11:11:27AM +0200, Marcus Wolf wrote:
> 
> Since the rule for kernel development seems to be, not to care about future,
> most probably you patch is fine, anyway.
> 

Yeah.  Deleting code if there is no user is required to move this code
out of staging...

I've worked in staging for a long time and I've seen patches from
thousands of people.  Simon really seems to understand kernel style and
that's less common than one might think.  It's a valuable skill.  If you
would just leave him to work then this driver would get fixed...

You're making it very difficult because you're complaining about the
work which *needs* to be done.  It's discouraging for people sending
patches.  This is very frustrating...  :(

On the naming, I think having a function which is named "enable" but
actually disables a feature is a non-starter.  What about we do either
one of these:
pi433_enable_(spi);
pi433_disable_(spi);
Or:
pi433_set_(spi, SETTING);

It's still a standard and we'll just decide on a case by case basis what
to use.  This is a tiny driver and it's really easy to grep for the
feature name to find the functions you want.  Plus all the config is
done in one location so it's really not that hard.

regards,
dan carpenter

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

Re: [PATCH 2/2] staging: fsl-mc: do not print error in case of defer probe error

[Greg KH]

On Wed, Dec 06, 2017 at 09:48:07PM +0530, Nipun Gupta wrote:
> Signed-off-by: Nipun Gupta 
> ---

I can't take patches without any changelog text :(

Please fix and resend the series.

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

Re: [PATCH] staging: rtl8712: Cleanup codestyle

[Greg Kroah-Hartman]

On Wed, Dec 06, 2017 at 05:31:38PM +0700, Neil Singh wrote:
> Fix following checkpatch.pl messages:
> 
> WARNING: line over 80 characters
>  1000: FILE: rtl871x_security.c:1000:
> 
> ERROR: code indent should use tabs where possible
>  1408: FILE: rtl871x_security.c:1408:
> 
> WARNING: please, no spaces at the start of a line
>  1408: FILE: rtl871x_security.c:1408:

That's multiple things, so this should be multiple patches...

thanks,

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

Re: [PATCH v2 04/11] staging: pi433: Rename enum optionOnOff in rf69_enum.h

[Marcus Wolf]

Sorry, for sending HTML as well - I am writing from my phone...

Yes, you will break something, when renaming.

Since order isn't changed, most probably old programs will still compile with 
old enum.h.

If we want to move from optionOnOff to bool, we will also break old progs.

Since driver is new (mainline for just something like 2 monthes) and stil under 
devel, I think we should "risk" it.

Gruß,

Marcus 

> Am 06.12.2017 um 11:44 schrieb Dan Carpenter :
> 
>> On Wed, Dec 06, 2017 at 12:31:31PM +0200, Marcus Wolf wrote:
>> 
>> 
>>> Am 06.12.2017 um 12:23 schrieb Dan Carpenter:
>>> On Wed, Dec 06, 2017 at 11:46:41AM +0200, Marcus Wolf wrote:
> diff --git a/drivers/staging/pi433/rf69_enum.h 
> b/drivers/staging/pi433/rf69_enum.h
> index babe597e2ec6..5247e9269de9 100644
> --- a/drivers/staging/pi433/rf69_enum.h
> +++ b/drivers/staging/pi433/rf69_enum.h
> @@ -18,9 +18,9 @@
>   #ifndef RF69_ENUM_H
>   #define RF69_ENUM_H
> -enum optionOnOff {
> -optionOff,
> -optionOn
> +enum option_on_off {
> +OPTION_OFF,
> +OPTION_ON
>   };
>   enum mode {
> 
 
 Hi Simon,
 
 nice work.
 
 Thank you very much for all the style fixes :-)
 
>>> 
>>> 
>>> Wow...  This was the one patch I thought was going to sink this
>>> patchset...
>> 
>> I don't get that. What do you mean?
>> 
>>> Isn't enum optionOnOff part of the userspace headers?
>>> 
>>> I thought we weren't allowed to change that.
>> 
>> All enums are for user space (or inteded to be used in userspace in future).
>> Didn't introduce enums for internal use.
> 
> So what I'm asking is if we do this change, does it break any userspace
> programs which are used *right now*.  In other words will programs stop
> compiling because we've renamed an enum?
> 
> regards,
> dan carpenter

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

Re: [PATCH v2 04/11] staging: pi433: Rename enum optionOnOff in rf69_enum.h

[Dan Carpenter]

On Wed, Dec 06, 2017 at 12:31:31PM +0200, Marcus Wolf wrote:
> 
> 
> Am 06.12.2017 um 12:23 schrieb Dan Carpenter:
> > On Wed, Dec 06, 2017 at 11:46:41AM +0200, Marcus Wolf wrote:
> > > > diff --git a/drivers/staging/pi433/rf69_enum.h 
> > > > b/drivers/staging/pi433/rf69_enum.h
> > > > index babe597e2ec6..5247e9269de9 100644
> > > > --- a/drivers/staging/pi433/rf69_enum.h
> > > > +++ b/drivers/staging/pi433/rf69_enum.h
> > > > @@ -18,9 +18,9 @@
> > > >#ifndef RF69_ENUM_H
> > > >#define RF69_ENUM_H
> > > > -enum optionOnOff {
> > > > -   optionOff,
> > > > -   optionOn
> > > > +enum option_on_off {
> > > > +   OPTION_OFF,
> > > > +   OPTION_ON
> > > >};
> > > >enum mode {
> > > > 
> > > 
> > > Hi Simon,
> > > 
> > > nice work.
> > > 
> > > Thank you very much for all the style fixes :-)
> > > 
> > 
> > 
> > Wow...  This was the one patch I thought was going to sink this
> > patchset...
> 
> I don't get that. What do you mean?
> 
> > Isn't enum optionOnOff part of the userspace headers?
> > 
> > I thought we weren't allowed to change that.
> 
> All enums are for user space (or inteded to be used in userspace in future).
> Didn't introduce enums for internal use.

So what I'm asking is if we do this change, does it break any userspace
programs which are used *right now*.  In other words will programs stop
compiling because we've renamed an enum?

regards,
dan carpenter

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

Re: [PATCH v2 06/11] staging: pi433: Split rf69_set_crc_enabled into two functions

[Marcus Wolf]

>>
>> rf69 -set/get - action
>> -> rf69_set_crc_enable
>
> No...  Simon's name is better.  His is shorter and makes more sense.


I disagree. If I am going to implement a new functionality and need to 
think about the naming of the function name, every time I need to change 
a register setting that's awfull.


I usually have code on one monitor and datasheet on the other. So if I 
want to set a bit/reg/whatever, I have the datasheet in front of my 
nose. I can easy write the code, if function names refer to the names in 
the datasheet and follow a strict naming convention. If the naming 
convetion is broken, I need to switch to the header and search manually 
for each register, I want to set.



There is so much potential in this young driver, that could be 
developed. Would be pitty, if all that wouldn't take place some day.



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

[PATCH] staging: rtl8712: Cleanup codestyle

[Neil Singh]

Fix following checkpatch.pl messages:

WARNING: line over 80 characters
 1000: FILE: rtl871x_security.c:1000:

ERROR: code indent should use tabs where possible
 1408: FILE: rtl871x_security.c:1408:

WARNING: please, no spaces at the start of a line
 1408: FILE: rtl871x_security.c:1408:

Signed-off-by: Neil Singh 
---
 drivers/staging/rtl8712/rtl871x_security.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_security.c 
b/drivers/staging/rtl8712/rtl871x_security.c
index 56d36f6..ed3a00f 100644
--- a/drivers/staging/rtl8712/rtl871x_security.c
+++ b/drivers/staging/rtl8712/rtl871x_security.c
@@ -997,8 +997,9 @@ static void construct_mic_header2(u8 *mic_header2, u8 
*mpdu, sint a4_exists,
 /* Builds the last MIC header block from*/
 /* header fields.   */
 //
-static void construct_ctr_preload(u8 *ctr_preload, sint a4_exists, sint 
qc_exists,
-  u8 *mpdu, u8 *pn_vector, sint c)
+static void construct_ctr_preload(u8 *ctr_preload,
+ sint a4_exists, sint qc_exists,
+ u8 *mpdu, u8 *pn_vector, sint c)
 {
sint i;
 
@@ -1405,7 +1406,7 @@ u32 r8712_aes_decrypt(struct _adapter *padapter, u8 
*precvframe)
 void r8712_use_tkipkey_handler(struct timer_list *t)
 {
struct _adapter *padapter =
-from_timer(padapter, t, securitypriv.tkip_timer);
+   from_timer(padapter, t, securitypriv.tkip_timer);
 
padapter->securitypriv.busetkipkey = true;
 }
-- 
2.1.4

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

Re: [PATCH v2 04/11] staging: pi433: Rename enum optionOnOff in rf69_enum.h

[Marcus Wolf]

Am 06.12.2017 um 12:23 schrieb Dan Carpenter:

On Wed, Dec 06, 2017 at 11:46:41AM +0200, Marcus Wolf wrote:

diff --git a/drivers/staging/pi433/rf69_enum.h 
b/drivers/staging/pi433/rf69_enum.h
index babe597e2ec6..5247e9269de9 100644
--- a/drivers/staging/pi433/rf69_enum.h
+++ b/drivers/staging/pi433/rf69_enum.h
@@ -18,9 +18,9 @@
   #ifndef RF69_ENUM_H
   #define RF69_ENUM_H
-enum optionOnOff {
-   optionOff,
-   optionOn
+enum option_on_off {
+   OPTION_OFF,
+   OPTION_ON
   };
   enum mode {



Hi Simon,

nice work.

Thank you very much for all the style fixes :-)




Wow...  This was the one patch I thought was going to sink this
patchset... 


I don't get that. What do you mean?


Isn't enum optionOnOff part of the userspace headers?

I thought we weren't allowed to change that.


All enums are for user space (or inteded to be used in userspace in 
future). Didn't introduce enums for internal use.


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

Re: [PATCH v2 04/11] staging: pi433: Rename enum optionOnOff in rf69_enum.h

[Dan Carpenter]

On Wed, Dec 06, 2017 at 11:46:41AM +0200, Marcus Wolf wrote:
> > diff --git a/drivers/staging/pi433/rf69_enum.h 
> > b/drivers/staging/pi433/rf69_enum.h
> > index babe597e2ec6..5247e9269de9 100644
> > --- a/drivers/staging/pi433/rf69_enum.h
> > +++ b/drivers/staging/pi433/rf69_enum.h
> > @@ -18,9 +18,9 @@
> >   #ifndef RF69_ENUM_H
> >   #define RF69_ENUM_H
> > -enum optionOnOff {
> > -   optionOff,
> > -   optionOn
> > +enum option_on_off {
> > +   OPTION_OFF,
> > +   OPTION_ON
> >   };
> >   enum mode {
> > 
> 
> Hi Simon,
> 
> nice work.
> 
> Thank you very much for all the style fixes :-)
> 


Wow...  This was the one patch I thought was going to sink this
patchset...  Isn't enum optionOnOff part of the userspace headers?

I thought we weren't allowed to change that.

regards,
dan carpenter

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

[PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC objects

[Nipun Gupta]

When DPRC probing is deferred (such as where IOMMU is not probed
before the fsl-mc bus), all the devices in the DPRC containers gets
initialized one after another. As IRQ's were allocated only once the
DPRC scanning is completed, the devices like DPIO which uses these
IRQ's at initalization fails. This patch allocates the IRQ resources
before scanning all the objects in the DPRC container.

Signed-off-by: Nipun Gupta 
---
 drivers/staging/fsl-mc/bus/dprc-driver.c | 49 ++--
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c 
b/drivers/staging/fsl-mc/bus/dprc-driver.c
index 06df528..7265431 100644
--- a/drivers/staging/fsl-mc/bus/dprc-driver.c
+++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
@@ -206,7 +206,8 @@ static void dprc_add_new_devices(struct fsl_mc_device 
*mc_bus_dev,
  * dprc_scan_objects - Discover objects in a DPRC
  *
  * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
- * @total_irq_count: total number of IRQs needed by objects in the DPRC.
+ * @total_irq_count: If argument is provided the function populates the
+ * total number of IRQs created by objects in the DPRC.
  *
  * Detects objects added and removed from a DPRC and synchronizes the
  * state of the Linux bus driver, MC by adding and removing
@@ -228,6 +229,7 @@ static int dprc_scan_objects(struct fsl_mc_device 
*mc_bus_dev,
int error;
unsigned int irq_count = mc_bus_dev->obj_desc.irq_count;
struct fsl_mc_obj_desc *child_obj_desc_array = NULL;
+   struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_bus_dev);
 
error = dprc_get_obj_count(mc_bus_dev->mc_io,
   0,
@@ -297,7 +299,26 @@ static int dprc_scan_objects(struct fsl_mc_device 
*mc_bus_dev,
}
}
 
-   *total_irq_count = irq_count;
+   /*
+* Allocate IRQ's before scanning objects in DPRC as some devices like
+* DPIO needs them on being probed.
+*/
+   if (dev_get_msi_domain(_bus_dev->dev) && !mc_bus->irq_resources) {
+   if (irq_count > FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS) {
+   dev_warn(_bus_dev->dev,
+"IRQs needed (%u) exceed IRQs preallocated 
(%u)\n",
+irq_count, FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
+   }
+
+   error = fsl_mc_populate_irq_pool(mc_bus,
+   FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
+   if (error < 0)
+   return error;
+   }
+
+   if (total_irq_count)
+   *total_irq_count = irq_count;
+
dprc_remove_devices(mc_bus_dev, child_obj_desc_array,
num_child_objects);
 
@@ -322,7 +343,6 @@ static int dprc_scan_objects(struct fsl_mc_device 
*mc_bus_dev,
 static int dprc_scan_container(struct fsl_mc_device *mc_bus_dev)
 {
int error;
-   unsigned int irq_count;
struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_bus_dev);
 
fsl_mc_init_all_resource_pools(mc_bus_dev);
@@ -331,29 +351,14 @@ static int dprc_scan_container(struct fsl_mc_device 
*mc_bus_dev)
 * Discover objects in the DPRC:
 */
mutex_lock(_bus->scan_mutex);
-   error = dprc_scan_objects(mc_bus_dev, _count);
+   error = dprc_scan_objects(mc_bus_dev, NULL);
mutex_unlock(_bus->scan_mutex);
-   if (error < 0)
-   goto error;
-
-   if (dev_get_msi_domain(_bus_dev->dev) && !mc_bus->irq_resources) {
-   if (irq_count > FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS) {
-   dev_warn(_bus_dev->dev,
-"IRQs needed (%u) exceed IRQs preallocated 
(%u)\n",
-irq_count, FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
-   }
-
-   error = fsl_mc_populate_irq_pool(
-   mc_bus,
-   FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
-   if (error < 0)
-   goto error;
+   if (error < 0) {
+   fsl_mc_cleanup_all_resource_pools(mc_bus_dev);
+   return error;
}
 
return 0;
-error:
-   fsl_mc_cleanup_all_resource_pools(mc_bus_dev);
-   return error;
 }
 
 /**
-- 
1.9.1

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

[PATCH 2/2] staging: fsl-mc: do not print error in case of defer probe error

[Nipun Gupta]

Signed-off-by: Nipun Gupta 
---
 drivers/staging/fsl-mc/bus/fsl-mc-bus.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c 
b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
index 409f2b9..10cee00 100644
--- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
+++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
@@ -171,7 +171,8 @@ static int fsl_mc_driver_probe(struct device *dev)
 
error = mc_drv->probe(mc_dev);
if (error < 0) {
-   dev_err(dev, "%s failed: %d\n", __func__, error);
+   if (error != -EPROBE_DEFER)
+   dev_err(dev, "%s failed: %d\n", __func__, error);
return error;
}
 
-- 
1.9.1

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

Re: [PATCH v2 06/11] staging: pi433: Split rf69_set_crc_enabled into two functions

[Marcus Wolf]

Am 06.12.2017 um 11:37 schrieb Dan Carpenter:

On Wed, Dec 06, 2017 at 11:05:22AM +0200, Marcus Wolf wrote:



Am 06.12.2017 um 00:08 schrieb Simon Sandström:

Splits rf69_set_crc_enabled(dev, enabled) into
rf69_enable_crc(dev) and rf69_disable_crc(dev).

Signed-off-by: Simon Sandström 
---
   drivers/staging/pi433/pi433_if.c | 22 --
   drivers/staging/pi433/rf69.c | 18 ++
   drivers/staging/pi433/rf69.h |  4 ++--
   3 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 2ae19ac565d1..614eec7dd904 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -216,7 +216,16 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
return ret;
}
SET_CHECKED(rf69_set_adressFiltering(dev->spi, 
rx_cfg->enable_address_filtering));
-   SET_CHECKED(rf69_set_crc_enable (dev->spi, rx_cfg->enable_crc));
+
+   if (rx_cfg->enable_crc == OPTION_ON) {
+   ret = rf69_enable_crc(dev->spi);
+   if (ret < 0)
+   return ret;
+   } else {
+   ret = rf69_disable_crc(dev->spi);
+   if (ret < 0)
+   return ret;
+   }


Why don't you use SET_CHECKED(...)?



Marcus, please don't introduce new uses of SET_CHECKED().  It has a
hidden return in it which is against kernel style and introduces very
predictable and avoidable bugs.  For example, in probe().


Ah ok.

Thanks for clarifiytion!

What a pitty - another bunch of extra lines of code...

Or is there an other construction, allowing for one line per register 
change? Something like


ret = rf69_set_xyz(...); if (ret) return ret;
ret = rf69_set_abc(...); if (ret) return ret;

is pretty ugly and voids the style guide...

Thx,

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

Re: [PATCH] staging: pi433: Fixes issue with bit shift in rf69_get_modulation

[Marcus Wolf]

Am 06.12.2017 um 11:02 schrieb Greg KH:

On Wed, Nov 08, 2017 at 07:13:56PM +0200, Marcus Wolf wrote:

Fixes issue with bit shift in rf69_get_modulation


What "issue"?



Signed-off-by: Marcus Wolf 
---
  drivers/staging/pi433/rf69.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index 290b419..c945b4b 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -102,7 +102,7 @@ enum modulation rf69_get_modulation(struct spi_device *spi)
  
  	currentValue = READ_REG(REG_DATAMODUL);
  
-	switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE >> 3) { // TODO improvement: change 3 to define

+   switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE) {


Doesn't this change the logic here?

thanks,

greg k-h



Hi Greg,

yes, it does.

This is one of the very few changes to pi433 driver, that does not 
modify the architecture or optics of the code, but really fixes a bug. 
This function wasn't working from the very beginning, and we had already 
several reports and patches (from me and otheres), announcing or trying 
to fix the bug. But so far all patches were skipped for some reason.



Please take the patch.

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

Re: [PATCH v2 04/11] staging: pi433: Rename enum optionOnOff in rf69_enum.h

[Marcus Wolf]

Am 06.12.2017 um 00:08 schrieb Simon Sandström:

Renames the enum optionOnOff and its values optionOn, optionOff to enum
option_on_off and OPTION_ON, OPTION_OFF. Fixes checkpatch.pl warnings:
"Avoid CamelCase: , , ".

Signed-off-by: Simon Sandström 
---
  drivers/staging/pi433/pi433_if.c  | 34 ++---
  drivers/staging/pi433/pi433_if.h  | 16 +++---
  drivers/staging/pi433/rf69.c  | 45 ++-
  drivers/staging/pi433/rf69.h  | 15 -
  drivers/staging/pi433/rf69_enum.h |  6 +++---
  5 files changed, 63 insertions(+), 53 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index b8efe6a74a2a..4f6830f369e9 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -198,7 +198,7 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
/* packet config */
/* enable */
SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
-   if (rx_cfg->enable_sync == optionOn)
+   if (rx_cfg->enable_sync == OPTION_ON)
{
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, 
afterSyncInterrupt));
}
@@ -206,7 +206,7 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
{
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
}
-   if (rx_cfg->enable_length_byte == optionOn) {
+   if (rx_cfg->enable_length_byte == OPTION_ON) {
ret = rf69_set_packet_format(dev->spi, packetLengthVar);
if (ret < 0)
return ret;
@@ -220,14 +220,14 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
  
  	/* lengths */

SET_CHECKED(rf69_set_sync_size(dev->spi, rx_cfg->sync_length));
-   if (rx_cfg->enable_length_byte == optionOn)
+   if (rx_cfg->enable_length_byte == OPTION_ON)
{
SET_CHECKED(rf69_set_payload_length(dev->spi, 0xff));
}
else if (rx_cfg->fixed_message_length != 0)
{
payload_length = rx_cfg->fixed_message_length;
-   if (rx_cfg->enable_length_byte  == optionOn) payload_length++;
+   if (rx_cfg->enable_length_byte  == OPTION_ON) payload_length++;
if (rx_cfg->enable_address_filtering != filteringOff) 
payload_length++;
SET_CHECKED(rf69_set_payload_length(dev->spi, payload_length));
}
@@ -237,7 +237,7 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
}
  
  	/* values */

-   if (rx_cfg->enable_sync == optionOn)
+   if (rx_cfg->enable_sync == OPTION_ON)
{
SET_CHECKED(rf69_set_sync_values(dev->spi, 
rx_cfg->sync_pattern));
}
@@ -264,7 +264,7 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct 
pi433_tx_cfg *tx_cfg)
SET_CHECKED(rf69_set_tx_start_condition(dev->spi, 
tx_cfg->tx_start_condition));
  
  	/* packet format enable */

-   if (tx_cfg->enable_preamble == optionOn)
+   if (tx_cfg->enable_preamble == OPTION_ON)
{
SET_CHECKED(rf69_set_preamble_length(dev->spi, 
tx_cfg->preamble_length));
}
@@ -273,7 +273,7 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct 
pi433_tx_cfg *tx_cfg)
SET_CHECKED(rf69_set_preamble_length(dev->spi, 0));
}
SET_CHECKED(rf69_set_sync_enable  (dev->spi, tx_cfg->enable_sync));
-   if (tx_cfg->enable_length_byte == optionOn) {
+   if (tx_cfg->enable_length_byte == OPTION_ON) {
ret = rf69_set_packet_format(dev->spi, packetLengthVar);
if (ret < 0)
return ret;
@@ -285,7 +285,7 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct 
pi433_tx_cfg *tx_cfg)
SET_CHECKED(rf69_set_crc_enable   (dev->spi, tx_cfg->enable_crc));
  
  	/* configure sync, if enabled */

-   if (tx_cfg->enable_sync == optionOn) {
+   if (tx_cfg->enable_sync == OPTION_ON) {
SET_CHECKED(rf69_set_sync_size(dev->spi, tx_cfg->sync_length));
SET_CHECKED(rf69_set_sync_values(dev->spi, 
tx_cfg->sync_pattern));
}
@@ -400,7 +400,7 @@ pi433_receive(void *data)
}
  
  	/* length byte enabled? */

-   if (dev->rx_cfg.enable_length_byte == optionOn)
+   if (dev->rx_cfg.enable_length_byte == OPTION_ON)
{
retval = wait_event_interruptible(dev->fifo_wait_queue,
  dev->free_in_fifo < 
FIFO_SIZE);
@@ -525,11 +525,11 @@ pi433_tx_thread(void *data)
size = tx_cfg.fixed_message_length;
  
  		/* increase size, if len byte is requested */

-   if (tx_cfg.enable_length_byte == optionOn)
+   if (tx_cfg.enable_length_byte == OPTION_ON)
size++;
  
  		/* increase size, if adr byte is requested */

-  

Re: [PATCH v2 09/11] staging: pi433: Combine all rf69_set_amplifier_x()

[Marcus Wolf]

Am 06.12.2017 um 00:08 schrieb Simon Sandström:

Replaces the functions rf69_set_amplifier_1, _2, _3 with two
functions: rf69_enable_amplifier(dev, amp_mask) and
rf69_disable_amplifier(dev, amp_mask).

Signed-off-by: Simon Sandström 
---
  drivers/staging/pi433/pi433_if.c |  6 +++---
  drivers/staging/pi433/rf69.c | 46 
  drivers/staging/pi433/rf69.h |  8 ++-
  3 files changed, 9 insertions(+), 51 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 3b4170b9ba94..688d0cf00782 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -1126,9 +1126,9 @@ static int pi433_probe(struct spi_device *spi)
/* setup the radio module */
SET_CHECKED(rf69_set_mode   (spi, standby));
SET_CHECKED(rf69_set_data_mode  (spi, DATAMODUL_MODE_PACKET));
-   SET_CHECKED(rf69_set_amplifier_0(spi, OPTION_ON));
-   SET_CHECKED(rf69_set_amplifier_1(spi, OPTION_OFF));
-   SET_CHECKED(rf69_set_amplifier_2(spi, OPTION_OFF));
+   SET_CHECKED(rf69_enable_amplifier(spi, MASK_PALEVEL_PA0));
+   SET_CHECKED(rf69_disable_amplifier(spi, MASK_PALEVEL_PA1));
+   SET_CHECKED(rf69_disable_amplifier(spi, MASK_PALEVEL_PA2));
SET_CHECKED(rf69_set_output_power_level (spi, 13));
SET_CHECKED(rf69_set_antenna_impedance  (spi, fiftyOhm));
  
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c

index 4c9eb97978ef..c97c65ea8acd 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -262,52 +262,14 @@ int rf69_set_frequency(struct spi_device *spi, u32 
frequency)
return 0;
  }
  
-int rf69_set_amplifier_0(struct spi_device *spi,

-enum option_on_off option_on_off)
+int rf69_enable_amplifier(struct spi_device *spi, u8 amplifier_mask)
  {
-   #ifdef DEBUG
-   dev_dbg(>dev, "set: amp #0");
-   #endif
-
-   switch (option_on_off) {
-   case OPTION_ON:  return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) | 
 MASK_PALEVEL_PA0));
-   case OPTION_OFF: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) & 
~MASK_PALEVEL_PA0));
-   default:
-   dev_dbg(>dev, "set: illegal input param");
-   return -EINVAL;
-   }
-}
-
-int rf69_set_amplifier_1(struct spi_device *spi,
-enum option_on_off option_on_off)
-{
-   #ifdef DEBUG
-   dev_dbg(>dev, "set: amp #1");
-   #endif
-
-   switch (option_on_off) {
-   case OPTION_ON:  return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) | 
 MASK_PALEVEL_PA1));
-   case OPTION_OFF: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) & 
~MASK_PALEVEL_PA1));
-   default:
-   dev_dbg(>dev, "set: illegal input param");
-   return -EINVAL;
-   }
+   return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) | amplifier_mask));
  }
  
-int rf69_set_amplifier_2(struct spi_device *spi,

-enum option_on_off option_on_off)
+int rf69_disable_amplifier(struct spi_device *spi, u8 amplifier_mask)
  {
-   #ifdef DEBUG
-   dev_dbg(>dev, "set: amp #2");
-   #endif
-
-   switch (option_on_off) {
-   case OPTION_ON:  return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) | 
 MASK_PALEVEL_PA2));
-   case OPTION_OFF: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) & 
~MASK_PALEVEL_PA2));
-   default:
-   dev_dbg(>dev, "set: illegal input param");
-   return -EINVAL;
-   }
+   return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) & 
~amplifier_mask));
  }
  
  int rf69_set_output_power_level(struct spi_device *spi, u8 powerLevel)

diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 18296b4502f3..ba25ab6aeae7 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -33,12 +33,8 @@ int rf69_set_modulation_shaping(struct spi_device *spi, enum 
mod_shaping mod_sha
  int rf69_set_bit_rate(struct spi_device *spi, u16 bitRate);
  int rf69_set_deviation(struct spi_device *spi, u32 deviation);
  int rf69_set_frequency(struct spi_device *spi, u32 frequency);
-int rf69_set_amplifier_0(struct spi_device *spi,
-enum option_on_off option_on_off);
-int rf69_set_amplifier_1(struct spi_device *spi,
-enum option_on_off option_on_off);
-int rf69_set_amplifier_2(struct spi_device *spi,
-enum option_on_off option_on_off);
+int rf69_enable_amplifier(struct spi_device *spi, u8 amplifier_mask);
+int rf69_disable_amplifier(struct spi_device *spi, u8 amplifier_mask);
  int rf69_set_output_power_level(struct spi_device *spi, u8 powerLevel);
  int rf69_set_pa_ramp(struct spi_device *spi, enum paRamp paRamp);
  int rf69_set_antenna_impedance(struct spi_device *spi, enum antennaImpedance 

Re: [PATCH v2 06/11] staging: pi433: Split rf69_set_crc_enabled into two functions

[Dan Carpenter]

On Wed, Dec 06, 2017 at 11:05:22AM +0200, Marcus Wolf wrote:
> 
> 
> Am 06.12.2017 um 00:08 schrieb Simon Sandström:
> > Splits rf69_set_crc_enabled(dev, enabled) into
> > rf69_enable_crc(dev) and rf69_disable_crc(dev).
> > 
> > Signed-off-by: Simon Sandström 
> > ---
> >   drivers/staging/pi433/pi433_if.c | 22 --
> >   drivers/staging/pi433/rf69.c | 18 ++
> >   drivers/staging/pi433/rf69.h |  4 ++--
> >   3 files changed, 28 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/staging/pi433/pi433_if.c 
> > b/drivers/staging/pi433/pi433_if.c
> > index 2ae19ac565d1..614eec7dd904 100644
> > --- a/drivers/staging/pi433/pi433_if.c
> > +++ b/drivers/staging/pi433/pi433_if.c
> > @@ -216,7 +216,16 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
> > pi433_rx_cfg *rx_cfg)
> > return ret;
> > }
> > SET_CHECKED(rf69_set_adressFiltering(dev->spi, 
> > rx_cfg->enable_address_filtering));
> > -   SET_CHECKED(rf69_set_crc_enable (dev->spi, rx_cfg->enable_crc));
> > +
> > +   if (rx_cfg->enable_crc == OPTION_ON) {
> > +   ret = rf69_enable_crc(dev->spi);
> > +   if (ret < 0)
> > +   return ret;
> > +   } else {
> > +   ret = rf69_disable_crc(dev->spi);
> > +   if (ret < 0)
> > +   return ret;
> > +   }
> 
> Why don't you use SET_CHECKED(...)?
> 

Marcus, please don't introduce new uses of SET_CHECKED().  It has a
hidden return in it which is against kernel style and introduces very
predictable and avoidable bugs.  For example, in probe().

> I stil don't like this kind of changes - and not using SET_CHECKED makes it
> even worse, since that further increases code length.
> 
> The idea was to have the configuration as compact, as you can see in the
> receiver config section. It's a pitty that the packet config already needs
> such a huge number of exceptions due to technical reasons. We shouldn't
> further extend the numbers of exceptions and shouldn't extend the number of
> lines for setting a reg.
> 
> Initially this function was just like
> set_rx_cfg()
> {
> SET_CHECKED(...)
> SET_CHECKED(...)
> SET_CHECKED(...)
> SET_CHECKED(...)
> }
> 
> It should be easy,
> * to survey, which chip settings are touched, if set_rx_cfg is called.
> * to survey, that all params of the rx_cfg struct are taken care of.
> 
> The longer the function gets, the harder it is, to service it.
> I really would be happy, if we don't go this way.
> 
> 
> Anyway, please keep the naming convention of rf69.c:
> 
> rf69 -set/get - action
> -> rf69_set_crc_enable

No...  Simon's name is better.  His is shorter and makes more sense.  :(

regards,
dan carpenter

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

[PATCH] staging: lustre: Fix sparse, using plain integer as NULL pointer in lov_object_fiemap()

[Andrii]

Change 0 to NULL in lov_object_fiemap() in order to fix warning produced by
sparse

Signed-off-by: Andrii Vladyka 
Signed-off-by: Andreas Dilger 


diff --git a/drivers/staging/lustre/lustre/lov/lov_object.c 
b/drivers/staging/lustre/lustre/lov/lov_object.c
index 105b707..897cf2c 100644
--- a/drivers/staging/lustre/lustre/lov/lov_object.c
+++ b/drivers/staging/lustre/lustre/lov/lov_object.c
@@ -1335,7 +1335,7 @@ static int lov_object_fiemap(const struct lu_env *env, 
struct cl_object *obj,
int rc = 0;
int cur_stripe;
int stripe_count;
-   struct fiemap_state fs = { 0 };
+   struct fiemap_state fs = { NULL };
 
lsm = lov_lsm_addref(cl2lov(obj));
if (!lsm)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 08/11] staging: pi433: Remove enum data_mode

[Marcus Wolf]

Am 06.12.2017 um 00:08 schrieb Simon Sandström:

Call rf69_set_data_mode with DATAMODUL_MODE value directly.

Signed-off-by: Simon Sandström 
---
  drivers/staging/pi433/pi433_if.c  |  2 +-
  drivers/staging/pi433/rf69.c  | 15 ++-
  drivers/staging/pi433/rf69.h  |  2 +-
  drivers/staging/pi433/rf69_enum.h |  6 --
  4 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index fb500d062df8..3b4170b9ba94 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -1125,7 +1125,7 @@ static int pi433_probe(struct spi_device *spi)
  
  	/* setup the radio module */

SET_CHECKED(rf69_set_mode   (spi, standby));
-   SET_CHECKED(rf69_set_data_mode  (spi, packet));
+   SET_CHECKED(rf69_set_data_mode  (spi, DATAMODUL_MODE_PACKET));
SET_CHECKED(rf69_set_amplifier_0(spi, OPTION_ON));
SET_CHECKED(rf69_set_amplifier_1(spi, OPTION_OFF));
SET_CHECKED(rf69_set_amplifier_2(spi, OPTION_OFF));
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index e5b29bed35ef..4c9eb97978ef 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -61,20 +61,9 @@ int rf69_set_mode(struct spi_device *spi, enum mode mode)
  
  }
  
-int rf69_set_data_mode(struct spi_device *spi, enum dataMode dataMode)

+int rf69_set_data_mode(struct spi_device *spi, u8 data_mode)
  {
-   #ifdef DEBUG
-   dev_dbg(>dev, "set: data mode");
-   #endif
-
-   switch (dataMode) {
-   case packet:return WRITE_REG(REG_DATAMODUL, 
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODE) | DATAMODUL_MODE_PACKET);
-   case continuous:return WRITE_REG(REG_DATAMODUL, 
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODE) | DATAMODUL_MODE_CONTINUOUS);
-   case continuousNoSync:  return WRITE_REG(REG_DATAMODUL, 
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODE) | 
DATAMODUL_MODE_CONTINUOUS_NOSYNC);
-   default:
-   dev_dbg(>dev, "set: illegal input param");
-   return -EINVAL;
-   }
+   return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & 
~MASK_DATAMODUL_MODE) | data_mode);
  }
  
  int rf69_set_modulation(struct spi_device *spi, enum modulation modulation)

diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 177223451c87..18296b4502f3 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -26,7 +26,7 @@
  #define FIFO_THRESHOLD15  /* in byte */
  
  int rf69_set_mode(struct spi_device *spi, enum mode mode);

-int rf69_set_data_mode(struct spi_device *spi, enum dataMode dataMode);
+int rf69_set_data_mode(struct spi_device *spi, u8 data_mode);
  int rf69_set_modulation(struct spi_device *spi, enum modulation modulation);
  enum modulation rf69_get_modulation(struct spi_device *spi);
  int rf69_set_modulation_shaping(struct spi_device *spi, enum mod_shaping 
mod_shaping);
diff --git a/drivers/staging/pi433/rf69_enum.h 
b/drivers/staging/pi433/rf69_enum.h
index 97f615effca3..b0715b4eb6ac 100644
--- a/drivers/staging/pi433/rf69_enum.h
+++ b/drivers/staging/pi433/rf69_enum.h
@@ -31,12 +31,6 @@ enum mode {
receive
  };
  
-enum dataMode {

-   packet,
-   continuous,
-   continuousNoSync
-};
-
  enum modulation {
OOK,
FSK



Hi Simon,

this change is "closing a door".

The rf69 is able to work in an advanced packet mode or in a simple 
continuous mode.


The driver so far is using the advanced packet mode, only. But if it 
will support continuous mode some day, it will be necessary to configure 
this.


The open source projects fhem and domoticz already asked for such a 
change, since for their architecture, they'll need a pi433 working in 
continuous mode. But so far I am not planning to implement such a 
functionality.


Since the rule for kernel development seems to be, not to care about 
future, most probably you patch is fine, anyway.


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

Re: [PATCH v2 06/11] staging: pi433: Split rf69_set_crc_enabled into two functions

[Marcus Wolf]

Am 06.12.2017 um 00:08 schrieb Simon Sandström:

Splits rf69_set_crc_enabled(dev, enabled) into
rf69_enable_crc(dev) and rf69_disable_crc(dev).

Signed-off-by: Simon Sandström 
---
  drivers/staging/pi433/pi433_if.c | 22 --
  drivers/staging/pi433/rf69.c | 18 ++
  drivers/staging/pi433/rf69.h |  4 ++--
  3 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 2ae19ac565d1..614eec7dd904 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -216,7 +216,16 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
return ret;
}
SET_CHECKED(rf69_set_adressFiltering(dev->spi, 
rx_cfg->enable_address_filtering));
-   SET_CHECKED(rf69_set_crc_enable (dev->spi, rx_cfg->enable_crc));
+
+   if (rx_cfg->enable_crc == OPTION_ON) {
+   ret = rf69_enable_crc(dev->spi);
+   if (ret < 0)
+   return ret;
+   } else {
+   ret = rf69_disable_crc(dev->spi);
+   if (ret < 0)
+   return ret;
+   }


Why don't you use SET_CHECKED(...)?

I stil don't like this kind of changes - and not using SET_CHECKED makes 
it even worse, since that further increases code length.


The idea was to have the configuration as compact, as you can see in the 
receiver config section. It's a pitty that the packet config already 
needs such a huge number of exceptions due to technical reasons. We 
shouldn't further extend the numbers of exceptions and shouldn't extend 
the number of lines for setting a reg.


Initially this function was just like
set_rx_cfg()
{
SET_CHECKED(...)
SET_CHECKED(...)
SET_CHECKED(...)
SET_CHECKED(...)
}

It should be easy,
* to survey, which chip settings are touched, if set_rx_cfg is called.
* to survey, that all params of the rx_cfg struct are taken care of.

The longer the function gets, the harder it is, to service it.
I really would be happy, if we don't go this way.


Anyway, please keep the naming convention of rf69.c:

rf69 -set/get - action
-> rf69_set_crc_enable

Thanks,

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

Re: [PATCH] staging: android: ion: Check for register_shrinker() failure.

[Greg KH]

On Wed, Nov 29, 2017 at 10:33:39PM +0900, Tetsuo Handa wrote:
> register_shrinker() might return -ENOMEM error since Linux 3.12.
> But since callers of ion_device_add_heap() are not ready to receive an
> error and it is not simple enough to fix within this patch, this patch
> just prints a warning line when register_shrinker() failed.
> 
> Signed-off-by: Tetsuo Handa 
> Cc: John Stultz 
> Cc: Greg Kroah-Hartman 
> Cc: Michal Hocko 
> ---
>  drivers/staging/android/ion/ion_heap.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/android/ion/ion_heap.c 
> b/drivers/staging/android/ion/ion_heap.c
> index 91faa7f..56dcbd8 100644
> --- a/drivers/staging/android/ion/ion_heap.c
> +++ b/drivers/staging/android/ion/ion_heap.c
> @@ -312,5 +312,6 @@ void ion_heap_init_shrinker(struct ion_heap *heap)
>   heap->shrinker.scan_objects = ion_heap_shrink_scan;
>   heap->shrinker.seeks = DEFAULT_SEEKS;
>   heap->shrinker.batch = 0;
> - register_shrinker(>shrinker);
> + if (register_shrinker(>shrinker))
> + pr_err("Failed to create heap shrinker for %s\n", heap->name);

Shouldn't you fail the init function here?

Just reporting the issue isn't going to help anyone.

thanks,

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

Re: [PATCH] staging: android: ashmem: Check for register_shrinker() failure.

[Greg KH]

On Wed, Nov 29, 2017 at 10:32:00PM +0900, Tetsuo Handa wrote:
> register_shrinker() might return -ENOMEM error since Linux 3.12.
> 
> Signed-off-by: Tetsuo Handa 
> Cc: Robert Love 
> Cc: Marco Nelissen 
> Cc: John Stultz 
> Cc: Greg Kroah-Hartman 
> Cc: Michal Hocko 
> ---
>  drivers/staging/android/ashmem.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/android/ashmem.c 
> b/drivers/staging/android/ashmem.c
> index 0f695df..ab56f81 100644
> --- a/drivers/staging/android/ashmem.c
> +++ b/drivers/staging/android/ashmem.c
> @@ -862,12 +862,18 @@ static int __init ashmem_init(void)
>   goto out_free2;
>   }
>  
> - register_shrinker(_shrinker);
> + ret = register_shrinker(_shrinker);
> + if (unlikely(ret)) {

Never use unlikely/likely unless you can benchmark the speed change.

Hint, in init functions you never need this...

Please fix up and resend.

thanks,

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

Re: [PATCH] staging: pi433: Fixes issue with bit shift in rf69_get_modulation

[Greg KH]

On Wed, Nov 08, 2017 at 07:13:56PM +0200, Marcus Wolf wrote:
> Fixes issue with bit shift in rf69_get_modulation

What "issue"?

> 
> Signed-off-by: Marcus Wolf 
> ---
>  drivers/staging/pi433/rf69.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> index 290b419..c945b4b 100644
> --- a/drivers/staging/pi433/rf69.c
> +++ b/drivers/staging/pi433/rf69.c
> @@ -102,7 +102,7 @@ enum modulation rf69_get_modulation(struct spi_device 
> *spi)
>  
>   currentValue = READ_REG(REG_DATAMODUL);
>  
> - switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE >> 3) { // TODO 
> improvement: change 3 to define
> + switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE) {

Doesn't this change the logic here?

thanks,

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

Re: [PATCH] staging: lustre: Fix sparse, using plain integer as NULL pointer in lov_object_fiemap()

[gre...@linuxfoundation.org]

On Mon, Dec 04, 2017 at 12:44:32PM +0200, Andrii Vladyka wrote:
> Change 0 to NULL in lov_object_fiemap() in order to fix warning produced by
> sparse
> 
> Signed-off-by: Andrii Vladyka 
> Signed-off-by: Andreas Dilger 
> ---
> 
> diff --git a/drivers/staging/lustre/lustre/lov/lov_object.c 
> b/drivers/staging/lustre/lustre/lov/lov_object.c
> index 105b707..897cf2c 100644
> --- a/drivers/staging/lustre/lustre/lov/lov_object.c
> +++ b/drivers/staging/lustre/lustre/lov/lov_object.c
> @@ -1335,7 +1335,7 @@ static int lov_object_fiemap(const struct lu_env *env, 
> struct cl_object *obj,
>   int rc = 0;
>   int cur_stripe;
>   int stripe_count;
> - struct fiemap_state fs = { 0 };
> + struct fiemap_state fs = { NULL };
>   lsm = lov_lsm_addref(cl2lov(obj));
>   if (!lsm)

Patch is corrupted, and can not apply, please fix up your email client
and try it again.

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

Re: [PATCH v2 1/2] staging: lustre: check result of register_shrinker

[Greg KH]

On Mon, Dec 04, 2017 at 10:21:56PM +0300, Aliaksei Karaliou wrote:
> Lustre code lacks checking the result of register_shrinker()
> in several places. register_shrinker() was tagged __must_check
> recently so that sparse has started reporting it.
> 
> Signed-off-by: Aliaksei Karaliou 
> ---
>  drivers/staging/lustre/lustre/ldlm/ldlm_pool.c | 12 +---
>  drivers/staging/lustre/lustre/obdclass/lu_object.c |  5 +++--
>  drivers/staging/lustre/lustre/osc/osc_request.c|  4 +++-
>  drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c|  8 +++-
>  4 files changed, 22 insertions(+), 7 deletions(-)
> 
> v2: Style fixes, as suggested by Cheers, Andreas and Dan Carpenter.
> Added one more patch to address resource cleanup, suggested by Dan 
> Carpenter.
> 
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c 
> b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
> index da65d00a7811..9fef2d52d6c2 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
> @@ -1086,10 +1086,16 @@ int ldlm_pools_init(void)
>   int rc;
>  
>   rc = ldlm_pools_thread_start();
> - if (rc == 0)
> - register_shrinker(_pools_cli_shrinker);
> + if (rc)
> + return rc;
>  
> - return rc;
> + rc = register_shrinker(_pools_cli_shrinker);
> + if (rc) {
> + ldlm_pools_thread_stop();
> + return rc;
> + }
> +
> + return 0;
>  }
>  
>  void ldlm_pools_fini(void)
> diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c 
> b/drivers/staging/lustre/lustre/obdclass/lu_object.c
> index b938a3f9d50a..9e0256ca2079 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
> @@ -1951,7 +1951,7 @@ int lu_global_init(void)
>* inode, one for ea. Unfortunately setting this high value results in
>* lu_object/inode cache consuming all the memory.
>*/
> - register_shrinker(_site_shrinker);
> + result = register_shrinker(_site_shrinker);
>  
>   return result;

return register_shrinker()?

> - register_shrinker(_shrinker);
> + rc = register_shrinker(_shrinker);
> + if (rc) {
> + enc_pools_free();
> + return rc;

Drop the return rc, and then just do:

> + }
>  
>   return 0;

return rc;
there.  Makes the patch smaller.

thanks,

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