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

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Sat, Nov 09, 2024 at 12:35:13PM +0200, Abel Vesa wrote:
> 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.

Thanks for catching that. I'll send the fix.

-- 
heikki