[PATCH 1/1] leds: lgm: Improve Kconfig help

[PATCH 1/1] leds: lgm: Improve Kconfig help

[Rahul Tanwar]

Remove unnecessary Kconfig symbol LEDS_BLINK
Improve Kconfig help text to make it more useful.

Signed-off-by: Rahul Tanwar <rtanwar@maxlinear.com>
---
  drivers/leds/Kconfig              |  5 ++---
  drivers/leds/Makefile             |  2 +-
  drivers/leds/blink/Kconfig        | 28 +++++++++++++---------------
  drivers/leds/blink/Makefile       |  2 +-
  drivers/leds/blink/leds-lgm-sso.c |  4 ++--
  5 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index b6742b4231bf..4ca8cd594518 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -928,13 +928,12 @@ config LEDS_ACER_A500
           This option enables support for the Power Button LED of
           Acer Iconia Tab A500.

+source "drivers/leds/blink/Kconfig"
+
  comment "Flash and Torch LED drivers"
  source "drivers/leds/flash/Kconfig"

  comment "LED Triggers"
  source "drivers/leds/trigger/Kconfig"

-comment "LED Blink"
-source "drivers/leds/blink/Kconfig"
-
  endif # NEW_LEDS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 2a698df9da57..7e604d3028c8 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -110,4 +110,4 @@ obj-$(CONFIG_LEDS_CLASS_FLASH)              += flash/
  obj-$(CONFIG_LEDS_TRIGGERS)            += trigger/

  # LED Blink
-obj-$(CONFIG_LEDS_BLINK)                += blink/
+obj-y                                  += blink/
diff --git a/drivers/leds/blink/Kconfig b/drivers/leds/blink/Kconfig
index 6dedc58c47b3..2de2973fbc6b 100644
--- a/drivers/leds/blink/Kconfig
+++ b/drivers/leds/blink/Kconfig
@@ -1,21 +1,19 @@
-menuconfig LEDS_BLINK
-       bool "LED Blink support"
-       depends on LEDS_CLASS
-       help
-         This option enables blink support for the leds class.
-         If unsure, say Y.
-
-if LEDS_BLINK
-
-config LEDS_BLINK_LGM
-       tristate "LED support for Intel LGM SoC series"
+config LEDS_LGM
+       tristate "LED support for LGM SoC series"
         depends on GPIOLIB
         depends on LEDS_CLASS
         depends on MFD_SYSCON
         depends on OF
         help
-         Parallel to serial conversion, which is also called SSO 
controller,
-         can drive external shift register for LED outputs.
-         This enables LED support for Serial Shift Output controller(SSO).
+         This option enables support for LEDs connected to GPIO lines on
+         Lightning Mountain(LGM) SoC. These LEDs are driven by a Serial
+         Shift Output(SSO) controller. The driver supports hardware
+         blinking with a configurable LED update/blink frequency in two
+         modes, 2/4/8/10 Hz in low speed mode and 50/100/200/250 KHz in
+         high speed mode. The LEDs can be configured to be triggered by
+         SW/CPU or by hardware. Say 'Y' here if you are working on LGM
+         SoC based platform.
+
+         To compile this driver as a module, choose M here: the
+         module will be called leds-lgm-sso.

-endif # LEDS_BLINK
diff --git a/drivers/leds/blink/Makefile b/drivers/leds/blink/Makefile
index 2fa6c7b7b67e..fa5d04dccf13 100644
--- a/drivers/leds/blink/Makefile
+++ b/drivers/leds/blink/Makefile
@@ -1,2 +1,2 @@
  # SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_LEDS_BLINK_LGM)   += leds-lgm-sso.o
+obj-$(CONFIG_LEDS_LGM) += leds-lgm-sso.o
diff --git a/drivers/leds/blink/leds-lgm-sso.c 
b/drivers/leds/blink/leds-lgm-sso.c
index 7d5c9ca007d6..e41143a87e20 100644
--- a/drivers/leds/blink/leds-lgm-sso.c
+++ b/drivers/leds/blink/leds-lgm-sso.c
@@ -1,8 +1,8 @@
  // SPDX-License-Identifier: GPL-2.0
  /*
- * Intel Lightning Mountain SoC LED Serial Shift Output Controller driver
+ * Lightning Mountain SoC LED Serial Shift Output Controller driver
   *
- * Copyright (c) 2020 Intel Corporation.
+ * Copyright (c) 2021 MaxLinear, Inc.
   */

  #include <linux/bitfield.h>
--
2.17.1

Re: [PATCH 1/1] leds: lgm: Improve Kconfig help

[Randy Dunlap]

Hi,

For the leds/blink/Kconfig file at least, something has
changed all of the tabs to spaces.

Keywords in Kconfig files should be indented with one tab,
while help text should be indented with one tab + 2 spaces.


On 3/17/21 3:04 AM, Rahul Tanwar wrote:
> Remove unnecessary Kconfig symbol LEDS_BLINK
> Improve Kconfig help text to make it more useful.
> 
> Signed-off-by: Rahul Tanwar <rtanwar@maxlinear.com>
> ---
> 
> diff --git a/drivers/leds/blink/Kconfig b/drivers/leds/blink/Kconfig
> index 6dedc58c47b3..2de2973fbc6b 100644
> --- a/drivers/leds/blink/Kconfig
> +++ b/drivers/leds/blink/Kconfig
> @@ -1,21 +1,19 @@
> -menuconfig LEDS_BLINK
> -       bool "LED Blink support"
> -       depends on LEDS_CLASS
> -       help
> -         This option enables blink support for the leds class.
> -         If unsure, say Y.
> -
> -if LEDS_BLINK
> -
> -config LEDS_BLINK_LGM
> -       tristate "LED support for Intel LGM SoC series"
> +config LEDS_LGM
> +       tristate "LED support for LGM SoC series"
>          depends on GPIOLIB
>          depends on LEDS_CLASS
>          depends on MFD_SYSCON
>          depends on OF
>          help
> -         Parallel to serial conversion, which is also called SSO 
> controller,
> -         can drive external shift register for LED outputs.
> -         This enables LED support for Serial Shift Output controller(SSO).
> +         This option enables support for LEDs connected to GPIO lines on
> +         Lightning Mountain(LGM) SoC. These LEDs are driven by a Serial

                      Mountain (LGM)

> +         Shift Output(SSO) controller. The driver supports hardware

                  Output (SSO)

> +         blinking with a configurable LED update/blink frequency in two
> +         modes, 2/4/8/10 Hz in low speed mode and 50/100/200/250 KHz in
> +         high speed mode. The LEDs can be configured to be triggered by
> +         SW/CPU or by hardware. Say 'Y' here if you are working on LGM

Please spell out "software".

> +         SoC based platform.
> +
> +         To compile this driver as a module, choose M here: the
> +         module will be called leds-lgm-sso.
> 
> -endif # LEDS_BLINK

thanks.
-- 
~Randy

Re: [PATCH 1/1] leds: lgm: Improve Kconfig help

[Rahul Tanwar]

Hi Randy,

On 18/3/2021 11:02 am, Randy Dunlap wrote:
> This email was sent from outside of MaxLinear.
> 
> 
> Hi,
> 
> For the leds/blink/Kconfig file at least, something has
> changed all of the tabs to spaces.
> 
> Keywords in Kconfig files should be indented with one tab,
> while help text should be indented with one tab + 2 spaces.
>


Hmm, facing some IT issues with git send-email so i had to send it by 
other means. I will fix it in V1 by ensuring that i send using git.


> 
> On 3/17/21 3:04 AM, Rahul Tanwar wrote:
>> Remove unnecessary Kconfig symbol LEDS_BLINK
>> Improve Kconfig help text to make it more useful.
>>
>> Signed-off-by: Rahul Tanwar <rtanwar@maxlinear.com>
>> ---
>>
>> diff --git a/drivers/leds/blink/Kconfig b/drivers/leds/blink/Kconfig
>> index 6dedc58c47b3..2de2973fbc6b 100644
>> --- a/drivers/leds/blink/Kconfig
>> +++ b/drivers/leds/blink/Kconfig
>> @@ -1,21 +1,19 @@
>> -menuconfig LEDS_BLINK
>> -       bool "LED Blink support"
>> -       depends on LEDS_CLASS
>> -       help
>> -         This option enables blink support for the leds class.
>> -         If unsure, say Y.
>> -
>> -if LEDS_BLINK
>> -
>> -config LEDS_BLINK_LGM
>> -       tristate "LED support for Intel LGM SoC series"
>> +config LEDS_LGM
>> +       tristate "LED support for LGM SoC series"
>>           depends on GPIOLIB
>>           depends on LEDS_CLASS
>>           depends on MFD_SYSCON
>>           depends on OF
>>           help
>> -         Parallel to serial conversion, which is also called SSO
>> controller,
>> -         can drive external shift register for LED outputs.
>> -         This enables LED support for Serial Shift Output controller(SSO).
>> +         This option enables support for LEDs connected to GPIO lines on
>> +         Lightning Mountain(LGM) SoC. These LEDs are driven by a Serial
> 
>                        Mountain (LGM)
> 
>> +         Shift Output(SSO) controller. The driver supports hardware
> 
>                    Output (SSO)
> 
>> +         blinking with a configurable LED update/blink frequency in two
>> +         modes, 2/4/8/10 Hz in low speed mode and 50/100/200/250 KHz in
>> +         high speed mode. The LEDs can be configured to be triggered by
>> +         SW/CPU or by hardware. Say 'Y' here if you are working on LGM
> 
> Please spell out "software".
> 
>> +         SoC based platform.
>> +
>> +         To compile this driver as a module, choose M here: the
>> +         module will be called leds-lgm-sso.
>>
>> -endif # LEDS_BLINK
> 

Well noted about above improvements suggestions. Shall update in V1. Thanks.

Regards,
Rahul

> thanks.
> --
> ~Randy
> 
> 

Re: [PATCH 1/1] leds: lgm: Improve Kconfig help

[Arnd Bergmann]

On Thu, Mar 18, 2021 at 10:24 AM Rahul Tanwar <rtanwar@maxlinear.com> wrote:
>
> Hi Randy,
>
> On 18/3/2021 11:02 am, Randy Dunlap wrote:
> > This email was sent from outside of MaxLinear.
> >
> >
> > Hi,
> >
> > For the leds/blink/Kconfig file at least, something has
> > changed all of the tabs to spaces.
> >
> > Keywords in Kconfig files should be indented with one tab,
> > while help text should be indented with one tab + 2 spaces.
> >
>
>
> Hmm, facing some IT issues with git send-email so i had to send it by
> other means. I will fix it in V1 by ensuring that i send using git.

FYI, the usual convention is that 'v1' is the implied version when you
send a patch series for the first time. If you send an updated version,
you start counting at 'v2'.

       Arnd

Re: [PATCH 1/1] leds: lgm: Improve Kconfig help

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1903 bytes --]

Hi!

> Remove unnecessary Kconfig symbol LEDS_BLINK
> Improve Kconfig help text to make it more useful.
> 
> Signed-off-by: Rahul Tanwar <rtanwar@maxlinear.com>

> +++ b/drivers/leds/blink/Kconfig
> @@ -1,21 +1,19 @@
> -menuconfig LEDS_BLINK
> -       bool "LED Blink support"
> -       depends on LEDS_CLASS
> -       help
> -         This option enables blink support for the leds class.
> -         If unsure, say Y.
> -
> -if LEDS_BLINK
> -
> -config LEDS_BLINK_LGM
> -       tristate "LED support for Intel LGM SoC series"
> +config LEDS_LGM
> +       tristate "LED support for LGM SoC series"
>          depends on GPIOLIB
>          depends on LEDS_CLASS
>          depends on MFD_SYSCON
>          depends on OF
>          help
> -         Parallel to serial conversion, which is also called SSO 
> controller,
> -         can drive external shift register for LED outputs.
> -         This enables LED support for Serial Shift Output controller(SSO).
> +         This option enables support for LEDs connected to GPIO lines on
> +         Lightning Mountain(LGM) SoC. These LEDs are driven by a Serial
> +         Shift Output(SSO) controller. The driver supports hardware

What is Lightning Mountain? The codename is not widely known. Where
can we find that hardware? Notebooks? Phones? Only some development
boards?

If user is not likely to need the driver, say so.

> +         blinking with a configurable LED update/blink frequency in two
> +         modes, 2/4/8/10 Hz in low speed mode and 50/100/200/250
> KHz in

kHz? But I guess we don't need that here.

>    *
> - * Copyright (c) 2020 Intel Corporation.
> + * Copyright (c) 2021 MaxLinear, Inc.
>    */
> 

I don't think you can do that, and I don't think you should be doing
it in the same patch.

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 1/1] leds: lgm: Improve Kconfig help

[Rahul Tanwar]

Hi Pavel,

On 18/3/2021 3:55 pm, Pavel Machek wrote:
> Hi!
> 
>  > Remove unnecessary Kconfig symbol LEDS_BLINK
>  > Improve Kconfig help text to make it more useful.
>  >
>  > Signed-off-by: Rahul Tanwar <rtanwar@maxlinear.com>
> 
>  > +++ b/drivers/leds/blink/Kconfig
>  > @@ -1,21 +1,19 @@
>  > -menuconfig LEDS_BLINK
>  > - bool "LED Blink support"
>  > - depends on LEDS_CLASS
>  > - help
>  > - This option enables blink support for the leds class.
>  > - If unsure, say Y.
>  > -
>  > -if LEDS_BLINK
>  > -
>  > -config LEDS_BLINK_LGM
>  > - tristate "LED support for Intel LGM SoC series"
>  > +config LEDS_LGM
>  > + tristate "LED support for LGM SoC series"
>  > depends on GPIOLIB
>  > depends on LEDS_CLASS
>  > depends on MFD_SYSCON
>  > depends on OF
>  > help
>  > - Parallel to serial conversion, which is also called SSO
>  > controller,
>  > - can drive external shift register for LED outputs.
>  > - This enables LED support for Serial Shift Output controller(SSO).
>  > + This option enables support for LEDs connected to GPIO lines on
>  > + Lightning Mountain(LGM) SoC. These LEDs are driven by a Serial
>  > + Shift Output(SSO) controller. The driver supports hardware
> 
> What is Lightning Mountain? The codename is not widely known. Where
> can we find that hardware? Notebooks? Phones? Only some development
> boards?
> 

Lightning Mountain is generically a network processor with a primary 
targeted application as Gateway SoC. It has already been added as a 
valid Intel Atom processor variant in 
arch/x86/include/asm/intel-family.h as below:

#define INTEL_FAM6_ATOM_AIRMONT_NP	0x75 /* Lightning Mountain */

Please see [1].


> If user is not likely to need the driver, say so.
> 
>  > + blinking with a configurable LED update/blink frequency in two
>  > + modes, 2/4/8/10 Hz in low speed mode and 50/100/200/250
>  > KHz in
> 
> kHz? But I guess we don't need that here.
>

Well noted. Will update in V2.


>  > *
>  > - * Copyright (c) 2020 Intel Corporation.
>  > + * Copyright (c) 2021 MaxLinear, Inc.
>  > */
>  >
> 
> I don't think you can do that, and I don't think you should be doing
> it in the same patch.


Well noted. Will revert it back now and update later in a separate 
patch. Thanks.


> 

Regards,
Rahul

[1] 
https://github.com/torvalds/linux/blob/master/arch/x86/include/asm/intel-family.h

Re: [PATCH 1/1] leds: lgm: Improve Kconfig help

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1430 bytes --]

Hi!


> >  > help
> >  > - Parallel to serial conversion, which is also called SSO
> >  > controller,
> >  > - can drive external shift register for LED outputs.
> >  > - This enables LED support for Serial Shift Output controller(SSO).
> >  > + This option enables support for LEDs connected to GPIO lines on
> >  > + Lightning Mountain(LGM) SoC. These LEDs are driven by a Serial
> >  > + Shift Output(SSO) controller. The driver supports hardware
> > 
> > What is Lightning Mountain? The codename is not widely known. Where
> > can we find that hardware? Notebooks? Phones? Only some development
> > boards?
> > 
> 
> Lightning Mountain is generically a network processor with a primary 
> targeted application as Gateway SoC. It has already been added as a 
> valid Intel Atom processor variant in 
> arch/x86/include/asm/intel-family.h as below:

Yep, but Kconfig users are not going to read header files.

If the SoC is not shipping in any products, state so.

> >  > *
> >  > - * Copyright (c) 2020 Intel Corporation.
> >  > + * Copyright (c) 2021 MaxLinear, Inc.
> >  > */
> >  >
> > 
> > I don't think you can do that, and I don't think you should be doing
> > it in the same patch.
> 
> Well noted. Will revert it back now and update later in a separate 
> patch. Thanks.

Don't. You can't update copyright like that.

								Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 1/1] leds: lgm: Improve Kconfig help

[Rahul Tanwar]

Hi Pavel,

On 19/3/2021 4:37 am, Pavel Machek wrote:
> Hi!
> 
> 
>  > > > help
>  > > > - Parallel to serial conversion, which is also called SSO
>  > > > controller,
>  > > > - can drive external shift register for LED outputs.
>  > > > - This enables LED support for Serial Shift Output controller(SSO).
>  > > > + This option enables support for LEDs connected to GPIO lines on
>  > > > + Lightning Mountain(LGM) SoC. These LEDs are driven by a Serial
>  > > > + Shift Output(SSO) controller. The driver supports hardware
>  > >
>  > > What is Lightning Mountain? The codename is not widely known. Where
>  > > can we find that hardware? Notebooks? Phones? Only some development
>  > > boards?
>  > >
>  >
>  > Lightning Mountain is generically a network processor with a primary
>  > targeted application as Gateway SoC. It has already been added as a
>  > valid Intel Atom processor variant in
>  > arch/x86/include/asm/intel-family.h as below:
> 
> Yep, but Kconfig users are not going to read header files.
> 
> If the SoC is not shipping in any products, state so.
> 


Got your point. Will update the help text.


>  > > > *
>  > > > - * Copyright (c) 2020 Intel Corporation.
>  > > > + * Copyright (c) 2021 MaxLinear, Inc.
>  > > > */
>  > > >
>  > >
>  > > I don't think you can do that, and I don't think you should be doing
>  > > it in the same patch.
>  >
>  > Well noted. Will revert it back now and update later in a separate
>  > patch. Thanks.
> 
> Don't. You can't update copyright like that.
>

Well noted.

Regards,
Rahul


> Pavel
> -- 
> http://www.livejournal.com/~pavelmachek 
> <http://www.livejournal.com/~pavelmachek>