staging: pi433: change (struct pi433_tx_cfg)->bit_rate to be a u32

Message ID Y+srSuTdGmzmXG1I@jacob-Ubuntu
State New
Headers
Series staging: pi433: change (struct pi433_tx_cfg)->bit_rate to be a u32 |

Commit Message

Jacob Bai Feb. 14, 2023, 6:33 a.m. UTC
  Based on the TODO file and datasheet of pi433, the maximum bit rate for
transmitter is 300kbps when modulation been set to FSK. Hence, the size
should be u32 rather than u16.

Signed-off-by: Jacob Bai <jacob.bai.au@gmail.com>
---
 drivers/staging/pi433/Documentation/pi433.txt | 3 ++-
 drivers/staging/pi433/pi433_if.h              | 2 +-
 drivers/staging/pi433/rf69.c                  | 4 ++--
 drivers/staging/pi433/rf69.h                  | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)
  

Comments

Deepak R Varma Feb. 14, 2023, 7:58 a.m. UTC | #1
On Tue, Feb 14, 2023 at 05:33:46PM +1100, Jacob Bai wrote:
> Based on the TODO file and datasheet of pi433, the maximum bit rate for
> transmitter is 300kbps when modulation been set to FSK. Hence, the size
> should be u32 rather than u16.

Hi Jacob,
This change was already discussed recently. Please check the following link and
all the responses to this thread:

https://lore.kernel.org/all/Y9h42l%2F8EcPqn63x@combine-ThinkPad-S1-Yoga/

Thanks,
./drv

> 
> Signed-off-by: Jacob Bai <jacob.bai.au@gmail.com>
> ---
>  drivers/staging/pi433/Documentation/pi433.txt | 3 ++-
>  drivers/staging/pi433/pi433_if.h              | 2 +-
>  drivers/staging/pi433/rf69.c                  | 4 ++--
>  drivers/staging/pi433/rf69.h                  | 2 +-
>  4 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/pi433/Documentation/pi433.txt b/drivers/staging/pi433/Documentation/pi433.txt
> index 4a0d34b4ad37..b34d8e9f6d53 100644
> --- a/drivers/staging/pi433/Documentation/pi433.txt
> +++ b/drivers/staging/pi433/Documentation/pi433.txt
> @@ -78,7 +78,8 @@ rf params:
>  		Allowed values: 433050000...434790000
>  	bit_rate
>  		bit rate used for transmission.
> -		Allowed values: #####
> +		Allowed values when FSK: 1200...300000
> +		Allowed values when OOK: 1200...32768
>  	dev_frequency
>  		frequency deviation in case of FSK.
>  		Allowed values: 600...500000
> diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
> index 25ee0b77a32c..1f8ffaf02d99 100644
> --- a/drivers/staging/pi433/pi433_if.h
> +++ b/drivers/staging/pi433/pi433_if.h
> @@ -51,7 +51,7 @@ enum option_on_off {
>  #define PI433_TX_CFG_IOCTL_NR	0
>  struct pi433_tx_cfg {
>  	__u32			frequency;
> -	__u16			bit_rate;
> +	__u32			bit_rate;
>  	__u32			dev_frequency;
>  	enum modulation		modulation;
>  	enum mod_shaping	mod_shaping;
> diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> index 8c7fab6a46bb..0b90ca004dd6 100644
> --- a/drivers/staging/pi433/rf69.c
> +++ b/drivers/staging/pi433/rf69.c
> @@ -185,7 +185,7 @@ int rf69_set_modulation_shaping(struct spi_device *spi,
>  	}
>  }
>  
> -int rf69_set_bit_rate(struct spi_device *spi, u16 bit_rate)
> +int rf69_set_bit_rate(struct spi_device *spi, u32 bit_rate)
>  {
>  	int retval;
>  	u32 bit_rate_reg;
> @@ -201,7 +201,7 @@ int rf69_set_bit_rate(struct spi_device *spi, u16 bit_rate)
>  	}
>  
>  	// check input value
> -	if (bit_rate < 1200 || (mod == OOK && bit_rate > 32768)) {
> +	if (bit_rate < 1200 || bit_rate > 300000 || (mod == OOK && bit_rate > 32768)) {
>  		dev_dbg(&spi->dev, "setBitRate: illegal input param\n");
>  		return -EINVAL;
>  	}
> diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
> index 78fa0b8bab8b..46a1fb2d5329 100644
> --- a/drivers/staging/pi433/rf69.h
> +++ b/drivers/staging/pi433/rf69.h
> @@ -24,7 +24,7 @@ int rf69_set_data_mode(struct spi_device *spi, u8 data_mode);
>  int rf69_set_modulation(struct spi_device *spi, enum modulation modulation);
>  int rf69_set_modulation_shaping(struct spi_device *spi,
>  				enum mod_shaping mod_shaping);
> -int rf69_set_bit_rate(struct spi_device *spi, u16 bit_rate);
> +int rf69_set_bit_rate(struct spi_device *spi, u32 bit_rate);
>  int rf69_set_deviation(struct spi_device *spi, u32 deviation);
>  int rf69_set_frequency(struct spi_device *spi, u32 frequency);
>  int rf69_enable_amplifier(struct spi_device *spi, u8 amplifier_mask);
> -- 
> 2.34.1
> 
>
  
Dan Carpenter Feb. 14, 2023, 8:25 a.m. UTC | #2
On Tue, Feb 14, 2023 at 05:33:46PM +1100, Jacob Bai wrote:
> Based on the TODO file and datasheet of pi433, the maximum bit rate for
> transmitter is 300kbps when modulation been set to FSK. Hence, the size
> should be u32 rather than u16.
> 
> Signed-off-by: Jacob Bai <jacob.bai.au@gmail.com>
> ---

This breaks the user space API.

The TODO was updated recently to explain how to do this properly.

regards,
dan carpenter
  
Jacob Bai Feb. 14, 2023, 9:24 a.m. UTC | #3
On Tue, Feb 14, 2023 at 01:28:20PM +0530, Deepak R Varma wrote:
> On Tue, Feb 14, 2023 at 05:33:46PM +1100, Jacob Bai wrote:
> > Based on the TODO file and datasheet of pi433, the maximum bit rate for
> > transmitter is 300kbps when modulation been set to FSK. Hence, the size
> > should be u32 rather than u16.
> 
> Hi Jacob,
> This change was already discussed recently. Please check the following link and
> all the responses to this thread:
> 
> https://lore.kernel.org/all/Y9h42l%2F8EcPqn63x@combine-ThinkPad-S1-Yoga/
> 
> Thanks,
> ./drv
Thanks Deepak and Dan!

Regards,
Jacob
> 
> > 
> > Signed-off-by: Jacob Bai <jacob.bai.au@gmail.com>
> > ---
> >  drivers/staging/pi433/Documentation/pi433.txt | 3 ++-
> >  drivers/staging/pi433/pi433_if.h              | 2 +-
> >  drivers/staging/pi433/rf69.c                  | 4 ++--
> >  drivers/staging/pi433/rf69.h                  | 2 +-
> >  4 files changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/staging/pi433/Documentation/pi433.txt b/drivers/staging/pi433/Documentation/pi433.txt
> > index 4a0d34b4ad37..b34d8e9f6d53 100644
> > --- a/drivers/staging/pi433/Documentation/pi433.txt
> > +++ b/drivers/staging/pi433/Documentation/pi433.txt
> > @@ -78,7 +78,8 @@ rf params:
> >  		Allowed values: 433050000...434790000
> >  	bit_rate
> >  		bit rate used for transmission.
> > -		Allowed values: #####
> > +		Allowed values when FSK: 1200...300000
> > +		Allowed values when OOK: 1200...32768
> >  	dev_frequency
> >  		frequency deviation in case of FSK.
> >  		Allowed values: 600...500000
> > diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
> > index 25ee0b77a32c..1f8ffaf02d99 100644
> > --- a/drivers/staging/pi433/pi433_if.h
> > +++ b/drivers/staging/pi433/pi433_if.h
> > @@ -51,7 +51,7 @@ enum option_on_off {
> >  #define PI433_TX_CFG_IOCTL_NR	0
> >  struct pi433_tx_cfg {
> >  	__u32			frequency;
> > -	__u16			bit_rate;
> > +	__u32			bit_rate;
> >  	__u32			dev_frequency;
> >  	enum modulation		modulation;
> >  	enum mod_shaping	mod_shaping;
> > diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> > index 8c7fab6a46bb..0b90ca004dd6 100644
> > --- a/drivers/staging/pi433/rf69.c
> > +++ b/drivers/staging/pi433/rf69.c
> > @@ -185,7 +185,7 @@ int rf69_set_modulation_shaping(struct spi_device *spi,
> >  	}
> >  }
> >  
> > -int rf69_set_bit_rate(struct spi_device *spi, u16 bit_rate)
> > +int rf69_set_bit_rate(struct spi_device *spi, u32 bit_rate)
> >  {
> >  	int retval;
> >  	u32 bit_rate_reg;
> > @@ -201,7 +201,7 @@ int rf69_set_bit_rate(struct spi_device *spi, u16 bit_rate)
> >  	}
> >  
> >  	// check input value
> > -	if (bit_rate < 1200 || (mod == OOK && bit_rate > 32768)) {
> > +	if (bit_rate < 1200 || bit_rate > 300000 || (mod == OOK && bit_rate > 32768)) {
> >  		dev_dbg(&spi->dev, "setBitRate: illegal input param\n");
> >  		return -EINVAL;
> >  	}
> > diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
> > index 78fa0b8bab8b..46a1fb2d5329 100644
> > --- a/drivers/staging/pi433/rf69.h
> > +++ b/drivers/staging/pi433/rf69.h
> > @@ -24,7 +24,7 @@ int rf69_set_data_mode(struct spi_device *spi, u8 data_mode);
> >  int rf69_set_modulation(struct spi_device *spi, enum modulation modulation);
> >  int rf69_set_modulation_shaping(struct spi_device *spi,
> >  				enum mod_shaping mod_shaping);
> > -int rf69_set_bit_rate(struct spi_device *spi, u16 bit_rate);
> > +int rf69_set_bit_rate(struct spi_device *spi, u32 bit_rate);
> >  int rf69_set_deviation(struct spi_device *spi, u32 deviation);
> >  int rf69_set_frequency(struct spi_device *spi, u32 frequency);
> >  int rf69_enable_amplifier(struct spi_device *spi, u8 amplifier_mask);
> > -- 
> > 2.34.1
> > 
> > 
> 
>
  

Patch

diff --git a/drivers/staging/pi433/Documentation/pi433.txt b/drivers/staging/pi433/Documentation/pi433.txt
index 4a0d34b4ad37..b34d8e9f6d53 100644
--- a/drivers/staging/pi433/Documentation/pi433.txt
+++ b/drivers/staging/pi433/Documentation/pi433.txt
@@ -78,7 +78,8 @@  rf params:
 		Allowed values: 433050000...434790000
 	bit_rate
 		bit rate used for transmission.
-		Allowed values: #####
+		Allowed values when FSK: 1200...300000
+		Allowed values when OOK: 1200...32768
 	dev_frequency
 		frequency deviation in case of FSK.
 		Allowed values: 600...500000
diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
index 25ee0b77a32c..1f8ffaf02d99 100644
--- a/drivers/staging/pi433/pi433_if.h
+++ b/drivers/staging/pi433/pi433_if.h
@@ -51,7 +51,7 @@  enum option_on_off {
 #define PI433_TX_CFG_IOCTL_NR	0
 struct pi433_tx_cfg {
 	__u32			frequency;
-	__u16			bit_rate;
+	__u32			bit_rate;
 	__u32			dev_frequency;
 	enum modulation		modulation;
 	enum mod_shaping	mod_shaping;
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index 8c7fab6a46bb..0b90ca004dd6 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -185,7 +185,7 @@  int rf69_set_modulation_shaping(struct spi_device *spi,
 	}
 }
 
-int rf69_set_bit_rate(struct spi_device *spi, u16 bit_rate)
+int rf69_set_bit_rate(struct spi_device *spi, u32 bit_rate)
 {
 	int retval;
 	u32 bit_rate_reg;
@@ -201,7 +201,7 @@  int rf69_set_bit_rate(struct spi_device *spi, u16 bit_rate)
 	}
 
 	// check input value
-	if (bit_rate < 1200 || (mod == OOK && bit_rate > 32768)) {
+	if (bit_rate < 1200 || bit_rate > 300000 || (mod == OOK && bit_rate > 32768)) {
 		dev_dbg(&spi->dev, "setBitRate: illegal input param\n");
 		return -EINVAL;
 	}
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 78fa0b8bab8b..46a1fb2d5329 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -24,7 +24,7 @@  int rf69_set_data_mode(struct spi_device *spi, u8 data_mode);
 int rf69_set_modulation(struct spi_device *spi, enum modulation modulation);
 int rf69_set_modulation_shaping(struct spi_device *spi,
 				enum mod_shaping mod_shaping);
-int rf69_set_bit_rate(struct spi_device *spi, u16 bit_rate);
+int rf69_set_bit_rate(struct spi_device *spi, u32 bit_rate);
 int rf69_set_deviation(struct spi_device *spi, u32 deviation);
 int rf69_set_frequency(struct spi_device *spi, u32 frequency);
 int rf69_enable_amplifier(struct spi_device *spi, u8 amplifier_mask);