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

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

On 2015年02月16日 18:08, Sudeep Holla wrote:
>
>
> On 13/02/15 07:56, Hanjun Guo wrote:
>> 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].
>>
>
> IMO, it would be good if you work on that instead, so that there's no
> dependency created as it's needed for your ARM64 port anyway.

I think so, I just want to confirm that it will be no duplicate work
for us. I will prepare a patch.

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