diff mbox

[V2,01/10] qemu-clk: introduce qemu-clk qom object

Message ID 1485424060-12217-2-git-send-email-fred.konrad@greensocs.com (mailing list archive)
State New, archived
Headers show

Commit Message

KONRAD Frédéric Jan. 26, 2017, 9:47 a.m. UTC
From: KONRAD Frederic <fred.konrad@greensocs.com>

This introduces qemu-clk qom object.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 Makefile.objs             |  1 +
 include/qemu/qemu-clock.h | 40 +++++++++++++++++++++++++++++++++++++
 qemu-clock.c              | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+)
 create mode 100644 include/qemu/qemu-clock.h
 create mode 100644 qemu-clock.c

Comments

Cédric Le Goater Feb. 5, 2017, 3:25 p.m. UTC | #1
ello Frederic,

On 01/26/2017 10:47 AM, fred.konrad@greensocs.com wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> This introduces qemu-clk qom object.
> 
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> ---
>  Makefile.objs             |  1 +
>  include/qemu/qemu-clock.h | 40 +++++++++++++++++++++++++++++++++++++
>  qemu-clock.c              | 50 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 91 insertions(+)
>  create mode 100644 include/qemu/qemu-clock.h
>  create mode 100644 qemu-clock.c
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 01cef86..de0219d 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -78,6 +78,7 @@ common-obj-y += backends/
>  common-obj-$(CONFIG_SECCOMP) += qemu-seccomp.o
>  
>  common-obj-$(CONFIG_FDT) += device_tree.o
> +common-obj-y += qemu-clock.o
>  
>  ######################################################################
>  # qapi
> diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
> new file mode 100644
> index 0000000..e7acd68
> --- /dev/null
> +++ b/include/qemu/qemu-clock.h
> @@ -0,0 +1,40 @@
> +/*
> + * QEMU Clock
> + *
> + *  Copyright (C) 2016 : GreenSocs Ltd
> + *      http://www.greensocs.com/ , email: info@greensocs.com
> + *
> + *  Frederic Konrad <fred.konrad@greensocs.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.

May be we could shorten the GPL v2 statement to something like :
 
 * This code is licensed under the GPL version 2 or later. See the
 * COPYING file in the top-level directory.

I haven't been very good at that my self, but we could save 
quite a few lines in qemu if files were not repeating the 
License statement.


> + *
> + */
> +
> +#ifndef QEMU_CLOCK_H
> +#define QEMU_CLOCK_H
> +
> +#include "qemu/osdep.h"
> +#include "qom/object.h"
> +
> +#define TYPE_CLOCK "qemu-clk"
> +#define QEMU_CLOCK(obj) OBJECT_CHECK(struct qemu_clk, (obj), TYPE_CLOCK)
> +
> +typedef struct qemu_clk {
> +    /*< private >*/
> +    Object parent_obj;
> +} *qemu_clk;
>

CODING_STYLE says that we should use CamelCase for typedef. Also, I don't 
think the '*' is required. What's the idea ? 

> +#endif /* QEMU_CLOCK_H */
> +
> +
> diff --git a/qemu-clock.c b/qemu-clock.c
> new file mode 100644
> index 0000000..ceea98d
> --- /dev/null
> +++ b/qemu-clock.c
> @@ -0,0 +1,50 @@
> +/*
> + * QEMU Clock
> + *
> + *  Copyright (C) 2016 : GreenSocs Ltd
> + *      http://www.greensocs.com/ , email: info@greensocs.com
> + *
> + *  Frederic Konrad <fred.konrad@greensocs.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/qemu-clock.h"
> +#include "hw/hw.h"
> +#include "qemu/log.h"
> +
> +#ifndef DEBUG_QEMU_CLOCK
> +#define DEBUG_QEMU_CLOCK 0
> +#endif
> +
> +#define DPRINTF(fmt, args...) do {                                           \
> +    if (DEBUG_QEMU_CLOCK) {                                                  \
> +        qemu_log("%s: " fmt, __func__, ## args);                             \
> +    }                                                                        \
> +} while (0);

I think the trace framework should be used instead.

Thanks

C.

> +static const TypeInfo qemu_clk_info = {
> +    .name          = TYPE_CLOCK,
> +    .parent        = TYPE_OBJECT,
> +    .instance_size = sizeof(struct qemu_clk),
> +};
> +
> +static void qemu_clk_register_types(void)
> +{
> +    type_register_static(&qemu_clk_info);
> +}
> +
> +type_init(qemu_clk_register_types);
>
KONRAD Frédéric Feb. 7, 2017, 9:20 a.m. UTC | #2
On 02/05/2017 04:25 PM, Cédric Le Goater wrote:
> ello Frederic,

Hi Cédric,

Thanks for taking a look at this!

> 
> On 01/26/2017 10:47 AM, fred.konrad@greensocs.com wrote:
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> This introduces qemu-clk qom object.
>>
>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>> ---
>>  Makefile.objs             |  1 +
>>  include/qemu/qemu-clock.h | 40 +++++++++++++++++++++++++++++++++++++
>>  qemu-clock.c              | 50 +++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 91 insertions(+)
>>  create mode 100644 include/qemu/qemu-clock.h
>>  create mode 100644 qemu-clock.c
>>
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 01cef86..de0219d 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -78,6 +78,7 @@ common-obj-y += backends/
>>  common-obj-$(CONFIG_SECCOMP) += qemu-seccomp.o
>>  
>>  common-obj-$(CONFIG_FDT) += device_tree.o
>> +common-obj-y += qemu-clock.o
>>  
>>  ######################################################################
>>  # qapi
>> diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
>> new file mode 100644
>> index 0000000..e7acd68
>> --- /dev/null
>> +++ b/include/qemu/qemu-clock.h
>> @@ -0,0 +1,40 @@
>> +/*
>> + * QEMU Clock
>> + *
>> + *  Copyright (C) 2016 : GreenSocs Ltd
>> + *      http://www.greensocs.com/ , email: info@greensocs.com
>> + *
>> + *  Frederic Konrad <fred.konrad@greensocs.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation, either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along
>> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> 
> May be we could shorten the GPL v2 statement to something like :
>  
>  * This code is licensed under the GPL version 2 or later. See the
>  * COPYING file in the top-level directory.
> 
> I haven't been very good at that my self, but we could save 
> quite a few lines in qemu if files were not repeating the 
> License statement.

Well if it is really needed I can get rid of this few lines :).

> 
> 
>> + *
>> + */
>> +
>> +#ifndef QEMU_CLOCK_H
>> +#define QEMU_CLOCK_H
>> +
>> +#include "qemu/osdep.h"
>> +#include "qom/object.h"
>> +
>> +#define TYPE_CLOCK "qemu-clk"
>> +#define QEMU_CLOCK(obj) OBJECT_CHECK(struct qemu_clk, (obj), TYPE_CLOCK)
>> +
>> +typedef struct qemu_clk {
>> +    /*< private >*/
>> +    Object parent_obj;
>> +} *qemu_clk;
>>
> 
> CODING_STYLE says that we should use CamelCase for typedef. Also, I don't 
> think the '*' is required. What's the idea ? 

Your right, I tried to follow the gpio "style".. But maybe it's old and
we better go for the camelcase and without the pointer.

> 
>> +#endif /* QEMU_CLOCK_H */
>> +
>> +
>> diff --git a/qemu-clock.c b/qemu-clock.c
>> new file mode 100644
>> index 0000000..ceea98d
>> --- /dev/null
>> +++ b/qemu-clock.c
>> @@ -0,0 +1,50 @@
>> +/*
>> + * QEMU Clock
>> + *
>> + *  Copyright (C) 2016 : GreenSocs Ltd
>> + *      http://www.greensocs.com/ , email: info@greensocs.com
>> + *
>> + *  Frederic Konrad <fred.konrad@greensocs.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation, either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along
>> + * with this program; if not, see <http://www.gnu.org/licenses/>.
>> + *
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu/qemu-clock.h"
>> +#include "hw/hw.h"
>> +#include "qemu/log.h"
>> +
>> +#ifndef DEBUG_QEMU_CLOCK
>> +#define DEBUG_QEMU_CLOCK 0
>> +#endif
>> +
>> +#define DPRINTF(fmt, args...) do {                                           \
>> +    if (DEBUG_QEMU_CLOCK) {                                                  \
>> +        qemu_log("%s: " fmt, __func__, ## args);                             \
>> +    }                                                                        \
>> +} while (0);
> 
> I think the trace framework should be used instead.

Seems not really standard AFAIK.
Some files use printf, some other qemu_log etc..

Thanks,
Fred
> 
> Thanks
> 
> C.
> 
>> +static const TypeInfo qemu_clk_info = {
>> +    .name          = TYPE_CLOCK,
>> +    .parent        = TYPE_OBJECT,
>> +    .instance_size = sizeof(struct qemu_clk),
>> +};
>> +
>> +static void qemu_clk_register_types(void)
>> +{
>> +    type_register_static(&qemu_clk_info);
>> +}
>> +
>> +type_init(qemu_clk_register_types);
>>
> 
>
diff mbox

Patch

diff --git a/Makefile.objs b/Makefile.objs
index 01cef86..de0219d 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -78,6 +78,7 @@  common-obj-y += backends/
 common-obj-$(CONFIG_SECCOMP) += qemu-seccomp.o
 
 common-obj-$(CONFIG_FDT) += device_tree.o
+common-obj-y += qemu-clock.o
 
 ######################################################################
 # qapi
diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
new file mode 100644
index 0000000..e7acd68
--- /dev/null
+++ b/include/qemu/qemu-clock.h
@@ -0,0 +1,40 @@ 
+/*
+ * QEMU Clock
+ *
+ *  Copyright (C) 2016 : GreenSocs Ltd
+ *      http://www.greensocs.com/ , email: info@greensocs.com
+ *
+ *  Frederic Konrad <fred.konrad@greensocs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef QEMU_CLOCK_H
+#define QEMU_CLOCK_H
+
+#include "qemu/osdep.h"
+#include "qom/object.h"
+
+#define TYPE_CLOCK "qemu-clk"
+#define QEMU_CLOCK(obj) OBJECT_CHECK(struct qemu_clk, (obj), TYPE_CLOCK)
+
+typedef struct qemu_clk {
+    /*< private >*/
+    Object parent_obj;
+} *qemu_clk;
+
+#endif /* QEMU_CLOCK_H */
+
+
diff --git a/qemu-clock.c b/qemu-clock.c
new file mode 100644
index 0000000..ceea98d
--- /dev/null
+++ b/qemu-clock.c
@@ -0,0 +1,50 @@ 
+/*
+ * QEMU Clock
+ *
+ *  Copyright (C) 2016 : GreenSocs Ltd
+ *      http://www.greensocs.com/ , email: info@greensocs.com
+ *
+ *  Frederic Konrad <fred.konrad@greensocs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/qemu-clock.h"
+#include "hw/hw.h"
+#include "qemu/log.h"
+
+#ifndef DEBUG_QEMU_CLOCK
+#define DEBUG_QEMU_CLOCK 0
+#endif
+
+#define DPRINTF(fmt, args...) do {                                           \
+    if (DEBUG_QEMU_CLOCK) {                                                  \
+        qemu_log("%s: " fmt, __func__, ## args);                             \
+    }                                                                        \
+} while (0);
+
+static const TypeInfo qemu_clk_info = {
+    .name          = TYPE_CLOCK,
+    .parent        = TYPE_OBJECT,
+    .instance_size = sizeof(struct qemu_clk),
+};
+
+static void qemu_clk_register_types(void)
+{
+    type_register_static(&qemu_clk_info);
+}
+
+type_init(qemu_clk_register_types);