Re: [PATCH v2 3/7] firmware: stratix10-svc: Add initial support for asynchronous communication with Stratix 10 service channel

[Mahesh Rao <mahesh.rao@altera.com>]

On 23-05-2025 12:17 am, Dinh Nguyen wrote:
> On 5/22/25 05:33, Mahesh Rao wrote:
>>
>>
>> On 22-05-2025 01:48 am, Dinh Nguyen wrote:
>>> On 5/21/25 03:42, Mahesh Rao wrote:
>>>>
>>>>
>>>> On 19-05-2025 05:28 pm, Dinh Nguyen wrote:
>>>>> On 5/12/25 06:39, Mahesh Rao via B4 Relay wrote:
>>>>>> From: Mahesh Rao <mahesh.rao@altera.com>
>>>>>>
>>>
>>> <snip>
>>>
>>>>>> +
>>>>>> +/**
>>>>>> + * stratix10_svc_async_prepare_response - Prepare the response 
>>>>>> data for an asynchronous transaction.
>>>>>> + * @chan: Pointer to the service channel structure.
>>>>>> + * @handle: Pointer to the asynchronous handler structure.
>>>>>> + * @data: Pointer to the callback data structure.
>>>>>> + *
>>>>>> + * This function prepares the response data for an asynchronous 
>>>>>> transaction. It
>>>>>> + * extracts the response data from the SMC response structure and 
>>>>>> stores it in
>>>>>> + * the callback data structure. The function also logs the 
>>>>>> completion of the
>>>>>> + * asynchronous transaction.
>>>>>> + *
>>>>>> + * Return: 0 on success, -ENOENT if the command is invalid
>>>>>> + */
>>>>>> +static int stratix10_svc_async_prepare_response(struct 
>>>>>> stratix10_svc_chan *chan,
>>>>>> +                        struct stratix10_svc_async_handler *handle,
>>>>>> +                        struct stratix10_svc_cb_data *data)
>>>>>> +{
>>>>>> +    struct stratix10_svc_client_msg *p_msg =
>>>>>> +        (struct stratix10_svc_client_msg *)handle->msg;
>>>>>> +    struct stratix10_svc_controller *ctrl = chan->ctrl;
>>>>>> +
>>>>>> +    data->status = STRATIX10_GET_SDM_STATUS_CODE(handle->res.a1);
>>>>>> +
>>>>>> +    switch (p_msg->command) {
>>>>>> +    default:
>>>>>> +        dev_alert(ctrl->dev, "Invalid command\n ,%d", p_msg- 
>>>>>> >command);
>>>>>> +        return -ENOENT;
>>>>>> +    }
>>>>>
>>>>> What is the above code doing?
>>>>
>>>> This function prepares the response  for clients after retriving the 
>>>> response from the Arm Trusted Firmware using the polling call. 
>>>> Currently only the negative scenario is shown for incvalid command, 
>>>> the last patch in series adds command for hwmon for the positive 
>>>> scenario.
>>>>
>>>
>>> Okay, but this is confusing. Please just add this to the last patch 
>>> then.
>>>
>>
>> Shall I combine the last patch for adding hwmon commands here so that 
>> the framework has some command usage?.
>>
> 
> I like the idea of splitting up the patches to be small, but I don't 
> think combining them is the right thing to do either. Each patch should 
> be able to stand on it's own as well. The HWMON commands are fine in 
> their own patch. You don't need to put the switch statements in this 
> patch, because nothing is being done.
> 

ok sure will make the change