[PATCH 0/3] staging: pi433: Fix includes & macros.

[PATCH 0/3] staging: pi433: Fix includes & macros.

[Shahar Avidar]

1. Untangle include hierarchy. 
2. Update #minors the driver can accept.
3. Make use of general macro instead of magic number.

Shahar Avidar (3):
  staging: pi433: Use headers in appropriate files.
  staging: pi433: Reduce N_PI433_MINORS to conform with N_SPI_MINORS.
  staging: pi433: Make use of spi mode macro instead of magic number.

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


base-commit: bfa8f18691ed2e978e4dd51190569c434f93e268
-- 
2.34.2

[PATCH 1/3] staging: pi433: Use headers in appropriate files.

[Shahar Avidar]

Ensure rf69.c directly includes rf69_enum.h.
Move rf69_registers.h from header to the relevant source file.

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

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index b6c4917d515e..a351b7acfcff 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -45,6 +45,7 @@
 
 #include "pi433_if.h"
 #include "rf69.h"
+#include "rf69_registers.h"
 
 #define N_PI433_MINORS		BIT(MINORBITS) /*32*/	/* ... up to 256 */
 #define MAX_MSG_SIZE		900	/* min: FIFO_SIZE! */
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index 5a1c362badb6..bf802f097310 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -11,6 +11,7 @@
 #include <linux/units.h>
 
 #include "rf69.h"
+#include "rf69_enum.h"
 #include "rf69_registers.h"
 
 #define F_OSC (32 * HZ_PER_MHZ)
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 76f0f9896a52..dd6fa8af9b9c 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -9,7 +9,6 @@
 #define RF69_H
 
 #include "rf69_enum.h"
-#include "rf69_registers.h"
 
 #define FIFO_SIZE	66		/* bytes */
 
-- 
2.34.1

[PATCH 2/3] staging: pi433: Reduce N_PI433_MINORS to conform with N_SPI_MINORS.

[Shahar Avidar]

The driver can't actually support 2^20 devices.
Current RasPis GPIO pins can actually support only 1 device.
Other or future platforms might support more.

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

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index a351b7acfcff..9fc93fa454b1 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -47,7 +47,7 @@
 #include "rf69.h"
 #include "rf69_registers.h"
 
-#define N_PI433_MINORS		BIT(MINORBITS) /*32*/	/* ... up to 256 */
+#define N_PI433_MINORS		32
 #define MAX_MSG_SIZE		900	/* min: FIFO_SIZE! */
 #define MSG_FIFO_SIZE		65536   /* 65536 = 2^16  */
 #define FIFO_THRESHOLD	15		/* bytes */
-- 
2.34.1

Re: [PATCH 2/3] staging: pi433: Reduce N_PI433_MINORS to conform with N_SPI_MINORS.

[Dan Carpenter]

On Sun, Mar 24, 2024 at 11:32:00AM +0200, Shahar Avidar wrote:
> The driver can't actually support 2^20 devices.
> Current RasPis GPIO pins can actually support only 1 device.
> Other or future platforms might support more.
> 

Why are you doing this?  Is it just a clean up or does it have some
effect on runtime?  And if it does have an effect on runtime, what does
that look like to the user?  This all needs to be in the commit message.

regards,
dan carpenter

[PATCH 3/3] staging: pi433: Make use of spi mode macro instead of magic number.

[Shahar Avidar]

Use SPI_MODE_0 to setup spi mode.

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

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 9fc93fa454b1..2a22342eda69 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -1164,7 +1164,7 @@ static int pi433_probe(struct spi_device *spi)
 	struct dentry		*entry;
 
 	/* setup spi parameters */
-	spi->mode = 0x00;
+	spi->mode = SPI_MODE_0;
 	spi->bits_per_word = 8;
 	/*
 	 * spi->max_speed_hz = 10000000;
-- 
2.34.1

Re: [PATCH 0/3] staging: pi433: Fix includes & macros.

[Andy Shevchenko]

On Sun, Mar 24, 2024 at 11:31:58AM +0200, Shahar Avidar wrote:
> 1. Untangle include hierarchy. 
> 2. Update #minors the driver can accept.
> 3. Make use of general macro instead of magic number.

> Shahar Avidar (3):
>   staging: pi433: Use headers in appropriate files.
>   staging: pi433: Reduce N_PI433_MINORS to conform with N_SPI_MINORS.
>   staging: pi433: Make use of spi mode macro instead of magic number.

For patches 1 and 3
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/3] staging: pi433: Fix includes & macros.

[Greg KH]

On Mon, Mar 25, 2024 at 11:18:42AM +0200, Andy Shevchenko wrote:
> On Sun, Mar 24, 2024 at 11:31:58AM +0200, Shahar Avidar wrote:
> > 1. Untangle include hierarchy. 
> > 2. Update #minors the driver can accept.
> > 3. Make use of general macro instead of magic number.
> 
> > Shahar Avidar (3):
> >   staging: pi433: Use headers in appropriate files.
> >   staging: pi433: Reduce N_PI433_MINORS to conform with N_SPI_MINORS.
> >   staging: pi433: Make use of spi mode macro instead of magic number.
> 
> For patches 1 and 3
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

That's impossible for b4 to parse, it would have applied this to all of
the commits if I had taken them :(

thanks,

greg k-h

Re: [PATCH 0/3] staging: pi433: Fix includes & macros.

[Andy Shevchenko]

On Mon, Mar 25, 2024 at 07:05:46PM +0100, Greg KH wrote:
> On Mon, Mar 25, 2024 at 11:18:42AM +0200, Andy Shevchenko wrote:
> > On Sun, Mar 24, 2024 at 11:31:58AM +0200, Shahar Avidar wrote:
> > > 1. Untangle include hierarchy. 
> > > 2. Update #minors the driver can accept.
> > > 3. Make use of general macro instead of magic number.
> > 
> > > Shahar Avidar (3):
> > >   staging: pi433: Use headers in appropriate files.
> > >   staging: pi433: Reduce N_PI433_MINORS to conform with N_SPI_MINORS.
> > >   staging: pi433: Make use of spi mode macro instead of magic number.
> > 
> > For patches 1 and 3
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> That's impossible for b4 to parse, it would have applied this to all of
> the commits if I had taken them :(

You can apply only patches 1 and 3 as long as they are independent to patch 2.

	b4 am -st -P 1,3 ...

or we can wait for the author to react on the comments and issue a new version.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/3] staging: pi433: Fix includes & macros.

[Greg KH]

On Mon, Mar 25, 2024 at 08:41:50PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 25, 2024 at 07:05:46PM +0100, Greg KH wrote:
> > On Mon, Mar 25, 2024 at 11:18:42AM +0200, Andy Shevchenko wrote:
> > > On Sun, Mar 24, 2024 at 11:31:58AM +0200, Shahar Avidar wrote:
> > > > 1. Untangle include hierarchy. 
> > > > 2. Update #minors the driver can accept.
> > > > 3. Make use of general macro instead of magic number.
> > > 
> > > > Shahar Avidar (3):
> > > >   staging: pi433: Use headers in appropriate files.
> > > >   staging: pi433: Reduce N_PI433_MINORS to conform with N_SPI_MINORS.
> > > >   staging: pi433: Make use of spi mode macro instead of magic number.
> > > 
> > > For patches 1 and 3
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > That's impossible for b4 to parse, it would have applied this to all of
> > the commits if I had taken them :(
> 
> You can apply only patches 1 and 3 as long as they are independent to patch 2.
> 
> 	b4 am -st -P 1,3 ...

I don't do that, it would not scale at all :)

Re: [PATCH 0/3] staging: pi433: Fix includes & macros.

[Andy Shevchenko]

On Mon, Mar 25, 2024 at 07:51:16PM +0100, Greg KH wrote:
> On Mon, Mar 25, 2024 at 08:41:50PM +0200, Andy Shevchenko wrote:
> > On Mon, Mar 25, 2024 at 07:05:46PM +0100, Greg KH wrote:
> > > On Mon, Mar 25, 2024 at 11:18:42AM +0200, Andy Shevchenko wrote:
> > > > On Sun, Mar 24, 2024 at 11:31:58AM +0200, Shahar Avidar wrote:
> > > > > 1. Untangle include hierarchy. 
> > > > > 2. Update #minors the driver can accept.
> > > > > 3. Make use of general macro instead of magic number.
> > > > 
> > > > > Shahar Avidar (3):
> > > > >   staging: pi433: Use headers in appropriate files.
> > > > >   staging: pi433: Reduce N_PI433_MINORS to conform with N_SPI_MINORS.
> > > > >   staging: pi433: Make use of spi mode macro instead of magic number.
> > > > 
> > > > For patches 1 and 3
> > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > 
> > > That's impossible for b4 to parse, it would have applied this to all of
> > > the commits if I had taken them :(
> > 
> > You can apply only patches 1 and 3 as long as they are independent to patch 2.
> > 
> > 	b4 am -st -P 1,3 ...
> 
> I don't do that, it would not scale at all :)

Yeah, this requires too much maintainer's involvement.

Alternative option, in case you are fine with patch 2, is to drop my tags and
apply.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/3] staging: pi433: Fix includes & macros.

[Greg KH]

On Mon, Mar 25, 2024 at 09:12:16PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 25, 2024 at 07:51:16PM +0100, Greg KH wrote:
> > On Mon, Mar 25, 2024 at 08:41:50PM +0200, Andy Shevchenko wrote:
> > > On Mon, Mar 25, 2024 at 07:05:46PM +0100, Greg KH wrote:
> > > > On Mon, Mar 25, 2024 at 11:18:42AM +0200, Andy Shevchenko wrote:
> > > > > On Sun, Mar 24, 2024 at 11:31:58AM +0200, Shahar Avidar wrote:
> > > > > > 1. Untangle include hierarchy. 
> > > > > > 2. Update #minors the driver can accept.
> > > > > > 3. Make use of general macro instead of magic number.
> > > > > 
> > > > > > Shahar Avidar (3):
> > > > > >   staging: pi433: Use headers in appropriate files.
> > > > > >   staging: pi433: Reduce N_PI433_MINORS to conform with N_SPI_MINORS.
> > > > > >   staging: pi433: Make use of spi mode macro instead of magic number.
> > > > > 
> > > > > For patches 1 and 3
> > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > 
> > > > That's impossible for b4 to parse, it would have applied this to all of
> > > > the commits if I had taken them :(
> > > 
> > > You can apply only patches 1 and 3 as long as they are independent to patch 2.
> > > 
> > > 	b4 am -st -P 1,3 ...
> > 
> > I don't do that, it would not scale at all :)
> 
> Yeah, this requires too much maintainer's involvement.
> 
> Alternative option, in case you are fine with patch 2, is to drop my tags and
> apply.

I wasn't fine with them, I'll wait for a new version of this series to
be posted before looking at them again.

thanks,

greg k-h