diff mbox series

[v4,34/37] omap-gpio: remove PROP_PTR

Message ID 20191120152442.26657-35-marcandre.lureau@redhat.com
State New
Headers show
Series Clean-ups: qom-ify serial and remove QDEV_PROP_PTR | expand

Commit Message

Marc-André Lureau Nov. 20, 2019, 3:24 p.m. UTC 
Since clocks are not QOM objects, replace PROP_PTR of clocks with
setters methods.

Move/adapt the existing TODO comment about a clock framework.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/arm/omap1.c        |  2 +-
 hw/arm/omap2.c        | 13 +++++++------
 hw/gpio/omap_gpio.c   | 42 +++++++++++++++---------------------------
 include/hw/arm/omap.h | 33 +++++++++++++++++++++++++++++----
 4 files changed, 52 insertions(+), 38 deletions(-)

Comments

Philippe Mathieu-Daudé Dec. 15, 2019, 5:44 a.m. UTC | #1 
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.
> 
> Move/adapt the existing TODO comment about a clock framework.
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   hw/arm/omap1.c        |  2 +-
>   hw/arm/omap2.c        | 13 +++++++------
>   hw/gpio/omap_gpio.c   | 42 +++++++++++++++---------------------------
>   include/hw/arm/omap.h | 33 +++++++++++++++++++++++++++++----
>   4 files changed, 52 insertions(+), 38 deletions(-)
> 
> diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
> index 807e5f70d1..761cc17ea9 100644
> --- a/hw/arm/omap1.c
> +++ b/hw/arm/omap1.c
> @@ -4012,7 +4012,7 @@ struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion *dram,
>   
>       s->gpio = qdev_create(NULL, "omap-gpio");
>       qdev_prop_set_int32(s->gpio, "mpu_model", s->mpu_model);
> -    qdev_prop_set_ptr(s->gpio, "clk", omap_findclk(s, "arm_gpio_ck"));
> +    omap_gpio_set_clk(OMAP1_GPIO(s->gpio), omap_findclk(s, "arm_gpio_ck"));
>       qdev_init_nofail(s->gpio);
>       sysbus_connect_irq(SYS_BUS_DEVICE(s->gpio), 0,
>                          qdev_get_gpio_in(s->ih[0], OMAP_INT_GPIO_BANK1));
> diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
> index 171e2d0472..e1c11de5ce 100644
> --- a/hw/arm/omap2.c
> +++ b/hw/arm/omap2.c
> @@ -2449,13 +2449,14 @@ struct omap_mpu_state_s *omap2420_mpu_init(MemoryRegion *sdram,
>   
>       s->gpio = qdev_create(NULL, "omap2-gpio");
>       qdev_prop_set_int32(s->gpio, "mpu_model", s->mpu_model);
> -    qdev_prop_set_ptr(s->gpio, "iclk", omap_findclk(s, "gpio_iclk"));
> -    qdev_prop_set_ptr(s->gpio, "fclk0", omap_findclk(s, "gpio1_dbclk"));
> -    qdev_prop_set_ptr(s->gpio, "fclk1", omap_findclk(s, "gpio2_dbclk"));
> -    qdev_prop_set_ptr(s->gpio, "fclk2", omap_findclk(s, "gpio3_dbclk"));
> -    qdev_prop_set_ptr(s->gpio, "fclk3", omap_findclk(s, "gpio4_dbclk"));
> +    omap2_gpio_set_iclk(OMAP2_GPIO(s->gpio), omap_findclk(s, "gpio_iclk"));
> +    omap2_gpio_set_fclk(OMAP2_GPIO(s->gpio), 0, omap_findclk(s, "gpio1_dbclk"));
> +    omap2_gpio_set_fclk(OMAP2_GPIO(s->gpio), 1, omap_findclk(s, "gpio2_dbclk"));
> +    omap2_gpio_set_fclk(OMAP2_GPIO(s->gpio), 2, omap_findclk(s, "gpio3_dbclk"));
> +    omap2_gpio_set_fclk(OMAP2_GPIO(s->gpio), 3, omap_findclk(s, "gpio4_dbclk"));
>       if (s->mpu_model == omap2430) {
> -        qdev_prop_set_ptr(s->gpio, "fclk4", omap_findclk(s, "gpio5_dbclk"));
> +        omap2_gpio_set_fclk(OMAP2_GPIO(s->gpio), 4,
> +                            omap_findclk(s, "gpio5_dbclk"));
>       }
>       qdev_init_nofail(s->gpio);
>       busdev = SYS_BUS_DEVICE(s->gpio);
> diff --git a/hw/gpio/omap_gpio.c b/hw/gpio/omap_gpio.c
> index 41e1aa798c..85c16897ae 100644
> --- a/hw/gpio/omap_gpio.c
> +++ b/hw/gpio/omap_gpio.c
> @@ -40,10 +40,6 @@ struct omap_gpio_s {
>       uint16_t pins;
>   };
>   
> -#define TYPE_OMAP1_GPIO "omap-gpio"
> -#define OMAP1_GPIO(obj) \
> -    OBJECT_CHECK(struct omap_gpif_s, (obj), TYPE_OMAP1_GPIO)
> -
>   struct omap_gpif_s {
>       SysBusDevice parent_obj;
>   
> @@ -212,10 +208,6 @@ struct omap2_gpio_s {
>       uint8_t delay;
>   };
>   
> -#define TYPE_OMAP2_GPIO "omap2-gpio"
> -#define OMAP2_GPIO(obj) \
> -    OBJECT_CHECK(struct omap2_gpif_s, (obj), TYPE_OMAP2_GPIO)
> -
>   struct omap2_gpif_s {
>       SysBusDevice parent_obj;
>   
> @@ -747,21 +739,13 @@ static void omap2_gpio_realize(DeviceState *dev, Error **errp)
>       }
>   }
>   
> -/* Using qdev pointer properties for the clocks is not ideal.
> - * qdev should support a generic means of defining a 'port' with
> - * an arbitrary interface for connecting two devices. Then we
> - * could reframe the omap clock API in terms of clock ports,
> - * and get some type safety. For now the best qdev provides is
> - * passing an arbitrary pointer.
> - * (It's not possible to pass in the string which is the clock
> - * name, because this device does not have the necessary information
> - * (ie the struct omap_mpu_state_s*) to do the clockname to pointer
> - * translation.)
> - */
> +void omap_gpio_set_clk(omap_gpif *gpio, omap_clk clk)
> +{
> +    gpio->clk = clk;
> +}
>   
>   static Property omap_gpio_properties[] = {
>       DEFINE_PROP_INT32("mpu_model", struct omap_gpif_s, mpu_model, 0),
> -    DEFINE_PROP_PTR("clk", struct omap_gpif_s, clk),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   
> @@ -784,15 +768,19 @@ static const TypeInfo omap_gpio_info = {
>       .class_init    = omap_gpio_class_init,
>   };
>   
> +void omap2_gpio_set_iclk(omap2_gpif *gpio, omap_clk clk)
> +{
> +    gpio->iclk = clk;
> +}
> +
> +void omap2_gpio_set_fclk(omap2_gpif *gpio, uint8_t i, omap_clk clk)
> +{
> +    assert(i <= 5);
> +    gpio->fclk[i] = clk;
> +}
> +
>   static Property omap2_gpio_properties[] = {
>       DEFINE_PROP_INT32("mpu_model", struct omap2_gpif_s, mpu_model, 0),
> -    DEFINE_PROP_PTR("iclk", struct omap2_gpif_s, iclk),
> -    DEFINE_PROP_PTR("fclk0", struct omap2_gpif_s, fclk[0]),
> -    DEFINE_PROP_PTR("fclk1", struct omap2_gpif_s, fclk[1]),
> -    DEFINE_PROP_PTR("fclk2", struct omap2_gpif_s, fclk[2]),
> -    DEFINE_PROP_PTR("fclk3", struct omap2_gpif_s, fclk[3]),
> -    DEFINE_PROP_PTR("fclk4", struct omap2_gpif_s, fclk[4]),
> -    DEFINE_PROP_PTR("fclk5", struct omap2_gpif_s, fclk[5]),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   
> diff --git a/include/hw/arm/omap.h b/include/hw/arm/omap.h
> index 39a295ba20..6be386d0e2 100644
> --- a/include/hw/arm/omap.h
> +++ b/include/hw/arm/omap.h
> @@ -77,6 +77,16 @@ 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.
> + *
> + * qdev should support a generic means of defining a 'port' with
> + * an arbitrary interface for connecting two devices. Then we
> + * could reframe the omap clock API in terms of clock ports,
> + * and get some type safety. For now the best qdev provides is
> + * passing an arbitrary pointer.
> + * (It's not possible to pass in the string which is the clock
> + * name, because this device does not have the necessary information
> + * (ie the struct omap_mpu_state_s*) to do the clockname to pointer
> + * translation.)
>    */
>   void omap_intc_set_iclk(omap_intr_handler *intc, omap_clk clk);
>   void omap_intc_set_fclk(omap_intr_handler *intc, omap_clk clk);
> @@ -87,13 +97,28 @@ void omap_intc_set_fclk(omap_intr_handler *intc, omap_clk clk);
>   
>   typedef struct OMAPI2CState OMAPI2CState;
>   
> -/*
> - * TODO: Ideally we should have a clock framework that
> - * let us wire these clocks up with QOM properties or links.
> - */
> +/* TODO: clock framework (see above) */
>   void omap_i2c_set_iclk(OMAPI2CState *i2c, omap_clk clk);
>   void omap_i2c_set_fclk(OMAPI2CState *i2c, omap_clk clk);
>   
> +/* omap_gpio.c */
> +#define TYPE_OMAP1_GPIO "omap-gpio"
> +#define OMAP1_GPIO(obj)                                         \
> +    OBJECT_CHECK(struct omap_gpif_s, (obj), TYPE_OMAP1_GPIO)
> +
> +#define TYPE_OMAP2_GPIO "omap2-gpio"
> +#define OMAP2_GPIO(obj)                                         \
> +    OBJECT_CHECK(struct omap2_gpif_s, (obj), TYPE_OMAP2_GPIO)
> +
> +typedef struct omap_gpif_s omap_gpif;
> +typedef struct omap2_gpif_s omap2_gpif;
> +
> +/* TODO: clock framework (see above) */
> +void omap_gpio_set_clk(omap_gpif *gpio, omap_clk clk);
> +
> +void omap2_gpio_set_iclk(omap2_gpif *gpio, omap_clk clk);
> +void omap2_gpio_set_fclk(omap2_gpif *gpio, uint8_t i, omap_clk clk);
> +
>   /* OMAP2 l4 Interconnect */
>   struct omap_l4_s;
>   struct omap_l4_region_s {
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
diff mbox series

Patch

diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
index 807e5f70d1..761cc17ea9 100644
--- a/hw/arm/omap1.c
+++ b/hw/arm/omap1.c
@@ -4012,7 +4012,7 @@  struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion *dram,
 
     s->gpio = qdev_create(NULL, "omap-gpio");
     qdev_prop_set_int32(s->gpio, "mpu_model", s->mpu_model);
-    qdev_prop_set_ptr(s->gpio, "clk", omap_findclk(s, "arm_gpio_ck"));
+    omap_gpio_set_clk(OMAP1_GPIO(s->gpio), omap_findclk(s, "arm_gpio_ck"));
     qdev_init_nofail(s->gpio);
     sysbus_connect_irq(SYS_BUS_DEVICE(s->gpio), 0,
                        qdev_get_gpio_in(s->ih[0], OMAP_INT_GPIO_BANK1));
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index 171e2d0472..e1c11de5ce 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -2449,13 +2449,14 @@  struct omap_mpu_state_s *omap2420_mpu_init(MemoryRegion *sdram,
 
     s->gpio = qdev_create(NULL, "omap2-gpio");
     qdev_prop_set_int32(s->gpio, "mpu_model", s->mpu_model);
-    qdev_prop_set_ptr(s->gpio, "iclk", omap_findclk(s, "gpio_iclk"));
-    qdev_prop_set_ptr(s->gpio, "fclk0", omap_findclk(s, "gpio1_dbclk"));
-    qdev_prop_set_ptr(s->gpio, "fclk1", omap_findclk(s, "gpio2_dbclk"));
-    qdev_prop_set_ptr(s->gpio, "fclk2", omap_findclk(s, "gpio3_dbclk"));
-    qdev_prop_set_ptr(s->gpio, "fclk3", omap_findclk(s, "gpio4_dbclk"));
+    omap2_gpio_set_iclk(OMAP2_GPIO(s->gpio), omap_findclk(s, "gpio_iclk"));
+    omap2_gpio_set_fclk(OMAP2_GPIO(s->gpio), 0, omap_findclk(s, "gpio1_dbclk"));
+    omap2_gpio_set_fclk(OMAP2_GPIO(s->gpio), 1, omap_findclk(s, "gpio2_dbclk"));
+    omap2_gpio_set_fclk(OMAP2_GPIO(s->gpio), 2, omap_findclk(s, "gpio3_dbclk"));
+    omap2_gpio_set_fclk(OMAP2_GPIO(s->gpio), 3, omap_findclk(s, "gpio4_dbclk"));
     if (s->mpu_model == omap2430) {
-        qdev_prop_set_ptr(s->gpio, "fclk4", omap_findclk(s, "gpio5_dbclk"));
+        omap2_gpio_set_fclk(OMAP2_GPIO(s->gpio), 4,
+                            omap_findclk(s, "gpio5_dbclk"));
     }
     qdev_init_nofail(s->gpio);
     busdev = SYS_BUS_DEVICE(s->gpio);
diff --git a/hw/gpio/omap_gpio.c b/hw/gpio/omap_gpio.c
index 41e1aa798c..85c16897ae 100644
--- a/hw/gpio/omap_gpio.c
+++ b/hw/gpio/omap_gpio.c
@@ -40,10 +40,6 @@  struct omap_gpio_s {
     uint16_t pins;
 };
 
-#define TYPE_OMAP1_GPIO "omap-gpio"
-#define OMAP1_GPIO(obj) \
-    OBJECT_CHECK(struct omap_gpif_s, (obj), TYPE_OMAP1_GPIO)
-
 struct omap_gpif_s {
     SysBusDevice parent_obj;
 
@@ -212,10 +208,6 @@  struct omap2_gpio_s {
     uint8_t delay;
 };
 
-#define TYPE_OMAP2_GPIO "omap2-gpio"
-#define OMAP2_GPIO(obj) \
-    OBJECT_CHECK(struct omap2_gpif_s, (obj), TYPE_OMAP2_GPIO)
-
 struct omap2_gpif_s {
     SysBusDevice parent_obj;
 
@@ -747,21 +739,13 @@  static void omap2_gpio_realize(DeviceState *dev, Error **errp)
     }
 }
 
-/* Using qdev pointer properties for the clocks is not ideal.
- * qdev should support a generic means of defining a 'port' with
- * an arbitrary interface for connecting two devices. Then we
- * could reframe the omap clock API in terms of clock ports,
- * and get some type safety. For now the best qdev provides is
- * passing an arbitrary pointer.
- * (It's not possible to pass in the string which is the clock
- * name, because this device does not have the necessary information
- * (ie the struct omap_mpu_state_s*) to do the clockname to pointer
- * translation.)
- */
+void omap_gpio_set_clk(omap_gpif *gpio, omap_clk clk)
+{
+    gpio->clk = clk;
+}
 
 static Property omap_gpio_properties[] = {
     DEFINE_PROP_INT32("mpu_model", struct omap_gpif_s, mpu_model, 0),
-    DEFINE_PROP_PTR("clk", struct omap_gpif_s, clk),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -784,15 +768,19 @@  static const TypeInfo omap_gpio_info = {
     .class_init    = omap_gpio_class_init,
 };
 
+void omap2_gpio_set_iclk(omap2_gpif *gpio, omap_clk clk)
+{
+    gpio->iclk = clk;
+}
+
+void omap2_gpio_set_fclk(omap2_gpif *gpio, uint8_t i, omap_clk clk)
+{
+    assert(i <= 5);
+    gpio->fclk[i] = clk;
+}
+
 static Property omap2_gpio_properties[] = {
     DEFINE_PROP_INT32("mpu_model", struct omap2_gpif_s, mpu_model, 0),
-    DEFINE_PROP_PTR("iclk", struct omap2_gpif_s, iclk),
-    DEFINE_PROP_PTR("fclk0", struct omap2_gpif_s, fclk[0]),
-    DEFINE_PROP_PTR("fclk1", struct omap2_gpif_s, fclk[1]),
-    DEFINE_PROP_PTR("fclk2", struct omap2_gpif_s, fclk[2]),
-    DEFINE_PROP_PTR("fclk3", struct omap2_gpif_s, fclk[3]),
-    DEFINE_PROP_PTR("fclk4", struct omap2_gpif_s, fclk[4]),
-    DEFINE_PROP_PTR("fclk5", struct omap2_gpif_s, fclk[5]),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/arm/omap.h b/include/hw/arm/omap.h
index 39a295ba20..6be386d0e2 100644
--- a/include/hw/arm/omap.h
+++ b/include/hw/arm/omap.h
@@ -77,6 +77,16 @@  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.
+ *
+ * qdev should support a generic means of defining a 'port' with
+ * an arbitrary interface for connecting two devices. Then we
+ * could reframe the omap clock API in terms of clock ports,
+ * and get some type safety. For now the best qdev provides is
+ * passing an arbitrary pointer.
+ * (It's not possible to pass in the string which is the clock
+ * name, because this device does not have the necessary information
+ * (ie the struct omap_mpu_state_s*) to do the clockname to pointer
+ * translation.)
  */
 void omap_intc_set_iclk(omap_intr_handler *intc, omap_clk clk);
 void omap_intc_set_fclk(omap_intr_handler *intc, omap_clk clk);
@@ -87,13 +97,28 @@  void omap_intc_set_fclk(omap_intr_handler *intc, omap_clk clk);
 
 typedef struct OMAPI2CState OMAPI2CState;
 
-/*
- * TODO: Ideally we should have a clock framework that
- * let us wire these clocks up with QOM properties or links.
- */
+/* TODO: clock framework (see above) */
 void omap_i2c_set_iclk(OMAPI2CState *i2c, omap_clk clk);
 void omap_i2c_set_fclk(OMAPI2CState *i2c, omap_clk clk);
 
+/* omap_gpio.c */
+#define TYPE_OMAP1_GPIO "omap-gpio"
+#define OMAP1_GPIO(obj)                                         \
+    OBJECT_CHECK(struct omap_gpif_s, (obj), TYPE_OMAP1_GPIO)
+
+#define TYPE_OMAP2_GPIO "omap2-gpio"
+#define OMAP2_GPIO(obj)                                         \
+    OBJECT_CHECK(struct omap2_gpif_s, (obj), TYPE_OMAP2_GPIO)
+
+typedef struct omap_gpif_s omap_gpif;
+typedef struct omap2_gpif_s omap2_gpif;
+
+/* TODO: clock framework (see above) */
+void omap_gpio_set_clk(omap_gpif *gpio, omap_clk clk);
+
+void omap2_gpio_set_iclk(omap2_gpif *gpio, omap_clk clk);
+void omap2_gpio_set_fclk(omap2_gpif *gpio, uint8_t i, omap_clk clk);
+
 /* OMAP2 l4 Interconnect */
 struct omap_l4_s;
 struct omap_l4_region_s {