Re: [PATCH 2/5] cmd646: create separate header and QOM type for CMD646_IDE

[Bernhard Beschow]

Am 9. Juni 2023 18:51:16 UTC schrieb Mark Cave-Ayland 
<mark.cave-ayland@ilande.co.uk>:
>This will enable CMD646-specific fields to be added to CMD6464IDEState in
>future.
>
>Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>---
> hw/ide/cmd646.c         |  4 +++-
> include/hw/ide/cmd646.h | 38 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 41 insertions(+), 1 deletion(-)
> create mode 100644 include/hw/ide/cmd646.h
>
>diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
>index 20f1e41d57..a3e227fba7 100644
>--- a/hw/ide/cmd646.c
>+++ b/hw/ide/cmd646.c
>@@ -33,6 +33,7 @@
> #include "sysemu/reset.h"
> 
> #include "hw/ide/pci.h"
>+#include "hw/ide/cmd646.h"
> #include "trace.h"
> 
> /* CMD646 specific */
>@@ -339,9 +340,10 @@ static void cmd646_ide_class_init(ObjectClass *klass, 
>void *data)
> }
> 
> static const TypeInfo cmd646_ide_info = {
>-    .name          = "cmd646-ide",
>+    .name          = TYPE_CMD646_IDE,
>     .parent        = TYPE_PCI_IDE,
>     .class_init    = cmd646_ide_class_init,
>+    .instance_size = sizeof(CMD646IDEState),
> };
> 
> static void cmd646_ide_register_types(void)
>diff --git a/include/hw/ide/cmd646.h b/include/hw/ide/cmd646.h
>new file mode 100644
>index 0000000000..4780b1132c
>--- /dev/null
>+++ b/include/hw/ide/cmd646.h
>@@ -0,0 +1,38 @@
>+/*
>+ * QEMU IDE Emulation: PCI cmd646 support.
>+ *
>+ * Copyright (c) 2003 Fabrice Bellard
>+ * Copyright (c) 2006 Openedhand Ltd.
>+ *
>+ * Permission is hereby granted, free of charge, to any person obtaining a 
>copy
>+ * of this software and associated documentation files (the "Software"), to 
>deal
>+ * in the Software without restriction, including without limitation the 
>rights
>+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>+ * copies of the Software, and to permit persons to whom the Software is
>+ * furnished to do so, subject to the following conditions:
>+ *
>+ * The above copyright notice and this permission notice shall be included in
>+ * all copies or substantial portions of the Software.
>+ *
>+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
>FROM,
>+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>+ * THE SOFTWARE.
>+ */
>+
>+#ifndef HW_IDE_CMD646_H
>+#define HW_IDE_CMD646_H
>+
>+#define TYPE_CMD646_IDE "cmd646-ide"
>+OBJECT_DECLARE_SIMPLE_TYPE(CMD646IDEState, CMD646_IDE)
>+
>+#include "hw/ide/pci.h"
>+
>+struct CMD646IDEState {
>+    PCIIDEState parent_obj;

No public / private sections here? From the naming this attribute is often 
considered private and the rest public. I guess what this scheme communicates 
is that `parent_obj` should be accessed via `IDE_PCI()` only.

>+};
>+
>+#endif