[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V2 01/10] qemu-clk: introduce qemu-clk qom objec
From: |
Frederic Konrad |
Subject: |
Re: [Qemu-devel] [PATCH V2 01/10] qemu-clk: introduce qemu-clk qom object |
Date: |
Tue, 7 Feb 2017 10:20:59 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.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, address@hidden wrote:
>> From: KONRAD Frederic <address@hidden>
>>
>> This introduces qemu-clk qom object.
>>
>> Signed-off-by: KONRAD Frederic <address@hidden>
>> ---
>> 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: address@hidden
>> + *
>> + * Frederic Konrad <address@hidden>
>> + *
>> + * 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: address@hidden
>> + *
>> + * Frederic Konrad <address@hidden>
>> + *
>> + * 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);
>>
>
>