Re: [PATCH] platform/chrome: cros_ec_proto: Fix deferred response payload handling

Tzung-Bi Shih <[email protected]>
Newsgroups dev.linux.lists.chrome-platform,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On Mon, Aug 10, 2026 at 02:43:29PM -0700, Rob Barnes wrote:
> Subject: [PATCH] platform/chrome: cros_ec_proto: Fix deferred response
> payload handling

The subject shouldn't be here.

> ... Save and restore orig_msg
> fields (command, outsize, version) around the resend request to
> prevent unintended caller struct mutations. Update existing KUnit
> tests and add test cases for 4-byte and 16-byte response
> payloads.

They are somehow redundant; can be removed.

> -static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev,
> uint32_t *result)

./scripts/checkpatch.pl --strict:
    ERROR: patch seems to be corrupt (line wrapped?)
resulting in the patch can't be applied (accompanying with other corrupts).

> +static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev,
> +					struct cros_ec_command *orig_msg)

./scripts/checkpatch.pl --strict:
    CHECK: Alignment should match open parenthesis

> @@ -170,8 +170,28 @@ static int cros_ec_wait_until_complete(struct
> cros_ec_device *ec_dev, uint32_t *
>  			break;
>  		}
> 
> -		if (!(status->flags & EC_COMMS_STATUS_PROCESSING))
> -			return ret;
> +		if (!(status->flags & EC_COMMS_STATUS_PROCESSING)) {
> +			/*
> +			 * If original command requested response payload, retrieve it via
> +			 * EC_CMD_RESEND_RESPONSE into orig_msg->data. Otherwise return 0
> +			 * for zero-payload commands.
> +			 */

How about exiting earlier if `orig_msg->insize` is 0?

> +			if (orig_msg->insize > 0) {
> +				uint32_t orig_cmd = orig_msg->command;
> +				uint32_t orig_outsize = orig_msg->outsize;
> +				uint32_t orig_version = orig_msg->version;

./scripts/checkpatch.pl --strict:
    CHECK: Prefer kernel type 'u32' over 'uint32_t'

> diff --git a/drivers/platform/chrome/cros_ec_proto_test.c
...
> -static void cros_ec_proto_test_cmd_xfer_in_progress_normal(struct kunit *test)
> +static void cros_ec_proto_test_cmd_xfer_in_progress_payload_0bytes(struct
                                                                    ^
							       0byte?

> +static void cros_ec_proto_test_cmd_xfer_in_progress_payload_4bytes(struct
> kunit *test)
> +{
...
> +	memset(&buf, 0, sizeof(buf));
> +	msg->version = 1;
> +	msg->command = 0x1234;
> +	msg->outsize = 2;
> +	msg->insize = sizeof(buf.data);
...
> +	ret = cros_ec_cmd_xfer(ec_dev, msg);
> +	KUNIT_EXPECT_EQ(test, ret, 4);
> +	KUNIT_EXPECT_EQ(test, msg->result, EC_RES_SUCCESS);
> +	KUNIT_EXPECT_EQ(test, msg->command, 0x1234);
> +	KUNIT_EXPECT_EQ(test, msg->outsize, 2);
> +	KUNIT_EXPECT_EQ(test, msg->version, 1);

If it wants to check the header is immutable, it should also check `insize`.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.