lkml.org 
[lkml]   [2021]   [Feb]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH v2 1/1] fpga: dfl: afu: harden port enable logic
    Date
    > 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.

    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
    > >
    > >

    \
     
     \ /
      Last update: 2021-02-04 02:58    [W:4.432 / U:0.432 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site