diff mbox series

[11/17] hw/misc/max111x: Create header file for documentation, TYPE_ macros

Message ID 20200628142429.17111-12-peter.maydell@linaro.org
State Superseded
Headers show
Series spitz: fix hacks, fix CID 1421913, various cleanups | expand

Commit Message

Peter Maydell June 28, 2020, 2:24 p.m. UTC 
Create a header file for the hw/misc/max111x device, in the
usual modern style for QOM devices:
 * definition of the TYPE_ constants and macros
 * definition of the device's state struct so that it can
   be embedded in other structs if desired
 * documentation of the interface

This allows us to use TYPE_MAX_1111 in the spitz.c code rather
than the string "max1111".

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
To be honest the main driver here is that I wanted a header
file to put the documentation comment in :-)
---
 include/hw/misc/max111x.h | 57 +++++++++++++++++++++++++++++++++++++++
 hw/arm/spitz.c            |  3 ++-
 hw/misc/max111x.c         | 25 +----------------
 MAINTAINERS               |  1 +
 4 files changed, 61 insertions(+), 25 deletions(-)
 create mode 100644 include/hw/misc/max111x.h

-- 
2.20.1

Comments

Philippe Mathieu-Daudé June 29, 2020, 8:29 a.m. UTC | #1 
On 6/28/20 4:24 PM, Peter Maydell wrote:
> Create a header file for the hw/misc/max111x device, in the

> usual modern style for QOM devices:

>  * definition of the TYPE_ constants and macros

>  * definition of the device's state struct so that it can

>    be embedded in other structs if desired

>  * documentation of the interface

> 

> This allows us to use TYPE_MAX_1111 in the spitz.c code rather

> than the string "max1111".

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

> To be honest the main driver here is that I wanted a header

> file to put the documentation comment in :-)

> ---

>  include/hw/misc/max111x.h | 57 +++++++++++++++++++++++++++++++++++++++

>  hw/arm/spitz.c            |  3 ++-

>  hw/misc/max111x.c         | 25 +----------------

>  MAINTAINERS               |  1 +

>  4 files changed, 61 insertions(+), 25 deletions(-)

>  create mode 100644 include/hw/misc/max111x.h

> 

> diff --git a/include/hw/misc/max111x.h b/include/hw/misc/max111x.h

> new file mode 100644

> index 00000000000..337ba63d588

> --- /dev/null

> +++ b/include/hw/misc/max111x.h

> @@ -0,0 +1,57 @@

> +/*

> + * Maxim MAX1110/1111 ADC chip emulation.

> + *

> + * Copyright (c) 2006 Openedhand Ltd.

> + * Written by Andrzej Zaborowski <balrog@zabor.org>

> + *

> + * This code is licensed under the GNU GPLv2.

> + *

> + * Contributions after 2012-01-13 are licensed under the terms of the

> + * GNU GPL, version 2 or (at your option) any later version.

> + */

> +

> +#ifndef HW_MISC_MAX111X_H

> +#define HW_MISC_MAX111X_H

> +

> +#include "hw/ssi/ssi.h"

> +#include "hw/irq.h"


"hw/irq.h" not needed as qemu_irq is forward-declared in
"qemu/typedefs.h".

> +

> +/*

> + * This is a model of the Maxim MAX1110/1111 ADC chip, which for QEMU

> + * is an SSI slave device. It has either 4 (max1110) or 8 (max1111)

> + * 8-bit ADC channels.

> + *

> + * QEMU interface:

> + *  + GPIO inputs 0..3 (for max1110) or 0..7 (for max1111): set the value

> + *    of each ADC input, as an unsigned 8-bit value

> + *  + GPIO output 0: interrupt line

> + *  + Properties "input0" to "input3" (max1110) or "input0" to "input7"

> + *    (max1111): initial reset values for ADC inputs.

> + *

> + * Known bugs:

> + *  + the interrupt line is not correctly implemented, and will never

> + *    be lowered once it has been asserted.

> + */

> +typedef struct {

> +    SSISlave parent_obj;

> +

> +    qemu_irq interrupt;

> +    /* Values of inputs at system reset (settable by QOM property) */

> +    uint8_t reset_input[8];

> +

> +    uint8_t tb1, rb2, rb3;

> +    int cycle;

> +

> +    uint8_t input[8];

> +    int inputs, com;

> +} MAX111xState;

> +

> +#define TYPE_MAX_111X "max111x"

> +

> +#define MAX_111X(obj) \

> +    OBJECT_CHECK(MAX111xState, (obj), TYPE_MAX_111X)


Nitpick, we can keep MAX_111X() + MAX111xState in "hw/misc/max111x.c"
until we get a consumer.

> +

> +#define TYPE_MAX_1110 "max1110"

> +#define TYPE_MAX_1111 "max1111"

> +

> +#endif

> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c

> index fa592aad6d6..1400a56729d 100644

> --- a/hw/arm/spitz.c

> +++ b/hw/arm/spitz.c

> @@ -29,6 +29,7 @@

>  #include "audio/audio.h"

>  #include "hw/boards.h"

>  #include "hw/sysbus.h"

> +#include "hw/misc/max111x.h"

>  #include "migration/vmstate.h"

>  #include "exec/address-spaces.h"

>  #include "cpu.h"

> @@ -732,7 +733,7 @@ static void spitz_ssp_attach(SpitzMachineState *sms)

>                            qdev_get_gpio_in(sms->mpu->gpio, SPITZ_GPIO_TP_INT));

>  

>      bus = qdev_get_child_bus(sms->mux, "ssi2");

> -    sms->max1111 = qdev_new("max1111");

> +    sms->max1111 = qdev_new(TYPE_MAX_1111);

>      max1111 = sms->max1111;

>      qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */,

>                          SPITZ_BATTERY_VOLT);

> diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c

> index 3a5cb838445..d41cfd92a8d 100644

> --- a/hw/misc/max111x.c

> +++ b/hw/misc/max111x.c

> @@ -11,34 +11,11 @@

>   */

>  

>  #include "qemu/osdep.h"

> -#include "hw/irq.h"


You still need "hw/irq.h" for qemu_irq_raise().

> -#include "hw/ssi/ssi.h"

> +#include "hw/misc/max111x.h"

>  #include "migration/vmstate.h"

>  #include "qemu/module.h"

>  #include "hw/qdev-properties.h"

>  

> -typedef struct {

> -    SSISlave parent_obj;

> -

> -    qemu_irq interrupt;

> -    /* Values of inputs at system reset (settable by QOM property) */

> -    uint8_t reset_input[8];

> -

> -    uint8_t tb1, rb2, rb3;

> -    int cycle;

> -

> -    uint8_t input[8];

> -    int inputs, com;

> -} MAX111xState;

> -

> -#define TYPE_MAX_111X "max111x"

> -

> -#define MAX_111X(obj) \

> -    OBJECT_CHECK(MAX111xState, (obj), TYPE_MAX_111X)

> -

> -#define TYPE_MAX_1110 "max1110"

> -#define TYPE_MAX_1111 "max1111"

> -

>  /* Control-byte bitfields */

>  #define CB_PD0		(1 << 0)

>  #define CB_PD1		(1 << 1)

> diff --git a/MAINTAINERS b/MAINTAINERS

> index 1b40446c739..550844d1514 100644

> --- a/MAINTAINERS

> +++ b/MAINTAINERS

> @@ -787,6 +787,7 @@ F: hw/gpio/max7310.c

>  F: hw/gpio/zaurus.c

>  F: hw/misc/mst_fpga.c

>  F: hw/misc/max111x.c

> +F: include/hw/misc/max111x.h

>  F: include/hw/arm/pxa.h

>  F: include/hw/arm/sharpsl.h

>  F: include/hw/display/tc6393xb.h

>
Peter Maydell June 29, 2020, 12:07 p.m. UTC | #2 
On Mon, 29 Jun 2020 at 09:29, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>

> On 6/28/20 4:24 PM, Peter Maydell wrote:

> > Create a header file for the hw/misc/max111x device, in the

> > usual modern style for QOM devices:

> >  * definition of the TYPE_ constants and macros

> >  * definition of the device's state struct so that it can

> >    be embedded in other structs if desired

> >  * documentation of the interface

> >

> > This allows us to use TYPE_MAX_1111 in the spitz.c code rather

> > than the string "max1111".

> >

> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> > ---


> Nitpick, we can keep MAX_111X() + MAX111xState in "hw/misc/max111x.c"

> until we get a consumer.


This is deliberate, as noted in the commit message.

thanks
-- PMM
Philippe Mathieu-Daudé June 29, 2020, 2:57 p.m. UTC | #3 
On 6/29/20 2:07 PM, Peter Maydell wrote:
> On Mon, 29 Jun 2020 at 09:29, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

>>

>> On 6/28/20 4:24 PM, Peter Maydell wrote:

>>> Create a header file for the hw/misc/max111x device, in the

>>> usual modern style for QOM devices:

>>>  * definition of the TYPE_ constants and macros

>>>  * definition of the device's state struct so that it can

>>>    be embedded in other structs if desired


Ah, fine.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


>>>  * documentation of the interface

>>>

>>> This allows us to use TYPE_MAX_1111 in the spitz.c code rather

>>> than the string "max1111".

>>>

>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

>>> ---

> 

>> Nitpick, we can keep MAX_111X() + MAX111xState in "hw/misc/max111x.c"

>> until we get a consumer.

> 

> This is deliberate, as noted in the commit message.

> 

> thanks

> -- PMM

>
diff mbox series

Patch

diff --git a/include/hw/misc/max111x.h b/include/hw/misc/max111x.h
new file mode 100644
index 00000000000..337ba63d588
--- /dev/null
+++ b/include/hw/misc/max111x.h
@@ -0,0 +1,57 @@ 
+/*
+ * Maxim MAX1110/1111 ADC chip emulation.
+ *
+ * Copyright (c) 2006 Openedhand Ltd.
+ * Written by Andrzej Zaborowski <balrog@zabor.org>
+ *
+ * This code is licensed under the GNU GPLv2.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#ifndef HW_MISC_MAX111X_H
+#define HW_MISC_MAX111X_H
+
+#include "hw/ssi/ssi.h"
+#include "hw/irq.h"
+
+/*
+ * This is a model of the Maxim MAX1110/1111 ADC chip, which for QEMU
+ * is an SSI slave device. It has either 4 (max1110) or 8 (max1111)
+ * 8-bit ADC channels.
+ *
+ * QEMU interface:
+ *  + GPIO inputs 0..3 (for max1110) or 0..7 (for max1111): set the value
+ *    of each ADC input, as an unsigned 8-bit value
+ *  + GPIO output 0: interrupt line
+ *  + Properties "input0" to "input3" (max1110) or "input0" to "input7"
+ *    (max1111): initial reset values for ADC inputs.
+ *
+ * Known bugs:
+ *  + the interrupt line is not correctly implemented, and will never
+ *    be lowered once it has been asserted.
+ */
+typedef struct {
+    SSISlave parent_obj;
+
+    qemu_irq interrupt;
+    /* Values of inputs at system reset (settable by QOM property) */
+    uint8_t reset_input[8];
+
+    uint8_t tb1, rb2, rb3;
+    int cycle;
+
+    uint8_t input[8];
+    int inputs, com;
+} MAX111xState;
+
+#define TYPE_MAX_111X "max111x"
+
+#define MAX_111X(obj) \
+    OBJECT_CHECK(MAX111xState, (obj), TYPE_MAX_111X)
+
+#define TYPE_MAX_1110 "max1110"
+#define TYPE_MAX_1111 "max1111"
+
+#endif
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index fa592aad6d6..1400a56729d 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -29,6 +29,7 @@ 
 #include "audio/audio.h"
 #include "hw/boards.h"
 #include "hw/sysbus.h"
+#include "hw/misc/max111x.h"
 #include "migration/vmstate.h"
 #include "exec/address-spaces.h"
 #include "cpu.h"
@@ -732,7 +733,7 @@  static void spitz_ssp_attach(SpitzMachineState *sms)
                           qdev_get_gpio_in(sms->mpu->gpio, SPITZ_GPIO_TP_INT));
 
     bus = qdev_get_child_bus(sms->mux, "ssi2");
-    sms->max1111 = qdev_new("max1111");
+    sms->max1111 = qdev_new(TYPE_MAX_1111);
     max1111 = sms->max1111;
     qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */,
                         SPITZ_BATTERY_VOLT);
diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
index 3a5cb838445..d41cfd92a8d 100644
--- a/hw/misc/max111x.c
+++ b/hw/misc/max111x.c
@@ -11,34 +11,11 @@ 
  */
 
 #include "qemu/osdep.h"
-#include "hw/irq.h"
-#include "hw/ssi/ssi.h"
+#include "hw/misc/max111x.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
 #include "hw/qdev-properties.h"
 
-typedef struct {
-    SSISlave parent_obj;
-
-    qemu_irq interrupt;
-    /* Values of inputs at system reset (settable by QOM property) */
-    uint8_t reset_input[8];
-
-    uint8_t tb1, rb2, rb3;
-    int cycle;
-
-    uint8_t input[8];
-    int inputs, com;
-} MAX111xState;
-
-#define TYPE_MAX_111X "max111x"
-
-#define MAX_111X(obj) \
-    OBJECT_CHECK(MAX111xState, (obj), TYPE_MAX_111X)
-
-#define TYPE_MAX_1110 "max1110"
-#define TYPE_MAX_1111 "max1111"
-
 /* Control-byte bitfields */
 #define CB_PD0		(1 << 0)
 #define CB_PD1		(1 << 1)
diff --git a/MAINTAINERS b/MAINTAINERS
index 1b40446c739..550844d1514 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -787,6 +787,7 @@  F: hw/gpio/max7310.c
 F: hw/gpio/zaurus.c
 F: hw/misc/mst_fpga.c
 F: hw/misc/max111x.c
+F: include/hw/misc/max111x.h
 F: include/hw/arm/pxa.h
 F: include/hw/arm/sharpsl.h
 F: include/hw/display/tc6393xb.h