Messages in this thread | | | Date | Wed, 3 Feb 2021 15:07:17 -0800 (PST) | From | matthew.gerlach@linux ... | Subject | Re: [PATCH v2 1/1] fpga: dfl: afu: harden port enable logic |
| |
On Wed, 3 Feb 2021, Russ Weight wrote:
> > > On 2/3/21 1:28 AM, Wu, Hao wrote: >>> Subject: Re: [PATCH v2 1/1] fpga: dfl: afu: harden port enable logic >>> >>> Sorry for the delay on this patch. It seemed like a lower priority patch than >>> others, since we haven't seen any issues with current products. Please my >>> responses inline. >>> >>> On 9/17/20 7:08 PM, Wu, Hao wrote: >>>>> -----Original Message----- >>>>> From: Russ Weight <russell.h.weight@intel.com> >>>>> Sent: Friday, September 18, 2020 2:32 AM >>>>> To: mdf@kernel.org; linux-fpga@vger.kernel.org; linux- >>>>> kernel@vger.kernel.org >>>>> Cc: trix@redhat.com; lgoncalv@redhat.com; Xu, Yilun <yilun.xu@intel.com>; >>>>> Wu, Hao <hao.wu@intel.com>; Gerlach, Matthew >>>>> <matthew.gerlach@intel.com>; Weight, Russell H >>>>> <russell.h.weight@intel.com> >>>>> Subject: [PATCH v2 1/1] fpga: dfl: afu: harden port enable logic >>>>> >>>>> Port enable is not complete until ACK = 0. Change >>>>> __afu_port_enable() to guarantee that the enable process >>>>> is complete by polling for ACK == 0. >>>> The description of this port reset ack bit is >>>> >>>> " After initiating a Port soft reset, SW should monitor this bit. HW >>>> will set this bit when all outstanding requests initiated by this port >>>> have been drained, and the minimum soft reset pulse width has >>>> elapsed. " >>>> >>>> But no description about what to do when clearing a Port soft reset >>>> to enable the port. >>>> >>>> So we need to understand clearly on why we need this change >>>> (e.g. what may happen without this change), and will it apply for all >>>> existing DFL devices and future ones, or just for one specific card. >>>> Could you please help? : ) >>> I touched bases with the hardware engineers. The recommendation to wait >>> for ACK to be cleared is new with OFS and is documented in the latest >>> OFS specification as follows (see step #4): >>> >>>> 3.7.1 AFU Soft Resets >>>> Software may cause a soft reset to be issued to the AFU as follows: >>>> 1. Assert the PortSoftReset field of the PORT_CONTROL register >>>> 2. Wait for the Port to acknowledge the soft reset by monitoring the >>>> PortSoftResetAck field of the PORT_CONTROL register, i.e. >>> PortSoftResetAck=1 >>>> 3. Deasserting the PortSoftReset field >>>> 4. Wait for the Port to acknowledge the soft reset de-assertion by monitoring >>> the >>>> PortSoftResetAck field of the PORT_CONTROL register, i.e. >>> PortSoftResetAck=0 >>>> This sequence ensures that outstanding transactions are suitably flushed and >>>> that the FIM minimum reset pulse width is respected. Failing to follow this >>>> sequence leaves the AFU in an undefined state. >>> The OFS specification has not been posted publicly, yet. >>> >>> Also, this is how it was explained to me: >>> >>>> In most scenario, port will be able to get out of reset soon enough >>>> when SW releases the port reset, especially on all the PAC products >>>> which have been verified before release. >>>> >>>> Polling for HW to clear the ACK is meant to handle the following scenarios: >>>> >>>> * Different platform can take variable period of time to get out of reset >>>> * Bug in the HW that hold the port in reset >>> So this change is not required for the currently released PAC cards, >>> but it is needed for OFS based products. I don't think there is any reason >>> to hold off on the patch, as it is still valid for current products. >> As you know, this driver is used for different cards, and we need to make >> sure new changes introduced in new version spec, don't break old products >> as we are sharing the same driver. and we are not sure if in the future some >> new products but still uses old specs, and then things may be broken if the >> driver which always perform new flow. Another method is that introduce 1 >> bit in hardware register to tell the driver to perform the additional steps, >> then it can avoid impacts to the old products. If this can't be done, then >> we at least need to verify this change on all existing hardware and suggest >> users to follow new spec only. > > According to the HW engineers, the RTL implementation has not changed; it is > the same as the RTL in the current PAC products. Polling for HW to clear the > ACK is something we could have (should have?) been doing all along. The timing
I also confirmed with HW engineers. The original specification was not precise. The code should have been doing this all along.
Matthew Gerlach
> hasn't been an issue for the current PAC products, as proven by our testing. > However, with OFS we cannot anticipate what the timing will be for customer > designed products, so the specification is calling out this requirement as a > precaution. > > I am using a development machine that has the older PAC devices installed. I > cleared port errors on these cards as a quick check, and the reset completes > without hanging - which indicates that the ACK bit is in fact getting cleared. > So there is not need for any device-specific conditional statements here. > > - Russ > >> >> Hao > >
| |