Re: [Qemu-devel] [RFC PATCH 06/11] introduce fixed-clock

[KONRAD Frederic]

Le 02/07/2016 à 01:07, Alistair Francis a écrit :
> On Mon, Jun 13, 2016 at 9:27 AM,  <address@hidden> wrote:
> > From: KONRAD Frederic <address@hidden>
> >
> > This is a fixed clock device.
> > It justs behave as an empty device with a parametrable output rate.
> >
> > Signed-off-by: KONRAD Frederic <address@hidden>
> > ---
> >   hw/misc/Makefile.objs         |  2 +
> >   hw/misc/fixed-clock.c         | 87 +++++++++++++++++++++++++++++++++++++++++++
> >   include/hw/misc/fixed-clock.h | 30 +++++++++++++++
> >   3 files changed, 119 insertions(+)
> >   create mode 100644 hw/misc/fixed-clock.c
> >   create mode 100644 include/hw/misc/fixed-clock.h
> >
> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> > index e504463..e8b8855 100644
> > --- a/hw/misc/Makefile.objs
> > +++ b/hw/misc/Makefile.objs
> > @@ -52,3 +52,5 @@ obj-$(CONFIG_MIPS_ITU) += mips_itu.o
> >   obj-$(CONFIG_PVPANIC) += pvpanic.o
> >   obj-$(CONFIG_EDU) += edu.o
> >   obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
> > +
> > +obj-y += fixed-clock.o
> > diff --git a/hw/misc/fixed-clock.c b/hw/misc/fixed-clock.c
> > new file mode 100644
> > index 0000000..c273a91
> > --- /dev/null
> > +++ b/hw/misc/fixed-clock.c
> > @@ -0,0 +1,87 @@
> > +/*
> > + * Fixed 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 "hw/qdev.h"
> > +#include "hw/misc/fixed-clock.h"
> > +#include "qemu/qemu-clock.h"
> > +#include "qapi/error.h"
> > +
> > +/* #define DEBUG_FIXED_CLOCK */
> Don't include this.
>
> > +
> > +#ifdef DEBUG_FIXED_CLOCK
> > +#define DPRINTF(fmt, ...) \
> > +do { printf("fixed-clock: " fmt , ## __VA_ARGS__); } while (0)
> It might be better to use __func__ here.
>
> It should also be qemu_log instead of printf().
>
> > +#else
> > +#define DPRINTF(fmt, ...) do { } while (0)
> > +#endif
> > +
> > +typedef struct {
> > +    DeviceState parent_obj;
> > +
> > +    uint32_t rate;
> > +    struct qemu_clk out;
> > +} FixedClock;
> Doesn't this need to be in the header file?

I think it's not necessary as we get the clock through the API?
> > +
> > +static Property fixed_clock_properties[] = {
> > +    DEFINE_PROP_UINT32("rate", FixedClock, rate, 0),
> > +    DEFINE_PROP_END_OF_LIST()
> > +};
> > +
> > +static void fixed_clock_realizefn(DeviceState *d, Error **errp)
> dev instead of d
>
> Thanks,
>
> Alistair
>
> > +{
> > +    FixedClock *s = FIXED_CLOCK(d);
> > +
> > +    qemu_clk_update_rate(&s->out, s->rate);
> > +}
> > +
> > +static void fixed_clock_instance_init(Object *obj)
> > +{
> > +    FixedClock *s = FIXED_CLOCK(obj);
> > +
> > +    object_initialize(&s->out, sizeof(s->out), TYPE_CLOCK);
> > +    qemu_clk_attach_to_device(DEVICE(obj), &s->out, "clk_out");
> > +}
> > +
> > +static void fixed_clock_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +    dc->realize = fixed_clock_realizefn;
> > +    dc->props = fixed_clock_properties;
> > +}
> > +
> > +static const TypeInfo fixed_clock_info = {
> > +    .name          = TYPE_FIXED_CLOCK,
> > +    .parent        = TYPE_DEVICE,
> > +    .instance_size = sizeof(FixedClock),
> > +    .instance_init = fixed_clock_instance_init,
> > +    .class_init    = fixed_clock_class_init,
> > +};
> > +
> > +static void fixed_clock_register_types(void)
> > +{
> > +    type_register_static(&fixed_clock_info);
> > +}
> > +
> > +type_init(fixed_clock_register_types);
> > diff --git a/include/hw/misc/fixed-clock.h b/include/hw/misc/fixed-clock.h
> > new file mode 100644
> > index 0000000..1376444
> > --- /dev/null
> > +++ b/include/hw/misc/fixed-clock.h
> > @@ -0,0 +1,30 @@
> > +/*
> > + * Fixed 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/>.
> > + *
> > + */
> > +
> > +#ifndef FIXED_CLOCK_H
> > +#define FIXED_CLOCK_H
> > +
> > +#define TYPE_FIXED_CLOCK "fixed-clock"
> > +#define FIXED_CLOCK(obj) OBJECT_CHECK(FixedClock, (obj), TYPE_FIXED_CLOCK)
> > +
> > +#endif /* FIXED_CLOCK_H */
> > --
> > 2.5.5