linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] staging: pi433: Fix includes & macros.
@ 2024-03-24  9:31 Shahar Avidar
  2024-03-24  9:31 ` [PATCH 1/3] staging: pi433: Use headers in appropriate files Shahar Avidar
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Shahar Avidar @ 2024-03-24  9:31 UTC (permalink / raw)
  To: gregkh, elder, andriy.shevchenko, robh, parthiban.veerasooran
  Cc: linux-staging, linux-kernel

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] staging: pi433: Use headers in appropriate files.
  2024-03-24  9:31 [PATCH 0/3] staging: pi433: Fix includes & macros Shahar Avidar
@ 2024-03-24  9:31 ` Shahar Avidar
  2024-03-24  9:32 ` [PATCH 2/3] staging: pi433: Reduce N_PI433_MINORS to conform with N_SPI_MINORS Shahar Avidar
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Shahar Avidar @ 2024-03-24  9:31 UTC (permalink / raw)
  To: gregkh, elder, andriy.shevchenko, robh, parthiban.veerasooran
  Cc: linux-staging, linux-kernel

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] staging: pi433: Reduce N_PI433_MINORS to conform with N_SPI_MINORS.
  2024-03-24  9:31 [PATCH 0/3] staging: pi433: Fix includes & macros Shahar Avidar
  2024-03-24  9:31 ` [PATCH 1/3] staging: pi433: Use headers in appropriate files Shahar Avidar
@ 2024-03-24  9:32 ` Shahar Avidar
  2024-03-25  5:52   ` Dan Carpenter
  2024-03-24  9:32 ` [PATCH 3/3] staging: pi433: Make use of spi mode macro instead of magic number Shahar Avidar
  2024-03-25  9:18 ` [PATCH 0/3] staging: pi433: Fix includes & macros Andy Shevchenko
  3 siblings, 1 reply; 11+ messages in thread
From: Shahar Avidar @ 2024-03-24  9:32 UTC (permalink / raw)
  To: gregkh, elder, andriy.shevchenko, robh, parthiban.veerasooran
  Cc: linux-staging, linux-kernel

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] staging: pi433: Make use of spi mode macro instead of magic number.
  2024-03-24  9:31 [PATCH 0/3] staging: pi433: Fix includes & macros Shahar Avidar
  2024-03-24  9:31 ` [PATCH 1/3] staging: pi433: Use headers in appropriate files Shahar Avidar
  2024-03-24  9:32 ` [PATCH 2/3] staging: pi433: Reduce N_PI433_MINORS to conform with N_SPI_MINORS Shahar Avidar
@ 2024-03-24  9:32 ` Shahar Avidar
  2024-03-25  9:18 ` [PATCH 0/3] staging: pi433: Fix includes & macros Andy Shevchenko
  3 siblings, 0 replies; 11+ messages in thread
From: Shahar Avidar @ 2024-03-24  9:32 UTC (permalink / raw)
  To: gregkh, elder, andriy.shevchenko, robh, parthiban.veerasooran
  Cc: linux-staging, linux-kernel

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] staging: pi433: Reduce N_PI433_MINORS to conform with N_SPI_MINORS.
  2024-03-24  9:32 ` [PATCH 2/3] staging: pi433: Reduce N_PI433_MINORS to conform with N_SPI_MINORS Shahar Avidar
@ 2024-03-25  5:52   ` Dan Carpenter
  0 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2024-03-25  5:52 UTC (permalink / raw)
  To: Shahar Avidar
  Cc: gregkh, elder, andriy.shevchenko, robh, parthiban.veerasooran,
	linux-staging, linux-kernel

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] staging: pi433: Fix includes & macros.
  2024-03-24  9:31 [PATCH 0/3] staging: pi433: Fix includes & macros Shahar Avidar
                   ` (2 preceding siblings ...)
  2024-03-24  9:32 ` [PATCH 3/3] staging: pi433: Make use of spi mode macro instead of magic number Shahar Avidar
@ 2024-03-25  9:18 ` Andy Shevchenko
  2024-03-25 18:05   ` Greg KH
  3 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2024-03-25  9:18 UTC (permalink / raw)
  To: Shahar Avidar
  Cc: gregkh, elder, robh, parthiban.veerasooran, linux-staging, linux-kernel

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] staging: pi433: Fix includes & macros.
  2024-03-25  9:18 ` [PATCH 0/3] staging: pi433: Fix includes & macros Andy Shevchenko
@ 2024-03-25 18:05   ` Greg KH
  2024-03-25 18:41     ` Andy Shevchenko
  0 siblings, 1 reply; 11+ messages in thread
From: Greg KH @ 2024-03-25 18:05 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Shahar Avidar, elder, robh, parthiban.veerasooran, linux-staging,
	linux-kernel

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] staging: pi433: Fix includes & macros.
  2024-03-25 18:05   ` Greg KH
@ 2024-03-25 18:41     ` Andy Shevchenko
  2024-03-25 18:51       ` Greg KH
  0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2024-03-25 18:41 UTC (permalink / raw)
  To: Greg KH
  Cc: Shahar Avidar, elder, robh, parthiban.veerasooran, linux-staging,
	linux-kernel

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] staging: pi433: Fix includes & macros.
  2024-03-25 18:41     ` Andy Shevchenko
@ 2024-03-25 18:51       ` Greg KH
  2024-03-25 19:12         ` Andy Shevchenko
  0 siblings, 1 reply; 11+ messages in thread
From: Greg KH @ 2024-03-25 18:51 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Shahar Avidar, elder, robh, parthiban.veerasooran, linux-staging,
	linux-kernel

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 :)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] staging: pi433: Fix includes & macros.
  2024-03-25 18:51       ` Greg KH
@ 2024-03-25 19:12         ` Andy Shevchenko
  2024-03-26  6:01           ` Greg KH
  0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2024-03-25 19:12 UTC (permalink / raw)
  To: Greg KH
  Cc: Shahar Avidar, elder, robh, parthiban.veerasooran, linux-staging,
	linux-kernel

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] staging: pi433: Fix includes & macros.
  2024-03-25 19:12         ` Andy Shevchenko
@ 2024-03-26  6:01           ` Greg KH
  0 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2024-03-26  6:01 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Shahar Avidar, elder, robh, parthiban.veerasooran, linux-staging,
	linux-kernel

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-03-26  6:01 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-24  9:31 [PATCH 0/3] staging: pi433: Fix includes & macros Shahar Avidar
2024-03-24  9:31 ` [PATCH 1/3] staging: pi433: Use headers in appropriate files Shahar Avidar
2024-03-24  9:32 ` [PATCH 2/3] staging: pi433: Reduce N_PI433_MINORS to conform with N_SPI_MINORS Shahar Avidar
2024-03-25  5:52   ` Dan Carpenter
2024-03-24  9:32 ` [PATCH 3/3] staging: pi433: Make use of spi mode macro instead of magic number Shahar Avidar
2024-03-25  9:18 ` [PATCH 0/3] staging: pi433: Fix includes & macros Andy Shevchenko
2024-03-25 18:05   ` Greg KH
2024-03-25 18:41     ` Andy Shevchenko
2024-03-25 18:51       ` Greg KH
2024-03-25 19:12         ` Andy Shevchenko
2024-03-26  6:01           ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).