Re: [Qemu-devel] [PATCH 9/9] tpm_tis: remove instance_finalize callback

2014-07-31 Thread Peter Crosthwaite
On Wed, Jul 30, 2014 at 8:27 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 It is never used, since ISA device are not hot-unpluggable.


Is it not good design practice though for the uninit to be correctly
implemented regardless of whether there is current-day usage? This
seems like the kind of patch that would get reverted if anyone finds a
reason to un-init a QOM object beyond existing hotplug use-cases.

Regards,
Peter

 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  hw/tpm/tpm_tis.c | 8 
  1 file changed, 8 deletions(-)

 diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
 index d398c16..82747ee 100644
 --- a/hw/tpm/tpm_tis.c
 +++ b/hw/tpm/tpm_tis.c
 @@ -896,13 +896,6 @@ static void tpm_tis_initfn(Object *obj)
  s-mmio);
  }

 -static void tpm_tis_uninitfn(Object *obj)
 -{
 -TPMState *s = TPM(obj);
 -
 -memory_region_del_subregion(get_system_memory(), s-mmio);
 -}
 -
  static void tpm_tis_class_init(ObjectClass *klass, void *data)
  {
  DeviceClass *dc = DEVICE_CLASS(klass);
 @@ -918,7 +911,6 @@ static const TypeInfo tpm_tis_info = {
  .parent = TYPE_ISA_DEVICE,
  .instance_size = sizeof(TPMState),
  .instance_init = tpm_tis_initfn,
 -.instance_finalize = tpm_tis_uninitfn,
  .class_init  = tpm_tis_class_init,
  };

 --
 1.8.3.1





Re: [Qemu-devel] [PATCH 9/9] tpm_tis: remove instance_finalize callback

2014-07-31 Thread Paolo Bonzini
Il 31/07/2014 14:00, Peter Crosthwaite ha scritto:
 On Wed, Jul 30, 2014 at 8:27 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 It is never used, since ISA device are not hot-unpluggable.

 
 Is it not good design practice though for the uninit to be correctly
 implemented regardless of whether there is current-day usage? This
 seems like the kind of patch that would get reverted if anyone finds a
 reason to un-init a QOM object beyond existing hotplug use-cases.

But even then, it should be an unrealize method, not an
instance_finalize, shouldn't it?

Paolo

 Regards,
 Peter
 
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  hw/tpm/tpm_tis.c | 8 
  1 file changed, 8 deletions(-)

 diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
 index d398c16..82747ee 100644
 --- a/hw/tpm/tpm_tis.c
 +++ b/hw/tpm/tpm_tis.c
 @@ -896,13 +896,6 @@ static void tpm_tis_initfn(Object *obj)
  s-mmio);
  }

 -static void tpm_tis_uninitfn(Object *obj)
 -{
 -TPMState *s = TPM(obj);
 -
 -memory_region_del_subregion(get_system_memory(), s-mmio);
 -}
 -
  static void tpm_tis_class_init(ObjectClass *klass, void *data)
  {
  DeviceClass *dc = DEVICE_CLASS(klass);
 @@ -918,7 +911,6 @@ static const TypeInfo tpm_tis_info = {
  .parent = TYPE_ISA_DEVICE,
  .instance_size = sizeof(TPMState),
  .instance_init = tpm_tis_initfn,
 -.instance_finalize = tpm_tis_uninitfn,
  .class_init  = tpm_tis_class_init,
  };

 --
 1.8.3.1






Re: [Qemu-devel] [PATCH 9/9] tpm_tis: remove instance_finalize callback

2014-07-31 Thread Peter Crosthwaite
On Thu, Jul 31, 2014 at 10:05 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 31/07/2014 14:00, Peter Crosthwaite ha scritto:
 On Wed, Jul 30, 2014 at 8:27 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 It is never used, since ISA device are not hot-unpluggable.


 Is it not good design practice though for the uninit to be correctly
 implemented regardless of whether there is current-day usage? This
 seems like the kind of patch that would get reverted if anyone finds a
 reason to un-init a QOM object beyond existing hotplug use-cases.

 But even then, it should be an unrealize method, not an
 instance_finalize, shouldn't it?


True. I guess this makes your patch half the solution.

Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com

 Paolo

 Regards,
 Peter

 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  hw/tpm/tpm_tis.c | 8 
  1 file changed, 8 deletions(-)

 diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
 index d398c16..82747ee 100644
 --- a/hw/tpm/tpm_tis.c
 +++ b/hw/tpm/tpm_tis.c
 @@ -896,13 +896,6 @@ static void tpm_tis_initfn(Object *obj)
  s-mmio);
  }

 -static void tpm_tis_uninitfn(Object *obj)
 -{
 -TPMState *s = TPM(obj);
 -
 -memory_region_del_subregion(get_system_memory(), s-mmio);
 -}
 -
  static void tpm_tis_class_init(ObjectClass *klass, void *data)
  {
  DeviceClass *dc = DEVICE_CLASS(klass);
 @@ -918,7 +911,6 @@ static const TypeInfo tpm_tis_info = {
  .parent = TYPE_ISA_DEVICE,
  .instance_size = sizeof(TPMState),
  .instance_init = tpm_tis_initfn,
 -.instance_finalize = tpm_tis_uninitfn,
  .class_init  = tpm_tis_class_init,
  };

 --
 1.8.3.1