Re: [kvm-unit-tests PATCH v9 11/12] s390x: css: msch, enable test

[Pierre Morel <pmorel@linux.ibm.com>]

On 2020-06-17 10:54, Cornelia Huck wrote:
> On Mon, 15 Jun 2020 11:32:00 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> A second step when testing the channel subsystem is to prepare a channel
>> for use.
...snip...
>> +
>> +	/* Read the SCHIB for this subchannel */
>> +	cc = stsch(schid, &schib);
>> +	if (cc) {
>> +		report_info("stsch failed with cc=%d", cc);
> 
> Mention the schid in the message?

Yes:
report_info("stsch: sch %08x failed with cc=%d", schid, cc);

> 
>> +		return cc;
>> +	}
>> +
>> +	if (pmcw->flags & PMCW_ENABLE) {
>> +		report_info("stsch: sch %08x already enabled", schid);
>> +		return 0;
>> +	}
>> +
>> +retry:
>> +	/* Update the SCHIB to enable the channel */
>> +	pmcw->flags |= PMCW_ENABLE;
>> +
>> +	/* Tell the CSS we want to modify the subchannel */
>> +	cc = msch(schid, &schib);
>> +	if (cc) {
>> +		/*
>> +		 * If the subchannel is status pending or
>> +		 * if a function is in progress,
>> +		 * we consider both cases as errors.
>> +		 */
>> +		report_info("msch failed with cc=%d", cc);

added schid here too

>> +		return cc;
>> +	}
>> +
>> +	/*
>> +	 * Read the SCHIB again to verify the enablement
>> +	 */
>> +	cc = stsch(schid, &schib);
>> +	if (cc) {
>> +		report_info("stsch failed with cc=%d", cc);
> 
> Also add the schid here? Maybe also add a marker to distinguish the two
> cases?

changed to:
report_info("stsch: updating sch %08x failed with cc=%d",schid, cc);
                     ^^^
> 
>> +		return cc;
>> +	}
>> +
>> +	if (pmcw->flags & PMCW_ENABLE) {
>> +		report_info("Subchannel %08x enabled after %d retries",
>> +			    schid, retry_count);
>> +		return 0;
>> +	}
>> +
>> +	if (retry_count++ < MAX_ENABLE_RETRIES) {
>> +		mdelay(10); /* the hardware was not ready, give it some time */
>> +		goto retry;
>> +	}
>> +
>> +	report_info("msch: enabling sch %08x failed after %d retries. pmcw flags: %x",
>> +		    schid, retry_count, pmcw->flags);
>> +	return -1;
>> +}
> 
> With the messages updated,
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> 

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen