[U-Boot] [PATCH] arm: socfpga: stratix10: Add generic FPGA reconfig mailbox API for S10

[Marek Vasut]

On 09/27/2018 07:08 AM, Ang, Chee Hong wrote:
> On Wed, 2018-09-26 at 16:53 +0200, Marek Vasut wrote:
>> On 09/26/2018 11:03 AM, chee.hong.ang at intel.com wrote:
>>>
>>> From: "Ang, Chee Hong" <chee.hong.ang at intel.com>
>>>
>>> Add a generic mailbox API for FPGA reconfig status which can be
>>> called by others. This new function accepts 2 different mailbox
>>> commands: CONFIG_STATUS or RECONFIG_STATUS.
>> What "others" can call this ?
> This is a common function used to check the readiness of the FPGA.
> FPGA configuration drivers will need to call this to make sure
> the FPGA configuration is successfully completed and running.
> These FPGA configuration drivers will be submitted soon after this
> patch.

So this should be in drivers/fpga/ ?

Also, don't add dead code, just submit this alongside the user of this code.

>>> Signed-off-by: Ang, Chee Hong <chee.hong.ang at intel.com>
>>> ---
>>>  arch/arm/mach-socfpga/include/mach/mailbox_s10.h |  3 +-
>>>  arch/arm/mach-socfpga/mailbox_s10.c              | 48
>>> ++++++++++++++++++++++++
>>>  2 files changed, 50 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/mach-socfpga/include/mach/mailbox_s10.h
>>> b/arch/arm/mach-socfpga/include/mach/mailbox_s10.h
>>> index 81a609d..660df35 100644
>>> --- a/arch/arm/mach-socfpga/include/mach/mailbox_s10.h
>>> +++ b/arch/arm/mach-socfpga/include/mach/mailbox_s10.h
>>> @@ -140,5 +140,6 @@ int mbox_qspi_open(void);
>>>  #endif
>>>  
>>>  int mbox_reset_cold(void);
>>> -
>>> +int mbox_get_fpga_config_status(u32 cmd);
>>> +int mbox_get_fpga_config_status_psci(u32 cmd);
>>>  #endif /* _MAILBOX_S10_H_ */
>>> diff --git a/arch/arm/mach-socfpga/mailbox_s10.c b/arch/arm/mach-
>>> socfpga/mailbox_s10.c
>>> index 0d906c3..345485c 100644
>>> --- a/arch/arm/mach-socfpga/mailbox_s10.c
>>> +++ b/arch/arm/mach-socfpga/mailbox_s10.c
>>> @@ -342,6 +342,54 @@ int mbox_reset_cold(void)
>>>  	return 0;
>>>  }
>>>  
>>> +/* Accepted commands: CONFIG_STATUS or RECONFIG_STATUS */
>>> +static __always_inline int mbox_get_fpga_config_status_common(u32
>>> cmd)
>> Why __always_inline ?
> This function is needed in 2 sections. Our u-boot run in normal code
> section and '__secure' section which mainly used for PSCI services.
> Refer to the 'mbox_get_fpga_config_status()' and
> 'mbox_get_fpga_config_status_psci()' below. These 2 functions run in 2
> different sections and they need to call this function.

But why does this need to be __always_inline ? The compiler can decide
what's best.

>>>
>>> +{
>>> +	u32 reconfig_status_resp_len;
>>> +	u32 reconfig_status_resp[RECONFIG_STATUS_RESPONSE_LEN];
>>> +	int ret;
>>> +
>>> +	reconfig_status_resp_len = RECONFIG_STATUS_RESPONSE_LEN;
>>> +	ret = mbox_send_cmd_common(MBOX_ID_UBOOT, cmd,
>>> +				   MBOX_CMD_DIRECT, 0, NULL, 0,
>>> +				   &reconfig_status_resp_len,
>>> +				   reconfig_status_resp);
>>> +	if (!ret) {
>> if (ret)
>>  return ret;
> Will be addressed in v2 patch.
>>
>> ...
>>
>>>
>>> +		/* Check for any error */
>>> +		ret = reconfig_status_resp[RECONFIG_STATUS_STATE];
>>> +		if (ret && ret != MBOX_CFGSTAT_STATE_CONFIG)
>>> +			return ret;
>>> +
>>> +		/* Make sure nStatus is not 0 */
>>> +		ret =
>>> reconfig_status_resp[RECONFIG_STATUS_PIN_STATUS];
>>> +		if (!(ret & RCF_PIN_STATUS_NSTATUS))
>>> +			return MBOX_CFGSTAT_STATE_ERROR_HARDWARE;
>>> +
>>> +		ret =
>>> reconfig_status_resp[RECONFIG_STATUS_SOFTFUNC_STATUS];
>>> +		if (ret & RCF_SOFTFUNC_STATUS_SEU_ERROR)
>>> +			return MBOX_CFGSTAT_STATE_ERROR_HARDWARE;
>>> +
>>> +		if ((ret & RCF_SOFTFUNC_STATUS_CONF_DONE) &&
>>> +		    (ret & RCF_SOFTFUNC_STATUS_INIT_DONE) &&
>>> +		    !reconfig_status_resp[RECONFIG_STATUS_STATE])
>>> +			return 0;	/* configuration success
>>> */
>>> +		else
>>> +			return MBOX_CFGSTAT_STATE_CONFIG;
>>> +	}
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +int mbox_get_fpga_config_status(u32 cmd)
>>> +{
>>> +	return mbox_get_fpga_config_status_common(cmd);
>>> +}
>>> +
>>> +int __secure mbox_get_fpga_config_status_psci(u32 cmd)
>>> +{
>>> +	return mbox_get_fpga_config_status_common(cmd);
>>> +}
>>> +
>>>  int mbox_send_cmd(u8 id, u32 cmd, u8 is_indirect, u32 len, u32
>>> *arg,
>>>  		  u8 urgent, u32 *resp_buf_len, u32 *resp_buf)
>>>  {
>>>


-- 
Best regards,
Marek Vasut