> 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@xxxxxxxxx> > >>>>> Sent: Friday, September 18, 2020 2:32 AM > >>>>> To: mdf@xxxxxxxxxx; linux-fpga@xxxxxxxxxxxxxxx; linux- > >>>>> kernel@xxxxxxxxxxxxxxx > >>>>> Cc: trix@xxxxxxxxxx; lgoncalv@xxxxxxxxxx; Xu, Yilun > <yilun.xu@xxxxxxxxx>; > >>>>> Wu, Hao <hao.wu@xxxxxxxxx>; Gerlach, Matthew > >>>>> <matthew.gerlach@xxxxxxxxx>; Weight, Russell H > >>>>> <russell.h.weight@xxxxxxxxx> > >>>>> 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. Thanks for this confirmation, then it sounds good to me. I think only Intel hardware is using this driver now, so if this is confirmed from hardware side, then we should be safe to take this one. Hao > > 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 > > > >
