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

2019-12-14 Thread 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.

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

Reviewed-by: Peter Maydell 
Signed-off-by: Marc-André Lureau 
---
  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",

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

2019-11-20 Thread Marc-André Lureau 
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 
Signed-off-by: Marc-André Lureau 
---
 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]),