linux-leds.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] leds: cros_ec: Implement review comments from Lee
@ 2024-06-14 10:04 Thomas Weißschuh
  2024-06-20  9:31 ` Lee Jones
  2024-06-20 17:13 ` (subset) " Lee Jones
  0 siblings, 2 replies; 10+ messages in thread
From: Thomas Weißschuh @ 2024-06-14 10:04 UTC (permalink / raw)
  To: Thomas Weißschuh, Pavel Machek, Lee Jones, Benson Leung,
	Guenter Roeck
  Cc: linux-leds, chrome-platform, linux-kernel, Thomas Weißschuh

Implement review comments from Lee as requested in [0] for
"leds: Add ChromeOS EC driver".

Changes:
* Inline DRV_NAME string constant.
* Use dev_err() instead of dev_warn() to report errors.
* Rename cros_ec_led_probe_led() to cros_ec_led_probe_one().
* Make loop variable "int" where they belong.
* Move "Unknown LED" comment to where it belongs.
* Register trigger during _probe().
* Use module_platform_driver() and drop all the custom boilerplate.

[0] https://lore.kernel.org/lkml/20240614093445.GF3029315@google.com/T/#m8750abdef6a968ace765645189170814196c9ce9

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/leds/leds-cros_ec.c | 50 +++++++++++++--------------------------------
 1 file changed, 14 insertions(+), 36 deletions(-)

diff --git a/drivers/leds/leds-cros_ec.c b/drivers/leds/leds-cros_ec.c
index 7bb21a587713..275522b81ea5 100644
--- a/drivers/leds/leds-cros_ec.c
+++ b/drivers/leds/leds-cros_ec.c
@@ -14,8 +14,6 @@
 #include <linux/platform_data/cros_ec_commands.h>
 #include <linux/platform_data/cros_ec_proto.h>
 
-#define DRV_NAME	"cros-ec-led"
-
 static const char * const cros_ec_led_functions[] = {
 	[EC_LED_ID_BATTERY_LED]            = LED_FUNCTION_CHARGING,
 	[EC_LED_ID_POWER_LED]              = LED_FUNCTION_POWER,
@@ -152,7 +150,7 @@ static int cros_ec_led_count_subleds(struct device *dev,
 
 		if (common_range != range) {
 			/* The multicolor LED API expects a uniform max_brightness */
-			dev_warn(dev, "Inconsistent LED brightness values\n");
+			dev_err(dev, "Inconsistent LED brightness values\n");
 			return -EINVAL;
 		}
 	}
@@ -176,22 +174,21 @@ static const char *cros_ec_led_get_color_name(struct led_classdev_mc *led_mc_cde
 	return led_get_color_name(color);
 }
 
-static int cros_ec_led_probe_led(struct device *dev, struct cros_ec_device *cros_ec,
+static int cros_ec_led_probe_one(struct device *dev, struct cros_ec_device *cros_ec,
 				 enum ec_led_id id)
 {
 	union cros_ec_led_cmd_data arg = {};
 	struct cros_ec_led_priv *priv;
 	struct led_classdev *led_cdev;
 	struct mc_subled *subleds;
-	int ret, num_subleds;
-	size_t i, subled;
+	int i, ret, num_subleds;
+	size_t subled;
 
 	arg.req.led_id = id;
 	arg.req.flags = EC_LED_FLAGS_QUERY;
 	ret = cros_ec_led_send_cmd(cros_ec, &arg);
-	/* Unknown LED, skip */
 	if (ret == -EINVAL)
-		return 0;
+		return 0; /* Unknown LED, skip */
 	if (ret == -EOPNOTSUPP)
 		return -ENODEV;
 	if (ret < 0)
@@ -247,11 +244,14 @@ static int cros_ec_led_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
 	struct cros_ec_device *cros_ec = ec_dev->ec_dev;
-	int ret = 0;
-	size_t i;
+	int i, ret = 0;
+
+	ret = devm_led_trigger_register(dev, &cros_ec_led_trigger);
+	if (ret)
+		return ret;
 
 	for (i = 0; i < EC_LED_ID_COUNT; i++) {
-		ret = cros_ec_led_probe_led(dev, cros_ec, i);
+		ret = cros_ec_led_probe_one(dev, cros_ec, i);
 		if (ret)
 			break;
 	}
@@ -260,38 +260,16 @@ static int cros_ec_led_probe(struct platform_device *pdev)
 }
 
 static const struct platform_device_id cros_ec_led_id[] = {
-	{ DRV_NAME, 0 },
+	{ "cros-ec-led", 0 },
 	{}
 };
 
 static struct platform_driver cros_ec_led_driver = {
-	.driver.name	= DRV_NAME,
+	.driver.name	= "cros-ec-led",
 	.probe		= cros_ec_led_probe,
 	.id_table	= cros_ec_led_id,
 };
-
-static int __init cros_ec_led_init(void)
-{
-	int ret;
-
-	ret = led_trigger_register(&cros_ec_led_trigger);
-	if (ret)
-		return ret;
-
-	ret = platform_driver_register(&cros_ec_led_driver);
-	if (ret)
-		led_trigger_unregister(&cros_ec_led_trigger);
-
-	return ret;
-};
-module_init(cros_ec_led_init);
-
-static void __exit cros_ec_led_exit(void)
-{
-	platform_driver_unregister(&cros_ec_led_driver);
-	led_trigger_unregister(&cros_ec_led_trigger);
-};
-module_exit(cros_ec_led_exit);
+module_platform_driver(cros_ec_led_driver);
 
 MODULE_DEVICE_TABLE(platform, cros_ec_led_id);
 MODULE_DESCRIPTION("ChromeOS EC LED Driver");

---
base-commit: b6774dd948171c478c7aa19318b1d7ae9cf6d7a4
change-id: 20240614-cros_ec-led-review-ec93abc65933

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>


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

* Re: [PATCH] leds: cros_ec: Implement review comments from Lee
  2024-06-14 10:04 [PATCH] leds: cros_ec: Implement review comments from Lee Thomas Weißschuh
@ 2024-06-20  9:31 ` Lee Jones
  2024-06-20 10:12   ` Thomas Weißschuh
  2024-06-20 17:13 ` (subset) " Lee Jones
  1 sibling, 1 reply; 10+ messages in thread
From: Lee Jones @ 2024-06-20  9:31 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Thomas Weißschuh, Pavel Machek, Benson Leung, Guenter Roeck,
	linux-leds, chrome-platform, linux-kernel

Definitely not seen a commit message like that before

> Implement review comments from Lee as requested in [0] for
> "leds: Add ChromeOS EC driver".
> 
> Changes:
> * Inline DRV_NAME string constant.
> * Use dev_err() instead of dev_warn() to report errors.
> * Rename cros_ec_led_probe_led() to cros_ec_led_probe_one().
> * Make loop variable "int" where they belong.
> * Move "Unknown LED" comment to where it belongs.
> * Register trigger during _probe().
> * Use module_platform_driver() and drop all the custom boilerplate.

If you're fixing many things, then I would expect to receive many
patches.  One patch for functional change please.  If you can reasonably
group fixes of similar elk, then please do.  However one patch that does
a bunch of different things is a no-go from me, sorry.

> [0] https://lore.kernel.org/lkml/20240614093445.GF3029315@google.com/T/#m8750abdef6a968ace765645189170814196c9ce9
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  drivers/leds/leds-cros_ec.c | 50 +++++++++++++--------------------------------
>  1 file changed, 14 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/leds/leds-cros_ec.c b/drivers/leds/leds-cros_ec.c
> index 7bb21a587713..275522b81ea5 100644
> --- a/drivers/leds/leds-cros_ec.c
> +++ b/drivers/leds/leds-cros_ec.c
> @@ -14,8 +14,6 @@
>  #include <linux/platform_data/cros_ec_commands.h>
>  #include <linux/platform_data/cros_ec_proto.h>
>  
> -#define DRV_NAME	"cros-ec-led"
> -
>  static const char * const cros_ec_led_functions[] = {
>  	[EC_LED_ID_BATTERY_LED]            = LED_FUNCTION_CHARGING,
>  	[EC_LED_ID_POWER_LED]              = LED_FUNCTION_POWER,
> @@ -152,7 +150,7 @@ static int cros_ec_led_count_subleds(struct device *dev,
>  
>  		if (common_range != range) {
>  			/* The multicolor LED API expects a uniform max_brightness */
> -			dev_warn(dev, "Inconsistent LED brightness values\n");
> +			dev_err(dev, "Inconsistent LED brightness values\n");
>  			return -EINVAL;
>  		}
>  	}
> @@ -176,22 +174,21 @@ static const char *cros_ec_led_get_color_name(struct led_classdev_mc *led_mc_cde
>  	return led_get_color_name(color);
>  }
>  
> -static int cros_ec_led_probe_led(struct device *dev, struct cros_ec_device *cros_ec,
> +static int cros_ec_led_probe_one(struct device *dev, struct cros_ec_device *cros_ec,
>  				 enum ec_led_id id)
>  {
>  	union cros_ec_led_cmd_data arg = {};
>  	struct cros_ec_led_priv *priv;
>  	struct led_classdev *led_cdev;
>  	struct mc_subled *subleds;
> -	int ret, num_subleds;
> -	size_t i, subled;
> +	int i, ret, num_subleds;
> +	size_t subled;
>  
>  	arg.req.led_id = id;
>  	arg.req.flags = EC_LED_FLAGS_QUERY;
>  	ret = cros_ec_led_send_cmd(cros_ec, &arg);
> -	/* Unknown LED, skip */
>  	if (ret == -EINVAL)
> -		return 0;
> +		return 0; /* Unknown LED, skip */
>  	if (ret == -EOPNOTSUPP)
>  		return -ENODEV;
>  	if (ret < 0)
> @@ -247,11 +244,14 @@ static int cros_ec_led_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
>  	struct cros_ec_device *cros_ec = ec_dev->ec_dev;
> -	int ret = 0;
> -	size_t i;
> +	int i, ret = 0;
> +
> +	ret = devm_led_trigger_register(dev, &cros_ec_led_trigger);
> +	if (ret)
> +		return ret;
>  
>  	for (i = 0; i < EC_LED_ID_COUNT; i++) {
> -		ret = cros_ec_led_probe_led(dev, cros_ec, i);
> +		ret = cros_ec_led_probe_one(dev, cros_ec, i);
>  		if (ret)
>  			break;
>  	}
> @@ -260,38 +260,16 @@ static int cros_ec_led_probe(struct platform_device *pdev)
>  }
>  
>  static const struct platform_device_id cros_ec_led_id[] = {
> -	{ DRV_NAME, 0 },
> +	{ "cros-ec-led", 0 },
>  	{}
>  };
>  
>  static struct platform_driver cros_ec_led_driver = {
> -	.driver.name	= DRV_NAME,
> +	.driver.name	= "cros-ec-led",
>  	.probe		= cros_ec_led_probe,
>  	.id_table	= cros_ec_led_id,
>  };
> -
> -static int __init cros_ec_led_init(void)
> -{
> -	int ret;
> -
> -	ret = led_trigger_register(&cros_ec_led_trigger);
> -	if (ret)
> -		return ret;
> -
> -	ret = platform_driver_register(&cros_ec_led_driver);
> -	if (ret)
> -		led_trigger_unregister(&cros_ec_led_trigger);
> -
> -	return ret;
> -};
> -module_init(cros_ec_led_init);
> -
> -static void __exit cros_ec_led_exit(void)
> -{
> -	platform_driver_unregister(&cros_ec_led_driver);
> -	led_trigger_unregister(&cros_ec_led_trigger);
> -};
> -module_exit(cros_ec_led_exit);
> +module_platform_driver(cros_ec_led_driver);
>  
>  MODULE_DEVICE_TABLE(platform, cros_ec_led_id);
>  MODULE_DESCRIPTION("ChromeOS EC LED Driver");
> 
> ---
> base-commit: b6774dd948171c478c7aa19318b1d7ae9cf6d7a4
> change-id: 20240614-cros_ec-led-review-ec93abc65933
> 
> Best regards,
> -- 
> Thomas Weißschuh <linux@weissschuh.net>
> 

-- 
Lee Jones [李琼斯]

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

* Re: [PATCH] leds: cros_ec: Implement review comments from Lee
  2024-06-20  9:31 ` Lee Jones
@ 2024-06-20 10:12   ` Thomas Weißschuh
  2024-06-20 12:27     ` Lee Jones
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Weißschuh @ 2024-06-20 10:12 UTC (permalink / raw)
  To: Lee Jones
  Cc: Pavel Machek, Benson Leung, Guenter Roeck, linux-leds,
	chrome-platform, linux-kernel

Hi Lee,

On 2024-06-20 10:31:14+0000, Lee Jones wrote:
> Definitely not seen a commit message like that before

I assumed that this patch would be squashed into the original commit.

My question in which form you wanted the changes should have included
"incremental series".

> > Implement review comments from Lee as requested in [0] for
> > "leds: Add ChromeOS EC driver".
> > 
> > Changes:
> > * Inline DRV_NAME string constant.
> > * Use dev_err() instead of dev_warn() to report errors.
> > * Rename cros_ec_led_probe_led() to cros_ec_led_probe_one().
> > * Make loop variable "int" where they belong.
> > * Move "Unknown LED" comment to where it belongs.
> > * Register trigger during _probe().
> > * Use module_platform_driver() and drop all the custom boilerplate.
> 
> If you're fixing many things, then I would expect to receive many
> patches.  One patch for functional change please.  If you can reasonably
> group fixes of similar elk, then please do.  However one patch that does
> a bunch of different things is a no-go from me, sorry.

Absolutely, if these changes are to end up as actual commits then they
need to look different.
I'll resend them as proper series.

> > [0] https://lore.kernel.org/lkml/20240614093445.GF3029315@google.com/T/#m8750abdef6a968ace765645189170814196c9ce9
> > 
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> >  drivers/leds/leds-cros_ec.c | 50 +++++++++++++--------------------------------
> >  1 file changed, 14 insertions(+), 36 deletions(-)

<snip>

Sorry for the confusion,
Thomas

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

* Re: [PATCH] leds: cros_ec: Implement review comments from Lee
  2024-06-20 10:12   ` Thomas Weißschuh
@ 2024-06-20 12:27     ` Lee Jones
  2024-06-20 14:38       ` Thomas Weißschuh
  0 siblings, 1 reply; 10+ messages in thread
From: Lee Jones @ 2024-06-20 12:27 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Pavel Machek, Benson Leung, Guenter Roeck, linux-leds,
	chrome-platform, linux-kernel

On Thu, 20 Jun 2024, Thomas Weißschuh wrote:

> Hi Lee,
> 
> On 2024-06-20 10:31:14+0000, Lee Jones wrote:
> > Definitely not seen a commit message like that before
> 
> I assumed that this patch would be squashed into the original commit.
> 
> My question in which form you wanted the changes should have included
> "incremental series".

Incremental means on top.

A lot of maintainers don't support history re-writes, but I've reserved
that right since forever, so I can squash it if you like.

> > > Implement review comments from Lee as requested in [0] for
> > > "leds: Add ChromeOS EC driver".
> > > 
> > > Changes:
> > > * Inline DRV_NAME string constant.
> > > * Use dev_err() instead of dev_warn() to report errors.
> > > * Rename cros_ec_led_probe_led() to cros_ec_led_probe_one().
> > > * Make loop variable "int" where they belong.
> > > * Move "Unknown LED" comment to where it belongs.
> > > * Register trigger during _probe().
> > > * Use module_platform_driver() and drop all the custom boilerplate.
> > 
> > If you're fixing many things, then I would expect to receive many
> > patches.  One patch for functional change please.  If you can reasonably
> > group fixes of similar elk, then please do.  However one patch that does
> > a bunch of different things is a no-go from me, sorry.
> 
> Absolutely, if these changes are to end up as actual commits then they
> need to look different.
> I'll resend them as proper series.
> 
> > > [0] https://lore.kernel.org/lkml/20240614093445.GF3029315@google.com/T/#m8750abdef6a968ace765645189170814196c9ce9
> > > 
> > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > > ---
> > >  drivers/leds/leds-cros_ec.c | 50 +++++++++++++--------------------------------
> > >  1 file changed, 14 insertions(+), 36 deletions(-)
> 
> <snip>
> 
> Sorry for the confusion,
> Thomas

-- 
Lee Jones [李琼斯]

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

* Re: [PATCH] leds: cros_ec: Implement review comments from Lee
  2024-06-20 12:27     ` Lee Jones
@ 2024-06-20 14:38       ` Thomas Weißschuh
  2024-06-20 17:15         ` Lee Jones
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Weißschuh @ 2024-06-20 14:38 UTC (permalink / raw)
  To: Lee Jones
  Cc: Pavel Machek, Benson Leung, Guenter Roeck, linux-leds,
	chrome-platform, linux-kernel

On 2024-06-20 13:27:41+0000, Lee Jones wrote:
> On Thu, 20 Jun 2024, Thomas Weißschuh wrote:
> 
> > Hi Lee,
> > 
> > On 2024-06-20 10:31:14+0000, Lee Jones wrote:
> > > Definitely not seen a commit message like that before
> > 
> > I assumed that this patch would be squashed into the original commit.
> > 
> > My question in which form you wanted the changes should have included
> > "incremental series".
> 
> Incremental means on top.
> 
> A lot of maintainers don't support history re-writes, but I've reserved
> that right since forever, so I can squash it if you like.

If it is already visible somewhere and a squash would inconvenience
anybody I'll resend it.
But if not I'd be happy about a squash.

(I couldn't and still can't find the public tree where driver is in)

> > > > Implement review comments from Lee as requested in [0] for
> > > > "leds: Add ChromeOS EC driver".
> > > > 
> > > > Changes:
> > > > * Inline DRV_NAME string constant.
> > > > * Use dev_err() instead of dev_warn() to report errors.
> > > > * Rename cros_ec_led_probe_led() to cros_ec_led_probe_one().
> > > > * Make loop variable "int" where they belong.
> > > > * Move "Unknown LED" comment to where it belongs.
> > > > * Register trigger during _probe().
> > > > * Use module_platform_driver() and drop all the custom boilerplate.
> > > 
> > > If you're fixing many things, then I would expect to receive many
> > > patches.  One patch for functional change please.  If you can reasonably
> > > group fixes of similar elk, then please do.  However one patch that does
> > > a bunch of different things is a no-go from me, sorry.
> > 
> > Absolutely, if these changes are to end up as actual commits then they
> > need to look different.
> > I'll resend them as proper series.
> > 
> > > > [0] https://lore.kernel.org/lkml/20240614093445.GF3029315@google.com/T/#m8750abdef6a968ace765645189170814196c9ce9
> > > > 
> > > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > > > ---
> > > >  drivers/leds/leds-cros_ec.c | 50 +++++++++++++--------------------------------
> > > >  1 file changed, 14 insertions(+), 36 deletions(-)
> > 
> > <snip>
> > 
> > Sorry for the confusion,
> > Thomas
> 
> -- 
> Lee Jones [李琼斯]

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

* Re: (subset) [PATCH] leds: cros_ec: Implement review comments from Lee
  2024-06-14 10:04 [PATCH] leds: cros_ec: Implement review comments from Lee Thomas Weißschuh
  2024-06-20  9:31 ` Lee Jones
@ 2024-06-20 17:13 ` Lee Jones
  1 sibling, 0 replies; 10+ messages in thread
From: Lee Jones @ 2024-06-20 17:13 UTC (permalink / raw)
  To: Thomas Weißschuh, Pavel Machek, Lee Jones, Benson Leung,
	Guenter Roeck, Thomas Weißschuh
  Cc: linux-leds, chrome-platform, linux-kernel

On Fri, 14 Jun 2024 12:04:29 +0200, Thomas Weißschuh wrote:
> Implement review comments from Lee as requested in [0] for
> "leds: Add ChromeOS EC driver".
> 
> Changes:
> * Inline DRV_NAME string constant.
> * Use dev_err() instead of dev_warn() to report errors.
> * Rename cros_ec_led_probe_led() to cros_ec_led_probe_one().
> * Make loop variable "int" where they belong.
> * Move "Unknown LED" comment to where it belongs.
> * Register trigger during _probe().
> * Use module_platform_driver() and drop all the custom boilerplate.
> 
> [...]

Applied, thanks!

[1/1] leds: cros_ec: Implement review comments from Lee
      commit: 1b95b0039874092a24f9ee58c8bedf68485221a7

--
Lee Jones [李琼斯]


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

* Re: [PATCH] leds: cros_ec: Implement review comments from Lee
  2024-06-20 14:38       ` Thomas Weißschuh
@ 2024-06-20 17:15         ` Lee Jones
  2024-06-20 19:02           ` Thomas Weißschuh
  0 siblings, 1 reply; 10+ messages in thread
From: Lee Jones @ 2024-06-20 17:15 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Pavel Machek, Benson Leung, Guenter Roeck, linux-leds,
	chrome-platform, linux-kernel

On Thu, 20 Jun 2024, Thomas Weißschuh wrote:

> On 2024-06-20 13:27:41+0000, Lee Jones wrote:
> > On Thu, 20 Jun 2024, Thomas Weißschuh wrote:
> > 
> > > Hi Lee,
> > > 
> > > On 2024-06-20 10:31:14+0000, Lee Jones wrote:
> > > > Definitely not seen a commit message like that before
> > > 
> > > I assumed that this patch would be squashed into the original commit.
> > > 
> > > My question in which form you wanted the changes should have included
> > > "incremental series".
> > 
> > Incremental means on top.
> > 
> > A lot of maintainers don't support history re-writes, but I've reserved
> > that right since forever, so I can squash it if you like.
> 
> If it is already visible somewhere and a squash would inconvenience
> anybody I'll resend it.
> But if not I'd be happy about a squash.
> 
> (I couldn't and still can't find the public tree where driver is in)

Odd, neither can I!  Okay applied the whole set again, squashed the
patch and submitted for testing.

-- 
Lee Jones [李琼斯]

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

* Re: [PATCH] leds: cros_ec: Implement review comments from Lee
  2024-06-20 17:15         ` Lee Jones
@ 2024-06-20 19:02           ` Thomas Weißschuh
  2024-06-21 10:40             ` Lee Jones
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Weißschuh @ 2024-06-20 19:02 UTC (permalink / raw)
  To: Lee Jones
  Cc: Pavel Machek, Benson Leung, Guenter Roeck, linux-leds,
	chrome-platform, linux-kernel

On 2024-06-20 18:15:11+0000, Lee Jones wrote:
> On Thu, 20 Jun 2024, Thomas Weißschuh wrote:
> 
> > On 2024-06-20 13:27:41+0000, Lee Jones wrote:
> > > On Thu, 20 Jun 2024, Thomas Weißschuh wrote:
> > > 
> > > > Hi Lee,
> > > > 
> > > > On 2024-06-20 10:31:14+0000, Lee Jones wrote:
> > > > > Definitely not seen a commit message like that before
> > > > 
> > > > I assumed that this patch would be squashed into the original commit.
> > > > 
> > > > My question in which form you wanted the changes should have included
> > > > "incremental series".
> > > 
> > > Incremental means on top.
> > > 
> > > A lot of maintainers don't support history re-writes, but I've reserved
> > > that right since forever, so I can squash it if you like.
> > 
> > If it is already visible somewhere and a squash would inconvenience
> > anybody I'll resend it.
> > But if not I'd be happy about a squash.
> > 
> > (I couldn't and still can't find the public tree where driver is in)
> 
> Odd, neither can I!  Okay applied the whole set again, squashed the
> patch and submitted for testing.

Thanks!

FYI:

The Ack you asked for in the cros_kbd_led_backlight series [0],
which should go through the LED tree (and has a MFD component),
was given by Tzung-Bi in [1].

(In case it fell through the cracks. If not, please disregard)


Thomas

[0] https://lore.kernel.org/lkml/20240526-cros_ec-kbd-led-framework-v3-0-ee577415a521@weissschuh.net/
[1] https://lore.kernel.org/lkml/ZmutW6vh7pD1HLf5@google.com/

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

* Re: [PATCH] leds: cros_ec: Implement review comments from Lee
  2024-06-20 19:02           ` Thomas Weißschuh
@ 2024-06-21 10:40             ` Lee Jones
  2024-06-21 10:44               ` Thomas Weißschuh
  0 siblings, 1 reply; 10+ messages in thread
From: Lee Jones @ 2024-06-21 10:40 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Pavel Machek, Benson Leung, Guenter Roeck, linux-leds,
	chrome-platform, linux-kernel

On Thu, 20 Jun 2024, Thomas Weißschuh wrote:

> On 2024-06-20 18:15:11+0000, Lee Jones wrote:
> > On Thu, 20 Jun 2024, Thomas Weißschuh wrote:
> > 
> > > On 2024-06-20 13:27:41+0000, Lee Jones wrote:
> > > > On Thu, 20 Jun 2024, Thomas Weißschuh wrote:
> > > > 
> > > > > Hi Lee,
> > > > > 
> > > > > On 2024-06-20 10:31:14+0000, Lee Jones wrote:
> > > > > > Definitely not seen a commit message like that before
> > > > > 
> > > > > I assumed that this patch would be squashed into the original commit.
> > > > > 
> > > > > My question in which form you wanted the changes should have included
> > > > > "incremental series".
> > > > 
> > > > Incremental means on top.
> > > > 
> > > > A lot of maintainers don't support history re-writes, but I've reserved
> > > > that right since forever, so I can squash it if you like.
> > > 
> > > If it is already visible somewhere and a squash would inconvenience
> > > anybody I'll resend it.
> > > But if not I'd be happy about a squash.
> > > 
> > > (I couldn't and still can't find the public tree where driver is in)
> > 
> > Odd, neither can I!  Okay applied the whole set again, squashed the
> > patch and submitted for testing.
> 
> Thanks!
> 
> FYI:
> 
> The Ack you asked for in the cros_kbd_led_backlight series [0],
> which should go through the LED tree (and has a MFD component),
> was given by Tzung-Bi in [1].
> 
> (In case it fell through the cracks. If not, please disregard)

Now I'm really confused.

This patch not for that set though, right?

You're talking about:

 mfd: cros_ec: Register keyboard backlight subdevice
 platform/chrome: cros_kbd_led_backlight: allow binding through MFD     <-- this one
 leds: class: Add flag to avoid automatic renaming of LED devices
 leds: class: Warn about name collisions earlier

But this fix-up patch belongs in:

 mfd: cros_ec: Register LED subdevice
 leds: Add ChromeOS EC driver                                           <-- this one
 leds: core: Unexport led_colors[] array
 leds: multicolor: Use led_get_color_name() function
 leds: core: Introduce led_get_color_name() function

Right?

> [0] https://lore.kernel.org/lkml/20240526-cros_ec-kbd-led-framework-v3-0-ee577415a521@weissschuh.net/
> [1] https://lore.kernel.org/lkml/ZmutW6vh7pD1HLf5@google.com/

-- 
Lee Jones [李琼斯]

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

* Re: [PATCH] leds: cros_ec: Implement review comments from Lee
  2024-06-21 10:40             ` Lee Jones
@ 2024-06-21 10:44               ` Thomas Weißschuh
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Weißschuh @ 2024-06-21 10:44 UTC (permalink / raw)
  To: Lee Jones
  Cc: Pavel Machek, Benson Leung, Guenter Roeck, linux-leds,
	chrome-platform, linux-kernel

On 2024-06-21 11:40:38+0000, Lee Jones wrote:
> On Thu, 20 Jun 2024, Thomas Weißschuh wrote:
> 
> > On 2024-06-20 18:15:11+0000, Lee Jones wrote:
> > > On Thu, 20 Jun 2024, Thomas Weißschuh wrote:
> > > 
> > > > On 2024-06-20 13:27:41+0000, Lee Jones wrote:
> > > > > On Thu, 20 Jun 2024, Thomas Weißschuh wrote:
> > > > > 
> > > > > > Hi Lee,
> > > > > > 
> > > > > > On 2024-06-20 10:31:14+0000, Lee Jones wrote:
> > > > > > > Definitely not seen a commit message like that before
> > > > > > 
> > > > > > I assumed that this patch would be squashed into the original commit.
> > > > > > 
> > > > > > My question in which form you wanted the changes should have included
> > > > > > "incremental series".
> > > > > 
> > > > > Incremental means on top.
> > > > > 
> > > > > A lot of maintainers don't support history re-writes, but I've reserved
> > > > > that right since forever, so I can squash it if you like.
> > > > 
> > > > If it is already visible somewhere and a squash would inconvenience
> > > > anybody I'll resend it.
> > > > But if not I'd be happy about a squash.
> > > > 
> > > > (I couldn't and still can't find the public tree where driver is in)
> > > 
> > > Odd, neither can I!  Okay applied the whole set again, squashed the
> > > patch and submitted for testing.
> > 
> > Thanks!
> > 
> > FYI:
> > 
> > The Ack you asked for in the cros_kbd_led_backlight series [0],
> > which should go through the LED tree (and has a MFD component),
> > was given by Tzung-Bi in [1].
> > 
> > (In case it fell through the cracks. If not, please disregard)
> 
> Now I'm really confused.
> 
> This patch not for that set though, right?
> 
> You're talking about:
> 
>  mfd: cros_ec: Register keyboard backlight subdevice
>  platform/chrome: cros_kbd_led_backlight: allow binding through MFD     <-- this one
>  leds: class: Add flag to avoid automatic renaming of LED devices
>  leds: class: Warn about name collisions earlier
> 
> But this fix-up patch belongs in:
> 
>  mfd: cros_ec: Register LED subdevice
>  leds: Add ChromeOS EC driver                                           <-- this one
>  leds: core: Unexport led_colors[] array
>  leds: multicolor: Use led_get_color_name() function
>  leds: core: Introduce led_get_color_name() function
> 
> Right?

Yes, all correct.
Which is why I referred to it explicitly as the "cros_kbd_led_backlight series".

It was meant as a heads-up. Mentioned here as I was writing with you anyways.

> > [0] https://lore.kernel.org/lkml/20240526-cros_ec-kbd-led-framework-v3-0-ee577415a521@weissschuh.net/
> > [1] https://lore.kernel.org/lkml/ZmutW6vh7pD1HLf5@google.com/

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

end of thread, other threads:[~2024-06-21 10:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-14 10:04 [PATCH] leds: cros_ec: Implement review comments from Lee Thomas Weißschuh
2024-06-20  9:31 ` Lee Jones
2024-06-20 10:12   ` Thomas Weißschuh
2024-06-20 12:27     ` Lee Jones
2024-06-20 14:38       ` Thomas Weißschuh
2024-06-20 17:15         ` Lee Jones
2024-06-20 19:02           ` Thomas Weißschuh
2024-06-21 10:40             ` Lee Jones
2024-06-21 10:44               ` Thomas Weißschuh
2024-06-20 17:13 ` (subset) " Lee Jones

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