* [PATCH v4 0/5] platform/chrome: cros_ec: miscellaneous cleanups
@ 2022-02-16 4:36 Tzung-Bi Shih
2022-02-16 4:36 ` [PATCH v4 1/5] platform/chrome: cros_ec: fix error handling in cros_ec_register() Tzung-Bi Shih
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Tzung-Bi Shih @ 2022-02-16 4:36 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, tzungbi, pmalani, linux-kernel
The 1st patch fixes unhandled undos in error handling path.
The rest of patches cleans drivers/platform/chrome/cros_ec.c.
Changes from v3:
(https://patchwork.kernel.org/project/chrome-platform/cover/20220209095703.517608-1-tzungbi@google.com/)
- Drop "platform/chrome: cros_ec: don't initialize `err` in cros_ec_register()".
- Rename the 3rd patch's title.
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 (5):
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: initialize `wake_enabled` 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 | 35 +++++++++++----------
include/linux/platform_data/cros_ec_proto.h | 3 --
2 files changed, 19 insertions(+), 19 deletions(-)
--
2.35.1.265.g69c8d7142f-goog
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 1/5] platform/chrome: cros_ec: fix error handling in cros_ec_register()
2022-02-16 4:36 [PATCH v4 0/5] platform/chrome: cros_ec: miscellaneous cleanups Tzung-Bi Shih
@ 2022-02-16 4:36 ` Tzung-Bi Shih
2022-02-16 5:44 ` Prashant Malani
2022-02-16 4:36 ` [PATCH v4 2/5] platform/chrome: cros_ec: remove unused variable `was_wake_device` Tzung-Bi Shih
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Tzung-Bi Shih @ 2022-02-16 4:36 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, tzungbi, pmalani, linux-kernel
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 v3:
(https://patchwork.kernel.org/project/chrome-platform/patch/20220209095703.517608-2-tzungbi@google.com/)
- Simplify by initializing the variables at the beginning.
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 | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index fc5aa1525d13..ff2a24b0c611 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -189,6 +189,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
ec_dev->max_request = sizeof(struct ec_params_hello);
ec_dev->max_response = sizeof(struct ec_response_get_protocol_info);
ec_dev->max_passthru = 0;
+ ec_dev->ec = NULL;
+ ec_dev->pd = NULL;
ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
if (!ec_dev->din)
@@ -245,18 +247,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 +278,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 +291,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
cros_ec_irq_thread(0, ec_dev);
return 0;
+exit:
+ platform_device_unregister(ec_dev->ec);
+ platform_device_unregister(ec_dev->pd);
+ return err;
}
EXPORT_SYMBOL(cros_ec_register);
--
2.35.1.265.g69c8d7142f-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 2/5] platform/chrome: cros_ec: remove unused variable `was_wake_device`
2022-02-16 4:36 [PATCH v4 0/5] platform/chrome: cros_ec: miscellaneous cleanups Tzung-Bi Shih
2022-02-16 4:36 ` [PATCH v4 1/5] platform/chrome: cros_ec: fix error handling in cros_ec_register() Tzung-Bi Shih
@ 2022-02-16 4:36 ` Tzung-Bi Shih
2022-02-16 4:36 ` [PATCH v4 3/5] platform/chrome: cros_ec: initialize `wake_enabled` in cros_ec_register() Tzung-Bi Shih
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Tzung-Bi Shih @ 2022-02-16 4:36 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, tzungbi, pmalani, linux-kernel
Reviewed-by: Prashant Malani <pmalani@chromium.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
No changes from v3.
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 ff2a24b0c611..25cd8df6e7b0 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -344,7 +344,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.1.265.g69c8d7142f-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 3/5] platform/chrome: cros_ec: initialize `wake_enabled` in cros_ec_register()
2022-02-16 4:36 [PATCH v4 0/5] platform/chrome: cros_ec: miscellaneous cleanups Tzung-Bi Shih
2022-02-16 4:36 ` [PATCH v4 1/5] platform/chrome: cros_ec: fix error handling in cros_ec_register() Tzung-Bi Shih
2022-02-16 4:36 ` [PATCH v4 2/5] platform/chrome: cros_ec: remove unused variable `was_wake_device` Tzung-Bi Shih
@ 2022-02-16 4:36 ` Tzung-Bi Shih
2022-02-16 5:47 ` Prashant Malani
2022-02-16 4:36 ` [PATCH v4 4/5] platform/chrome: cros_ec: sort header inclusion alphabetically Tzung-Bi Shih
2022-02-16 4:36 ` [PATCH v4 5/5] platform/chrome: cros_ec: append newline to all logs Tzung-Bi Shih
4 siblings, 1 reply; 9+ messages in thread
From: Tzung-Bi Shih @ 2022-02-16 4:36 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, tzungbi, pmalani, linux-kernel
`wake_enabled` indicates cros_ec_resume() needs to call
disable_irq_wake() to undo enable_irq_wake() in cros_ec_suspend().
Initialize `wake_enabled` in cros_ec_register() and determine the flag
in cros_ec_suspend() instead of reset-after-used in cros_ec_resume().
Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
Changes from v3:
(https://patchwork.kernel.org/project/chrome-platform/patch/20220209095703.517608-4-tzungbi@google.com/)
- Change the patch title.
- Simplify by initializing wake_enabled in cros_ec_register().
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 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index 25cd8df6e7b0..b6604a9ab315 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -191,6 +191,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
ec_dev->max_passthru = 0;
ec_dev->ec = NULL;
ec_dev->pd = NULL;
+ ec_dev->wake_enabled = false;
ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
if (!ec_dev->din)
@@ -383,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.1.265.g69c8d7142f-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 4/5] platform/chrome: cros_ec: sort header inclusion alphabetically
2022-02-16 4:36 [PATCH v4 0/5] platform/chrome: cros_ec: miscellaneous cleanups Tzung-Bi Shih
` (2 preceding siblings ...)
2022-02-16 4:36 ` [PATCH v4 3/5] platform/chrome: cros_ec: initialize `wake_enabled` in cros_ec_register() Tzung-Bi Shih
@ 2022-02-16 4:36 ` Tzung-Bi Shih
2022-02-16 4:36 ` [PATCH v4 5/5] platform/chrome: cros_ec: append newline to all logs Tzung-Bi Shih
4 siblings, 0 replies; 9+ messages in thread
From: Tzung-Bi Shih @ 2022-02-16 4:36 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, pmalani, linux-kernel, Guenter Roeck
Sort header inclusion alphabetically.
Reviewed-by: Guenter Roeck <groeck@google.com>
Reviewed-by: Prashant Malani <pmalani@chromium.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
Changes from v3:
(https://patchwork.kernel.org/project/chrome-platform/patch/20220209095703.517608-6-tzungbi@google.com/)
- Add R-b tags.
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 b6604a9ab315..a3921fe5813a 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.1.265.g69c8d7142f-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 5/5] platform/chrome: cros_ec: append newline to all logs
2022-02-16 4:36 [PATCH v4 0/5] platform/chrome: cros_ec: miscellaneous cleanups Tzung-Bi Shih
` (3 preceding siblings ...)
2022-02-16 4:36 ` [PATCH v4 4/5] platform/chrome: cros_ec: sort header inclusion alphabetically Tzung-Bi Shih
@ 2022-02-16 4:36 ` Tzung-Bi Shih
4 siblings, 0 replies; 9+ messages in thread
From: Tzung-Bi Shih @ 2022-02-16 4:36 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, pmalani, linux-kernel, Guenter Roeck
To be consistent, append newline ("\n") to all logs.
Reviewed-by: Guenter Roeck <groeck@google.com>
Reviewed-by: Prashant Malani <pmalani@chromium.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
Changes from v3:
(https://patchwork.kernel.org/project/chrome-platform/patch/20220209095703.517608-7-tzungbi@google.com/)
- Add R-b tags.
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 a3921fe5813a..a45bbc589928 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -216,7 +216,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;
}
@@ -267,7 +267,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) {
@@ -338,7 +338,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.1.265.g69c8d7142f-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/5] platform/chrome: cros_ec: fix error handling in cros_ec_register()
2022-02-16 4:36 ` [PATCH v4 1/5] platform/chrome: cros_ec: fix error handling in cros_ec_register() Tzung-Bi Shih
@ 2022-02-16 5:44 ` Prashant Malani
0 siblings, 0 replies; 9+ messages in thread
From: Prashant Malani @ 2022-02-16 5:44 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform, linux-kernel
On Tue, Feb 15, 2022 at 8:36 PM Tzung-Bi Shih <tzungbi@google.com> 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>
Reviewed-by: Prashant Malani <pmalani@chromium.org>
> ---
> Changes from v3:
> (https://patchwork.kernel.org/project/chrome-platform/patch/20220209095703.517608-2-tzungbi@google.com/)
> - Simplify by initializing the variables at the beginning.
>
> 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 | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
> index fc5aa1525d13..ff2a24b0c611 100644
> --- a/drivers/platform/chrome/cros_ec.c
> +++ b/drivers/platform/chrome/cros_ec.c
> @@ -189,6 +189,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
> ec_dev->max_request = sizeof(struct ec_params_hello);
> ec_dev->max_response = sizeof(struct ec_response_get_protocol_info);
> ec_dev->max_passthru = 0;
> + ec_dev->ec = NULL;
> + ec_dev->pd = NULL;
>
> ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
> if (!ec_dev->din)
> @@ -245,18 +247,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 +278,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 +291,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
> cros_ec_irq_thread(0, ec_dev);
>
> return 0;
> +exit:
> + platform_device_unregister(ec_dev->ec);
> + platform_device_unregister(ec_dev->pd);
> + return err;
> }
> EXPORT_SYMBOL(cros_ec_register);
>
> --
> 2.35.1.265.g69c8d7142f-goog
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 3/5] platform/chrome: cros_ec: initialize `wake_enabled` in cros_ec_register()
2022-02-16 4:36 ` [PATCH v4 3/5] platform/chrome: cros_ec: initialize `wake_enabled` in cros_ec_register() Tzung-Bi Shih
@ 2022-02-16 5:47 ` Prashant Malani
2022-02-16 7:45 ` Tzung-Bi Shih
0 siblings, 1 reply; 9+ messages in thread
From: Prashant Malani @ 2022-02-16 5:47 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform, linux-kernel
On Tue, Feb 15, 2022 at 8:37 PM Tzung-Bi Shih <tzungbi@google.com> wrote:
>
> `wake_enabled` indicates cros_ec_resume() needs to call
> disable_irq_wake() to undo enable_irq_wake() in cros_ec_suspend().
>
> Initialize `wake_enabled` in cros_ec_register() and determine the flag
> in cros_ec_suspend() instead of reset-after-used in cros_ec_resume().
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
One minor thing to consider, but regardless:
Reviewed-by: Prashant Malani <pmalani@chromium.org>
> ---
> Changes from v3:
> (https://patchwork.kernel.org/project/chrome-platform/patch/20220209095703.517608-4-tzungbi@google.com/)
> - Change the patch title.
> - Simplify by initializing wake_enabled in cros_ec_register().
>
> 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 | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
> index 25cd8df6e7b0..b6604a9ab315 100644
> --- a/drivers/platform/chrome/cros_ec.c
> +++ b/drivers/platform/chrome/cros_ec.c
> @@ -191,6 +191,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
> ec_dev->max_passthru = 0;
> ec_dev->ec = NULL;
> ec_dev->pd = NULL;
> + ec_dev->wake_enabled = false;
>
> ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
> if (!ec_dev->din)
> @@ -383,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;
> - }
> +
Better to leave it as is, and ensure "wake_enabled" is cleared after resume?
Will result in a smaller diff.
I'll leave it up to you.
-Prashant
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 3/5] platform/chrome: cros_ec: initialize `wake_enabled` in cros_ec_register()
2022-02-16 5:47 ` Prashant Malani
@ 2022-02-16 7:45 ` Tzung-Bi Shih
0 siblings, 0 replies; 9+ messages in thread
From: Tzung-Bi Shih @ 2022-02-16 7:45 UTC (permalink / raw)
To: Prashant Malani; +Cc: bleung, groeck, chrome-platform, linux-kernel
On Tue, Feb 15, 2022 at 09:47:09PM -0800, Prashant Malani wrote:
> On Tue, Feb 15, 2022 at 8:37 PM Tzung-Bi Shih <tzungbi@google.com> wrote:
> >
> > `wake_enabled` indicates cros_ec_resume() needs to call
> > disable_irq_wake() to undo enable_irq_wake() in cros_ec_suspend().
> >
> > Initialize `wake_enabled` in cros_ec_register() and determine the flag
> > in cros_ec_suspend() instead of reset-after-used in cros_ec_resume().
After reconsidering the 2 options in [1], I feel the flag needs to be set in
cros_ec_suspend() just in case if someone changes the wakeup capability per
[2].
[1]: https://patchwork.kernel.org/project/chrome-platform/patch/20220209045035.380615-4-tzungbi@google.com/#24739778
[2]: https://patchwork.kernel.org/project/chrome-platform/patch/20220209045035.380615-4-tzungbi@google.com/#24740205
Will change it back in the next version, pardon me.
> > @@ -383,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;
> > - }
> > +
>
> Better to leave it as is, and ensure "wake_enabled" is cleared after resume?
> Will result in a smaller diff.
No, cros_ec_suspend() uses the flag to tell cros_ec_resume(): don't forget to
call disable_irq_wake(). It shouldn't be reset after used by
cros_ec_resume().
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-02-16 7:46 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-16 4:36 [PATCH v4 0/5] platform/chrome: cros_ec: miscellaneous cleanups Tzung-Bi Shih
2022-02-16 4:36 ` [PATCH v4 1/5] platform/chrome: cros_ec: fix error handling in cros_ec_register() Tzung-Bi Shih
2022-02-16 5:44 ` Prashant Malani
2022-02-16 4:36 ` [PATCH v4 2/5] platform/chrome: cros_ec: remove unused variable `was_wake_device` Tzung-Bi Shih
2022-02-16 4:36 ` [PATCH v4 3/5] platform/chrome: cros_ec: initialize `wake_enabled` in cros_ec_register() Tzung-Bi Shih
2022-02-16 5:47 ` Prashant Malani
2022-02-16 7:45 ` Tzung-Bi Shih
2022-02-16 4:36 ` [PATCH v4 4/5] platform/chrome: cros_ec: sort header inclusion alphabetically Tzung-Bi Shih
2022-02-16 4:36 ` [PATCH v4 5/5] platform/chrome: cros_ec: append newline to all logs Tzung-Bi Shih
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).