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

[Hanjun Guo <hanjun.guo@linaro.org>]

On 2014年11月29日 07:26, Rafael J. Wysocki wrote:
> On Friday, November 28, 2014 07:05:09 PM Sudeep Holla wrote:
>> 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.
>
> Yes, it's better to rebase IMO.

Hi Sudeep, any updates for this patch? I'm working on introducing
typedef u32 phys_cpuid_t for x86 and ia64 for phys_id, and need
this CPU_PHYS_ID_INVALID macro, if you are not working on that, I
will restart the work to address the comments from Lorenzo and
Catalin [1].

[1]: https://lkml.org/lkml/2015/2/3/635

Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html