Re: [PATCH 08/23] powerpc/eeh: Refactor eeh_reset_pe_once()

[Gavin Shan <shangw@linux.vnet.ibm.com>]

On Sat, Jun 01, 2013 at 02:18:50PM +1000, Benjamin Herrenschmidt wrote:
>On Thu, 2013-05-30 at 16:23 +0800, Gavin Shan wrote:
>> The patch changes the criteria used to judge if the PE has been
>> resetted successfully. We needn't the PE status is exactly equal
>> to the combo: (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE).
>
>The comment above doesn't mean anything :-)
>
>I assume you meant to write "we shouldn't check that the
>returned PE status is exactly equal to the two flags X and Y,
>but instead only check that they are both set".
>

Thanks, Ben. I'll update it accordingly in next version :-)

>> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/platforms/pseries/eeh.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
>> index 39d2ea6..ffe34c4 100644
>> --- a/arch/powerpc/platforms/pseries/eeh.c
>> +++ b/arch/powerpc/platforms/pseries/eeh.c
>> @@ -527,7 +527,6 @@ static void eeh_reset_pe_once(struct eeh_pe *pe)
>>  	 * Partitionable Endpoint trumps hot-reset.
>>    	 */
>>  	eeh_pe_dev_traverse(pe, eeh_set_dev_freset, &freset);
>> -
>
>Unrelated churn
>

Ok. Will remove it in next version.

>>  	if (freset)
>>  		eeh_ops->reset(pe, EEH_RESET_FUNDAMENTAL);
>>  	else
>> @@ -565,6 +564,7 @@ static void eeh_reset_pe_once(struct eeh_pe *pe)
>>   */
>>  int eeh_reset_pe(struct eeh_pe *pe)
>>  {
>> +	int flags = (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
>>  	int i, rc;
>>  
>>  	/* Take three shots at resetting the bus */
>> @@ -572,7 +572,7 @@ int eeh_reset_pe(struct eeh_pe *pe)
>>  		eeh_reset_pe_once(pe);
>>  
>>  		rc = eeh_ops->wait_state(pe, PCI_BUS_RESET_WAIT_MSEC);
>> -		if (rc == (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE))
>> +		if ((rc & flags) == flags)
>>  			return 0;
>>  
>>  		if (rc < 0) {

Thanks,
Gavin