DRConnectorClass has a set_configured method, however:
  * There is only one implementation, and only ever likely to be one
  * There's exactly one caller, and that's (now) local
  * The implementation is very straightforward

So abolish the method entirely, and just open-code what we need.  We also
remove the tracepoints associated with it, since they don't look to be
terribly useful.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_drc.c         | 20 ++++----------------
 hw/ppc/trace-events        |  2 --
 include/hw/ppc/spapr_drc.h |  3 ---
 3 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index f5b7b68..025453b 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -199,18 +199,6 @@ static const char *get_name(sPAPRDRConnector *drc)
     return drc->name;
 }
 
-static void set_configured(sPAPRDRConnector *drc)
-{
-    trace_spapr_drc_set_configured(get_index(drc));
-
-    if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
-        /* guest should be not configuring an isolated device */
-        trace_spapr_drc_set_configured_skipping(get_index(drc));
-        return;
-    }
-    drc->configured = true;
-}
-
 /* has the guest been notified of device attachment? */
 static void set_signalled(sPAPRDRConnector *drc)
 {
@@ -745,7 +733,6 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data)
     drck->get_index = get_index;
     drck->get_type = get_type;
     drck->get_name = get_name;
-    drck->set_configured = set_configured;
     drck->entity_sense = entity_sense;
     drck->attach = attach;
     drck->detach = detach;
@@ -1113,7 +1100,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
     uint64_t wa_offset;
     uint32_t drc_index;
     sPAPRDRConnector *drc;
-    sPAPRDRConnectorClass *drck;
     sPAPRConfigureConnectorState *ccs;
     sPAPRDRCCResponse resp = SPAPR_DR_CC_RESPONSE_CONTINUE;
     int rc;
@@ -1133,7 +1119,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
         goto out;
     }
 
-    drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
     if (!drc->fdt) {
         trace_spapr_rtas_ibm_configure_connector_missing_fdt(drc_index);
         rc = SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE;
@@ -1170,10 +1155,13 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
         case FDT_END_NODE:
             ccs->fdt_depth--;
             if (ccs->fdt_depth == 0) {
+                sPAPRDRIsolationState state = drc->isolation_state;
                 /* done sending the device tree, don't need to track
                  * the state anymore
                  */
-                drck->set_configured(drc);
+                if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
+                    drc->configured = true;
+                }
                 spapr_ccs_remove(spapr, ccs);
                 ccs = NULL;
                 resp = SPAPR_DR_CC_RESPONSE_SUCCESS;
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index 43d265f..96ffc02 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -42,8 +42,6 @@ spapr_drc_set_isolation_state_deferring(uint32_t index) "drc: 0x%"PRIx32
 spapr_drc_set_indicator_state(uint32_t index, int state) "drc: 0x%"PRIx32", state: 0x%x"
 spapr_drc_set_allocation_state(uint32_t index, int state) "drc: 0x%"PRIx32", state: 0x%x"
 spapr_drc_set_allocation_state_finalizing(uint32_t index) "drc: 0x%"PRIx32
-spapr_drc_set_configured(uint32_t index) "drc: 0x%"PRIx32
-spapr_drc_set_configured_skipping(uint32_t index) "drc: 0x%"PRIx32", isolated device"
 spapr_drc_entity_sense(uint32_t index, int state) "drc: 0x%"PRIx32", state: 0x%x"
 spapr_drc_attach(uint32_t index) "drc: 0x%"PRIx32
 spapr_drc_detach(uint32_t index) "drc: 0x%"PRIx32
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 80db955..90f4953 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -177,9 +177,6 @@ typedef struct sPAPRDRConnectorClass {
 
     uint32_t (*entity_sense)(sPAPRDRConnector *drc, sPAPRDREntitySense *state);
 
-    /* QEMU interfaces for managing FDT/configure-connector */
-    void (*set_configured)(sPAPRDRConnector *drc);
-
     /* QEMU interfaces for managing hotplug operations */
     void (*attach)(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
                    int fdt_start_offset, bool coldplug, Error **errp);
-- 
2.9.4

On 01/06/2017 03:52, David Gibson wrote:
> DRConnectorClass has a set_configured method, however:
>   * There is only one implementation, and only ever likely to be one
>   * There's exactly one caller, and that's (now) local
>   * The implementation is very straightforward
> 
> So abolish the method entirely, and just open-code what we need.  We also
> remove the tracepoints associated with it, since they don't look to be
> terribly useful.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> ---
>  hw/ppc/spapr_drc.c         | 20 ++++----------------
>  hw/ppc/trace-events        |  2 --
>  include/hw/ppc/spapr_drc.h |  3 ---
>  3 files changed, 4 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index f5b7b68..025453b 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -199,18 +199,6 @@ static const char *get_name(sPAPRDRConnector *drc)
>      return drc->name;
>  }
>  
> -static void set_configured(sPAPRDRConnector *drc)
> -{
> -    trace_spapr_drc_set_configured(get_index(drc));
> -
> -    if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
> -        /* guest should be not configuring an isolated device */
> -        trace_spapr_drc_set_configured_skipping(get_index(drc));
> -        return;
> -    }
> -    drc->configured = true;
> -}
> -
>  /* has the guest been notified of device attachment? */
>  static void set_signalled(sPAPRDRConnector *drc)
>  {
> @@ -745,7 +733,6 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data)
>      drck->get_index = get_index;
>      drck->get_type = get_type;
>      drck->get_name = get_name;
> -    drck->set_configured = set_configured;
>      drck->entity_sense = entity_sense;
>      drck->attach = attach;
>      drck->detach = detach;
> @@ -1113,7 +1100,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
>      uint64_t wa_offset;
>      uint32_t drc_index;
>      sPAPRDRConnector *drc;
> -    sPAPRDRConnectorClass *drck;
>      sPAPRConfigureConnectorState *ccs;
>      sPAPRDRCCResponse resp = SPAPR_DR_CC_RESPONSE_CONTINUE;
>      int rc;
> @@ -1133,7 +1119,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
>          goto out;
>      }
>  
> -    drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
>      if (!drc->fdt) {
>          trace_spapr_rtas_ibm_configure_connector_missing_fdt(drc_index);
>          rc = SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE;
> @@ -1170,10 +1155,13 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
>          case FDT_END_NODE:
>              ccs->fdt_depth--;
>              if (ccs->fdt_depth == 0) {
> +                sPAPRDRIsolationState state = drc->isolation_state;
>                  /* done sending the device tree, don't need to track
>                   * the state anymore
>                   */
> -                drck->set_configured(drc);
> +                if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
> +                    drc->configured = true;
> +                }
>                  spapr_ccs_remove(spapr, ccs);
>                  ccs = NULL;
>                  resp = SPAPR_DR_CC_RESPONSE_SUCCESS;
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index 43d265f..96ffc02 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -42,8 +42,6 @@ spapr_drc_set_isolation_state_deferring(uint32_t index) "drc: 0x%"PRIx32
>  spapr_drc_set_indicator_state(uint32_t index, int state) "drc: 0x%"PRIx32", state: 0x%x"
>  spapr_drc_set_allocation_state(uint32_t index, int state) "drc: 0x%"PRIx32", state: 0x%x"
>  spapr_drc_set_allocation_state_finalizing(uint32_t index) "drc: 0x%"PRIx32
> -spapr_drc_set_configured(uint32_t index) "drc: 0x%"PRIx32
> -spapr_drc_set_configured_skipping(uint32_t index) "drc: 0x%"PRIx32", isolated device"
>  spapr_drc_entity_sense(uint32_t index, int state) "drc: 0x%"PRIx32", state: 0x%x"
>  spapr_drc_attach(uint32_t index) "drc: 0x%"PRIx32
>  spapr_drc_detach(uint32_t index) "drc: 0x%"PRIx32
> diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> index 80db955..90f4953 100644
> --- a/include/hw/ppc/spapr_drc.h
> +++ b/include/hw/ppc/spapr_drc.h
> @@ -177,9 +177,6 @@ typedef struct sPAPRDRConnectorClass {
>  
>      uint32_t (*entity_sense)(sPAPRDRConnector *drc, sPAPRDREntitySense *state);
>  
> -    /* QEMU interfaces for managing FDT/configure-connector */
> -    void (*set_configured)(sPAPRDRConnector *drc);
> -
>      /* QEMU interfaces for managing hotplug operations */
>      void (*attach)(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
>                     int fdt_start_offset, bool coldplug, Error **errp);
> 

Quoting David Gibson (2017-05-31 20:52:17)
> DRConnectorClass has a set_configured method, however:
>   * There is only one implementation, and only ever likely to be one
>   * There's exactly one caller, and that's (now) local
>   * The implementation is very straightforward
> 
> So abolish the method entirely, and just open-code what we need.  We also
> remove the tracepoints associated with it, since they don't look to be
> terribly useful.

Dropping the method makes sense, but the 'configured' state affects a
lot of the state-transitions throughout the code so I think it may
be useful to keep the traces.

> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr_drc.c         | 20 ++++----------------
>  hw/ppc/trace-events        |  2 --
>  include/hw/ppc/spapr_drc.h |  3 ---
>  3 files changed, 4 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index f5b7b68..025453b 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -199,18 +199,6 @@ static const char *get_name(sPAPRDRConnector *drc)
>      return drc->name;
>  }
> 
> -static void set_configured(sPAPRDRConnector *drc)
> -{
> -    trace_spapr_drc_set_configured(get_index(drc));
> -
> -    if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
> -        /* guest should be not configuring an isolated device */
> -        trace_spapr_drc_set_configured_skipping(get_index(drc));
> -        return;
> -    }
> -    drc->configured = true;
> -}
> -
>  /* has the guest been notified of device attachment? */
>  static void set_signalled(sPAPRDRConnector *drc)
>  {
> @@ -745,7 +733,6 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data)
>      drck->get_index = get_index;
>      drck->get_type = get_type;
>      drck->get_name = get_name;
> -    drck->set_configured = set_configured;
>      drck->entity_sense = entity_sense;
>      drck->attach = attach;
>      drck->detach = detach;
> @@ -1113,7 +1100,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
>      uint64_t wa_offset;
>      uint32_t drc_index;
>      sPAPRDRConnector *drc;
> -    sPAPRDRConnectorClass *drck;
>      sPAPRConfigureConnectorState *ccs;
>      sPAPRDRCCResponse resp = SPAPR_DR_CC_RESPONSE_CONTINUE;
>      int rc;
> @@ -1133,7 +1119,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
>          goto out;
>      }
> 
> -    drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
>      if (!drc->fdt) {
>          trace_spapr_rtas_ibm_configure_connector_missing_fdt(drc_index);
>          rc = SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE;
> @@ -1170,10 +1155,13 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
>          case FDT_END_NODE:
>              ccs->fdt_depth--;
>              if (ccs->fdt_depth == 0) {
> +                sPAPRDRIsolationState state = drc->isolation_state;
>                  /* done sending the device tree, don't need to track
>                   * the state anymore
>                   */
> -                drck->set_configured(drc);
> +                if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
> +                    drc->configured = true;
> +                }
>                  spapr_ccs_remove(spapr, ccs);
>                  ccs = NULL;
>                  resp = SPAPR_DR_CC_RESPONSE_SUCCESS;
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index 43d265f..96ffc02 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -42,8 +42,6 @@ spapr_drc_set_isolation_state_deferring(uint32_t index) "drc: 0x%"PRIx32
>  spapr_drc_set_indicator_state(uint32_t index, int state) "drc: 0x%"PRIx32", state: 0x%x"
>  spapr_drc_set_allocation_state(uint32_t index, int state) "drc: 0x%"PRIx32", state: 0x%x"
>  spapr_drc_set_allocation_state_finalizing(uint32_t index) "drc: 0x%"PRIx32
> -spapr_drc_set_configured(uint32_t index) "drc: 0x%"PRIx32
> -spapr_drc_set_configured_skipping(uint32_t index) "drc: 0x%"PRIx32", isolated device"
>  spapr_drc_entity_sense(uint32_t index, int state) "drc: 0x%"PRIx32", state: 0x%x"
>  spapr_drc_attach(uint32_t index) "drc: 0x%"PRIx32
>  spapr_drc_detach(uint32_t index) "drc: 0x%"PRIx32
> diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> index 80db955..90f4953 100644
> --- a/include/hw/ppc/spapr_drc.h
> +++ b/include/hw/ppc/spapr_drc.h
> @@ -177,9 +177,6 @@ typedef struct sPAPRDRConnectorClass {
> 
>      uint32_t (*entity_sense)(sPAPRDRConnector *drc, sPAPRDREntitySense *state);
> 
> -    /* QEMU interfaces for managing FDT/configure-connector */
> -    void (*set_configured)(sPAPRDRConnector *drc);
> -
>      /* QEMU interfaces for managing hotplug operations */
>      void (*attach)(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
>                     int fdt_start_offset, bool coldplug, Error **errp);
> -- 
> 2.9.4
> 
> 

On Thu,  1 Jun 2017 11:52:17 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> DRConnectorClass has a set_configured method, however:
>   * There is only one implementation, and only ever likely to be one
>   * There's exactly one caller, and that's (now) local
>   * The implementation is very straightforward
> 
> So abolish the method entirely, and just open-code what we need.  We also
> remove the tracepoints associated with it, since they don't look to be
> terribly useful.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr_drc.c         | 20 ++++----------------
>  hw/ppc/trace-events        |  2 --
>  include/hw/ppc/spapr_drc.h |  3 ---
>  3 files changed, 4 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index f5b7b68..025453b 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -199,18 +199,6 @@ static const char *get_name(sPAPRDRConnector *drc)
>      return drc->name;
>  }
>  
> -static void set_configured(sPAPRDRConnector *drc)
> -{
> -    trace_spapr_drc_set_configured(get_index(drc));
> -
> -    if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
> -        /* guest should be not configuring an isolated device */
> -        trace_spapr_drc_set_configured_skipping(get_index(drc));
> -        return;
> -    }
> -    drc->configured = true;
> -}
> -
>  /* has the guest been notified of device attachment? */
>  static void set_signalled(sPAPRDRConnector *drc)
>  {
> @@ -745,7 +733,6 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data)
>      drck->get_index = get_index;
>      drck->get_type = get_type;
>      drck->get_name = get_name;
> -    drck->set_configured = set_configured;
>      drck->entity_sense = entity_sense;
>      drck->attach = attach;
>      drck->detach = detach;
> @@ -1113,7 +1100,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
>      uint64_t wa_offset;
>      uint32_t drc_index;
>      sPAPRDRConnector *drc;
> -    sPAPRDRConnectorClass *drck;
>      sPAPRConfigureConnectorState *ccs;
>      sPAPRDRCCResponse resp = SPAPR_DR_CC_RESPONSE_CONTINUE;
>      int rc;
> @@ -1133,7 +1119,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
>          goto out;
>      }
>  
> -    drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
>      if (!drc->fdt) {
>          trace_spapr_rtas_ibm_configure_connector_missing_fdt(drc_index);
>          rc = SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE;
> @@ -1170,10 +1155,13 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
>          case FDT_END_NODE:
>              ccs->fdt_depth--;
>              if (ccs->fdt_depth == 0) {
> +                sPAPRDRIsolationState state = drc->isolation_state;
>                  /* done sending the device tree, don't need to track
>                   * the state anymore
>                   */
> -                drck->set_configured(drc);
> +                if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
> +                    drc->configured = true;
> +                }
>                  spapr_ccs_remove(spapr, ccs);
>                  ccs = NULL;
>                  resp = SPAPR_DR_CC_RESPONSE_SUCCESS;
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index 43d265f..96ffc02 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -42,8 +42,6 @@ spapr_drc_set_isolation_state_deferring(uint32_t index) "drc: 0x%"PRIx32
>  spapr_drc_set_indicator_state(uint32_t index, int state) "drc: 0x%"PRIx32", state: 0x%x"
>  spapr_drc_set_allocation_state(uint32_t index, int state) "drc: 0x%"PRIx32", state: 0x%x"
>  spapr_drc_set_allocation_state_finalizing(uint32_t index) "drc: 0x%"PRIx32
> -spapr_drc_set_configured(uint32_t index) "drc: 0x%"PRIx32
> -spapr_drc_set_configured_skipping(uint32_t index) "drc: 0x%"PRIx32", isolated device"
>  spapr_drc_entity_sense(uint32_t index, int state) "drc: 0x%"PRIx32", state: 0x%x"
>  spapr_drc_attach(uint32_t index) "drc: 0x%"PRIx32
>  spapr_drc_detach(uint32_t index) "drc: 0x%"PRIx32
> diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> index 80db955..90f4953 100644
> --- a/include/hw/ppc/spapr_drc.h
> +++ b/include/hw/ppc/spapr_drc.h
> @@ -177,9 +177,6 @@ typedef struct sPAPRDRConnectorClass {
>  
>      uint32_t (*entity_sense)(sPAPRDRConnector *drc, sPAPRDREntitySense *state);
>  
> -    /* QEMU interfaces for managing FDT/configure-connector */
> -    void (*set_configured)(sPAPRDRConnector *drc);
> -
>      /* QEMU interfaces for managing hotplug operations */
>      void (*attach)(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
>                     int fdt_start_offset, bool coldplug, Error **errp);