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
Ensure rf69.c directly includes rf69_enum.h.
Move rf69_registers.h from header to the relevant source file.
Signed-off-by: Shahar Avidar <[email protected]>
---
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
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 <[email protected]>
---
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
Use SPI_MODE_0 to setup spi mode.
Signed-off-by: Shahar Avidar <[email protected]>
---
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
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
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 <[email protected]>
--
With Best Regards,
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 <[email protected]>
>
> 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
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 <[email protected]>
> >
> > 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 :)
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 <[email protected]>
> > >
> > > 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
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 <[email protected]>
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
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 <[email protected]>
> > > >
> > > > 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