Re: [PATCH V2 2/4] drivers/fpga/amd: Add communication channel

[Christophe JAILLET <christophe.jaillet@wanadoo.fr>]

Le 10/12/2024 à 19:37, Yidong Zhang a écrit :
> The communication channel (comm_chan) service is between versal-pci and the
> user PF driver. When the user PF driver requests PL data download, the
> comm_chan service will handle the request by versal_pci_load_xclbin.

...

> +enum comm_chan_req_ops {
> +	COMM_CHAN_REQ_OPS_UNKNOWN		= 0,
> +	COMM_CHAN_REQ_OPS_HOT_RESET		= 5,
> +	COMM_CHAN_REQ_OPS_GET_PROTOCOL_VERSION	= 19,
> +	COMM_CHAN_REQ_OPS_LOAD_XCLBIN_UUID	= 20,
> +	COMM_CHAN_REQ_OPS_MAX,

Unneeded comma after a terminator.

> +};

...

> +static void comm_chan_check_request(struct work_struct *w)
> +{
> +	struct comm_chan_device *ccdev = to_ccdev_work(w);
> +	u32 status = 0, request[COMM_CHAN_DATA_LEN] = {0};

These 2 initialisations are not needed.

> +	struct comm_chan_hw_msg *hw_msg;
> +	u8 type, eom;
> +	int i;
> +
> +	status = comm_chan_read(ccdev, COMM_CHAN_REG_IS_OFF);
> +	if (!STATUS_IS_READY(status))
> +		return;
> +	if (STATUS_IS_ERROR(status)) {
> +		vdev_err(ccdev->vdev, "An error has occurred with comms");
> +		return;
> +	}
> +
> +	/* ACK status */
> +	comm_chan_write(ccdev, COMM_CHAN_REG_IS_OFF, status);
> +
> +	for (i = 0; i < COMM_CHAN_DATA_LEN; i++)
> +		request[i] = comm_chan_read(ccdev, COMM_CHAN_REG_RDDATA_OFF);
> +

...

CJ