diff mbox series

[v4,1/4] leds: simatic-ipc-leds-gpio: add terminating entries to gpio tables

Message ID 20230524124628.32295-2-henning.schild@siemens.com
State New
Headers show
Series leds: simatic-ipc-leds-gpio: split up | expand

Commit Message

Henning Schild May 24, 2023, 12:46 p.m. UTC
The entries do not seem to be stricly needed when the number of entries
is given via the number of LEDs. But adding them is a safeguard should
anyone ever iterate over the tables to their end, it also gets us in
line with other drivers that register "leds-gpio" tables.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 drivers/leds/simple/simatic-ipc-leds-gpio.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Andy Shevchenko May 27, 2023, 8:54 a.m. UTC | #1
On Wed, May 24, 2023 at 02:46:25PM +0200, Henning Schild wrote:
> The entries do not seem to be stricly needed when the number of entries
> is given via the number of LEDs. But adding them is a safeguard should
> anyone ever iterate over the tables to their end, it also gets us in
> line with other drivers that register "leds-gpio" tables.

Reported-by?
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  drivers/leds/simple/simatic-ipc-leds-gpio.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio.c b/drivers/leds/simple/simatic-ipc-leds-gpio.c
> index e8d329b5a68c..1a1cfdad6218 100644
> --- a/drivers/leds/simple/simatic-ipc-leds-gpio.c
> +++ b/drivers/leds/simple/simatic-ipc-leds-gpio.c
> @@ -28,6 +28,7 @@ static struct gpiod_lookup_table simatic_ipc_led_gpio_table_127e = {
>  		GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 51, NULL, 5, GPIO_ACTIVE_LOW),
>  		GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 56, NULL, 6, GPIO_ACTIVE_LOW),
>  		GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 59, NULL, 7, GPIO_ACTIVE_HIGH),
> +		{} /* Terminating entry */
>  	},
>  };
>  
> @@ -42,6 +43,7 @@ static struct gpiod_lookup_table simatic_ipc_led_gpio_table_227g = {
>  		GPIO_LOOKUP_IDX("gpio-f7188x-2", 5, NULL, 5, GPIO_ACTIVE_LOW),
>  		GPIO_LOOKUP_IDX("gpio-f7188x-3", 6, NULL, 6, GPIO_ACTIVE_HIGH),
>  		GPIO_LOOKUP_IDX("gpio-f7188x-3", 7, NULL, 7, GPIO_ACTIVE_HIGH),
> +		{} /* Terminating entry */
>  	}
>  };
>  
> -- 
> 2.39.3
>
Henning Schild May 30, 2023, 3:11 p.m. UTC | #2
Am Sat, 27 May 2023 11:54:08 +0300
schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:

> On Wed, May 24, 2023 at 02:46:25PM +0200, Henning Schild wrote:
> > The entries do not seem to be stricly needed when the number of
> > entries is given via the number of LEDs. But adding them is a
> > safeguard should anyone ever iterate over the tables to their end,
> > it also gets us in line with other drivers that register
> > "leds-gpio" tables.  
> 
> Reported-by?
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

I think we could do

Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

on merge. But i would not want to send the whole series again for that
one line.

Thanks!
Henning

> > Signed-off-by: Henning Schild <henning.schild@siemens.com>
> > ---
> >  drivers/leds/simple/simatic-ipc-leds-gpio.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio.c
> > b/drivers/leds/simple/simatic-ipc-leds-gpio.c index
> > e8d329b5a68c..1a1cfdad6218 100644 ---
> > a/drivers/leds/simple/simatic-ipc-leds-gpio.c +++
> > b/drivers/leds/simple/simatic-ipc-leds-gpio.c @@ -28,6 +28,7 @@
> > static struct gpiod_lookup_table simatic_ipc_led_gpio_table_127e =
> > { GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 51, NULL, 5,
> > GPIO_ACTIVE_LOW), GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 56, NULL,
> > 6, GPIO_ACTIVE_LOW), GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 59,
> > NULL, 7, GPIO_ACTIVE_HIGH),
> > +		{} /* Terminating entry */
> >  	},
> >  };
> >  
> > @@ -42,6 +43,7 @@ static struct gpiod_lookup_table
> > simatic_ipc_led_gpio_table_227g = {
> > GPIO_LOOKUP_IDX("gpio-f7188x-2", 5, NULL, 5, GPIO_ACTIVE_LOW),
> > GPIO_LOOKUP_IDX("gpio-f7188x-3", 6, NULL, 6, GPIO_ACTIVE_HIGH),
> > GPIO_LOOKUP_IDX("gpio-f7188x-3", 7, NULL, 7, GPIO_ACTIVE_HIGH),
> > +		{} /* Terminating entry */
> >  	}
> >  };
> >  
> > -- 
> > 2.39.3
> >   
>
Andy Shevchenko June 1, 2023, 4:47 p.m. UTC | #3
On Tue, May 30, 2023 at 05:11:00PM +0200, Henning Schild wrote:
> Am Sat, 27 May 2023 11:54:08 +0300
> schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> 
> > On Wed, May 24, 2023 at 02:46:25PM +0200, Henning Schild wrote:
> > > The entries do not seem to be stricly needed when the number of
> > > entries is given via the number of LEDs. But adding them is a
> > > safeguard should anyone ever iterate over the tables to their end,
> > > it also gets us in line with other drivers that register
> > > "leds-gpio" tables.  
> > 
> > Reported-by?
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> I think we could do
> 
> Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> on merge. But i would not want to send the whole series again for that
> one line.

Since you added it, `b4` will happily take it, I believe no manual work even
needed for that, thank you!
Lee Jones June 8, 2023, 5:25 p.m. UTC | #4
On Wed, 24 May 2023, Henning Schild wrote:

> The entries do not seem to be stricly needed when the number of entries
> is given via the number of LEDs. But adding them is a safeguard should
> anyone ever iterate over the tables to their end, it also gets us in
> line with other drivers that register "leds-gpio" tables.
> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  drivers/leds/simple/simatic-ipc-leds-gpio.c | 2 ++
>  1 file changed, 2 insertions(+)

Applied, thanks
Lee Jones June 8, 2023, 5:30 p.m. UTC | #5
On Thu, 01 Jun 2023, Andy Shevchenko wrote:

> On Tue, May 30, 2023 at 05:11:00PM +0200, Henning Schild wrote:
> > Am Sat, 27 May 2023 11:54:08 +0300
> > schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> > 
> > > On Wed, May 24, 2023 at 02:46:25PM +0200, Henning Schild wrote:
> > > > The entries do not seem to be stricly needed when the number of
> > > > entries is given via the number of LEDs. But adding them is a
> > > > safeguard should anyone ever iterate over the tables to their end,
> > > > it also gets us in line with other drivers that register
> > > > "leds-gpio" tables.  
> > > 
> > > Reported-by?
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > I think we could do
> > 
> > Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > on merge. But i would not want to send the whole series again for that
> > one line.
> 
> Since you added it, `b4` will happily take it, I believe no manual work even
> needed for that, thank you!

b4 didn't pick this up.  Nor the whole-series Ack provided by Hans.

I added both manually.
Andy Shevchenko June 9, 2023, 2:25 p.m. UTC | #6
On Thu, Jun 08, 2023 at 06:30:27PM +0100, Lee Jones wrote:
> On Thu, 01 Jun 2023, Andy Shevchenko wrote:
> > On Tue, May 30, 2023 at 05:11:00PM +0200, Henning Schild wrote:
> > > Am Sat, 27 May 2023 11:54:08 +0300
> > > schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> > > 
> > > > On Wed, May 24, 2023 at 02:46:25PM +0200, Henning Schild wrote:
> > > > > The entries do not seem to be stricly needed when the number of
> > > > > entries is given via the number of LEDs. But adding them is a
> > > > > safeguard should anyone ever iterate over the tables to their end,
> > > > > it also gets us in line with other drivers that register
> > > > > "leds-gpio" tables.  
> > > > 
> > > > Reported-by?
> > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > 
> > > I think we could do
> > > 
> > > Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > 
> > > on merge. But i would not want to send the whole series again for that
> > > one line.
> > 
> > Since you added it, `b4` will happily take it, I believe no manual work even
> > needed for that, thank you!
> 
> b4 didn't pick this up.  Nor the whole-series Ack provided by Hans.
> 
> I added both manually.

There is an option to take this

  -t, --apply-cover-trailers
                        Apply trailers sent to the cover letter to all patches

have you tried it?
Lee Jones June 12, 2023, 11:27 a.m. UTC | #7
On Fri, 09 Jun 2023, Andy Shevchenko wrote:

> On Thu, Jun 08, 2023 at 06:30:27PM +0100, Lee Jones wrote:
> > On Thu, 01 Jun 2023, Andy Shevchenko wrote:
> > > On Tue, May 30, 2023 at 05:11:00PM +0200, Henning Schild wrote:
> > > > Am Sat, 27 May 2023 11:54:08 +0300
> > > > schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> > > > 
> > > > > On Wed, May 24, 2023 at 02:46:25PM +0200, Henning Schild wrote:
> > > > > > The entries do not seem to be stricly needed when the number of
> > > > > > entries is given via the number of LEDs. But adding them is a
> > > > > > safeguard should anyone ever iterate over the tables to their end,
> > > > > > it also gets us in line with other drivers that register
> > > > > > "leds-gpio" tables.  
> > > > > 
> > > > > Reported-by?
> > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > 
> > > > I think we could do
> > > > 
> > > > Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > 
> > > > on merge. But i would not want to send the whole series again for that
> > > > one line.
> > > 
> > > Since you added it, `b4` will happily take it, I believe no manual work even
> > > needed for that, thank you!
> > 
> > b4 didn't pick this up.  Nor the whole-series Ack provided by Hans.
> > 
> > I added both manually.
> 
> There is an option to take this
> 
>   -t, --apply-cover-trailers
>                         Apply trailers sent to the cover letter to all patches
> 
> have you tried it?

Doesn't look like it:

  b4 am -3 -slt -P_ -o - ${id} | git am -3 --reject

Can't remember if I had it before then removed it, or never had it.

I'll attempt to add it now and see what happens.
Lee Jones June 12, 2023, 11:29 a.m. UTC | #8
On Mon, 12 Jun 2023, Lee Jones wrote:

> On Fri, 09 Jun 2023, Andy Shevchenko wrote:
> 
> > On Thu, Jun 08, 2023 at 06:30:27PM +0100, Lee Jones wrote:
> > > On Thu, 01 Jun 2023, Andy Shevchenko wrote:
> > > > On Tue, May 30, 2023 at 05:11:00PM +0200, Henning Schild wrote:
> > > > > Am Sat, 27 May 2023 11:54:08 +0300
> > > > > schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> > > > > 
> > > > > > On Wed, May 24, 2023 at 02:46:25PM +0200, Henning Schild wrote:
> > > > > > > The entries do not seem to be stricly needed when the number of
> > > > > > > entries is given via the number of LEDs. But adding them is a
> > > > > > > safeguard should anyone ever iterate over the tables to their end,
> > > > > > > it also gets us in line with other drivers that register
> > > > > > > "leds-gpio" tables.  
> > > > > > 
> > > > > > Reported-by?
> > > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > > 
> > > > > I think we could do
> > > > > 
> > > > > Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > > 
> > > > > on merge. But i would not want to send the whole series again for that
> > > > > one line.
> > > > 
> > > > Since you added it, `b4` will happily take it, I believe no manual work even
> > > > needed for that, thank you!
> > > 
> > > b4 didn't pick this up.  Nor the whole-series Ack provided by Hans.
> > > 
> > > I added both manually.
> > 
> > There is an option to take this
> > 
> >   -t, --apply-cover-trailers
> >                         Apply trailers sent to the cover letter to all patches
> > 
> > have you tried it?
> 
> Doesn't look like it:
> 
>   b4 am -3 -slt -P_ -o - ${id} | git am -3 --reject

No wait - it's there.

> Can't remember if I had it before then removed it, or never had it.
> 
> I'll attempt to add it now and see what happens.
diff mbox series

Patch

diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio.c b/drivers/leds/simple/simatic-ipc-leds-gpio.c
index e8d329b5a68c..1a1cfdad6218 100644
--- a/drivers/leds/simple/simatic-ipc-leds-gpio.c
+++ b/drivers/leds/simple/simatic-ipc-leds-gpio.c
@@ -28,6 +28,7 @@  static struct gpiod_lookup_table simatic_ipc_led_gpio_table_127e = {
 		GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 51, NULL, 5, GPIO_ACTIVE_LOW),
 		GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 56, NULL, 6, GPIO_ACTIVE_LOW),
 		GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 59, NULL, 7, GPIO_ACTIVE_HIGH),
+		{} /* Terminating entry */
 	},
 };
 
@@ -42,6 +43,7 @@  static struct gpiod_lookup_table simatic_ipc_led_gpio_table_227g = {
 		GPIO_LOOKUP_IDX("gpio-f7188x-2", 5, NULL, 5, GPIO_ACTIVE_LOW),
 		GPIO_LOOKUP_IDX("gpio-f7188x-3", 6, NULL, 6, GPIO_ACTIVE_HIGH),
 		GPIO_LOOKUP_IDX("gpio-f7188x-3", 7, NULL, 7, GPIO_ACTIVE_HIGH),
+		{} /* Terminating entry */
 	}
 };