Re: [PATCH 0/3] ACPI: Fix for supporting > 256 processor declaration limit

[Alexey Starikovskiy <aystarik@gmail.com>]

Hi Myron,

It is great to see that this functionality found some care after
2+ years :)
Please check your patches to conform to patch submission guidelines,
namely 80 char string limit... :) scripts/checkpatch.pl is quite good for
such purpose.

Myron Stowe wrote:
> Len, Alexey:
>
> The following three item patch series fixes an issue with the introduction
> of > 256 processor declaration support: "Allow processor to be declared with
> the Device() instead of Processor()" (git SHA 11bf04c4).
>
> The root issue is in the lsapic mapping logic of drivers/acpi/processor_core.c.
> Currently, the logic tries both types of matches irregardless of declaration
> type and relies on one failing.  According to the spec - lsapic mapping is
> dependent on how the processor object was declared: CPUs declared using the
> "Processor" statement should use the Local SAPIC's 'processor_id', and CPUs
> declared using the "Device" statement should use the 'uid'.
>
> Reference: Section 8.4 Declaring Processors; Section 5.2.11.13 Local SAPIC
> Structure.  "Advanced Configuration and Power Interface Specification",
> Revision 3.0b, October 2006.
>
>
> [PATCH 1/3] disambiguates the processor declaration type that is currently
> conflated so that subsequent logic can behave based explicitly on the
> declaration's type.  I expect the disambiguation this patch introduces will
> also be advantageous when extending the > 256 processor support for x86 via
> x2APIC.
>
> [PATCH 2/3] addresses the root issue as stated above.  With respect to this
> patch I'm ambivalent about the 'printk' introduced in "map_lsapic_id()" -
> perhaps it should be removed?
>
>   
Errors causing change from default behaviour should be visible, so
printk is ok, please only make it conform to 80 char string length limit.
> [PATCH 3/3] is non-functional white space/spelling fixes in the related code. 
>
>
> While the specific fix is ia64 focused the underlying code affected is common
> to both x86 and ia64.
That was intentional to have it common, as the less frequently used
case will rotten without care.
>   I have tested on the following platform/namespace
> combinations:
>   ia64 with "Processor" type namespace processor declarations,
>   ia64 with "Device" type namespace processor declarations,
>   x86 with "Processor" type namespace processor declarations.
>
>   
I had no machine with second variant, so it probably didn't work there, 
right?
> Note that this patch series does *not* handle "Device" type processor
> declarations that contain a string type _UID object under the processor
> device's scope (I am currently not aware of any platforms that have such to
> test against).
>
>   
Original didn't do it either -- it is too difficult and does not give 
any advantage
to HW vendors over numeral ID.

Regards,
Alex.