Hi, On 7/10/22 12:15, Greg KH wrote: > On Fri, Jul 08, 2022 at 11:34:40AM -0700, Joseph, Jithu wrote: >> >> >> On 7/8/2022 8:28 AM, Greg KH wrote: >>> On Fri, Jul 08, 2022 at 08:19:38AM -0700, Jithu Joseph wrote: >>>> Existing implementation limits IFS image to be loaded only from >>>> a default file-name (ff-mm-ss.scan). >>>> >> >>> >>> Ick, but now what namespace are you saying that path is in? If you need >>> debugging stuff, then put the api/interface in debugfs and use it there, >>> don't overload the existing sysfs api to do something different here. >> >> The namespace related confusion could be because, the original commit message >> was not using full path-names. The below write-up tries to be more clear on this >> >> Existing implementation limits IFS images to be loaded only from >> a default file-name /lib/firmware/intel/ifs/ff-mm-ss.scan. >> >> But there are situations where there may be multiple scan files >> that can be run on a particular system stored in /lib/firmware/intel/ifs >> >> E.g. >> 1. Because test contents are larger than the memory reserved for IFS by BIOS >> 2. To provide increased test coverage >> 3. Custom test files to debug certain specific issues in the field >> >> Renaming each of these to ff-mm-ss.scan and then loading might be >> possible in some environments. But on systems where /lib is read-only >> this is not a practical solution. >> >> Extend the semantics of the driver /sys/devices/virtual/misc/intel_ifs_0/reload >> file: >> >> Writing "1" remains the legacy behavior to load from the default >> ff-mm-ss.scan file. >> >> Writing some other string is interpreted as a filename in >> /lib/firmware/intel/ifs to be loaded instead of the default file. > > Ick, you are overloading an existing sysfs file to do different things > based on random stuff. This is a brand-new api that you are already > messing with in crazy ways. Why not just revert the whole thing and > start over as obviously this was not tested well with real devices. > > And what is wrong with a firmware file called '1'? :) Actually the Intel IFS stuff has landed in 5.19-rc# so it is a bit late(ish) for dropping it now. But I do agree overloading the reload attribute is ugly, why not just add a new /sys/devices/virtual/misc/intel_ifs_0/filename rw sysfs attribute and use that to allow the user to specify a filename to load different from the default one. Then to load the <whatever> test firmware the user can do: echo "<whatever>" > /sys/devices/virtual/misc/intel_ifs_0/filename echo > /sys/devices/virtual/misc/intel_ifs_0/reload that seems much cleaner to me ? Regards, Hans
