All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: pi433: fix frequency deviation check
@ 2022-01-03  8:11 Paulo Miguel Almeida
  2022-01-03 10:00 ` Fabio Aiuto
  2022-01-03 13:27 ` Greg KH
  0 siblings, 2 replies; 5+ messages in thread
From: Paulo Miguel Almeida @ 2022-01-03  8:11 UTC (permalink / raw)
  To: gregkh, paulo.miguel.almeida.rodenas, realwakka
  Cc: linux-staging, linux-kernel

rf69 datasheet states that frequency deviation must exceed 600 Hz but
also that frequency deviation + (bitrate / 2) should be less than equal
to 500 kHz to ensure proper modulation.

This patch validates that both conditions are met so RF intersymbol
interference is "avoided" where possible.

Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
---
 drivers/staging/pi433/rf69.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index e62e61ef4d27..cc22915fd489 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -255,17 +255,29 @@ int rf69_set_deviation(struct spi_device *spi, u32 deviation)
 	int retval;
 	u64 f_reg;
 	u64 f_step;
+	u32 bit_rate_reg;
+	u32 bit_rate;
 	u8 msb;
 	u8 lsb;
 	u64 factor = 1000000; // to improve precision of calculation
 
-	// TODO: Dependency to bitrate
-	if (deviation < 600 || deviation > 500000) {
-		dev_dbg(&spi->dev, "set_deviation: illegal input param");
+	// calculate bit rate
+	bit_rate_reg = rf69_read_reg(spi, REG_BITRATE_MSB) << 8;
+	bit_rate_reg |= rf69_read_reg(spi, REG_BITRATE_LSB);
+	bit_rate = F_OSC / bit_rate_reg;
+
+	/*
+	 * frequency deviation must exceed 600 Hz and but not exceed
+	 * 500kHz when taking bitrate dependency into consideration
+	 * to ensure proper modulation
+	 */
+	if (deviation < 600 || (deviation + (bit_rate / 2)) > 500000) {
+		dev_dbg(&spi->dev,
+			"set_deviation: illegal input param: %u", deviation);
 		return -EINVAL;
 	}
 
-	// calculat f step
+	// calculate f step
 	f_step = F_OSC * factor;
 	do_div(f_step, 524288); //  524288 = 2^19
 
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] staging: pi433: fix frequency deviation check
  2022-01-03  8:11 [PATCH] staging: pi433: fix frequency deviation check Paulo Miguel Almeida
@ 2022-01-03 10:00 ` Fabio Aiuto
  2022-01-03 13:27 ` Greg KH
  1 sibling, 0 replies; 5+ messages in thread
From: Fabio Aiuto @ 2022-01-03 10:00 UTC (permalink / raw)
  To: Paulo Miguel Almeida; +Cc: gregkh, realwakka, linux-staging, linux-kernel

Hello Paulo,

thanks for your patch.

On Mon, Jan 03, 2022 at 09:11:35PM +1300, Paulo Miguel Almeida wrote:
> rf69 datasheet states that frequency deviation must exceed 600 Hz but
> also that frequency deviation + (bitrate / 2) should be less than equal
> to 500 kHz to ensure proper modulation.
> 
> This patch validates that both conditions are met so RF intersymbol
> interference is "avoided" where possible.
> 
> Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
> ---
>  drivers/staging/pi433/rf69.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> index e62e61ef4d27..cc22915fd489 100644
> --- a/drivers/staging/pi433/rf69.c
> +++ b/drivers/staging/pi433/rf69.c
> @@ -255,17 +255,29 @@ int rf69_set_deviation(struct spi_device *spi, u32 deviation)
>  	int retval;
>  	u64 f_reg;
>  	u64 f_step;
> +	u32 bit_rate_reg;
> +	u32 bit_rate;
>  	u8 msb;
>  	u8 lsb;
>  	u64 factor = 1000000; // to improve precision of calculation
>  
> -	// TODO: Dependency to bitrate
> -	if (deviation < 600 || deviation > 500000) {
> -		dev_dbg(&spi->dev, "set_deviation: illegal input param");
> +	// calculate bit rate
> +	bit_rate_reg = rf69_read_reg(spi, REG_BITRATE_MSB) << 8;
> +	bit_rate_reg |= rf69_read_reg(spi, REG_BITRATE_LSB);
> +	bit_rate = F_OSC / bit_rate_reg;
> +
> +	/*
> +	 * frequency deviation must exceed 600 Hz and but not exceed
> +	 * 500kHz when taking bitrate dependency into consideration
> +	 * to ensure proper modulation
> +	 */
> +	if (deviation < 600 || (deviation + (bit_rate / 2)) > 500000) {
> +		dev_dbg(&spi->dev,
> +			"set_deviation: illegal input param: %u", deviation);
>  		return -EINVAL;
>  	}

this should go in a separate patch IMO

>  
> -	// calculat f step
> +	// calculate f step
>  	f_step = F_OSC * factor;
>  	do_div(f_step, 524288); //  524288 = 2^19
>  
> -- 
> 2.25.4
> 
> 

thank you,

fabio

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] staging: pi433: fix frequency deviation check
  2022-01-03  8:11 [PATCH] staging: pi433: fix frequency deviation check Paulo Miguel Almeida
  2022-01-03 10:00 ` Fabio Aiuto
@ 2022-01-03 13:27 ` Greg KH
  2022-01-03 22:23   ` [PATCH v2] " Paulo Miguel Almeida
  2022-01-03 22:28   ` [PATCH] " Paulo Miguel Almeida
  1 sibling, 2 replies; 5+ messages in thread
From: Greg KH @ 2022-01-03 13:27 UTC (permalink / raw)
  To: Paulo Miguel Almeida; +Cc: realwakka, linux-staging, linux-kernel

On Mon, Jan 03, 2022 at 09:11:35PM +1300, Paulo Miguel Almeida wrote:
> rf69 datasheet states that frequency deviation must exceed 600 Hz but
> also that frequency deviation + (bitrate / 2) should be less than equal
> to 500 kHz to ensure proper modulation.
> 
> This patch validates that both conditions are met so RF intersymbol
> interference is "avoided" where possible.
> 
> Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
> ---
>  drivers/staging/pi433/rf69.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> index e62e61ef4d27..cc22915fd489 100644
> --- a/drivers/staging/pi433/rf69.c
> +++ b/drivers/staging/pi433/rf69.c
> @@ -255,17 +255,29 @@ int rf69_set_deviation(struct spi_device *spi, u32 deviation)
>  	int retval;
>  	u64 f_reg;
>  	u64 f_step;
> +	u32 bit_rate_reg;
> +	u32 bit_rate;
>  	u8 msb;
>  	u8 lsb;
>  	u64 factor = 1000000; // to improve precision of calculation
>  
> -	// TODO: Dependency to bitrate
> -	if (deviation < 600 || deviation > 500000) {
> -		dev_dbg(&spi->dev, "set_deviation: illegal input param");
> +	// calculate bit rate
> +	bit_rate_reg = rf69_read_reg(spi, REG_BITRATE_MSB) << 8;
> +	bit_rate_reg |= rf69_read_reg(spi, REG_BITRATE_LSB);
> +	bit_rate = F_OSC / bit_rate_reg;
> +
> +	/*
> +	 * frequency deviation must exceed 600 Hz and but not exceed
> +	 * 500kHz when taking bitrate dependency into consideration
> +	 * to ensure proper modulation
> +	 */
> +	if (deviation < 600 || (deviation + (bit_rate / 2)) > 500000) {
> +		dev_dbg(&spi->dev,
> +			"set_deviation: illegal input param: %u", deviation);
>  		return -EINVAL;
>  	}
>  
> -	// calculat f step
> +	// calculate f step

This change should be a separate patch.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH v2] staging: pi433: fix frequency deviation check
  2022-01-03 13:27 ` Greg KH
@ 2022-01-03 22:23   ` Paulo Miguel Almeida
  2022-01-03 22:28   ` [PATCH] " Paulo Miguel Almeida
  1 sibling, 0 replies; 5+ messages in thread
From: Paulo Miguel Almeida @ 2022-01-03 22:23 UTC (permalink / raw)
  To: gregkh, paulo.miguel.almeida.rodenas, realwakka
  Cc: linux-staging, linux-kernel

rf69 datasheet states that frequency deviation must exceed 600 Hz but
also that frequency deviation + (bitrate / 2) should be less than equal
to 500 kHz to ensure proper modulation.

This patch validates that both conditions are met so RF intersymbol
interference is less likely to happen due to misconfiguration of the uC

Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
---
v2: remove typo correction from this patch - Req Greg k-h, Fabio A.
v1: https://lore.kernel.org/lkml/20220103081135.GA11642@mail.google.com/
---
 drivers/staging/pi433/rf69.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index e62e61ef4d27..d64df072d8e8 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -255,13 +255,25 @@ int rf69_set_deviation(struct spi_device *spi, u32 deviation)
 	int retval;
 	u64 f_reg;
 	u64 f_step;
+	u32 bit_rate_reg;
+	u32 bit_rate;
 	u8 msb;
 	u8 lsb;
 	u64 factor = 1000000; // to improve precision of calculation
 
-	// TODO: Dependency to bitrate
-	if (deviation < 600 || deviation > 500000) {
-		dev_dbg(&spi->dev, "set_deviation: illegal input param");
+	// calculate bit rate
+	bit_rate_reg = rf69_read_reg(spi, REG_BITRATE_MSB) << 8;
+	bit_rate_reg |= rf69_read_reg(spi, REG_BITRATE_LSB);
+	bit_rate = F_OSC / bit_rate_reg;
+
+	/*
+	 * frequency deviation must exceed 600 Hz but not exceed
+	 * 500kHz when taking bitrate dependency into consideration
+	 * to ensure proper modulation
+	 */
+	if (deviation < 600 || (deviation + (bit_rate / 2)) > 500000) {
+		dev_dbg(&spi->dev,
+			"set_deviation: illegal input param: %u", deviation);
 		return -EINVAL;
 	}
 
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] staging: pi433: fix frequency deviation check
  2022-01-03 13:27 ` Greg KH
  2022-01-03 22:23   ` [PATCH v2] " Paulo Miguel Almeida
@ 2022-01-03 22:28   ` Paulo Miguel Almeida
  1 sibling, 0 replies; 5+ messages in thread
From: Paulo Miguel Almeida @ 2022-01-03 22:28 UTC (permalink / raw)
  To: Greg KH, fabioaiuto83; +Cc: realwakka, linux-staging, linux-kernel

On Mon, Jan 03, 2022 at 02:27:10PM +0100, Greg KH wrote:
> On Mon, Jan 03, 2022 at 09:11:35PM +1300, Paulo Miguel Almeida wrote:
> > -	// calculat f step
> > +	// calculate f step
> 
> This change should be a separate patch.
>

Thanks Fabio and Greg for reviewing my patch. I just sent a v2 of this
patch with that line untouched.

thanks,

Paulo A.

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-03 22:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-03  8:11 [PATCH] staging: pi433: fix frequency deviation check Paulo Miguel Almeida
2022-01-03 10:00 ` Fabio Aiuto
2022-01-03 13:27 ` Greg KH
2022-01-03 22:23   ` [PATCH v2] " Paulo Miguel Almeida
2022-01-03 22:28   ` [PATCH] " Paulo Miguel Almeida

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.