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
