Re: [PATCH v2] usb: typec: ucsi: Convert connector specific commands to bitmaps

[Abel Vesa <abel.vesa@linaro.org>]

On 24-11-06 17:06:05, Heikki Krogerus wrote:
> That allows the fields in those command data structures to
> be easily validated. If an unsupported field is accessed, a
> warning is generated.
> 
> This will not force UCSI version checks to be made in every
> place where these data structures are accessed, but it will
> make it easier to pinpoint issues that are caused by the
> unconditional accesses to those fields, and perhaps more
> importantly, allow those issues to be noticed immediately.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> 
> Changed since v1:
> - The CONCAP macro now reads the correct member.
> 
> I added the version to all the field definitions that need it instead
> of adding a complete macro for each of those fields. I think that
> should be enough to make the version clear to everybody during use as
> well as review.
> 
> ---
>  drivers/usb/typec/ucsi/psy.c       |  28 ++--
>  drivers/usb/typec/ucsi/trace.h     |  28 ++--
>  drivers/usb/typec/ucsi/ucsi.c      | 121 +++++++-------
>  drivers/usb/typec/ucsi/ucsi.h      | 252 +++++++++++++++++------------
>  drivers/usb/typec/ucsi/ucsi_acpi.c |   7 +-
>  5 files changed, 240 insertions(+), 196 deletions(-)
> 

[...]

> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index e430a0ca4a2b..974a441155e1 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c

[...]

> @@ -1778,7 +1775,7 @@ static int ucsi_init(struct ucsi *ucsi)
>  
>  	/* Get PPM capabilities */
>  	command = UCSI_GET_CAPABILITY;
> -	ret = ucsi_send_command(ucsi, command, &ucsi->cap, sizeof(ucsi->cap));
> +	ret = ucsi_send_command(ucsi, command, &ucsi->cap, UCSI_GET_CAPABILITY_SIZE);

This fails for version < UCSI_VERSION_2_0. Size is wrong.

>  	if (ret < 0)
>  		goto err_reset;
>  

[...]