| Message ID | 1453863283-7562-10-git-send-email-zxq_yx_007@163.com |
|---|---|
| State | New |
| Headers | show |
On 27 January 2016 at 02:54, xiaoqiang zhao <zxq_yx_007@163.com> wrote: > * split milkymist_sysctl_init into milkymist_sysctl_info.instance_init and milkymist_sysctl_realize I think the "info" in this function name is wrong ? > * use DeviceClass::realize instead of SysBusDeviceClass::init Please make sure you line wrap your commit messages (at somewhere around 70-72 columns is usual). Otherwise: Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Again, you should cc the maintainer for this device. thanks -- PMM
在 2016年02月16日 02:14, Peter Maydell 写道: > On 27 January 2016 at 02:54, xiaoqiang zhao<zxq_yx_007@163.com> wrote: >> >* split milkymist_sysctl_init into milkymist_sysctl_info.instance_init and milkymist_sysctl_realize > I think the "info" in this function name is wrong ? > I can not understand , can you give me more details?
On 16 February 2016 at 09:34, hitmoon <zxq_yx_007@163.com> wrote: > > > 在 2016年02月16日 02:14, Peter Maydell 写道: >> >> On 27 January 2016 at 02:54, xiaoqiang zhao<zxq_yx_007@163.com> wrote: >>> >>> >* split milkymist_sysctl_init into milkymist_sysctl_info.instance_init >>> > and milkymist_sysctl_realize >> >> I think the "info" in this function name is wrong ? >> > I can not understand , can you give me more details? The two functions which you have split the old milkymist_sysctl_init() into are named "milkymist_sysctl_init()" and "milkymist_sysctl_realize()". It confused me that you said "split FUNCTION into STRUCT.FIELDNAME and FUNCTION"; I expected to read "split FUNCTION into FUNCTION and FUNCTION". If you want you could just say "Split the old SysBus init function into an instance_init and a Device realize function." (at the moment your two bullet points in the commit message are actually both describing the same thing.) thanks -- PMM
在 2016年02月16日 17:41, Peter Maydell 写道: > On 16 February 2016 at 09:34, hitmoon <zxq_yx_007@163.com> wrote: >> >> 在 2016年02月16日 02:14, Peter Maydell 写道: >>> On 27 January 2016 at 02:54, xiaoqiang zhao<zxq_yx_007@163.com> wrote: >>>>> * split milkymist_sysctl_init into milkymist_sysctl_info.instance_init >>>>> and milkymist_sysctl_realize >>> I think the "info" in this function name is wrong ? >>> >> I can not understand , can you give me more details? > The two functions which you have split the old > milkymist_sysctl_init() into are named "milkymist_sysctl_init()" > and "milkymist_sysctl_realize()". It confused me that you > said "split FUNCTION into STRUCT.FIELDNAME and FUNCTION"; > I expected to read "split FUNCTION into FUNCTION and FUNCTION". > > If you want you could just say > "Split the old SysBus init function into an instance_init > and a Device realize function." > (at the moment your two bullet points in the commit message > are actually both describing the same thing.) > > thanks > -- PMM I see ;-)
I have sent the v3, which fix this problem. > 在 2016年2月16日，02:14，Peter Maydell <peter.maydell@linaro.org> 写道： > >> On 27 January 2016 at 02:54, xiaoqiang zhao <zxq_yx_007@163.com> wrote: >> * split milkymist_sysctl_init into milkymist_sysctl_info.instance_init and milkymist_sysctl_realize > > I think the "info" in this function name is wrong ? > >> * use DeviceClass::realize instead of SysBusDeviceClass::init > > Please make sure you line wrap your commit messages (at somewhere > around 70-72 columns is usual). > > Otherwise: > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > Again, you should cc the maintainer for this device. > > thanks > -- PMM >
diff --git a/hw/timer/milkymist-sysctl.c b/hw/timer/milkymist-sysctl.c index 30535a4..e29c823 100644 --- a/hw/timer/milkymist-sysctl.c +++ b/hw/timer/milkymist-sysctl.c @@ -269,9 +269,10 @@ static void milkymist_sysctl_reset(DeviceState *d) s->regs[R_GPIO_IN] = s->strappings; } -static int milkymist_sysctl_init(SysBusDevice *dev) +static void milkymist_sysctl_init(Object *obj) { - MilkymistSysctlState *s = MILKYMIST_SYSCTL(dev); + MilkymistSysctlState *s = MILKYMIST_SYSCTL(obj); + SysBusDevice *dev = SYS_BUS_DEVICE(obj); sysbus_init_irq(dev, &s->gpio_irq); sysbus_init_irq(dev, &s->timer0_irq); @@ -281,14 +282,18 @@ static int milkymist_sysctl_init(SysBusDevice *dev) s->bh1 = qemu_bh_new(timer1_hit, s); s->ptimer0 = ptimer_init(s->bh0); s->ptimer1 = ptimer_init(s->bh1); - ptimer_set_freq(s->ptimer0, s->freq_hz); - ptimer_set_freq(s->ptimer1, s->freq_hz); memory_region_init_io(&s->regs_region, OBJECT(s), &sysctl_mmio_ops, s, "milkymist-sysctl", R_MAX * 4); sysbus_init_mmio(dev, &s->regs_region); +} - return 0; +static void milkymist_sysctl_realize(DeviceState *dev, Error **errp) +{ + MilkymistSysctlState *s = MILKYMIST_SYSCTL(dev); + + ptimer_set_freq(s->ptimer0, s->freq_hz); + ptimer_set_freq(s->ptimer1, s->freq_hz); } static const VMStateDescription vmstate_milkymist_sysctl = { @@ -318,9 +323,8 @@ static Property milkymist_sysctl_properties[] = { static void milkymist_sysctl_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - k->init = milkymist_sysctl_init; + dc->realize = milkymist_sysctl_realize; dc->reset = milkymist_sysctl_reset; dc->vmsd = &vmstate_milkymist_sysctl; dc->props = milkymist_sysctl_properties; @@ -330,6 +334,7 @@ static const TypeInfo milkymist_sysctl_info = { .name = TYPE_MILKYMIST_SYSCTL, .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(MilkymistSysctlState), + .instance_init = milkymist_sysctl_init, .class_init = milkymist_sysctl_class_init, };
* split milkymist_sysctl_init into milkymist_sysctl_info.instance_init and milkymist_sysctl_realize * use DeviceClass::realize instead of SysBusDeviceClass::init Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com> --- hw/timer/milkymist-sysctl.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)