On Mon, May 27, 2019 at 02:21:14PM +0200, Igor Mammedov wrote:
>On Thu, 11 Apr 2019 15:17:39 +0800
>Wei Yang <richardw.y...@linux.intel.com> wrote:
>
>> pc_memory_pre_plug() is called during hotplug for both pc-dimm and
>> nvdimm. This is more proper to check apci hotplug capability before
>> check nvdimm specific capability.
>not sure what this about.
>Currently we are checking if ACPI is enabled
> if (!pcms->acpi_dev || !acpi_enabled) { ...
>before nvdimm check and it looks better to me that we cancel
>nvdimm hotplug earlier than passing it to
> hotplug_handler_pre_plug(pcms->acpi_dev, dev, &local_err)
>with this patch ACPI device handler will be called before
>nvdimm check happens, so it's +1 unnecessary call chain in
>the case of nvdimm, which I'd rather not have.
>
>Are there any issues with current call flow?
>(commit message doesn't really explaining why we need this patch)
>
My idea is to check more generic requirement and then specific one.
For example, the call flow looks like this:
pc_memory_pre_plug
piix4_device_pre_plug_cb | ich9_pm_device_pre_plug_cb
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) &&
!lpc->pm.acpi_memory_hotplug.is_enabled)
if (is_nvdimm && !ms->nvdimms_state->is_enabled)
In hotplug_handler_pre_plug(), it checks the acpi hotplug capability. And then
if it has memory hotplug capability and is nvdimm, we check whether nvdimm is
enabled.
This is why I suggest to change the order here. No functional issue for
current code.
--
Wei Yang
Help you, Help me
