On Mon, Mar 25, 2019 at 07:42:27PM -0500, Corey Minyard wrote: > On Sun, Mar 24, 2019 at 07:05:23PM +0100, Ernest Esene wrote: > > Categorize devices in "uncategorised devices" section > > This patch is based on BiteSizedTask. > > > > Signed-off-by: Ernest Esene <erok...@gmail.com> > > I'm not 100% sure the use of this field. A couple > of comments on the IPMI one inline. > > > --- > > hw/dma/i82374.c | 2 ++ > > hw/i386/amd_iommu.c | 2 ++ > > hw/i386/intel_iommu.c | 2 ++ > > hw/i386/pc_piix.c | 1 + > > hw/ipmi/ipmi_bmc_extern.c | 2 ++ > > hw/ipmi/ipmi_bmc_sim.c | 2 ++ > > hw/ipmi/isa_ipmi_bt.c | 2 ++ > > hw/ipmi/isa_ipmi_kcs.c | 2 ++ > > hw/mem/nvdimm.c | 1 + > > hw/mem/pc-dimm.c | 1 + > > hw/tpm/tpm_tis.c | 3 +++ > > 11 files changed, 20 insertions(+) > > > > diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c > > index bf0b7ee0..39049c4d 100644 > > --- a/hw/ipmi/ipmi_bmc_extern.c > > +++ b/hw/ipmi/ipmi_bmc_extern.c > > @@ -526,6 +526,8 @@ static void ipmi_bmc_extern_class_init(ObjectClass *oc, > > void *data) > > dc->hotpluggable = false; > > dc->realize = ipmi_bmc_extern_realize; > > dc->props = ipmi_bmc_extern_properties; > > + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > > + dc->desc = "IPMI Baseboard management controller"; > > This is not exactly a bridge. None of the categories seem > to fit, though, a management device would be the best > category, but that's not available. misc is probably the > best. > > Also, the description might be betters a: "IPMI external > baseboard management controller" to distinguish it from > the next one... > > > } > > > > static const TypeInfo ipmi_bmc_extern_type = { > > diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c > > index 9b509f82..95a096fa 100644 > > --- a/hw/ipmi/ipmi_bmc_sim.c > > +++ b/hw/ipmi/ipmi_bmc_sim.c > > @@ -2016,6 +2016,8 @@ static void ipmi_sim_class_init(ObjectClass *oc, void > > *data) > > dc->realize = ipmi_sim_realize; > > dc->props = ipmi_sim_properties; > > bk->handle_command = ipmi_sim_handle_command; > > + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > > + dc->desc = "IPMI Baseboard management controller"; > > This is definitely not a bridge, same basic comment as above, > but this is an internal simulator of the device. For the > description, perhaps: "IPMI simulated baseboard management > controller" > > > } > > > > static const TypeInfo ipmi_sim_type = { > > diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c > > index 8bbb1fa7..9ca3402e 100644 > > --- a/hw/ipmi/isa_ipmi_bt.c > > +++ b/hw/ipmi/isa_ipmi_bt.c > > @@ -541,6 +541,8 @@ static void isa_ipmi_bt_class_init(ObjectClass *oc, > > void *data) > > > > dc->realize = isa_ipmi_bt_realize; > > dc->props = ipmi_isa_properties; > > + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > > + dc->desc = "ISA IPMI BT System Interface"; > > I'm ok with these being bridges, that seem accurate, and the > description looks good. Same for KCS below. > > Thanks, > > -corey > Thank you corey.
Ernest
signature.asc
Description: PGP signature