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

[Tzung-Bi Shih <tzungbi@kernel.org>]

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`.