Hi,
On 3/13/23 22:34, Joseph, Jithu wrote:
>
>
> On 3/13/2023 7:46 AM, Hans de Goede wrote:
>> Hi Jithu,
>>
>> On 3/1/23 02:59, Jithu Joseph wrote:
>
>>>
>>> +struct ifs_const_data {
>>> + int integrity_cap_bit;
>>> + int test_num;
>>> +};
>>> +
>>
>> This is a description of the specific capabilties / bits of
>> the IFS on e.g. Saphire Rapids, so please name this appropriately
>> for example:
>>
>> struct ifs_hw_caps {
>> int integrity_cap_bit;
>> int test_num;
>> };
>
> This can be renamed to ifs_test_caps as it holds test specific fields.
Ack.
>>
>> You got this exactly the wrong way around, there should be a single
>>
>> static const struct ifs_hw_caps saphire_rapids_caps = {
>> .integrity_cap_bit = MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT,
>> .test_num = 0,
>> };
>>
>> And then struct ifs_device { } should have a "const struct ifs_hw_caps *hw_caps"
>> which gets initialized to point to &saphire_rapids_caps. So that your const
>> data is actually const.
>>
>> Where as since the r/w data's lifetime is couple to the misc-device lifetime
>> there is no need to dynamically allocate it just keep that embedded, so that
>> together you get:
>
> Noted
>
>>
>> struct ifs_device {
>> const struct ifs_hw_caps *hw_caps;
>> struct ifs_data data;
>> struct miscdevice misc;
>> };
>>
>
> The initialization portion, taking into account your suggestion above, translates to:
Yes, assuming we go with 1 ifs_device per test type.
Regards,
Hans
> static const struct ifs_test_caps scan_test = {
> .integrity_cap_bit = MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT,
> .test_num = IFS_TYPE_SAF,
> };
>
> static const struct ifs_test_caps array_test = {
> .integrity_cap_bit = MSR_INTEGRITY_CAPS_ARRAY_BIST_BIT,
> .test_num = IFS_TYPE_ARRAY_BIST,
> };
>
> static struct ifs_device ifs_devices[] = {
> [IFS_TYPE_SAF] = {
> .test_caps = &scan_test,
> .misc = {
> .name = "intel_ifs_0",
> .minor = MISC_DYNAMIC_MINOR,
> .groups = plat_ifs_groups,
> },
> },
> [IFS_TYPE_ARRAY_BIST] = {
> .test_caps = &array_test,
> .misc = {
> .name = "intel_ifs_1",
> .minor = MISC_DYNAMIC_MINOR,
> .groups = plat_ifs_array_groups,
> },
> },
> };
>
> Jithu
>
