All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/6] platform/chrome: cros_ec: miscellaneous cleanups
@ 2022-02-09  9:56 Tzung-Bi Shih
  2022-02-09  9:56 ` [PATCH v3 1/6] platform/chrome: cros_ec: fix error handling in cros_ec_register() Tzung-Bi Shih
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Tzung-Bi Shih @ 2022-02-09  9:56 UTC (permalink / raw)
  To: bleung, groeck; +Cc: chrome-platform, tzungbi, pmalani

The 1st patch fixes unhandled undos in error handling path.

The rest of patches cleans drivers/platform/chrome/cros_ec.c.

Changes from v2:
(https://patchwork.kernel.org/project/chrome-platform/cover/20220209045035.380615-1-tzungbi@google.com/)
- Fix review comments in 1st and 2nd patch.

Changes from v1:
(https://lore.kernel.org/lkml/20220125101527.1812887-1-tzungbi@google.com/T/#u)
- Use imperative mood in commit messages.
- Use IS_ERR_OR_NULL() in 1st patch.

Tzung-Bi Shih (6):
  platform/chrome: cros_ec: fix error handling in cros_ec_register()
  platform/chrome: cros_ec: remove unused variable `was_wake_device`
  platform/chrome: cros_ec: determine `wake_enabled` in
    cros_ec_suspend()
  platform/chrome: cros_ec: don't initialize `err` in cros_ec_register()
  platform/chrome: cros_ec: sort header inclusion alphabetically
  platform/chrome: cros_ec: append newline to all logs

 drivers/platform/chrome/cros_ec.c           | 37 +++++++++++----------
 include/linux/platform_data/cros_ec_proto.h |  3 --
 2 files changed, 20 insertions(+), 20 deletions(-)

-- 
2.35.0.263.gb82422642f-goog


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

* [PATCH v3 1/6] platform/chrome: cros_ec: fix error handling in cros_ec_register()
  2022-02-09  9:56 [PATCH v3 0/6] platform/chrome: cros_ec: miscellaneous cleanups Tzung-Bi Shih
@ 2022-02-09  9:56 ` Tzung-Bi Shih
  2022-02-15 21:39   ` Prashant Malani
  2022-02-09  9:56 ` [PATCH v3 2/6] platform/chrome: cros_ec: remove unused variable `was_wake_device` Tzung-Bi Shih
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Tzung-Bi Shih @ 2022-02-09  9:56 UTC (permalink / raw)
  To: bleung, groeck; +Cc: chrome-platform, tzungbi, pmalani

Fix cros_ec_register() to unregister platform devices if
blocking_notifier_chain_register() fails.

Also use the single exit path to handle the platform device
unregistration.

Fixes: 42cd0ab476e2 ("platform/chrome: cros_ec: Query EC protocol version if EC transitions between RO/RW")
Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
Changes from v2:
(https://patchwork.kernel.org/project/chrome-platform/patch/20220209045035.380615-2-tzungbi@google.com/)
- Fix grammar error in commit message.
- Change the code that don't rely on zeroed memory.
- Remove unnecessary `if` checks before calling platform_device_unregister().

Changes from v1:
(https://lore.kernel.org/lkml/20220125101527.1812887-1-tzungbi@google.com/T/#u)
- Use imperative mood in commit message.
- Use IS_ERR_OR_NULL() in 1st patch.

 drivers/platform/chrome/cros_ec.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index fc5aa1525d13..15f599d721a1 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -245,18 +245,16 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 		if (IS_ERR(ec_dev->pd)) {
 			dev_err(ec_dev->dev,
 				"Failed to create CrOS PD platform device\n");
-			platform_device_unregister(ec_dev->ec);
-			return PTR_ERR(ec_dev->pd);
+			err = PTR_ERR(ec_dev->pd);
+			goto exit;
 		}
 	}
 
 	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
 		err = devm_of_platform_populate(dev);
 		if (err) {
-			platform_device_unregister(ec_dev->pd);
-			platform_device_unregister(ec_dev->ec);
 			dev_err(dev, "Failed to register sub-devices\n");
-			return err;
+			goto exit;
 		}
 	}
 
@@ -278,7 +276,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 		err = blocking_notifier_chain_register(&ec_dev->event_notifier,
 						      &ec_dev->notifier_ready);
 		if (err)
-			return err;
+			goto exit;
 	}
 
 	dev_info(dev, "Chrome EC device registered\n");
@@ -291,6 +289,11 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 		cros_ec_irq_thread(0, ec_dev);
 
 	return 0;
+exit:
+	if (ec_dev->max_passthru)
+		platform_device_unregister(ec_dev->pd);
+	platform_device_unregister(ec_dev->ec);
+	return err;
 }
 EXPORT_SYMBOL(cros_ec_register);
 
-- 
2.35.0.263.gb82422642f-goog


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

* [PATCH v3 2/6] platform/chrome: cros_ec: remove unused variable `was_wake_device`
  2022-02-09  9:56 [PATCH v3 0/6] platform/chrome: cros_ec: miscellaneous cleanups Tzung-Bi Shih
  2022-02-09  9:56 ` [PATCH v3 1/6] platform/chrome: cros_ec: fix error handling in cros_ec_register() Tzung-Bi Shih
@ 2022-02-09  9:56 ` Tzung-Bi Shih
  2022-02-09  9:57 ` [PATCH v3 3/6] platform/chrome: cros_ec: determine `wake_enabled` in cros_ec_suspend() Tzung-Bi Shih
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Tzung-Bi Shih @ 2022-02-09  9:56 UTC (permalink / raw)
  To: bleung, groeck; +Cc: chrome-platform, tzungbi, pmalani

Reviewed-by: Prashant Malani <pmalani@chromium.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
Changes from v2:
(https://patchwork.kernel.org/project/chrome-platform/patch/20220209045035.380615-3-tzungbi@google.com/)
- Add pmalani's R-b tag.
- Remove redundant commit message.

Changes from v1:
(https://lore.kernel.org/lkml/20220125101527.1812887-1-tzungbi@google.com/T/#u)
- Use imperative mood in commit message.

 drivers/platform/chrome/cros_ec.c           | 1 -
 include/linux/platform_data/cros_ec_proto.h | 3 ---
 2 files changed, 4 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index 15f599d721a1..c9c90f98baf4 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -343,7 +343,6 @@ int cros_ec_suspend(struct cros_ec_device *ec_dev)
 		ec_dev->wake_enabled = !enable_irq_wake(ec_dev->irq);
 
 	disable_irq(ec_dev->irq);
-	ec_dev->was_wake_device = ec_dev->wake_enabled;
 	ec_dev->suspended = true;
 
 	return 0;
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
index df3c78c92ca2..c65971ec90ea 100644
--- a/include/linux/platform_data/cros_ec_proto.h
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -76,8 +76,6 @@ struct cros_ec_command {
  * struct cros_ec_device - Information about a ChromeOS EC device.
  * @phys_name: Name of physical comms layer (e.g. 'i2c-4').
  * @dev: Device pointer for physical comms device
- * @was_wake_device: True if this device was set to wake the system from
- *                   sleep at the last suspend.
  * @cros_class: The class structure for this device.
  * @cmd_readmem: Direct read of the EC memory-mapped region, if supported.
  *     @offset: Is within EC_LPC_ADDR_MEMMAP region.
@@ -137,7 +135,6 @@ struct cros_ec_device {
 	/* These are used by other drivers that want to talk to the EC */
 	const char *phys_name;
 	struct device *dev;
-	bool was_wake_device;
 	struct class *cros_class;
 	int (*cmd_readmem)(struct cros_ec_device *ec, unsigned int offset,
 			   unsigned int bytes, void *dest);
-- 
2.35.0.263.gb82422642f-goog


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

* [PATCH v3 3/6] platform/chrome: cros_ec: determine `wake_enabled` in cros_ec_suspend()
  2022-02-09  9:56 [PATCH v3 0/6] platform/chrome: cros_ec: miscellaneous cleanups Tzung-Bi Shih
  2022-02-09  9:56 ` [PATCH v3 1/6] platform/chrome: cros_ec: fix error handling in cros_ec_register() Tzung-Bi Shih
  2022-02-09  9:56 ` [PATCH v3 2/6] platform/chrome: cros_ec: remove unused variable `was_wake_device` Tzung-Bi Shih
@ 2022-02-09  9:57 ` Tzung-Bi Shih
  2022-02-09  9:57 ` [PATCH v3 4/6] platform/chrome: cros_ec: don't initialize `err` in cros_ec_register() Tzung-Bi Shih
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Tzung-Bi Shih @ 2022-02-09  9:57 UTC (permalink / raw)
  To: bleung, groeck; +Cc: chrome-platform, tzungbi, pmalani

`wake_enabled` indicates cros_ec_resume() needs to call
disable_irq_wake() to undo enable_irq_wake() in cros_ec_suspend().

Determine `wake_enabled` in cros_ec_suspend() instead of
reset-after-used in cros_ec_resume().

Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
No changes from v2.

Changes from v1:
(https://lore.kernel.org/lkml/20220125101527.1812887-1-tzungbi@google.com/T/#u)
- Use imperative mood in commit message.

 drivers/platform/chrome/cros_ec.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index c9c90f98baf4..07dd5cc35d7c 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -341,6 +341,8 @@ int cros_ec_suspend(struct cros_ec_device *ec_dev)
 
 	if (device_may_wakeup(dev))
 		ec_dev->wake_enabled = !enable_irq_wake(ec_dev->irq);
+	else
+		ec_dev->wake_enabled = false;
 
 	disable_irq(ec_dev->irq);
 	ec_dev->suspended = true;
@@ -382,10 +384,9 @@ int cros_ec_resume(struct cros_ec_device *ec_dev)
 		dev_dbg(ec_dev->dev, "Error %d sending resume event to ec",
 			ret);
 
-	if (ec_dev->wake_enabled) {
+	if (ec_dev->wake_enabled)
 		disable_irq_wake(ec_dev->irq);
-		ec_dev->wake_enabled = 0;
-	}
+
 	/*
 	 * Let the mfd devices know about events that occur during
 	 * suspend. This way the clients know what to do with them.
-- 
2.35.0.263.gb82422642f-goog


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

* [PATCH v3 4/6] platform/chrome: cros_ec: don't initialize `err` in cros_ec_register()
  2022-02-09  9:56 [PATCH v3 0/6] platform/chrome: cros_ec: miscellaneous cleanups Tzung-Bi Shih
                   ` (2 preceding siblings ...)
  2022-02-09  9:57 ` [PATCH v3 3/6] platform/chrome: cros_ec: determine `wake_enabled` in cros_ec_suspend() Tzung-Bi Shih
@ 2022-02-09  9:57 ` Tzung-Bi Shih
  2022-02-15 21:39   ` Prashant Malani
  2022-02-09  9:57 ` [PATCH v3 5/6] platform/chrome: cros_ec: sort header inclusion alphabetically Tzung-Bi Shih
  2022-02-09  9:57 ` [PATCH v3 6/6] platform/chrome: cros_ec: append newline to all logs Tzung-Bi Shih
  5 siblings, 1 reply; 13+ messages in thread
From: Tzung-Bi Shih @ 2022-02-09  9:57 UTC (permalink / raw)
  To: bleung, groeck; +Cc: chrome-platform, tzungbi, pmalani

`err` in cros_ec_register() doesn't need to be initialized because it is going
to be overridden in the following code soon.

Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
No changes from v2.

Changes from v1:
(https://lore.kernel.org/lkml/20220125101527.1812887-1-tzungbi@google.com/T/#u)
- Use imperative mood in commit message.

 drivers/platform/chrome/cros_ec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index 07dd5cc35d7c..618b8b4c42ef 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -182,7 +182,7 @@ static int cros_ec_ready_event(struct notifier_block *nb,
 int cros_ec_register(struct cros_ec_device *ec_dev)
 {
 	struct device *dev = ec_dev->dev;
-	int err = 0;
+	int err;
 
 	BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
 
-- 
2.35.0.263.gb82422642f-goog


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

* [PATCH v3 5/6] platform/chrome: cros_ec: sort header inclusion alphabetically
  2022-02-09  9:56 [PATCH v3 0/6] platform/chrome: cros_ec: miscellaneous cleanups Tzung-Bi Shih
                   ` (3 preceding siblings ...)
  2022-02-09  9:57 ` [PATCH v3 4/6] platform/chrome: cros_ec: don't initialize `err` in cros_ec_register() Tzung-Bi Shih
@ 2022-02-09  9:57 ` Tzung-Bi Shih
  2022-02-15 21:40   ` Prashant Malani
  2022-02-09  9:57 ` [PATCH v3 6/6] platform/chrome: cros_ec: append newline to all logs Tzung-Bi Shih
  5 siblings, 1 reply; 13+ messages in thread
From: Tzung-Bi Shih @ 2022-02-09  9:57 UTC (permalink / raw)
  To: bleung, groeck; +Cc: chrome-platform, tzungbi, pmalani

Sort header inclusion alphabetically.

Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
No changes from v2.

Changes from v1:
(https://lore.kernel.org/lkml/20220125101527.1812887-1-tzungbi@google.com/T/#u)
- Use imperative mood in commit message.

 drivers/platform/chrome/cros_ec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index 618b8b4c42ef..2c4e81f3fef9 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -9,12 +9,12 @@
  * battery charging and regulator control, firmware update.
  */
 
-#include <linux/of_platform.h>
 #include <linux/interrupt.h>
-#include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/of_platform.h>
 #include <linux/platform_data/cros_ec_commands.h>
 #include <linux/platform_data/cros_ec_proto.h>
+#include <linux/slab.h>
 #include <linux/suspend.h>
 
 #include "cros_ec.h"
-- 
2.35.0.263.gb82422642f-goog


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

* [PATCH v3 6/6] platform/chrome: cros_ec: append newline to all logs
  2022-02-09  9:56 [PATCH v3 0/6] platform/chrome: cros_ec: miscellaneous cleanups Tzung-Bi Shih
                   ` (4 preceding siblings ...)
  2022-02-09  9:57 ` [PATCH v3 5/6] platform/chrome: cros_ec: sort header inclusion alphabetically Tzung-Bi Shih
@ 2022-02-09  9:57 ` Tzung-Bi Shih
  2022-02-15 21:40   ` Prashant Malani
  5 siblings, 1 reply; 13+ messages in thread
From: Tzung-Bi Shih @ 2022-02-09  9:57 UTC (permalink / raw)
  To: bleung, groeck; +Cc: chrome-platform, tzungbi, pmalani

To be consistent, append newline ("\n") to all logs.

Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
No changes from v2.

Changes from v1:
(https://lore.kernel.org/lkml/20220125101527.1812887-1-tzungbi@google.com/T/#u)
- Use imperative mood in commit message.

 drivers/platform/chrome/cros_ec.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index 2c4e81f3fef9..639d5a651b76 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -213,7 +213,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
 						"chromeos-ec", ec_dev);
 		if (err) {
-			dev_err(dev, "Failed to request IRQ %d: %d",
+			dev_err(dev, "Failed to request IRQ %d: %d\n",
 				ec_dev->irq, err);
 			return err;
 		}
@@ -264,7 +264,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 	 */
 	err = cros_ec_sleep_event(ec_dev, 0);
 	if (err < 0)
-		dev_dbg(ec_dev->dev, "Error %d clearing sleep event to ec",
+		dev_dbg(ec_dev->dev, "Error %d clearing sleep event to ec\n",
 			err);
 
 	if (ec_dev->mkbp_event_supported) {
@@ -336,7 +336,7 @@ int cros_ec_suspend(struct cros_ec_device *ec_dev)
 
 	ret = cros_ec_sleep_event(ec_dev, sleep_event);
 	if (ret < 0)
-		dev_dbg(ec_dev->dev, "Error %d sending suspend event to ec",
+		dev_dbg(ec_dev->dev, "Error %d sending suspend event to ec\n",
 			ret);
 
 	if (device_may_wakeup(dev))
@@ -381,7 +381,7 @@ int cros_ec_resume(struct cros_ec_device *ec_dev)
 
 	ret = cros_ec_sleep_event(ec_dev, sleep_event);
 	if (ret < 0)
-		dev_dbg(ec_dev->dev, "Error %d sending resume event to ec",
+		dev_dbg(ec_dev->dev, "Error %d sending resume event to ec\n",
 			ret);
 
 	if (ec_dev->wake_enabled)
-- 
2.35.0.263.gb82422642f-goog


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

* Re: [PATCH v3 1/6] platform/chrome: cros_ec: fix error handling in cros_ec_register()
  2022-02-09  9:56 ` [PATCH v3 1/6] platform/chrome: cros_ec: fix error handling in cros_ec_register() Tzung-Bi Shih
@ 2022-02-15 21:39   ` Prashant Malani
  2022-02-16  4:11     ` Tzung-Bi Shih
  0 siblings, 1 reply; 13+ messages in thread
From: Prashant Malani @ 2022-02-15 21:39 UTC (permalink / raw)
  To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform

On Feb 09 17:56, Tzung-Bi Shih wrote:
> Fix cros_ec_register() to unregister platform devices if
> blocking_notifier_chain_register() fails.
>
> Also use the single exit path to handle the platform device
> unregistration.
>
> Fixes: 42cd0ab476e2 ("platform/chrome: cros_ec: Query EC protocol version if EC transitions between RO/RW")
> Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
> ---
> Changes from v2:
> (https://patchwork.kernel.org/project/chrome-platform/patch/20220209045035.380615-2-tzungbi@google.com/)
> - Fix grammar error in commit message.
> - Change the code that don't rely on zeroed memory.
> - Remove unnecessary `if` checks before calling platform_device_unregister().
>
> Changes from v1:
> (https://lore.kernel.org/lkml/20220125101527.1812887-1-tzungbi@google.com/T/#u)
> - Use imperative mood in commit message.
> - Use IS_ERR_OR_NULL() in 1st patch.
>
>  drivers/platform/chrome/cros_ec.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
> index fc5aa1525d13..15f599d721a1 100644
> --- a/drivers/platform/chrome/cros_ec.c
> +++ b/drivers/platform/chrome/cros_ec.c
> @@ -245,18 +245,16 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>               if (IS_ERR(ec_dev->pd)) {
>                       dev_err(ec_dev->dev,
>                               "Failed to create CrOS PD platform device\n");
> -                     platform_device_unregister(ec_dev->ec);
> -                     return PTR_ERR(ec_dev->pd);
> +                     err = PTR_ERR(ec_dev->pd);
> +                     goto exit;
>               }
>       }
>
>       if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
>               err = devm_of_platform_populate(dev);
>               if (err) {
> -                     platform_device_unregister(ec_dev->pd);
> -                     platform_device_unregister(ec_dev->ec);
>                       dev_err(dev, "Failed to register sub-devices\n");
> -                     return err;
> +                     goto exit;
>               }
>       }
>
> @@ -278,7 +276,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>               err = blocking_notifier_chain_register(&ec_dev->event_notifier,
>                                                     &ec_dev->notifier_ready);
>               if (err)
> -                     return err;
> +                     goto exit;
>       }
>
>       dev_info(dev, "Chrome EC device registered\n");
> @@ -291,6 +289,11 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>               cros_ec_irq_thread(0, ec_dev);
>
>       return 0;
> +exit:
> +     if (ec_dev->max_passthru)

This just complicates things, by depending on another variable. Just initialize
ec_dev->pd and ec_dev->ec to NULL at the beginning instead.


> +             platform_device_unregister(ec_dev->pd);
> +     platform_device_unregister(ec_dev->ec);
> +     return err;
>  }
>  EXPORT_SYMBOL(cros_ec_register);
>
> --
> 2.35.0.263.gb82422642f-goog
>

-Prashant

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

* Re: [PATCH v3 4/6] platform/chrome: cros_ec: don't initialize `err` in cros_ec_register()
  2022-02-09  9:57 ` [PATCH v3 4/6] platform/chrome: cros_ec: don't initialize `err` in cros_ec_register() Tzung-Bi Shih
@ 2022-02-15 21:39   ` Prashant Malani
  2022-02-16  4:12     ` Tzung-Bi Shih
  0 siblings, 1 reply; 13+ messages in thread
From: Prashant Malani @ 2022-02-15 21:39 UTC (permalink / raw)
  To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform

On Feb 09 17:57, Tzung-Bi Shih wrote:
> `err` in cros_ec_register() doesn't need to be initialized because it is going
> to be overridden in the following code soon.

As mentioned in the previous version [1], please drop this patch, since
it is not necessary (doesn't meaningfully improve readability, doesn't
fix any bugs).

[1] https://lore.kernel.org/chrome-platform/CACeCKaeRz2D-DdPRHbrvywTMr2w-MPuGZ+-mBhNtWCapgm=ZXw@mail.gmail.com/

>
> Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
> ---
> No changes from v2.
>
> Changes from v1:
> (https://lore.kernel.org/lkml/20220125101527.1812887-1-tzungbi@google.com/T/#u)
> - Use imperative mood in commit message.
>
>  drivers/platform/chrome/cros_ec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
> index 07dd5cc35d7c..618b8b4c42ef 100644
> --- a/drivers/platform/chrome/cros_ec.c
> +++ b/drivers/platform/chrome/cros_ec.c
> @@ -182,7 +182,7 @@ static int cros_ec_ready_event(struct notifier_block *nb,
>  int cros_ec_register(struct cros_ec_device *ec_dev)
>  {
>       struct device *dev = ec_dev->dev;
> -     int err = 0;
> +     int err;
>
>       BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
>
> --
> 2.35.0.263.gb82422642f-goog
>

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

* Re: [PATCH v3 5/6] platform/chrome: cros_ec: sort header inclusion alphabetically
  2022-02-09  9:57 ` [PATCH v3 5/6] platform/chrome: cros_ec: sort header inclusion alphabetically Tzung-Bi Shih
@ 2022-02-15 21:40   ` Prashant Malani
  0 siblings, 0 replies; 13+ messages in thread
From: Prashant Malani @ 2022-02-15 21:40 UTC (permalink / raw)
  To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform

On Feb 09 17:57, Tzung-Bi Shih wrote:
> Sort header inclusion alphabetically.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
Reviewed-by: Prashant Malani <pmalani@chromium.org>


> ---
> No changes from v2.
>
> Changes from v1:
> (https://lore.kernel.org/lkml/20220125101527.1812887-1-tzungbi@google.com/T/#u)
> - Use imperative mood in commit message.
>
>  drivers/platform/chrome/cros_ec.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
> index 618b8b4c42ef..2c4e81f3fef9 100644
> --- a/drivers/platform/chrome/cros_ec.c
> +++ b/drivers/platform/chrome/cros_ec.c
> @@ -9,12 +9,12 @@
>   * battery charging and regulator control, firmware update.
>   */
>
> -#include <linux/of_platform.h>
>  #include <linux/interrupt.h>
> -#include <linux/slab.h>
>  #include <linux/module.h>
> +#include <linux/of_platform.h>
>  #include <linux/platform_data/cros_ec_commands.h>
>  #include <linux/platform_data/cros_ec_proto.h>
> +#include <linux/slab.h>
>  #include <linux/suspend.h>
>
>  #include "cros_ec.h"
> --
> 2.35.0.263.gb82422642f-goog
>

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

* Re: [PATCH v3 6/6] platform/chrome: cros_ec: append newline to all logs
  2022-02-09  9:57 ` [PATCH v3 6/6] platform/chrome: cros_ec: append newline to all logs Tzung-Bi Shih
@ 2022-02-15 21:40   ` Prashant Malani
  0 siblings, 0 replies; 13+ messages in thread
From: Prashant Malani @ 2022-02-15 21:40 UTC (permalink / raw)
  To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform

On Feb 09 17:57, Tzung-Bi Shih wrote:
> To be consistent, append newline ("\n") to all logs.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
Reviewed-by: Prashant Malani <pmalani@chromium.org>


> ---
> No changes from v2.
>
> Changes from v1:
> (https://lore.kernel.org/lkml/20220125101527.1812887-1-tzungbi@google.com/T/#u)
> - Use imperative mood in commit message.
>
>  drivers/platform/chrome/cros_ec.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
> index 2c4e81f3fef9..639d5a651b76 100644
> --- a/drivers/platform/chrome/cros_ec.c
> +++ b/drivers/platform/chrome/cros_ec.c
> @@ -213,7 +213,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>                                               IRQF_TRIGGER_LOW | IRQF_ONESHOT,
>                                               "chromeos-ec", ec_dev);
>               if (err) {
> -                     dev_err(dev, "Failed to request IRQ %d: %d",
> +                     dev_err(dev, "Failed to request IRQ %d: %d\n",
>                               ec_dev->irq, err);
>                       return err;
>               }
> @@ -264,7 +264,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>        */
>       err = cros_ec_sleep_event(ec_dev, 0);
>       if (err < 0)
> -             dev_dbg(ec_dev->dev, "Error %d clearing sleep event to ec",
> +             dev_dbg(ec_dev->dev, "Error %d clearing sleep event to ec\n",
>                       err);
>
>       if (ec_dev->mkbp_event_supported) {
> @@ -336,7 +336,7 @@ int cros_ec_suspend(struct cros_ec_device *ec_dev)
>
>       ret = cros_ec_sleep_event(ec_dev, sleep_event);
>       if (ret < 0)
> -             dev_dbg(ec_dev->dev, "Error %d sending suspend event to ec",
> +             dev_dbg(ec_dev->dev, "Error %d sending suspend event to ec\n",
>                       ret);
>
>       if (device_may_wakeup(dev))
> @@ -381,7 +381,7 @@ int cros_ec_resume(struct cros_ec_device *ec_dev)
>
>       ret = cros_ec_sleep_event(ec_dev, sleep_event);
>       if (ret < 0)
> -             dev_dbg(ec_dev->dev, "Error %d sending resume event to ec",
> +             dev_dbg(ec_dev->dev, "Error %d sending resume event to ec\n",
>                       ret);
>
>       if (ec_dev->wake_enabled)
> --
> 2.35.0.263.gb82422642f-goog
>

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

* Re: [PATCH v3 1/6] platform/chrome: cros_ec: fix error handling in cros_ec_register()
  2022-02-15 21:39   ` Prashant Malani
@ 2022-02-16  4:11     ` Tzung-Bi Shih
  0 siblings, 0 replies; 13+ messages in thread
From: Tzung-Bi Shih @ 2022-02-16  4:11 UTC (permalink / raw)
  To: Prashant Malani; +Cc: bleung, groeck, chrome-platform

On Tue, Feb 15, 2022 at 01:39:51PM -0800, Prashant Malani wrote:
> On Feb 09 17:56, Tzung-Bi Shih wrote:
> > +exit:
> > +     if (ec_dev->max_passthru)
> 
> This just complicates things, by depending on another variable. Just initialize
> ec_dev->pd and ec_dev->ec to NULL at the beginning instead.

Ack, will fix in the next version.

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

* Re: [PATCH v3 4/6] platform/chrome: cros_ec: don't initialize `err` in cros_ec_register()
  2022-02-15 21:39   ` Prashant Malani
@ 2022-02-16  4:12     ` Tzung-Bi Shih
  0 siblings, 0 replies; 13+ messages in thread
From: Tzung-Bi Shih @ 2022-02-16  4:12 UTC (permalink / raw)
  To: Prashant Malani; +Cc: bleung, groeck, chrome-platform

On Tue, Feb 15, 2022 at 01:39:57PM -0800, Prashant Malani wrote:
> On Feb 09 17:57, Tzung-Bi Shih wrote:
> > `err` in cros_ec_register() doesn't need to be initialized because it is going
> > to be overridden in the following code soon.
> 
> As mentioned in the previous version [1], please drop this patch, since
> it is not necessary (doesn't meaningfully improve readability, doesn't
> fix any bugs).
> 
> [1] https://lore.kernel.org/chrome-platform/CACeCKaeRz2D-DdPRHbrvywTMr2w-MPuGZ+-mBhNtWCapgm=ZXw@mail.gmail.com/

Ack, will drop it in the next version.

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

end of thread, other threads:[~2022-02-16  4:12 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-09  9:56 [PATCH v3 0/6] platform/chrome: cros_ec: miscellaneous cleanups Tzung-Bi Shih
2022-02-09  9:56 ` [PATCH v3 1/6] platform/chrome: cros_ec: fix error handling in cros_ec_register() Tzung-Bi Shih
2022-02-15 21:39   ` Prashant Malani
2022-02-16  4:11     ` Tzung-Bi Shih
2022-02-09  9:56 ` [PATCH v3 2/6] platform/chrome: cros_ec: remove unused variable `was_wake_device` Tzung-Bi Shih
2022-02-09  9:57 ` [PATCH v3 3/6] platform/chrome: cros_ec: determine `wake_enabled` in cros_ec_suspend() Tzung-Bi Shih
2022-02-09  9:57 ` [PATCH v3 4/6] platform/chrome: cros_ec: don't initialize `err` in cros_ec_register() Tzung-Bi Shih
2022-02-15 21:39   ` Prashant Malani
2022-02-16  4:12     ` Tzung-Bi Shih
2022-02-09  9:57 ` [PATCH v3 5/6] platform/chrome: cros_ec: sort header inclusion alphabetically Tzung-Bi Shih
2022-02-15 21:40   ` Prashant Malani
2022-02-09  9:57 ` [PATCH v3 6/6] platform/chrome: cros_ec: append newline to all logs Tzung-Bi Shih
2022-02-15 21:40   ` Prashant Malani

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.