[Skiboot] [PATCH V6 08/21] core/pldm: Encode GetPLDMVersion response

[Christophe Lombard]

Le 12/04/2023 à 16:51, Frederic Barrat a écrit :
>
>
> On 13/09/2022 12:26, Christophe Lombard wrote:
>> The GetPLDMVersion command can be used to retrieve the PLDM base
>> specification versions that the PLDM terminus supports, as well as the
>> PLDM Type specification versions supported for each PLDM Type.
>>
>> The reported version for Type 0 (PLDMbase) shall be encoded as 
>> 0xF1F1F000.
>>
>> Reviewed-by: Abhishek Singh Tomar <abhishek at linux.ibm.com>
>> Signed-off-by: Christophe Lombard <clombard at linux.vnet.ibm.com>
>> ---
>>   core/pldm/pldm-responder.c | 84 ++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 84 insertions(+)
>>
>> diff --git a/core/pldm/pldm-responder.c b/core/pldm/pldm-responder.c
>> index 88545685..230b9c05 100644
>> --- a/core/pldm/pldm-responder.c
>> +++ b/core/pldm/pldm-responder.c
>> @@ -208,6 +208,7 @@ static struct pldm_cmd pldm_base_get_types = {
>>   /*
>>    * Extended error codes defined for the Base command set.
>>    */
>> +#define INVALID_TRANSFER_OPERATION_FLAG        0x80
>
>
> Spec says:
> INVALID_DATA_TRANSFER_HANDLE=128 (0x80),
> INVALID_TRANSFER_OPERATION_FLAG=129 (0x81),
>

You are right ! Thanks

>
>>   #define INVALID_PLDM_TYPE_IN_REQUEST_DATA      0x83
>>   #define INVALID_PLDM_VERSION_IN_REQUEST_DATA   0x84
>>   @@ -294,6 +295,88 @@ static struct pldm_cmd pldm_base_get_commands = {
>>       .handler = base_get_commands_handler,
>>   };
>>   +/*
>> + * GetPLDMVersion (0x03)
>> + * The GetPLDMVersion command can be used to retrieve the PLDM base
>> + * specification versions that the PLDM terminus supports, as well as
>> + * the PLDM Type specification versions supported for each PLDM Type.
>> + */
>> +static int base_get_version_handler(const struct pldm_rx_data *req)
>> +{
>> +    uint32_t tmp[2];
>
>
> Can we rename 'tmp' to something more descriptive, like 'VersionData'?
> A comment describing the expected encoding from the spec would also 
> help the first time reader.
>
>   Fred
>

ok, I will add more explanation and change the name of the variable.