Re: [PATCH v4 32/37] omap-intc: remove PROP_PTR

[Philippe Mathieu-Daudé]

On 11/20/19 4:24 PM, Marc-André Lureau wrote:
> Since clocks are not QOM objects, replace PROP_PTR of clocks with
> setters methods.
>
> (in theory there should probably be different methods for omap1 &
> omap2 intc, but this is left as a future improvement)
>
> Reviewed-by: Peter Maydell <address@hidden>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>   hw/arm/omap1.c        |  4 ++--
>   hw/arm/omap2.c        |  4 ++--
>   hw/intc/omap_intc.c   | 17 ++++++++++-------
>   include/hw/arm/omap.h | 14 ++++++++++++++
>   4 files changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
> index 6ce038a453..1afd1d3d7f 100644
> --- a/hw/arm/omap1.c
> +++ b/hw/arm/omap1.c
> @@ -3889,7 +3889,7 @@ struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion 
> *dram,
>   
>       s->ih[0] = qdev_create(NULL, "omap-intc");
>       qdev_prop_set_uint32(s->ih[0], "size", 0x100);
> -    qdev_prop_set_ptr(s->ih[0], "clk", omap_findclk(s, "arminth_ck"));
> +    omap_intc_set_iclk(OMAP_INTC(s->ih[0]), omap_findclk(s, "arminth_ck"));
>       qdev_init_nofail(s->ih[0]);
>       busdev = SYS_BUS_DEVICE(s->ih[0]);
>       sysbus_connect_irq(busdev, 0,
> @@ -3899,7 +3899,7 @@ struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion 
> *dram,
>       sysbus_mmio_map(busdev, 0, 0xfffecb00);
>       s->ih[1] = qdev_create(NULL, "omap-intc");
>       qdev_prop_set_uint32(s->ih[1], "size", 0x800);
> -    qdev_prop_set_ptr(s->ih[1], "clk", omap_findclk(s, "arminth_ck"));
> +    omap_intc_set_iclk(OMAP_INTC(s->ih[1]), omap_findclk(s, "arminth_ck"));
>       qdev_init_nofail(s->ih[1]);
>       busdev = SYS_BUS_DEVICE(s->ih[1]);
>       sysbus_connect_irq(busdev, 0,
> diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
> index 457f152bac..1d7cc435ef 100644
> --- a/hw/arm/omap2.c
> +++ b/hw/arm/omap2.c
> @@ -2308,8 +2308,8 @@ struct omap_mpu_state_s *omap2420_mpu_init(MemoryRegion 
> *sdram,
>       /* Actually mapped at any 2K boundary in the ARM11 private-peripheral if 
> */
>       s->ih[0] = qdev_create(NULL, "omap2-intc");
>       qdev_prop_set_uint8(s->ih[0], "revision", 0x21);
> -    qdev_prop_set_ptr(s->ih[0], "fclk", omap_findclk(s, "mpu_intc_fclk"));
> -    qdev_prop_set_ptr(s->ih[0], "iclk", omap_findclk(s, "mpu_intc_iclk"));
> +    omap_intc_set_fclk(OMAP_INTC(s->ih[0]), omap_findclk(s, "mpu_intc_fclk"));
> +    omap_intc_set_iclk(OMAP_INTC(s->ih[0]), omap_findclk(s, "mpu_intc_iclk"));
>       qdev_init_nofail(s->ih[0]);
>       busdev = SYS_BUS_DEVICE(s->ih[0]);
>       sysbus_connect_irq(busdev, 0,
> diff --git a/hw/intc/omap_intc.c b/hw/intc/omap_intc.c
> index 854b709ca0..73bb1c2af4 100644
> --- a/hw/intc/omap_intc.c
> +++ b/hw/intc/omap_intc.c
> @@ -38,10 +38,6 @@ struct omap_intr_handler_bank_s {
>       unsigned char priority[32];
>   };
>   
> -#define TYPE_OMAP_INTC "common-omap-intc"
> -#define OMAP_INTC(obj) \
> -    OBJECT_CHECK(struct omap_intr_handler_s, (obj), TYPE_OMAP_INTC)
> -
>   struct omap_intr_handler_s {
>       SysBusDevice parent_obj;
>   
> @@ -391,9 +387,18 @@ static void omap_intc_realize(DeviceState *dev, Error **errp)
>       }
>   }
>   
> +void omap_intc_set_iclk(omap_intr_handler *intc, omap_clk clk)
> +{
> +    intc->iclk = clk;
> +}
> +
> +void omap_intc_set_fclk(omap_intr_handler *intc, omap_clk clk)
> +{
> +    intc->fclk = clk;
> +}
> +
>   static Property omap_intc_properties[] = {
>       DEFINE_PROP_UINT32("size", struct omap_intr_handler_s, size, 0x100),
> -    DEFINE_PROP_PTR("clk", struct omap_intr_handler_s, iclk),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   
> @@ -647,8 +652,6 @@ static void omap2_intc_realize(DeviceState *dev, Error **errp)
>   static Property omap2_intc_properties[] = {
>       DEFINE_PROP_UINT8("revision", struct omap_intr_handler_s,
>       revision, 0x21),
> -    DEFINE_PROP_PTR("iclk", struct omap_intr_handler_s, iclk),
> -    DEFINE_PROP_PTR("fclk", struct omap_intr_handler_s, fclk),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   
> diff --git a/include/hw/arm/omap.h b/include/hw/arm/omap.h
> index f3aa670036..bcecf19f89 100644
> --- a/include/hw/arm/omap.h
> +++ b/include/hw/arm/omap.h
> @@ -67,6 +67,20 @@ void omap_clk_setrate(omap_clk clk, int divide, int 
> multiply);
>   int64_t omap_clk_getrate(omap_clk clk);
>   void omap_clk_reparent(omap_clk clk, omap_clk parent);
>   
> +/* omap_intc.c */
> +#define TYPE_OMAP_INTC "common-omap-intc"
> +#define OMAP_INTC(obj)                                              \
> +    OBJECT_CHECK(omap_intr_handler, (obj), TYPE_OMAP_INTC)
> +
> +typedef struct omap_intr_handler_s omap_intr_handler;
> +
> +/*
> + * TODO: Ideally we should have a clock framework that
> + * let us wire these clocks up with QOM properties or links.
> + */
> +void omap_intc_set_iclk(omap_intr_handler *intc, omap_clk clk);
> +void omap_intc_set_fclk(omap_intr_handler *intc, omap_clk clk);
> +
>   /* OMAP2 l4 Interconnect */
>   struct omap_l4_s;
>   struct omap_l4_region_s {

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>