Re: [PATCH] fpga: altera-cvp: allow interrupt to continue next time

From: Xu Yilun <yilun.xu@intel.com>
To: tien.sung.ang@intel.com
Cc: dinh.nguyen@intel.com, hao.wu@intel.com,
	linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org,
	mdf@kernel.org, trix@redhat.com
Subject: Re: [PATCH] fpga: altera-cvp: allow interrupt to continue next time
Date: Fri, 3 Jun 2022 18:14:43 +0800	[thread overview]
Message-ID: <20220603101443.GB238410@yilunxu-OptiPlex-7050> (raw)
In-Reply-To: <20220531022004.2314963-1-tien.sung.ang@intel.com>

On Tue, May 31, 2022 at 10:20:04AM +0800, tien.sung.ang@intel.com wrote:
> >> CFG_READY signal/bit may time-out due to firmware not responding
> >> within the given time-out. This time varies due to numerous
> >> factors like size of bitstream and others.
> >> This time-out error does not impact the result of the CvP
> >> previous transactions. The CvP driver shall then, respond with
> 
> >Do you mean the reprogramming is successful even if you find the time
> >out in write_complete()? Then return 0 is better?
> Based on the information given by the Intel FPGA firmware team,
> CFG_READY is essential to indicate if the current FPGA 
> configuration session is indeed a success. There are 
> cases we test in the lab whereby, CFG_READY stays invalid and
> the tests performed subsequently to verify the FPGA functionality
> could not detect the failed session. A failed FPGA 
> configuration session means, the new bitstream wasn't 
> successfully configured and tests ran later will just be passing
> on the previous working bitstream version. In short, CFG_READY
> is esential, and an error indicating the time-out is a must.
> Another example, using an incorrect SOF/Design FPGA results
> in CFG_READY being invalid. The user must be informed of a 
> potential error. 
> I will correct the wordings i used earlier that says that
> the timoeut error does not impact the results of the CvP
> previous transactions. It may so if the firmware has some sort
> of error. 

Understood. But with your new comment why you must change the error
code to -EAGAIN rather than timeout?

I think you may change your commit message. The main change is adding
the error handling. The error code change is minor, even not necessary
if you don't have a strong reason.

Thanks,
Yilun

> 
> >And could you specify what the time-out mean on write_init() phase?
> I could not really understand your question. We set huge 
> time-outs of ~10seconds. Every wait for the firmware to respond
> is potentially a hazard. The firmware CvP is has it's limitation
> unfortunately. 