Re: [PATCH v2 1/3] usb: typec: ucsi: Add support for message out data structure

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

Hi Dmitry,

Please check the comment from Greg. I think we need to revert
back to the original. Please let us know if you still disagree.

But I tend to agree with Greg. There are too many parameters for the
functions like this. Let's just supply a separate function that can be
used to fill the message_out like Pooja originally suggested.

> > diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
> > index 6b92f296e985..c0426bb1e81b 100644
> > --- a/drivers/usb/typec/ucsi/ucsi_acpi.c
> > +++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
> > @@ -86,6 +86,21 @@ static int ucsi_acpi_read_message_in(struct ucsi *ucsi, void *val, size_t val_le
> >   	return 0;
> >   }
> > +static int ucsi_acpi_write_message_out(struct ucsi *ucsi, void *data, size_t data_len)
> > +{
> > +	struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
> > +
> > +	if (!data || !data_len)
> > +		return -EINVAL;
> > +
> > +	if (ucsi->version <= UCSI_VERSION_1_2)
> > +		memcpy(ua->base + UCSI_MESSAGE_OUT, data, data_len);
> > +	else
> > +		memcpy(ua->base + UCSIv2_MESSAGE_OUT, data, data_len);
> > +
> > +	return ucsi_acpi_dsm(ua, UCSI_DSM_FUNC_WRITE);
> 
> Why do we need extra WRITE? Isn't the one performed by
> ucsi_acpi_async_control() not enough?

This is actually a good point!

Syncing the mailbox before there is a command will confuse at least
some of the EC firmwares. But more importantly, the previous command
may still be in the control field, no? So that write may actually
cause the previous command to be executed again.

So Pooja, please drop that write.

thanks,

-- 
heikki