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

[Yidong Zhang <yidong.zhang@amd.com>]

On 1/26/25 02:19, Christophe JAILLET wrote:
> Caution: This message originated from an External Source. Use proper 
> caution when opening attachments, clicking links, or responding.
> 
> 
> 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.

Will fix this.

> 
>> +};
> 
> ...
> 
>> +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.

Will fix this.

> 
>> +     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