| Message ID | 20190227075101.6263-5-richardw.yang@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nvdimm: clean up | expand |
On Wed, 27 Feb 2019 15:51:01 +0800 Wei Yang <richardw.yang@linux.intel.com> wrote: > Currently we initialize nvdimm device like below: > > device_set_realized > dc->realize > nvdimm_plug > nvdimm_build_fit_buffer > acpi_build > nvdimm_build_acpi > > This shows nvdimm's acpi stuff is prepared in two places: > > * device plug stage > * acpi_build stage > > It would be more proper to build fit buffer in nvdimm_build_acpi, which > consolidate all acpi related initialization together. > > This patch moves FIT build in nvdimm_build_acpi. Does hotplug work after this change? > > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > --- > hw/acpi/nvdimm.c | 6 +----- > hw/i386/pc.c | 5 ----- > include/hw/mem/nvdimm.h | 1 - > 3 files changed, 1 insertion(+), 11 deletions(-) > > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c > index 2457c1aa44..6fc0c65ee0 100644 > --- a/hw/acpi/nvdimm.c > +++ b/hw/acpi/nvdimm.c > @@ -425,11 +425,6 @@ static void nvdimm_build_fit_buffer(AcpiNVDIMMState *state) > fit_buf->dirty = true; > } > > -void nvdimm_plug(AcpiNVDIMMState *state) > -{ > - nvdimm_build_fit_buffer(state); > -} > - > static void nvdimm_build_nfit(AcpiNVDIMMState *state, GArray *table_offsets, > GArray *table_data, BIOSLinker *linker) > { > @@ -1338,6 +1333,7 @@ void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data, > return; > } > > + nvdimm_build_fit_buffer(state); > nvdimm_build_nfit(state, table_offsets, table_data, linker); > g_slist_free(device_list); > } > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 64d9e756fa..686b610c50 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -2099,17 +2099,12 @@ static void pc_memory_plug(HotplugHandler *hotplug_dev, > { > Error *local_err = NULL; > PCMachineState *pcms = PC_MACHINE(hotplug_dev); > - bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM); > > pc_dimm_plug(PC_DIMM(dev), MACHINE(pcms), &local_err); > if (local_err) { > goto out; > } > > - if (is_nvdimm) { > - nvdimm_plug(&pcms->acpi_nvdimm_state); > - } > - > hotplug_handler_plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &error_abort); > out: > error_propagate(errp, local_err); > diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h > index c5c9b3c7f8..30869b4834 100644 > --- a/include/hw/mem/nvdimm.h > +++ b/include/hw/mem/nvdimm.h > @@ -148,6 +148,5 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io, > void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data, > BIOSLinker *linker, AcpiNVDIMMState *state, > uint32_t ram_slots); > -void nvdimm_plug(AcpiNVDIMMState *state); > void nvdimm_acpi_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev); > #endif
On Wed, Mar 06, 2019 at 05:14:06PM +0100, Igor Mammedov wrote: >On Wed, 27 Feb 2019 15:51:01 +0800 >Wei Yang <richardw.yang@linux.intel.com> wrote: > >> Currently we initialize nvdimm device like below: >> >> device_set_realized >> dc->realize >> nvdimm_plug >> nvdimm_build_fit_buffer >> acpi_build >> nvdimm_build_acpi >> >> This shows nvdimm's acpi stuff is prepared in two places: >> >> * device plug stage >> * acpi_build stage >> >> It would be more proper to build fit buffer in nvdimm_build_acpi, which >> consolidate all acpi related initialization together. >> >> This patch moves FIT build in nvdimm_build_acpi. > >Does hotplug work after this change? > Looks no. I missed to test this case. >> >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >> --- >> hw/acpi/nvdimm.c | 6 +----- >> hw/i386/pc.c | 5 ----- >> include/hw/mem/nvdimm.h | 1 - >> 3 files changed, 1 insertion(+), 11 deletions(-) >> >> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c >> index 2457c1aa44..6fc0c65ee0 100644 >> --- a/hw/acpi/nvdimm.c >> +++ b/hw/acpi/nvdimm.c >> @@ -425,11 +425,6 @@ static void nvdimm_build_fit_buffer(AcpiNVDIMMState *state) >> fit_buf->dirty = true; >> } >> >> -void nvdimm_plug(AcpiNVDIMMState *state) >> -{ >> - nvdimm_build_fit_buffer(state); >> -} >> - >> static void nvdimm_build_nfit(AcpiNVDIMMState *state, GArray *table_offsets, >> GArray *table_data, BIOSLinker *linker) >> { >> @@ -1338,6 +1333,7 @@ void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data, >> return; >> } >> >> + nvdimm_build_fit_buffer(state); >> nvdimm_build_nfit(state, table_offsets, table_data, linker); >> g_slist_free(device_list); >> } >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> index 64d9e756fa..686b610c50 100644 >> --- a/hw/i386/pc.c >> +++ b/hw/i386/pc.c >> @@ -2099,17 +2099,12 @@ static void pc_memory_plug(HotplugHandler *hotplug_dev, >> { >> Error *local_err = NULL; >> PCMachineState *pcms = PC_MACHINE(hotplug_dev); >> - bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM); >> >> pc_dimm_plug(PC_DIMM(dev), MACHINE(pcms), &local_err); >> if (local_err) { >> goto out; >> } >> >> - if (is_nvdimm) { >> - nvdimm_plug(&pcms->acpi_nvdimm_state); >> - } >> - >> hotplug_handler_plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &error_abort); >> out: >> error_propagate(errp, local_err); >> diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h >> index c5c9b3c7f8..30869b4834 100644 >> --- a/include/hw/mem/nvdimm.h >> +++ b/include/hw/mem/nvdimm.h >> @@ -148,6 +148,5 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io, >> void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data, >> BIOSLinker *linker, AcpiNVDIMMState *state, >> uint32_t ram_slots); >> -void nvdimm_plug(AcpiNVDIMMState *state); >> void nvdimm_acpi_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev); >> #endif
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c index 2457c1aa44..6fc0c65ee0 100644 --- a/hw/acpi/nvdimm.c +++ b/hw/acpi/nvdimm.c @@ -425,11 +425,6 @@ static void nvdimm_build_fit_buffer(AcpiNVDIMMState *state) fit_buf->dirty = true; } -void nvdimm_plug(AcpiNVDIMMState *state) -{ - nvdimm_build_fit_buffer(state); -} - static void nvdimm_build_nfit(AcpiNVDIMMState *state, GArray *table_offsets, GArray *table_data, BIOSLinker *linker) { @@ -1338,6 +1333,7 @@ void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data, return; } + nvdimm_build_fit_buffer(state); nvdimm_build_nfit(state, table_offsets, table_data, linker); g_slist_free(device_list); } diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 64d9e756fa..686b610c50 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -2099,17 +2099,12 @@ static void pc_memory_plug(HotplugHandler *hotplug_dev, { Error *local_err = NULL; PCMachineState *pcms = PC_MACHINE(hotplug_dev); - bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM); pc_dimm_plug(PC_DIMM(dev), MACHINE(pcms), &local_err); if (local_err) { goto out; } - if (is_nvdimm) { - nvdimm_plug(&pcms->acpi_nvdimm_state); - } - hotplug_handler_plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &error_abort); out: error_propagate(errp, local_err); diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h index c5c9b3c7f8..30869b4834 100644 --- a/include/hw/mem/nvdimm.h +++ b/include/hw/mem/nvdimm.h @@ -148,6 +148,5 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io, void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data, BIOSLinker *linker, AcpiNVDIMMState *state, uint32_t ram_slots); -void nvdimm_plug(AcpiNVDIMMState *state); void nvdimm_acpi_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev); #endif
Currently we initialize nvdimm device like below: device_set_realized dc->realize nvdimm_plug nvdimm_build_fit_buffer acpi_build nvdimm_build_acpi This shows nvdimm's acpi stuff is prepared in two places: * device plug stage * acpi_build stage It would be more proper to build fit buffer in nvdimm_build_acpi, which consolidate all acpi related initialization together. This patch moves FIT build in nvdimm_build_acpi. Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> --- hw/acpi/nvdimm.c | 6 +----- hw/i386/pc.c | 5 ----- include/hw/mem/nvdimm.h | 1 - 3 files changed, 1 insertion(+), 11 deletions(-)