On 1/19/23 05:28, Greg KH wrote: > On Wed, Jan 18, 2023 at 08:36:00PM -0500, Tianfei Zhang wrote: >> 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 <tianfei.zhang@xxxxxxxxx> >> --- >> 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@xxxxxxxxx> >> +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? > >> + >> +What: /sys/bus/pci/slots/X-X/image_load >> +Date: May 2023 >> +KernelVersion: 6.3 >> +Contact: Tianfei Zhang <tianfei.zhang@xxxxxxxxx> >> +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) Thanks, - Russ > That feels wrong. > > thanks, > > greg k-h