[PATCH net-next v6 1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()

[PATCH net-next v6 1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()

[Andy Shevchenko]

LED core provides a helper to parse default state from firmware node.
Use it instead of custom implementation.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
---
v6: wrapped long lines (Simon, Jakub)
 drivers/net/dsa/hirschmann/hellcreek_ptp.c | 45 ++++++++++++----------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/drivers/net/dsa/hirschmann/hellcreek_ptp.c b/drivers/net/dsa/hirschmann/hellcreek_ptp.c
index b28baab6d56a..3e44ccb7db84 100644
--- a/drivers/net/dsa/hirschmann/hellcreek_ptp.c
+++ b/drivers/net/dsa/hirschmann/hellcreek_ptp.c
@@ -297,7 +297,8 @@ static enum led_brightness hellcreek_led_is_gm_get(struct led_classdev *ldev)
 static int hellcreek_led_setup(struct hellcreek *hellcreek)
 {
 	struct device_node *leds, *led = NULL;
-	const char *label, *state;
+	enum led_default_state state;
+	const char *label;
 	int ret = -EINVAL;
 
 	of_node_get(hellcreek->dev->of_node);
@@ -318,16 +319,17 @@ static int hellcreek_led_setup(struct hellcreek *hellcreek)
 	ret = of_property_read_string(led, "label", &label);
 	hellcreek->led_sync_good.name = ret ? "sync_good" : label;
 
-	ret = of_property_read_string(led, "default-state", &state);
-	if (!ret) {
-		if (!strcmp(state, "on"))
-			hellcreek->led_sync_good.brightness = 1;
-		else if (!strcmp(state, "off"))
-			hellcreek->led_sync_good.brightness = 0;
-		else if (!strcmp(state, "keep"))
-			hellcreek->led_sync_good.brightness =
-				hellcreek_get_brightness(hellcreek,
-							 STATUS_OUT_SYNC_GOOD);
+	state = led_init_default_state_get(of_fwnode_handle(led));
+	switch (state) {
+	case LEDS_DEFSTATE_ON:
+		hellcreek->led_sync_good.brightness = 1;
+		break;
+	case LEDS_DEFSTATE_KEEP:
+		hellcreek->led_sync_good.brightness =
+			hellcreek_get_brightness(hellcreek, STATUS_OUT_SYNC_GOOD);
+		break;
+	default:
+		hellcreek->led_sync_good.brightness = 0;
 	}
 
 	hellcreek->led_sync_good.max_brightness = 1;
@@ -344,16 +346,17 @@ static int hellcreek_led_setup(struct hellcreek *hellcreek)
 	ret = of_property_read_string(led, "label", &label);
 	hellcreek->led_is_gm.name = ret ? "is_gm" : label;
 
-	ret = of_property_read_string(led, "default-state", &state);
-	if (!ret) {
-		if (!strcmp(state, "on"))
-			hellcreek->led_is_gm.brightness = 1;
-		else if (!strcmp(state, "off"))
-			hellcreek->led_is_gm.brightness = 0;
-		else if (!strcmp(state, "keep"))
-			hellcreek->led_is_gm.brightness =
-				hellcreek_get_brightness(hellcreek,
-							 STATUS_OUT_IS_GM);
+	state = led_init_default_state_get(of_fwnode_handle(led));
+	switch (state) {
+	case LEDS_DEFSTATE_ON:
+		hellcreek->led_is_gm.brightness = 1;
+		break;
+	case LEDS_DEFSTATE_KEEP:
+		hellcreek->led_is_gm.brightness =
+			hellcreek_get_brightness(hellcreek, STATUS_OUT_IS_GM);
+		break;
+	default:
+		hellcreek->led_is_gm.brightness = 0;
 	}
 
 	hellcreek->led_is_gm.max_brightness = 1;
-- 
2.39.2

Re: [PATCH net-next v6 1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()

[kernel test robot]

Hi Andy,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/net-dsa-hellcreek-Get-rid-of-custom-led_init_default_state_get/20230315-021931
patch link:    https://lore.kernel.org/r/20230314181824.56881-1-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH net-next v6 1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()
config: i386-randconfig-a015-20230313 (https://download.01.org/0day-ci/archive/20230315/202303150831.vgyKe8FD-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/fdd54417a75386e7ad47065c21403835b7fda94a
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Andy-Shevchenko/net-dsa-hellcreek-Get-rid-of-custom-led_init_default_state_get/20230315-021931
        git checkout fdd54417a75386e7ad47065c21403835b7fda94a
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/dsa/hirschmann/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303150831.vgyKe8FD-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/net/dsa/hirschmann/hellcreek_ptp.c:322:10: error: implicit declaration of function 'led_init_default_state_get' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
           state = led_init_default_state_get(of_fwnode_handle(led));
                   ^
   1 error generated.


vim +/led_init_default_state_get +322 drivers/net/dsa/hirschmann/hellcreek_ptp.c

   292	
   293	/* There two available LEDs internally called sync_good and is_gm. However, the
   294	 * user might want to use a different label and specify the default state. Take
   295	 * those properties from device tree.
   296	 */
   297	static int hellcreek_led_setup(struct hellcreek *hellcreek)
   298	{
   299		struct device_node *leds, *led = NULL;
   300		enum led_default_state state;
   301		const char *label;
   302		int ret = -EINVAL;
   303	
   304		of_node_get(hellcreek->dev->of_node);
   305		leds = of_find_node_by_name(hellcreek->dev->of_node, "leds");
   306		if (!leds) {
   307			dev_err(hellcreek->dev, "No LEDs specified in device tree!\n");
   308			return ret;
   309		}
   310	
   311		hellcreek->status_out = 0;
   312	
   313		led = of_get_next_available_child(leds, led);
   314		if (!led) {
   315			dev_err(hellcreek->dev, "First LED not specified!\n");
   316			goto out;
   317		}
   318	
   319		ret = of_property_read_string(led, "label", &label);
   320		hellcreek->led_sync_good.name = ret ? "sync_good" : label;
   321	
 > 322		state = led_init_default_state_get(of_fwnode_handle(led));
   323		switch (state) {
   324		case LEDS_DEFSTATE_ON:
   325			hellcreek->led_sync_good.brightness = 1;
   326			break;
   327		case LEDS_DEFSTATE_KEEP:
   328			hellcreek->led_sync_good.brightness =
   329				hellcreek_get_brightness(hellcreek, STATUS_OUT_SYNC_GOOD);
   330			break;
   331		default:
   332			hellcreek->led_sync_good.brightness = 0;
   333		}
   334	
   335		hellcreek->led_sync_good.max_brightness = 1;
   336		hellcreek->led_sync_good.brightness_set = hellcreek_led_sync_good_set;
   337		hellcreek->led_sync_good.brightness_get = hellcreek_led_sync_good_get;
   338	
   339		led = of_get_next_available_child(leds, led);
   340		if (!led) {
   341			dev_err(hellcreek->dev, "Second LED not specified!\n");
   342			ret = -EINVAL;
   343			goto out;
   344		}
   345	
   346		ret = of_property_read_string(led, "label", &label);
   347		hellcreek->led_is_gm.name = ret ? "is_gm" : label;
   348	
   349		state = led_init_default_state_get(of_fwnode_handle(led));
   350		switch (state) {
   351		case LEDS_DEFSTATE_ON:
   352			hellcreek->led_is_gm.brightness = 1;
   353			break;
   354		case LEDS_DEFSTATE_KEEP:
   355			hellcreek->led_is_gm.brightness =
   356				hellcreek_get_brightness(hellcreek, STATUS_OUT_IS_GM);
   357			break;
   358		default:
   359			hellcreek->led_is_gm.brightness = 0;
   360		}
   361	
   362		hellcreek->led_is_gm.max_brightness = 1;
   363		hellcreek->led_is_gm.brightness_set = hellcreek_led_is_gm_set;
   364		hellcreek->led_is_gm.brightness_get = hellcreek_led_is_gm_get;
   365	
   366		/* Set initial state */
   367		if (hellcreek->led_sync_good.brightness == 1)
   368			hellcreek_set_brightness(hellcreek, STATUS_OUT_SYNC_GOOD, 1);
   369		if (hellcreek->led_is_gm.brightness == 1)
   370			hellcreek_set_brightness(hellcreek, STATUS_OUT_IS_GM, 1);
   371	
   372		/* Register both leds */
   373		led_classdev_register(hellcreek->dev, &hellcreek->led_sync_good);
   374		led_classdev_register(hellcreek->dev, &hellcreek->led_is_gm);
   375	
   376		ret = 0;
   377	
   378	out:
   379		of_node_put(leds);
   380	
   381		return ret;
   382	}
   383	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH net-next v6 1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()

[Andy Shevchenko]

On Wed, Mar 15, 2023 at 09:05:25AM +0800, kernel test robot wrote:
> Hi Andy,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on net-next/master]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/net-dsa-hellcreek-Get-rid-of-custom-led_init_default_state_get/20230315-021931
> patch link:    https://lore.kernel.org/r/20230314181824.56881-1-andriy.shevchenko%40linux.intel.com
> patch subject: [PATCH net-next v6 1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()
> config: i386-randconfig-a015-20230313 (https://download.01.org/0day-ci/archive/20230315/202303150831.vgyKe8FD-lkp@intel.com/config)
> compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/intel-lab-lkp/linux/commit/fdd54417a75386e7ad47065c21403835b7fda94a
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Andy-Shevchenko/net-dsa-hellcreek-Get-rid-of-custom-led_init_default_state_get/20230315-021931
>         git checkout fdd54417a75386e7ad47065c21403835b7fda94a
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/dsa/hirschmann/
> 
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202303150831.vgyKe8FD-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
> >> drivers/net/dsa/hirschmann/hellcreek_ptp.c:322:10: error: implicit declaration of function 'led_init_default_state_get' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
>            state = led_init_default_state_get(of_fwnode_handle(led));
>                    ^
>    1 error generated.

I can not reproduce it.

I have downloaded net-next and applied the only patch on top.
I have downloaded the above mentioned kernel configuration and
repeated the steps with `make ... oldconfig; make W=1 ...`

Can you shed a light on what's going on here?

Note, the bug is impossibly related to my patch because the new API is in the
same header as already used from the LEDS framework. If it's reproducible, it
should be also without my patch.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net-next v6 1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()

[Nathan Chancellor]

On Wed, Mar 15, 2023 at 07:06:35PM +0200, Andy Shevchenko wrote:
> On Wed, Mar 15, 2023 at 09:05:25AM +0800, kernel test robot wrote:
> > Hi Andy,
> > 
> > I love your patch! Yet something to improve:
> > 
> > [auto build test ERROR on net-next/master]
> > 
> > url:    https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/net-dsa-hellcreek-Get-rid-of-custom-led_init_default_state_get/20230315-021931
> > patch link:    https://lore.kernel.org/r/20230314181824.56881-1-andriy.shevchenko%40linux.intel.com
> > patch subject: [PATCH net-next v6 1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()
> > config: i386-randconfig-a015-20230313 (https://download.01.org/0day-ci/archive/20230315/202303150831.vgyKe8FD-lkp@intel.com/config)
> > compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
> > reproduce (this is a W=1 build):
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # https://github.com/intel-lab-lkp/linux/commit/fdd54417a75386e7ad47065c21403835b7fda94a
> >         git remote add linux-review https://github.com/intel-lab-lkp/linux
> >         git fetch --no-tags linux-review Andy-Shevchenko/net-dsa-hellcreek-Get-rid-of-custom-led_init_default_state_get/20230315-021931
> >         git checkout fdd54417a75386e7ad47065c21403835b7fda94a
> >         # save the config file
> >         mkdir build_dir && cp config build_dir/.config
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/dsa/hirschmann/
> > 
> > If you fix the issue, kindly add following tag where applicable
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Link: https://lore.kernel.org/oe-kbuild-all/202303150831.vgyKe8FD-lkp@intel.com/
> > 
> > All errors (new ones prefixed by >>):
> > 
> > >> drivers/net/dsa/hirschmann/hellcreek_ptp.c:322:10: error: implicit declaration of function 'led_init_default_state_get' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
> >            state = led_init_default_state_get(of_fwnode_handle(led));
> >                    ^
> >    1 error generated.
> 
> I can not reproduce it.
> 
> I have downloaded net-next and applied the only patch on top.
> I have downloaded the above mentioned kernel configuration and
> repeated the steps with `make ... oldconfig; make W=1 ...`
> 
> Can you shed a light on what's going on here?
> 
> Note, the bug is impossibly related to my patch because the new API is in the
> same header as already used from the LEDS framework. If it's reproducible, it
> should be also without my patch.

If you modify the GitHub link above the 'git remote' command above from
'commit' to 'commits', you can see that your patch was applied on top of
mainline commit 5b7c4cabbb65 ("Merge tag 'net-next-6.3' of
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next"), which
was before the pull that moved led_init_default_state_get() into
include/linux/leds.h, commit e4bc15889506 ("Merge tag 'leds-next-6.3' of
git://git.kernel.org/pub/scm/linux/kernel/git/lee/leds"). Not sure why
that was the base that was chosen but it explains the error.

Cheers,
Nathan

Re: [PATCH net-next v6 1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()

[Jakub Kicinski]

On Wed, 15 Mar 2023 12:51:54 -0700 Nathan Chancellor wrote:
> If you modify the GitHub link above the 'git remote' command above from
> 'commit' to 'commits', you can see that your patch was applied on top of
> mainline commit 5b7c4cabbb65 ("Merge tag 'net-next-6.3' of
> git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next"), which
> was before the pull that moved led_init_default_state_get() into
> include/linux/leds.h, commit e4bc15889506 ("Merge tag 'leds-next-6.3' of
> git://git.kernel.org/pub/scm/linux/kernel/git/lee/leds"). Not sure why
> that was the base that was chosen but it explains the error.

Because they still haven't moved to using the main branch of netdev
trees, they try to pull master :| I'll email them directly, I think
they don't see the in-reply messages.

Re: [PATCH net-next v6 1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()

[Philip Li]

On Wed, Mar 15, 2023 at 02:15:38PM -0700, Jakub Kicinski wrote:
> On Wed, 15 Mar 2023 12:51:54 -0700 Nathan Chancellor wrote:
> > If you modify the GitHub link above the 'git remote' command above from
> > 'commit' to 'commits', you can see that your patch was applied on top of
> > mainline commit 5b7c4cabbb65 ("Merge tag 'net-next-6.3' of
> > git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next"), which
> > was before the pull that moved led_init_default_state_get() into
> > include/linux/leds.h, commit e4bc15889506 ("Merge tag 'leds-next-6.3' of
> > git://git.kernel.org/pub/scm/linux/kernel/git/lee/leds"). Not sure why
> > that was the base that was chosen but it explains the error.
> 
> Because they still haven't moved to using the main branch of netdev
> trees, they try to pull master :| I'll email them directly, I think
> they don't see the in-reply messages.

Sorry for missing the early information, we didn't notice it and still
apply to the wrong branch. We will resolve this asap to use main branch
instead.

Thanks

> 

Re: [PATCH net-next v6 1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()

[Michal Swiatkowski]

On Tue, Mar 14, 2023 at 08:18:24PM +0200, Andy Shevchenko wrote:
> LED core provides a helper to parse default state from firmware node.
> Use it instead of custom implementation.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
> ---
> v6: wrapped long lines (Simon, Jakub)
>  drivers/net/dsa/hirschmann/hellcreek_ptp.c | 45 ++++++++++++----------
>  1 file changed, 24 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/dsa/hirschmann/hellcreek_ptp.c b/drivers/net/dsa/hirschmann/hellcreek_ptp.c
> index b28baab6d56a..3e44ccb7db84 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek_ptp.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek_ptp.c
> @@ -297,7 +297,8 @@ static enum led_brightness hellcreek_led_is_gm_get(struct led_classdev *ldev)
>  static int hellcreek_led_setup(struct hellcreek *hellcreek)
>  {
>  	struct device_node *leds, *led = NULL;
> -	const char *label, *state;
> +	enum led_default_state state;
> +	const char *label;
>  	int ret = -EINVAL;
>  
>  	of_node_get(hellcreek->dev->of_node);
> @@ -318,16 +319,17 @@ static int hellcreek_led_setup(struct hellcreek *hellcreek)
>  	ret = of_property_read_string(led, "label", &label);
>  	hellcreek->led_sync_good.name = ret ? "sync_good" : label;
>  
> -	ret = of_property_read_string(led, "default-state", &state);
> -	if (!ret) {
> -		if (!strcmp(state, "on"))
> -			hellcreek->led_sync_good.brightness = 1;
> -		else if (!strcmp(state, "off"))
> -			hellcreek->led_sync_good.brightness = 0;
> -		else if (!strcmp(state, "keep"))
> -			hellcreek->led_sync_good.brightness =
> -				hellcreek_get_brightness(hellcreek,
> -							 STATUS_OUT_SYNC_GOOD);
> +	state = led_init_default_state_get(of_fwnode_handle(led));
> +	switch (state) {
> +	case LEDS_DEFSTATE_ON:
> +		hellcreek->led_sync_good.brightness = 1;
> +		break;
> +	case LEDS_DEFSTATE_KEEP:
> +		hellcreek->led_sync_good.brightness =
> +			hellcreek_get_brightness(hellcreek, STATUS_OUT_SYNC_GOOD);
> +		break;
> +	default:
> +		hellcreek->led_sync_good.brightness = 0;
>  	}
>  
>  	hellcreek->led_sync_good.max_brightness = 1;
> @@ -344,16 +346,17 @@ static int hellcreek_led_setup(struct hellcreek *hellcreek)
>  	ret = of_property_read_string(led, "label", &label);
>  	hellcreek->led_is_gm.name = ret ? "is_gm" : label;
>  
> -	ret = of_property_read_string(led, "default-state", &state);
> -	if (!ret) {
> -		if (!strcmp(state, "on"))
> -			hellcreek->led_is_gm.brightness = 1;
> -		else if (!strcmp(state, "off"))
> -			hellcreek->led_is_gm.brightness = 0;
> -		else if (!strcmp(state, "keep"))
> -			hellcreek->led_is_gm.brightness =
> -				hellcreek_get_brightness(hellcreek,
> -							 STATUS_OUT_IS_GM);
> +	state = led_init_default_state_get(of_fwnode_handle(led));
> +	switch (state) {
> +	case LEDS_DEFSTATE_ON:
> +		hellcreek->led_is_gm.brightness = 1;
> +		break;
> +	case LEDS_DEFSTATE_KEEP:
> +		hellcreek->led_is_gm.brightness =
> +			hellcreek_get_brightness(hellcreek, STATUS_OUT_IS_GM);
> +		break;
> +	default:
> +		hellcreek->led_is_gm.brightness = 0;

Hi,

You have to fix implict declaration of the led_init_default_state_get().

I wonder if the code duplication here can be avoided:
static void set_led_brightness(hellcreek, led, *brightness, status)
{
	enum led_default_state state =
		led_init_default_state_get(of_fwnode_handle(led));

	switch(state) {
	case LEDS_DEFSTATE_ON:
		*brightness = 1;
		break;
	case LEDS_DEFSTATE_KEEP:
		*brightness = hellcreek_get_brightness(hellcreek, status);
		break;
	default:
		*brightness = 0;
	}
}

and call it like:
set_led_brightness(heelcreek, led, &heelcreek->...,
		   STATUS_OUT_IS_GM/SYNC_GOOD);

Only suggestion, patch looks good.

Thanks,
Michal

>  	}
>  
>  	hellcreek->led_is_gm.max_brightness = 1;
> -- 
> 2.39.2
> 

Re: [PATCH net-next v6 1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()

[Andy Shevchenko]

On Wed, Mar 15, 2023 at 06:57:37AM +0100, Michal Swiatkowski wrote:
> On Tue, Mar 14, 2023 at 08:18:24PM +0200, Andy Shevchenko wrote:
> > LED core provides a helper to parse default state from firmware node.
> > Use it instead of custom implementation.

...

> You have to fix implict declaration of the led_init_default_state_get().

Seems like users have to choose between 'select NEW_LEDS' and
'depends on NEW_LEDS' in the Kconfig.

> I wonder if the code duplication here can be avoided:

Whether or not this is out of the scope of this patch.
Feel free to submit one :-)

...

> Only suggestion, patch looks good.

Thank you!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net-next v6 1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()

[Michal Swiatkowski]

On Wed, Mar 15, 2023 at 03:16:42PM +0200, Andy Shevchenko wrote:
> On Wed, Mar 15, 2023 at 06:57:37AM +0100, Michal Swiatkowski wrote:
> > On Tue, Mar 14, 2023 at 08:18:24PM +0200, Andy Shevchenko wrote:
> > > LED core provides a helper to parse default state from firmware node.
> > > Use it instead of custom implementation.
> 
> ...
> 
> > You have to fix implict declaration of the led_init_default_state_get().
> 
> Seems like users have to choose between 'select NEW_LEDS' and
> 'depends on NEW_LEDS' in the Kconfig.
> 
> > I wonder if the code duplication here can be avoided:
> 
> Whether or not this is out of the scope of this patch.
> Feel free to submit one :-)
> 
> ...
>

Reasonable ;)
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

> > Only suggestion, patch looks good.
> 
> Thank you!
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH net-next v6 1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()

[Andy Shevchenko]

On Wed, Mar 15, 2023 at 06:57:37AM +0100, Michal Swiatkowski wrote:
> On Tue, Mar 14, 2023 at 08:18:24PM +0200, Andy Shevchenko wrote:

...

> You have to fix implict declaration of the led_init_default_state_get().

FWIW, I have spent more than a couple of hours on this and have no idea
how it's even possible. If there is a bug it must have been reproduced
without my patch.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net-next v6 1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 14 Mar 2023 20:18:24 +0200 you wrote:
> LED core provides a helper to parse default state from firmware node.
> Use it instead of custom implementation.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
> ---
> v6: wrapped long lines (Simon, Jakub)
>  drivers/net/dsa/hirschmann/hellcreek_ptp.c | 45 ++++++++++++----------
>  1 file changed, 24 insertions(+), 21 deletions(-)

Here is the summary with links:
  - [net-next,v6,1/1] net: dsa: hellcreek: Get rid of custom led_init_default_state_get()
    https://git.kernel.org/netdev/net-next/c/d565263b7d83

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html