Re: [1/4] powerpc/powernv: Sync header with firmware

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

On Thu, Sep 25, 2014 at 02:27:47PM +1000, Michael Ellerman wrote:
>On Tue, 2014-26-08 at 07:56:16 UTC, Gavin Shan wrote:
>> From: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>> 
>> The patch synchronizes firmware header file (opal.h) for PCI error
>> injection.
>> 
>> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
>> index 4593a93..9113653 100644
>> --- a/arch/powerpc/include/asm/opal.h
>> +++ b/arch/powerpc/include/asm/opal.h
>> @@ -200,6 +201,33 @@ enum OpalPciErrorSeverity {
>>  	OPAL_EEH_SEV_INF	= 5
>>  };
>>  
>> +enum OpalErrinjctType {
>> +	OpalErrinjctTypeIoaBusError	= 0,
>> +	OpalErrinjctTypeIoaBusError64	= 1,
>> +
>> +	/* IoaBusError & IoaBusError64 */
>> +	OpalEjtIoaLoadMemAddr		= 0,
>> +	OpalEjtIoaLoadMemData		= 1,
>> +	OpalEjtIoaLoadIoAddr		= 2,
>> +	OpalEjtIoaLoadIoData		= 3,
>> +	OpalEjtIoaLoadConfigAddr	= 4,
>> +	OpalEjtIoaLoadConfigData	= 5,
>> +	OpalEjtIoaStoreMemAddr		= 6,
>> +	OpalEjtIoaStoreMemData		= 7,
>> +	OpalEjtIoaStoreIoAddr		= 8,
>> +	OpalEjtIoaStoreIoData		= 9,
>> +	OpalEjtIoaStoreConfigAddr	= 10,
>> +	OpalEjtIoaStoreConfigData	= 11,
>> +	OpalEjtIoaDmaReadMemAddr	= 12,
>> +	OpalEjtIoaDmaReadMemData	= 13,
>> +	OpalEjtIoaDmaReadMemMaster	= 14,
>> +	OpalEjtIoaDmaReadMemTarget	= 15,
>> +	OpalEjtIoaDmaWriteMemAddr	= 16,
>> +	OpalEjtIoaDmaWriteMemData	= 17,
>> +	OpalEjtIoaDmaWriteMemMaster	= 18,
>> +	OpalEjtIoaDmaWriteMemTarget	= 19,
>> +};
>
>I realise these come from the skiboot source, but they're just too ugly.
>
>Please use kernel style naming, like most of the rest of the file, eg:
>
>	OPAL_ERR_INJECT_IOA_BUS_ERR
>

After thinking it for more, I decided to take this way because the specfic
function types will potentially be used when we're going to support error
injection from userland (QEMU). 

The new revision has folded all your comments (for other patches as well).
Michael, I'm not sure it's good way to send all my patches for 3.18, or I
just need send revised ones?

Thanks,
Gavin

>Also this enum seems to contain two separate types, the first two values are
>the "type", and the rest are "functions".
>
>The only usage I see is:
>
>	/* Sanity check on error type */
>	if (type < OpalErrinjctTypeIoaBusError   ||
>	    type > OpalErrinjctTypeIoaBusError64 ||
>	    function < OpalEjtIoaLoadMemAddr     ||
>	    function > OpalEjtIoaDmaWriteMemTarget) {
>		pr_warn("%s: Invalid error type %d-%d\n",
>			__func__, type, function);
>		return -ERANGE;
>	}
>
>So we could also just do:
>
># define OPAL_ERR_INJECT_TYPE_MIN	0
># define OPAL_ERR_INJECT_TYPE_MAX	1
>
># define OPAL_ERR_INJECT_FUNC_MIN	0
># define OPAL_ERR_INJECT_FUNC_MAX	19
>
>	if (type < OPAL_ERR_INJECT_TYPE_MIN ||
>	    type > OPAL_ERR_INJECT_TYPE_MAX ||
>	    function < OPAL_ERR_INJECT_FUNC_MIN ||
>	    function > OPAL_ERR_INJECT_FUNC_MIN)
>	{
>		pr_warn("%s: Invalid error type %d-%d\n", __func__, type, function);
>		return -ERANGE;
>	}
>