Re: [PATCH] fpga: dfl: afu: update initialization of port_hdr driver

From: matthew . gerlach
Date: Wed Jan 24 2024 - 14:40:50 EST




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