On Tue, 23 Jan 2024, Xu Yilun wrote:
On Mon, Jan 22, 2024 at 09:24:33AM -0800, Matthew Gerlach wrote:
Revision 2 of the Device Feature List (DFL) Port feature has
slightly different requirements than revision 1. Revision 2
does not need the port to reset at driver startup. In fact,
Please help illustrate what's the difference between Revision 1 & 2, and
why revision 2 needs not.
I will update the commit message to clarify the differences between
revision 1 and 2.
performing a port reset during driver initialization can cause
driver race conditions when the port is connected to a different
Please reorganize this part, in this description there seems be a
software racing bug and the patch is a workaround. But the fact is port
reset shouldn't been done for a new HW.
Reorganizing the commit message a bit will help to clarify why port reset
should not be performed during driver initialization with revision 2 of
the hardware.
BTW: Is there a way to tell whether the port is connected to a different
PF? Any guarantee that revision 3, 4 ... would need a port reset or not?
The use of revision 2 of the port_hdr IP block indicates that the port can
be connected multiple PFs, but there is nothing explicitly stating which
PFs the port is connected to.
It is hard to predict the requirements and implementation of a future
revision of an IP block. If a requirement of a future revision is to work
with existing software, then the future revision would not require a port
reset at driver initialization.
Thanks,
Yilun
PCIe Physical Function (PF) than the management PF performing
the actual port reset.
Signed-off-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
---
drivers/fpga/dfl-afu-main.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index c0a75ca360d6..7d7f80cd264f 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -417,7 +417,18 @@ static const struct attribute_group port_hdr_group = {
static int port_hdr_init(struct platform_device *pdev,
struct dfl_feature *feature)
{
- port_reset(pdev);
+ void __iomem *base;
+ u8 rev;
+
+ base = dfl_get_feature_ioaddr_by_id(&pdev->dev, PORT_FEATURE_ID_HEADER);
+
+ rev = dfl_feature_revision(base);
+
+ if (rev < 2)
+ port_reset(pdev);
+
+ if (rev > 2)
+ dev_info(&pdev->dev, "unexpected port feature revision, %u\n", rev);
Remove the print. It is indicating an error but the function returns OK.
The message is intended to be informational, but I'll remove it because it
could be confusing.
Thanks,
Yilun
Thanks for the feedback.
return 0;
}
--
2.34.1
