[PATCH 0/4] Fix defines in rf69.h

[PATCH 0/4] Fix defines in rf69.h

[Shahar Avidar]

This patchset fixes several misuses of the define statement in rf69.h
Duplicted defines.
Define in header instead of source file.
Unused define.

Shahar Avidar (4):
  staging: pi433: Remove a duplicated F_OSC define
  staging: pi433: Remove the unused FREQUENCY define
  staging: pi433: Remove a duplicated FIFO_SIZE define
  staging: pi433: Move FIFO_THRESHOLD define to source file

 drivers/staging/pi433/pi433_if.c | 1 +
 drivers/staging/pi433/rf69.c     | 3 +--
 drivers/staging/pi433/rf69.h     | 4 ----
 3 files changed, 2 insertions(+), 6 deletions(-)


base-commit: 455c5e12a3b7d08c2ab47b7dd54944901c69cdcd
-- 
2.34.1

[PATCH 1/4] staging: pi433: Remove a duplicated F_OSC define

[Shahar Avidar]

F_OSC is already defined & only used by rf69.c source file
Also fix define comment

Signed-off-by: Shahar Avidar <ikobh7@gmail.com>
---
 drivers/staging/pi433/rf69.c | 2 +-
 drivers/staging/pi433/rf69.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index 8c7fab6a46bb..d7e2dbe70d7c 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -12,7 +12,7 @@
 #include "rf69.h"
 #include "rf69_registers.h"
 
-#define F_OSC	  32000000 /* in Hz */
+#define F_OSC	  32000000 /* Hz */
 #define FIFO_SIZE 66	   /* in byte */
 
 /*-------------------------------------------------------------------------*/
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 78fa0b8bab8b..52e43a909b03 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -12,7 +12,6 @@
 #include "rf69_registers.h"
 
 /* NOTE: Modifying FREQUENCY value impacts CE certification */
-#define F_OSC		32000000	/* Hz */
 #define FREQUENCY	433920000	/* Hz */
 #define FIFO_SIZE	66		/* bytes */
 #define FIFO_THRESHOLD	15		/* bytes */
-- 
2.34.1

[PATCH 2/4] staging: pi433: Remove the unused FREQUENCY define

[Shahar Avidar]

FREQUENCY is not being used, delete its comment

Signed-off-by: Shahar Avidar <ikobh7@gmail.com>
---
 drivers/staging/pi433/rf69.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 52e43a909b03..e63e87fd6cce 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -11,8 +11,6 @@
 #include "rf69_enum.h"
 #include "rf69_registers.h"
 
-/* NOTE: Modifying FREQUENCY value impacts CE certification */
-#define FREQUENCY	433920000	/* Hz */
 #define FIFO_SIZE	66		/* bytes */
 #define FIFO_THRESHOLD	15		/* bytes */
 
-- 
2.34.1

[PATCH 3/4] staging: pi433: Remove a duplicated FIFO_SIZE define

[Shahar Avidar]

FIFO_SIZE is being used in both rf69.c & pi433_if.c source files
It is also already defined in rf69.h header file

Signed-off-by: Shahar Avidar <ikobh7@gmail.com>
---
 drivers/staging/pi433/rf69.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index d7e2dbe70d7c..4bfb1a98328f 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -13,7 +13,6 @@
 #include "rf69_registers.h"
 
 #define F_OSC	  32000000 /* Hz */
-#define FIFO_SIZE 66	   /* in byte */
 
 /*-------------------------------------------------------------------------*/
 
-- 
2.34.1

[PATCH 4/4] staging: pi433: Move FIFO_THRESHOLD define to source file

[Shahar Avidar]

FIFO_THRESHOLD is only being used by pi433_if.c source file

Signed-off-by: Shahar Avidar <ikobh7@gmail.com>
---
 drivers/staging/pi433/pi433_if.c | 1 +
 drivers/staging/pi433/rf69.h     | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 0ec3130225db..b6c4917d515e 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -49,6 +49,7 @@
 #define N_PI433_MINORS		BIT(MINORBITS) /*32*/	/* ... up to 256 */
 #define MAX_MSG_SIZE		900	/* min: FIFO_SIZE! */
 #define MSG_FIFO_SIZE		65536   /* 65536 = 2^16  */
+#define FIFO_THRESHOLD	15		/* bytes */
 #define NUM_DIO			2
 
 static dev_t pi433_dev;
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index e63e87fd6cce..76f0f9896a52 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -12,7 +12,6 @@
 #include "rf69_registers.h"
 
 #define FIFO_SIZE	66		/* bytes */
-#define FIFO_THRESHOLD	15		/* bytes */
 
 u8 rf69_read_reg(struct spi_device *spi, u8 addr);
 int rf69_get_version(struct spi_device *spi);
-- 
2.34.1

Re: [PATCH 1/4] staging: pi433: Remove a duplicated F_OSC define

[Andy Shevchenko]

On Sun, Feb 25, 2024 at 07:33:38PM +0200, Shahar Avidar wrote:
> F_OSC is already defined & only used by rf69.c source file
> Also fix define comment

You missed periods at the end of the sentences.

...

> -#define F_OSC	  32000000 /* in Hz */
> +#define F_OSC	  32000000 /* Hz */

Instead of having a comment you can

  #include <linux/units.h>
  ...
  #define F_OSC	  (32 * HZ_PER_MHZ)

which will be more robust code (no need to count 0s).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/4] staging: pi433: Remove a duplicated FIFO_SIZE define

[Andy Shevchenko]

On Sun, Feb 25, 2024 at 07:33:40PM +0200, Shahar Avidar wrote:
> FIFO_SIZE is being used in both rf69.c & pi433_if.c source files
> It is also already defined in rf69.h header file

You can done this before patch 1, so the code will be cleaner already.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/4] staging: pi433: Move FIFO_THRESHOLD define to source file

[Andy Shevchenko]

On Sun, Feb 25, 2024 at 07:33:41PM +0200, Shahar Avidar wrote:
> FIFO_THRESHOLD is only being used by pi433_if.c source file

Same issue (and seems in all of your commit messages) — missing periods
at the end of the sentences. Respect English grammar, please.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/4] staging: pi433: Remove a duplicated F_OSC define

[Shahar Avidar]

On 26/02/2024 16:45, Andy Shevchenko wrote:
> On Sun, Feb 25, 2024 at 07:33:38PM +0200, Shahar Avidar wrote:
>> F_OSC is already defined & only used by rf69.c source file
>> Also fix define comment
> 
> You missed periods at the end of the sentences.

Thank you for noticing.

>> -#define F_OSC	  32000000 /* in Hz */
>> +#define F_OSC	  32000000 /* Hz */
> 
> Instead of having a comment you can
> 
>    #include <linux/units.h>
>    ...
>    #define F_OSC	  (32 * HZ_PER_MHZ)
> 
> which will be more robust code (no need to count 0s).
> 

All comments are fixed in v2 which was just sent.
Please note I sent the updated patchset twice, I forgot to add v2 the 
first time...

-- 
Regards,

Shahar