Quoting Bharata B Rao (2015-08-19 01:56:10) > drck->set_isolation_state() can return error. For such a case ensure > correct error is returned by rtas_set_indicator() instead of always > returning success. > > TODO: rtas_st(, , uint32 val) => the return value uint32, but > drck->set_[allocation/indicator/isolation]_state() is returning int. > Should we change this return value to uint32_t to match with rtas_st() > argument ?
I wouldn't bother too much aligning the types unless we go to the extent of documenting these interfaces as returning rtas error codes. That's not really the case currently, and there's a lot of rtas errors that don't really need to be determined by DRC code so I think it's more trouble than it's worth. For now I think it's better to just check for ret != 0 and set return values explicitly in rtas code based on what the drc errors entail. > > Signed-off-by: Bharata B Rao <bhar...@linux.vnet.ibm.com> > --- > hw/ppc/spapr_rtas.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c > index e99e25f..96729b4 100644 > --- a/hw/ppc/spapr_rtas.c > +++ b/hw/ppc/spapr_rtas.c > @@ -374,6 +374,7 @@ static void rtas_set_indicator(PowerPCCPU *cpu, > sPAPRMachineState *spapr, > uint32_t sensor_state; > sPAPRDRConnector *drc; > sPAPRDRConnectorClass *drck; > + int ret; > > if (nargs != 3 || nret != 1) { > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > @@ -413,19 +414,19 @@ static void rtas_set_indicator(PowerPCCPU *cpu, > sPAPRMachineState *spapr, > spapr_ccs_remove(spapr, ccs); > } > } > - drck->set_isolation_state(drc, sensor_state); > + ret = drck->set_isolation_state(drc, sensor_state); > break; > case RTAS_SENSOR_TYPE_DR: > - drck->set_indicator_state(drc, sensor_state); > + ret = drck->set_indicator_state(drc, sensor_state); > break; > case RTAS_SENSOR_TYPE_ALLOCATION_STATE: > - drck->set_allocation_state(drc, sensor_state); > + ret = drck->set_allocation_state(drc, sensor_state); > break; > default: > goto out_unimplemented; > } > > - rtas_st(rets, 0, RTAS_OUT_SUCCESS); > + rtas_st(rets, 0, ret); > return; > > out_unimplemented: > -- > 2.1.0 >