On Mon, Jun 17, 2019 at 01:34:51PM +0100, Valentin Schneider wrote: > 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 How about the "the processor structure flag being examined" ? > > + * > > + * 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: > I agree with that. I am also fine if you want to change the code to return 0 or 1 based on the flag value. It then aligns well with comment under acpi_pptt_cpu_is_thread. Either way, we just need consistency. -- Regards, Sudeep
