linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: pi433: Fix rf69_set_tx_cfg() logic
@ 2019-03-27  8:11 Sidong Yang
  2019-03-27 11:59 ` Dan Carpenter
  0 siblings, 1 reply; 2+ messages in thread
From: Sidong Yang @ 2019-03-27  8:11 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Sidong Yang, Michael Straube, devel, linux-kernel

Moved code to configure sync to where check enable_sync option before.
There is no need to check enable_sync twice. Configuring sync should be
executed immediately after enabling sync.

Signed-off-by: Sidong Yang <realwakka@gmail.com>
---
 drivers/staging/pi433/pi433_if.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 53928af696a6..0a48d6cb9547 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -318,10 +318,17 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct pi433_tx_cfg *tx_cfg)
 			return ret;
 	}
 
+	/* configure sync, if enabled */
 	if (tx_cfg->enable_sync == OPTION_ON) {
 		ret = rf69_enable_sync(dev->spi);
 		if (ret < 0)
 			return ret;
+		ret = rf69_set_sync_size(dev->spi, tx_cfg->sync_length);
+		if (ret < 0)
+			return ret;
+		ret = rf69_set_sync_values(dev->spi, tx_cfg->sync_pattern);
+		if (ret < 0)
+			return ret;
 	} else {
 		ret = rf69_disable_sync(dev->spi);
 		if (ret < 0)
@@ -348,16 +355,6 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct pi433_tx_cfg *tx_cfg)
 			return ret;
 	}
 
-	/* configure sync, if enabled */
-	if (tx_cfg->enable_sync == OPTION_ON) {
-		ret = rf69_set_sync_size(dev->spi, tx_cfg->sync_length);
-		if (ret < 0)
-			return ret;
-		ret = rf69_set_sync_values(dev->spi, tx_cfg->sync_pattern);
-		if (ret < 0)
-			return ret;
-	}
-
 	return 0;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] staging: pi433: Fix rf69_set_tx_cfg() logic
  2019-03-27  8:11 [PATCH] staging: pi433: Fix rf69_set_tx_cfg() logic Sidong Yang
@ 2019-03-27 11:59 ` Dan Carpenter
  0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2019-03-27 11:59 UTC (permalink / raw)
  To: Sidong Yang; +Cc: Greg Kroah-Hartman, devel, linux-kernel

On Wed, Mar 27, 2019 at 08:11:20AM +0000, Sidong Yang wrote:
> Moved code to configure sync to where check enable_sync option before.
> There is no need to check enable_sync twice. Configuring sync should be
> executed immediately after enabling sync.
> 
> Signed-off-by: Sidong Yang <realwakka@gmail.com>
> ---
>  drivers/staging/pi433/pi433_if.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
> index 53928af696a6..0a48d6cb9547 100644
> --- a/drivers/staging/pi433/pi433_if.c
> +++ b/drivers/staging/pi433/pi433_if.c
> @@ -318,10 +318,17 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct pi433_tx_cfg *tx_cfg)
>  			return ret;
>  	}
>  
> +	/* configure sync, if enabled */

This comment is obvious.  Just delete it.

>  	if (tx_cfg->enable_sync == OPTION_ON) {
>  		ret = rf69_enable_sync(dev->spi);
>  		if (ret < 0)
>  			return ret;
> +		ret = rf69_set_sync_size(dev->spi, tx_cfg->sync_length);
> +		if (ret < 0)
> +			return ret;
> +		ret = rf69_set_sync_values(dev->spi, tx_cfg->sync_pattern);
> +		if (ret < 0)
> +			return ret;

It's weird that we enable sync before we et the size or sync values...

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-03-27 11:59 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-27  8:11 [PATCH] staging: pi433: Fix rf69_set_tx_cfg() logic Sidong Yang
2019-03-27 11:59 ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).