Hi Chris,
On 11/11/25 01:38, Christophe JAILLET wrote:
Caution: This message originated from an External Source. Use proper
caution when opening attachments, clicking links, or responding.
Le 11/11/2025 à 02:15, David Zhang a écrit :
This patch introduces a new PCI driver for AMD Versal-based accelerator
cards.
The driver provides basic module and PCI device initialization, based on
BAR resources used to establish a hardware queue-based ring buffer
between
the PCIe host and the Versal Management Runtime (VMR) service running on
the embedded SoC. This interface enables firmware management and board
health monitoring.
Key features:
- PCI probe and BAR resource initialization.
- Integration with configfs for firmware management
- Compatibility check using firmware-reported UUIDs
The base firmware image is expected under /lib/firmware/xilinx/<fw_name>
and can be programmed to the device through the configfs interface.
Firmware transfer is handled via a remote queue service (added in a later
patch).
Co-developed-by: DMG Karthik <[email protected]>
Signed-off-by: DMG Karthik <[email protected]>
Co-developed-by: Nishad Saraf <[email protected]>
Signed-off-by: Nishad Saraf <[email protected]>
Signed-off-by: David Zhang <[email protected]>
---
...
+static int versal_pci_device_setup(struct versal_pci_device *vdev)
+{
+ int ret;
+
+ ret = versal_pci_fw_init(vdev);
+ if (ret) {
+ vdev_err(vdev, "Failed to init fw, err %d", ret);
+ goto comm_chan_fini;
+ }
+
+ ret = versal_pci_cfs_init(vdev);
+ if (ret) {
Do we need to call versal_pci_fw_fini()?
(here or in the error handling path to be future proof)
+ vdev_err(vdev, "Failed to init configfs subsys, err %d",
ret);
+ goto comm_chan_fini;
+ }
+
+ return 0;
+
+comm_chan_fini:
+
+ return ret;
+}
...
diff --git a/drivers/accel/amd_vpci/versal-pci.h
b/drivers/accel/amd_vpci/versal-pci.h
new file mode 100644
index 000000000000..ca309aee87ad
--- /dev/null
+++ b/drivers/accel/amd_vpci/versal-pci.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Driver for Versal PCIe device
+ *
+ * Copyright (C) 2025 Advanced Micro Devices, Inc. All rights reserved.
+ */
+
+#ifndef __VERSAL_PCI_H
+#define __VERSAL_PCI_H
+
+#include <linux/configfs.h>
+#include <linux/firmware.h>
+
+#define MGMT_BAR 0
+
+#define vdev_info(vdev, fmt,
args...) \
+ dev_info(&(vdev)->pdev->dev, "%s: "fmt, __func__, ##args)
\n could be added after fmt, as it is not included in the messages
themselves, when used.
I tested this with "\n" on latest linux kernel, it will give us one
extra line. I think latest linux has no need to add this extra "\n" anymore.
Meanwhile, I will address all your other comments and also update the
copyright to 2026.
Happy new year to everyone!
Thanks,
David
Same for the other macro below.
+
+#define vdev_warn(vdev, fmt,
args...) \
+ dev_warn(&(vdev)->pdev->dev, "%s: "fmt, __func__, ##args)
+
+#define vdev_err(vdev, fmt, args...) \
+ dev_err(&(vdev)->pdev->dev, "%s: "fmt, __func__, ##args)
+
+#define vdev_dbg(vdev, fmt, args...) \
+ dev_dbg(&(vdev)->pdev->dev, fmt, ##args)
...
CJ
