lkml.org 
[lkml]   [2019]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2 1/2] ACPI/PPTT: Add support for ACPI 6.3 thread flag
    From
    Date
    Hi Jeremy,

    Few nits below.

    Also, I had a look at the other PPTT processor flags that were introduced
    in 6.3, and the only other one being used is ACPI_LEAF_NODE in
    acpi_pptt_leaf_node(). However that one already has a handle on the table
    header, so the check_acpi_cpu_flag() isn't of much help there.

    I don't believe the other existing flags will benefit from the helper since
    they are more about describing the PPTT tree, but I think it doesn't hurt
    to keep it around for potential future flags.

    On 14/06/2019 23:31, Jeremy Linton wrote:
    [...]
    > @@ -517,6 +517,43 @@ static int find_acpi_cpu_topology_tag(unsigned int cpu, int level, int flag)
    > return retval;
    > }
    >
    > +/**
    > + * check_acpi_cpu_flag() - Determine if CPU node has a flag set
    > + * @cpu: Kernel logical CPU number
    > + * @rev: The PPTT revision defining the flag
    > + * @flag: The flag itself
    > + *
    > + * Check the node representing a CPU for a given flag.
    > + *
    > + * Return: -ENOENT if the PPTT doesn't exist, the CPU cannot be found or
    > + * the table revision isn't new enough.
    > + * Otherwise returns flag value
    > + */

    Nit: strictly speaking we're not returning the flag value but its mask
    applied to the flags field. I don't think anyone will care about getting
    the actual flag value, but it should be made obvious in the doc:

    -ENOENT if ...
    0 if the flag isn't set
    > 0 if it is set.

    [...]
    > @@ -581,6 +618,21 @@ int cache_setup_acpi(unsigned int cpu)
    > return status;
    > }
    >
    > +/**
    > + * acpi_pptt_cpu_is_thread() - Determine if CPU is a thread
    > + * @cpu: Kernel logical CPU number
    > + *
    > + *

    Nit: extra newline

    > + * Return: 1, a thread
    > + * 0, not a thread
    > + * -ENOENT ,if the PPTT doesn't exist, the CPU cannot be found or
    > + * the table revision isn't new enough.
    > + */
    > +int acpi_pptt_cpu_is_thread(unsigned int cpu)
    > +{
    > + return check_acpi_cpu_flag(cpu, 2, ACPI_PPTT_ACPI_PROCESSOR_IS_THREAD);
    > +}
    > +
    > /**
    > * find_acpi_cpu_topology() - Determine a unique topology value for a given CPU
    > * @cpu: Kernel logical CPU number
    > @@ -641,7 +693,6 @@ int find_acpi_cpu_cache_topology(unsigned int cpu, int level)
    [...]

    \
     
     \ /
      Last update: 2019-06-17 14:35    [W:4.030 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site