On Mon, 2025-03-03 at 17:21 +0100, Stefano Garzarella wrote:
> On Sat, Mar 01, 2025 at 03:45:10AM +0200, Jarkko Sakkinen wrote:
> > On Fri, Feb 28, 2025 at 06:07:17PM +0100, Stefano Garzarella wrote:
> > > + int (*send_recv)(struct tpm_chip *chip, u8 *buf, size_t
> > > buf_len,
> > > + size_t to_send);
> >
> > Please describe the meaning and purpose of to_send.
>
> Sure, I'll add in the commit description.
It's always a command, right? So better be more concerete than
"to_send", e.g. "cmd_len".
I'd do instead:
if (!chip->send)
goto out_recv;
And change recv into:
int (*recv)(struct tpm_chip *chip, u8 *buf, size_t buf_len,
cmd_len);
Those who don't need the last parameter, can ignore it.
This also reduces meaningless possible states for the ops structure
such as "send_recv and send or recv defined", i.e. makes it overall
more mutually exclusive.
>
> Should I add documentation in the code as well?
>
> The other callbacks don't have that, but if you think it's useful we
> can
> start with that, I mean something like this:
>
> /**
> * send_recv() - send the command and receive the response
> on the same
> * buffer in a single call.
> *
> * @chip: The TPM chip
> * @buf: A buffer used to both send the command and receive
> the response
> * @buf_len: The size of the buffer
> * @to_send: Number of bytes in the buffer to send
> *
> * Return: number of received bytes on success, negative
> error code on
> * failure.
> */
> int (*send_recv)(struct tpm_chip *chip, u8 *buf, size_t
> buf_len,
> size_t to_send);
I would not document in callback level as their implementation is not global.
This is probably stance also taken by file_operations, vm_ops and many other
places with "ops" structure.
>
> Thanks,
> Stefano
>
>
BR, Jarkko
