2022-02-16 07:17:33

by Tzung-Bi Shih

Subject: [PATCH v4 1/5] platform/chrome: cros_ec: fix error handling in cros_ec_register()

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 <[email protected]>
---
Changes from v3:
(https://patchwork.kernel.org/project/chrome-platform/patch/[email protected]/)
- Simplify by initializing the variables at the beginning.

Changes from v2:
(https://patchwork.kernel.org/project/chrome-platform/patch/[email protected]/)
- 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/[email protected]/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

2022-02-16 07:30:55

by Prashant Malani

Subject: Re: [PATCH v4 1/5] platform/chrome: cros_ec: fix error handling in cros_ec_register()

On Tue, Feb 15, 2022 at 8:36 PM Tzung-Bi Shih <[email protected]> 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 <[email protected]>
Reviewed-by: Prashant Malani <[email protected]>

> ---
> Changes from v3:
> (https://patchwork.kernel.org/project/chrome-platform/patch/[email protected]/)
> - Simplify by initializing the variables at the beginning.
>
> Changes from v2:
> (https://patchwork.kernel.org/project/chrome-platform/patch/[email protected]/)
> - 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/[email protected]/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
>