On Fri, Jan 20, 2023 at 02:38:43PM -0800, Russ Weight wrote:

On 1/19/23 05:28, Greg KH wrote:

quoted

On Wed, Jan 18, 2023 at 08:36:00PM -0500, Tianfei Zhang wrote:

quoted

Implement the image_load and available_images callback functions
for fpgahp driver. This patch leverages some APIs from pciehp
driver to implement the device reconfiguration below the PCI hotplug
bridge.

Here are the steps for a process of image load.
1. remove all PFs and VFs except the PF0.
2. remove all non-reserved devices of PF0.
3. trigger a image load via BMC.
4. disable the link of the hotplug bridge.
5. remove all reserved devices under PF0 and PCI devices
   below the hotplug bridge.
6. wait for image load done via BMC, e.g. 10s.
7. re-enable the link of the hotplug bridge.
8. re-enumerate PCI devices below the hotplug bridge.

Signed-off-by: Tianfei Zhang <redacted>
---
 Documentation/ABI/testing/sysfs-driver-fpgahp |  21 ++
 MAINTAINERS                                   |   1 +
 drivers/pci/hotplug/fpgahp.c                  | 179 ++++++++++++++++++
 3 files changed, 201 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-fpgahp

diff --git a/Documentation/ABI/testing/sysfs-driver-fpgahp b/Documentation/ABI/testing/sysfs-driver-fpgahp
new file mode 100644
index 000000000000..8d4b1bfc4012
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-fpgahp

@@ -0,0 +1,21 @@
+What:		/sys/bus/pci/slots/X-X/available_images
+Date:		May 2023
+KernelVersion:	6.3
+Contact:	Tianfei Zhang <tianfei.zhang@intel.com>
+Description:	Read-only. This file returns a space separated list of
+		key words that may be written into the image_load file
+		described below. These keywords decribe an FPGA, BMC,
+		or firmware image in FLASH or EEPROM storage that may
+		be loaded.

No, sysfs is "one value per file", why is this a list?

And what exactly defines the values in this list?

quoted

+
+What:		/sys/bus/pci/slots/X-X/image_load
+Date:		May 2023
+KernelVersion:	6.3
+Contact:	Tianfei Zhang [off-list ref]
+Description:	Write-only. A key word may be written to this file to
+		trigger a new image loading of an FPGA, BMC, or firmware
+		image from FLASH or EEPROM. Refer to the available_images
+		file for a list of supported key words for the underlying
+		device.
+		Writing an unsupported string to this file will result in
+		EINVAL being returned.

Why is this a separate file from the "read the list" file?

The intended usage is like this:

$ cat available_images
bmc_factory bmc_user fpga_factory fpga_user1 fpga_user2
$ echo bmc_user > image_load

This specifies which image stored in flash that you want to have activated
on the device.

An existing example of something like this is in the tracing code:
available_tracers and current_tracer

Would it be preferable to just create a file for each possible image,
and echo 1 to trigger the event? (echo 1 > bmc_user)

That would make things much more simpler overall and not force people to
have to parse a sysfs file, which is the main reason we created sysfs in
the first place.

thanks,

greg k-h