[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <8ece6a84-b235-4881-ad25-c0f4a9e732a6@altera.com>
Date: Fri, 23 May 2025 14:50:06 +0530
From: Mahesh Rao <mahesh.rao@...era.com>
To: Dinh Nguyen <dinguyen@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>
Cc: Matthew Gerlach <matthew.gerlach@...era.com>,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 3/7] firmware: stratix10-svc: Add initial support for
asynchronous communication with Stratix 10 service channel
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@...era.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
Powered by blists - more mailing lists