Re: [PATCH 1/3] ACPI / processor: remove incorrect comparison of phys_id

[Sudeep Holla <sudeep.holla@arm.com>]

Hi Rafael,

On 26/11/14 22:26, Rafael J. Wysocki wrote:
> On Wednesday, November 26, 2014 10:33:04 AM Sudeep Holla wrote:
>> Hi Hanjun,
>>
>> On 26/11/14 09:53, Hanjun Guo wrote:
>>> Hi Sudeep,
>>>
>>> On 2014-11-25 22:48, Sudeep Holla wrote:

[...]

>>>>
>>>> This patch removes that incorrect comparision in
>>>> acpi_processor_hotadd_init as the lone user of this function is
>>>> already checking for correctness of phys_id before calling it.
>>>
>>> if (apic_id < 0) acpi_handle_debug(pr->handle, "failed to get CPU
>>> APIC ID.\n");
>>>
>>> it only check the value and print debug message but no returns, so I
>>> think the check in the following patch is still needed.
>>>
>>
>> Agreed, but that's something we need to fix in the logic and not by
>> changing these identifiers to signed values in the structures.
>
> I'd rather not change data structures just because of what one funtion returns.
>
> Instead, I'd do something like
>
> 	#define CPU_PHYS_ID_INVALID	(u32)(-1)
>
> change the function in question to return CPU_PHYS_ID_INVALID instead of -1
> and change the check to
>
> 	if (phys_id == CPU_PHYS_ID_INVALID)
> 		...
>

Do I need to rebase this on top of Hanjun's cleanups to convert apic_id
to phys_id ? Since the variable is getting renamed it will conflict.

Regards,
Sudeep