[PATCH] platform/chrome: cros_ec: Separate logic for getting panic info

[PATCH] platform/chrome: cros_ec: Separate logic for getting panic info

[Rob Barnes]

Create a separate function called cros_ec_get_panicinfo for getting
panic info from EC.

Currently cros_ec_create_panicinfo is the only caller.

Signed-off-by: Rob Barnes <robbarnes@google.com>
---

 drivers/platform/chrome/cros_ec_debugfs.c | 46 ++++++++++++++++++-----
 1 file changed, 36 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index a98c529d8c698..def7b2f2dd7f9 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -400,24 +400,50 @@ static void cros_ec_cleanup_console_log(struct cros_ec_debugfs *debug_info)
 	}
 }
 
-static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
+/**
+ * Returns the size of the panicinfo data fetched from the EC
+ */
+static int cros_ec_get_panicinfo(struct cros_ec_device *ec_dev, uint8_t *data,
+				 int data_size)
 {
-	struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
-	int ret;
+	int ret = 0;
 	struct cros_ec_command *msg;
-	int insize;
 
-	insize = ec_dev->max_response;
+	if (!data || data_size <= 0)
+		return -EINVAL;
 
-	msg = devm_kzalloc(debug_info->ec->dev,
-			sizeof(*msg) + insize, GFP_KERNEL);
+	msg = devm_kzalloc(ec_dev->dev, sizeof(*msg) + data_size, GFP_KERNEL);
 	if (!msg)
 		return -ENOMEM;
 
 	msg->command = EC_CMD_GET_PANIC_INFO;
-	msg->insize = insize;
+	msg->insize = data_size;
 
 	ret = cros_ec_cmd_xfer_status(ec_dev, msg);
+
+	if (ret < 0)
+		goto free;
+
+	memcpy(data, msg->data, data_size);
+
+free:
+	devm_kfree(ec_dev->dev, msg);
+	return ret;
+}
+
+static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
+{
+	struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
+	int ret;
+	void *data;
+
+	data = devm_kzalloc(debug_info->ec->dev, ec_dev->max_response,
+			    GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	ret = cros_ec_get_panicinfo(ec_dev, data, ec_dev->max_response);
+
 	if (ret < 0) {
 		ret = 0;
 		goto free;
@@ -427,7 +453,7 @@ static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
 	if (ret == 0)
 		goto free;
 
-	debug_info->panicinfo_blob.data = msg->data;
+	debug_info->panicinfo_blob.data = data;
 	debug_info->panicinfo_blob.size = ret;
 
 	debugfs_create_blob("panicinfo", S_IFREG | 0444, debug_info->dir,
@@ -436,7 +462,7 @@ static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
 	return 0;
 
 free:
-	devm_kfree(debug_info->ec->dev, msg);
+	devm_kfree(debug_info->ec->dev, data);
 	return ret;
 }
 
-- 
2.39.2

Re: [PATCH] platform/chrome: cros_ec: Separate logic for getting panic info

[Tzung-Bi Shih]

On Thu, Apr 06, 2023 at 04:14:14PM +0000, Rob Barnes wrote:
> -static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
> +/**
> + * Returns the size of the panicinfo data fetched from the EC
> + */
> +static int cros_ec_get_panicinfo(struct cros_ec_device *ec_dev, uint8_t *data,
> +				 int data_size)
>  {
> -	struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
> -	int ret;
> +	int ret = 0;

`ret` doesn't need to be initialized.  It will be overridden soon anyway.

> -	insize = ec_dev->max_response;
> +	if (!data || data_size <= 0)
> +		return -EINVAL;

How about `data_size > ec_dev->max_response`?

> -	msg = devm_kzalloc(debug_info->ec->dev,
> -			sizeof(*msg) + insize, GFP_KERNEL);
> +	msg = devm_kzalloc(ec_dev->dev, sizeof(*msg) + data_size, GFP_KERNEL);

In the case, it doesn't have to be devm anymore.

>  	ret = cros_ec_cmd_xfer_status(ec_dev, msg);
> +
> +	if (ret < 0)
> +		goto free;

If possible, please remove the extra blank line to make them look as a whole.

> +free:
> +	devm_kfree(ec_dev->dev, msg);

Given that the function controls the memory's lifecycle, it doesn't have to
be devm.

> +static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
> +{
> +	struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
[...]
> +	ret = cros_ec_get_panicinfo(ec_dev, data, ec_dev->max_response);
> +
>  	if (ret < 0) {
>  		ret = 0;
>  		goto free;

Ditto, please remove the extra blank line if possible.

[PATCH v2] platform/chrome: cros_ec: Separate logic for getting panic info

[Rob Barnes]

Create a separate function called cros_ec_get_panicinfo for getting
panic info from EC.

Currently cros_ec_create_panicinfo is the only caller.

Signed-off-by: Rob Barnes <robbarnes@google.com>
---
Changelog since v1:
- Minor cleanup

drivers/platform/chrome/cros_ec_debugfs.c | 42 ++++++++++++++++++-----
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index 9044c6bab770..58d1d7f6c94f 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -400,24 +400,48 @@ static void cros_ec_cleanup_console_log(struct cros_ec_debugfs *debug_info)
 	}
 }
 
-static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
+/**
+ * Returns the size of the panicinfo data fetched from the EC
+ */
+static int cros_ec_get_panicinfo(struct cros_ec_device *ec_dev, uint8_t *data,
+				 int data_size)
 {
-	struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
 	int ret;
 	struct cros_ec_command *msg;
-	int insize;
 
-	insize = ec_dev->max_response;
+	if (!data || data_size <= 0 || data_size > ec_dev->max_response)
+		return -EINVAL;
 
-	msg = devm_kzalloc(debug_info->ec->dev,
-			sizeof(*msg) + insize, GFP_KERNEL);
+	msg = kzalloc(sizeof(*msg) + data_size, GFP_KERNEL);
 	if (!msg)
 		return -ENOMEM;
 
 	msg->command = EC_CMD_GET_PANIC_INFO;
-	msg->insize = insize;
+	msg->insize = data_size;
 
 	ret = cros_ec_cmd_xfer_status(ec_dev, msg);
+	if (ret < 0)
+		goto free;
+
+	memcpy(data, msg->data, data_size);
+
+free:
+	kfree(msg);
+	return ret;
+}
+
+static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
+{
+	struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
+	int ret;
+	void *data;
+
+	data = devm_kzalloc(debug_info->ec->dev, ec_dev->max_response,
+			    GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	ret = cros_ec_get_panicinfo(ec_dev, data, ec_dev->max_response);
 	if (ret < 0) {
 		ret = 0;
 		goto free;
@@ -427,7 +451,7 @@ static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
 	if (ret == 0)
 		goto free;
 
-	debug_info->panicinfo_blob.data = msg->data;
+	debug_info->panicinfo_blob.data = data;
 	debug_info->panicinfo_blob.size = ret;
 
 	debugfs_create_blob("panicinfo", S_IFREG | 0444, debug_info->dir,
@@ -436,7 +460,7 @@ static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
 	return 0;
 
 free:
-	devm_kfree(debug_info->ec->dev, msg);
+	devm_kfree(debug_info->ec->dev, data);
 	return ret;
 }
 
-- 
2.40.0.577.gac1e443424-goog

Re: [PATCH v2] platform/chrome: cros_ec: Separate logic for getting panic info

[patchwork-bot+chrome-platform]

Hello:

This patch was applied to chrome-platform/linux.git (for-kernelci)
by Tzung-Bi Shih <tzungbi@kernel.org>:

On Mon, 10 Apr 2023 16:58:17 +0000 you wrote:
> Create a separate function called cros_ec_get_panicinfo for getting
> panic info from EC.
> 
> Currently cros_ec_create_panicinfo is the only caller.
> 
> Signed-off-by: Rob Barnes <robbarnes@google.com>
> 
> [...]

Here is the summary with links:
  - [v2] platform/chrome: cros_ec: Separate logic for getting panic info
    https://git.kernel.org/chrome-platform/c/14bb09b32f43

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

Re: [PATCH v2] platform/chrome: cros_ec: Separate logic for getting panic info

[patchwork-bot+chrome-platform]

Hello:

This patch was applied to chrome-platform/linux.git (for-next)
by Tzung-Bi Shih <tzungbi@kernel.org>:

On Mon, 10 Apr 2023 16:58:17 +0000 you wrote:
> Create a separate function called cros_ec_get_panicinfo for getting
> panic info from EC.
> 
> Currently cros_ec_create_panicinfo is the only caller.
> 
> Signed-off-by: Rob Barnes <robbarnes@google.com>
> 
> [...]

Here is the summary with links:
  - [v2] platform/chrome: cros_ec: Separate logic for getting panic info
    https://git.kernel.org/chrome-platform/c/14bb09b32f43

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