Re: [PATCHv2] fpga: stratix10-soc: remove the pre-set reconfiguration condition

From: Russ Weight <russell.h.weight@intel.com>
To: Richard Gong <richard.gong@linux.intel.com>, mdf@kernel.org
Cc: linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org,
	dinguyen@kernel.org, Richard Gong <richard.gong@intel.com>
Subject: Re: [PATCHv2] fpga: stratix10-soc: remove the pre-set reconfiguration condition
Date: Fri, 29 May 2020 09:18:23 -0700	[thread overview]
Message-ID: <3adbce08-78c3-7300-f2f8-1e12639af340@intel.com> (raw)
In-Reply-To: <1d9b21df-7421-b25e-5139-f297e24d99d4@linux.intel.com>

Minor comment below.

- Russ

On 5/29/20 6:15 AM, Richard Gong wrote:
> Hi Moritz,
>
> Sorry for asking.
>
> When you get chance, can you review my version 2 patch submitted on 
> 05/15/20?
>
> Regards,
> Richard
>
> On 5/15/20 9:35 AM, richard.gong@linux.intel.com wrote:
>> From: Richard Gong <richard.gong@intel.com>
>>
>> The reconfiguration mode is pre-set by driver as the full 
>> reconfiguration.
>> As a result, user have to change code and recompile the drivers if he or
>> she wants to perform a partial reconfiguration. Removing the pre-set
>> reconfiguration condition so that user can select full or partial
>> reconfiguration via overlay device tree without recompiling the drivers.
>>
>> Also add an error message if the configuration request is failure.
s/is failure/fails/
>>
>> Signed-off-by: Richard Gong <richard.gong@intel.com>
>> ---
>> v2: define and use constant values
>> ---
>>   drivers/fpga/stratix10-soc.c | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
>> index 44b7c56..4d52a80 100644
>> --- a/drivers/fpga/stratix10-soc.c
>> +++ b/drivers/fpga/stratix10-soc.c
>> @@ -14,9 +14,13 @@
>>   /*
>>    * FPGA programming requires a higher level of privilege (EL3), per 
>> the SoC
>>    * design.
>> + * SoC firmware supports full and partial reconfiguration.
>>    */
>>   #define NUM_SVC_BUFS    4
>>   #define SVC_BUF_SIZE    SZ_512K
>> +#define FULL_RECONFIG_FLAG    0
>> +#define PARTIAL_RECONFIG_FLAG    1
>> +
>>     /* Indicates buffer is in use if set */
>>   #define SVC_BUF_LOCK    0
>> @@ -182,12 +186,12 @@ static int s10_ops_write_init(struct 
>> fpga_manager *mgr,
>>       uint i;
>>       int ret;
>>   -    ctype.flags = 0;
>>       if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
>>           dev_dbg(dev, "Requesting partial reconfiguration.\n");
>> -        ctype.flags |= BIT(COMMAND_RECONFIG_FLAG_PARTIAL);
>> +        ctype.flags = PARTIAL_RECONFIG_FLAG;
>>       } else {
>>           dev_dbg(dev, "Requesting full reconfiguration.\n");
>> +        ctype.flags = FULL_RECONFIG_FLAG;
>>       }
>> reinit_completion(&priv->status_return_completion);
>> @@ -210,6 +214,7 @@ static int s10_ops_write_init(struct fpga_manager 
>> *mgr,
>>         ret = 0;
>>       if (!test_and_clear_bit(SVC_STATUS_OK, &priv->status)) {
>> +        dev_err(dev, "RECONFIG_REQUEST failed\n");
>>           ret = -ETIMEDOUT;
>>           goto init_done;
>>       }
>>