Re: [PATCH v2 06/14] greybus: cpc: introduce CPC header structure

["Jérôme Pouiller" <jerome.pouiller@silabs.com>]

On Thursday 15 January 2026 16:57:59 Central European Standard Time Damien Riégel wrote:
> CPC main features are reliable transmission and remote's receive window
> management. To implement these features, an additional header is needed.
> This header is prepended to all Greybus messages.
> 
> Reliable transmission: to make transmission reliable, messages are
> sequenced and acknowledged. That constitutes two bytes of the header,
> one for the sequence number, one for the acknowledgment number. If a
> message is not acked in a timely manner, a retransmission mechanism will
> attempt another transmission. That mechanism will be implemented in a
> future patch set.
> 
> Remote's receive window: the remote advertises the number of reception
> buffers that are available on this cport. The other peer must take care
> of not sending more messages than advertised by the remote. This is a
> sort of flow control. That accounts for one byte in the header.
> 
> The remaining byte carries some flags. For instance, there is a flag to
> indicate if it's a CPC message or a Greybus message.
> 
> Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
> ---
> Changes in v2:
>   - Minor tweaks to structure documentation
> 
>  drivers/greybus/cpc/header.h | 44 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 drivers/greybus/cpc/header.h
> 
> diff --git a/drivers/greybus/cpc/header.h b/drivers/greybus/cpc/header.h
> new file mode 100644
> index 00000000000..af283fec492
> --- /dev/null
> +++ b/drivers/greybus/cpc/header.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2025, Silicon Laboratories, Inc.
> + */
> +
> +#ifndef __CPC_HEADER_H
> +#define __CPC_HEADER_H
> +
> +#include <linux/greybus.h>
> +#include <linux/types.h>
> +
> +#define CPC_HEADER_MAX_RX_WINDOW U8_MAX
> +
> +/**
> + * struct cpc header - Representation of CPC header.
> + * @ctrl_flags: contains the type of frame and other control flags.
> + * @recv_wnd: number of buffers that the cport can receive without blocking.
> + * @seq: sequence number.
> + * @ack: acknowledge number, indicate to the remote the next sequence number
> + *	 this peer expects to see.
> + *
> + * Each peer can confirm reception of frames by setting the acknowledgment number to the next frame
> + * it expects to see, i.e. setting the ack number to X effectively acknowledges frames with sequence
> + * number up to X-1.
> + *
> + * CPC is designed around the concept that each cport has its pool of reception buffers. The number
> + * of buffers in a pool is advertised to the remote via the @recv_wnd attribute. This acts as
> + * software flow-control, and a peer shall not send frames to a remote if the @recv_wnd is zero.
> + *
> + * The eighth-bit (0x80) of the control byte indicates if the frame targets CPC or Greybus. If the
> + * bit is set, the frame should be interpreted as a CPC control frame. For simplicity, control
> + * frames have the same encoding as Greybus frames.
> + */
> +struct cpc_header {
> +	__u8 ctrl_flags;
> +	__u8 recv_wnd;
> +	__u8 seq;
> +	__u8 ack;
> +} __packed;
> +
> +#define CPC_HEADER_SIZE (sizeof(struct cpc_header))
> +#define GREYBUS_HEADER_SIZE (sizeof(struct gb_operation_msg_hdr))

Generally, when I see such macros in the code, I suspect some black
magic behind. So, I have to check the definitions.

I suggest to not introduce these trivial symbols and just use
"sizeof(struct ...)" inline in the code.



-- 
Jérôme Pouiller