Re: [PATCH v1 10/12] PCI: hotplug: implement the hotplug_slot_ops callback for fpgahp

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jan 20, 2023 at 02:38:43PM -0800, Russ Weight wrote:
> 
> 
> 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)

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



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux