lkml.org 
[lkml]   [2023]   [Mar]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v3 1/8] platform/x86/intel/ifs: Reorganize driver data
    From
    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
    >

    \
     
     \ /
      Last update: 2023-03-27 01:03    [W:4.277 / U:0.076 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site