[Skiboot] [PATCH 1/7] phb4/capp: Update and re-factor phb4_set_capi_mode()

Vaibhav Jain vaibhav at linux.ibm.com
Mon Sep 17 18:17:12 AEST 2018


Thanks for reviewing this patch Andrew.

Andrew Donnellan <andrew.donnellan at au1.ibm.com> writes:

>> +	/* No one else fiddle with capp while we modify its state */
>>   	lock(&capi_lock);
>> -	chip->capp_phb4_attached_mask |= 1 << p->index;
>> -	unlock(&capi_lock);
>
> Is there any way to release the lock earlier, like we currently do here? 
> As is, we could be holding capi_lock for quite a while (some of the 
> timeouts in enable_capi_mode() are up to 5 seconds) and we only need to 
> hold capi_lock while we're touching the attached_mask, AIUI.

Good point though we will need to hold the lock to prevent someone from
calling phb4_set_capi_mode() with a conflicting value and stomping on
us. I can probably replace lock with try_lock and return OPAL_BUSY in
that case leaving the caller to take recovery steps.


>> +				       (mode == OPAL_PHB_CAPI_MODE_DMA_TVT1 ?
>> +					/* Max perf for DMA */
>> +					(CAPP_MIN_STQ_ENGINES |
>> +					 CAPP_MAX_DMA_READ_ENGINES) :
>> +					/* Max perf for CAPP STQ */
>> +					(CAPP_MAX_STQ_ENGINES |
>> +					 CAPP_MIN_DMA_READ_ENGINES)));
>
Fair point will fix this in next iteration of this patchset.


-- 
Vaibhav Jain <vaibhav at linux.vnet.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.



More information about the Skiboot mailing list