Improve detection of Flashstor (FS67xx) devices, implement force_device to override detection #27
Conversation
|
Turned it into a draft because it hasn't been tested yet (except with my AS5402T Nimbustor Gen2) |
DanielGibson
force-pushed
the
detect-flashtor
branch
2 times, most recently
from
a1a25be to
4b0d9a9
Compare
|
(already fixed) |
DanielGibson
force-pushed
the
detect-flashtor
branch
from
a1abb52 to
463649a
Compare
|
I just fixed the aforementioned issue with MODULE_DEVICE_TABLE(), was easier/quicker than I expected. |
DanielGibson
force-pushed
the
detect-flashtor
branch
from
463649a to
cc12e21
Compare
|
Hi, following up here on the conversation from #21 . I tested the new Compared to I also tried loading with the The LEDs are still a bit confused, but at least I can control most of them properly now! We can open perhaps a new issue to look into tweaking them. In any case, at least when it comes to the |
|
Great, thanks for testing! Good to hear my detection logic works :)
oops, you're right! We can as well try to tweak the LEDs here if they're wrong. What specifically is wrong/confused about them? |
DanielGibson
changed the title
DanielGibson
force-pushed
the
detect-flashtor
branch
from
cc12e21 to
043cbbd
Compare
|
I think this is ready for review. I think potential tweaks for the LEDs don't impair that - they should only affect a few lines in the |
|
I'm confused why you removed See the original PR if you're confused as to why I didn't use the DISK LED structs for NVMe drives. |
|
Damn, you're right, sorry! |
DanielGibson
force-pushed
the
detect-flashtor
branch
from
043cbbd to
ac1deb9
Compare
|
Fixed (force-pushed the commits without that change) @mgc8 Was your issue with the LEDs with the NVME LED (and thus either due to my change, or due to Linux not being able to flash LEDs based on NVME activity), or are there other issues? |
the AS67xx devices shouldn't have those packet switches from `lspci` on a FS6712X: 02:00.0 PCI bridge: ASMedia Technology Inc. ASM2806 4-Port PCIe x2 Gen3 Packet Switch (rev 01) 03:00.0 PCI bridge: ASMedia Technology Inc. ASM2806 4-Port PCIe x2 Gen3 Packet Switch (rev 01) (... and several more ...) `lspci -v -nn` then revealed that their PCI vendor ID and product ID are 1b21:2806 (vendor 0xb21 product 0x2806)
can be used like `modprobe asustor force_device=AS66xx` or by adding /etc/modprobe.d/asustor.conf with a line like options asustor force_device=FS67xx (Also ran clang-format)
Instead of using dmi_first_match(asustor_systems) to figure out what kind of ASUSTOR system we're on, I added custom pci_matches[] to asustor_driver_data and wrote custom logic to find the matching system based on both DMI data *and* PCI devices. It's a bit ugly that I had to copy the dmi_matches() code from drivers/firmware/dmi_scan.c, but as that function (or equivalent ones) aren't exported by dmi_scan.c (it's static), it was unavoidable. On the upside, this helped getting rid of the special-casing of FS67xx in asustor_init() and is potentially useful for matching other systems in the future. It should even be a good base for implementing matching of other kinds of devices (than PCI/PCIe), if that becomes necessary. NOTE: While the asustor_systems array isn't fed into dmi_first_match() anymore, it still needs to use the dmi_system_id type so it can be used with MODULE_DEVICE_TABLE(), which creates the alias entries that can be seen in `modinfo asustor`, like alias: dmi*:svn*Insyde*:pn*AS61xx*: and is also used to automatically load the asustor kernel module on boot if the DMI entries in those aliases match the current system
DanielGibson
force-pushed
the
detect-flashtor
branch
from
ac1deb9 to
41ee0a2
Compare
|
I can confirm that the latest version with the generalised detection still works fine on my FS6712X.
No, in my case it's not the NVMe LED. Let me try to explain the LED status in more detail. In Taking them one-by-one:
When the machine boots, the green status led comes on blinking, this can be turned off with the What used to be possible but doesn't seem to be any more with the current setup was to turn off the entire large red side LED, which is quite distracting. The If I can provide more information or videos/photos, please let me know. |
|
The disk triggers won't work with the FS67xx, because they don't work with NVME drives at all, see #21 (comment)
(I assume that
Weird that brightness=0 doesn't work. Does it work for one of the colors (red or green) at least? @romracer: Is it the same with your device? |
Okay, so that's the known NVMe LED issue then, all clear. But it's the same with pretty much every other trigger (
Ah, yes, the red part works fine, sorry for not being more clear. I can turn the red bit on, and then it gets that combined orange colour, and if I make the green status bit blink, then it basically flicks between deep red and orange. The green won't turn fully off though, no matter what I set |
|
If the usb-host trigger works but others don't then that sounds more like an issue with the triggers than with "our" kernel modules. I just tested the green status LED blinking with my AS5402T. |
Does setting to 0 turn it on? I seem to remember if the default state is backwards, the brightness is backwards, or something like that. I'll have to digest some of these comments and check this module later this evening or this week. But I use the following in a startup script on TrueNAS SCALE and it works as expected (with my changes):
I noticed this too, but only when connected to 10GbE. Is yours also connected to 10GbE? I swear it was blue on 1GbE, but I couldn't do my probing to find red while connected at 10GbE. |
|
I did most requested changes, but I still have to do something about that dmi_matches() license issue |
|
Ok I figured out how to build |
DanielGibson
force-pushed
the
detect-flashtor
branch
3 times, most recently
from
2f7c0f6 to
e61ad9d
Compare
|
Since i just found this now, maybe the following can help for FS6706T - it has no PCIe bridges at all from what I see in the output of lspci: Unfortunately I dont have the time right now to test the change but will do as soon as possible, but my guess is that just reducing the amount of these bridges to 0 will correct the detection mechanism for FS6706T as well. { 0x1b21, 0x2806, 1, DEVICE_COUNT_MAX }, -> { 0x1b21, 0x2806, 0, DEVICE_COUNT_MAX }, Maybe a proposal would be to differentiate between the two FS models based on their NICs but I dont know the ID used by 10G NIC: Also I'm willing to test something if needed as soon as I have some spare time. |
|
Thanks a lot! It seems like your device has more Intel PCI bridges than the FS6712X (or the AS67xx). UPDATE/TODO: I also only have one NVME SSD in my NAS, but it has 4 slots - I wonder if more of those intel PCI bridges would turn up if I used the other 3 NVME slots Another candidate might be Maybe one of those can be used, but I'll have to take a closer look at data from other AS67xx devices first. ANOTHER UPDATE: Shit, at least the AS6706 has all the aforementioned PCI bridges and the audio device: #27 (comment) |
|
Oh right, for the sake of completeness, can you also post the output of: |
|
Hi, yes, you are right i totally forgot the AS67xxx machine. |
asustor_main.c is licensed under GPLv2 or later, while asustor_gpl2.c is licensed under GPLv2 only. dmi_matches() is moved from asustor(_main).c to asustor_gpl2.c, because it's copied from drivers/firmware/dmi_scan.c which is released under GPLv2 only. asustor.c had to be renamed to asustor_main.c, because apparently the kernel build system only supports a source file with the same name as the module if that module is built only from that file. I also bumped the DRIVER_VERSION (used for DKMS) from v0.1 to v0.2 to hopefully avoid conflicts due to all these changes
.. and update the link to the block device LED trigger to the (AFAIK) latest (last?) attempt by Ian Pilcher (v13) Though TBH it seems like neither is ever gonna happen, because the block subsystem maintainers don't even want to provide an API that allows accessing block device data based on the device name (like sda), and generally seem to be unfriendly, so maybe the best solution would be to implement this in the userland (periodically check disks read/write stats and control blinking based on that)...
turns out that the gpio_device* returned by gpio_device_find_by_label() must be "freed" (or the refcounter decremented) by calling gpio_device_put(dev). While fixing that, I refactored the code for getting the GPIO chip's base number a bit so we don't have to use the (deprecated) gpio_device_get_chip() anymore.
Looks like the Flashtor for 6 SSDs doesn't have any PCIe device that the *A*S67xx devices don't have. So now we painstakingly detect every AS67xx and FS67xx separately. On the upside, this allows exposing only the SATA LEDs that actually exist on each specific AS67xx device. This also means that the overrides (force_device kernel module argument) for those devices have changed and are now "AS6702", "AS6704", "AS6706" instead of "AS67xx" and "FS6706", "FS6712" instead of "FS67xx"
DanielGibson
force-pushed
the
detect-flashtor
branch
from
b184681 to
9c9cc66
Compare
|
I updated the code for this, now the AS67xx and FS67xx devices are detected separately as "AS6702" (also used for AS5402T), "AS6704" (also used for AS5404T), "AS6706", "FS6706" and "FS6712". Of course this means that the code should be tested on all those devices. Testing means: Check out my branch ( Afterwards check if Thanks in advance! |
|
Hi @DanielGibson, All seems to work well on my end, good idea on using negative check for SATA and all PCIe bridges! Some further information that i managed to dig out from my experiments some time ago is the LED to GPIO mapping using the gpio_it87 driver: Further more I noted also the inversion on the pwm control for the brightness, but i have no idea if this is still valid: What I really can't wrap my head around is that Asustor does this detection most likely on something stored in emmc boot partitions or maybe in the SPI eeprom where the BIOS is stored, but I could not find a single clue in their firmware update files or ACPI tables dissasembly. The furthest i could get is that they pull this info from /etc/nas.conf for their scripts. |
|
Thanks a lot @brunomatic! You're right about the inverted pwm control for brightness, it's the same on my AS5402T. Not sure if we can do anything about that though |
Hi @DanielGibson , sorry for the delay lately, the end of the year period tends to get a bit crazy... I have checked out the latest version of your branch for a clean build, and I can confirm that it compiles and loads fine, yielding the expected result on the FS6712x:
I tried enabling those GPIOs as per your comments here, but I didn't see any change with the red side/middle LEDs: @brunomatic Can you please provide more details as to how you identified those GPIO numbers and process you use for testing them? On quick check they don't line up with what we already have here: |
Be careful with
That would indeed be a good way to differentiate them. The FS6712x comes with the Aquantia AQC113 10GbE controller, ID |
|
Thanks a lot, @mgc8 !
The problem wasn't telling FS6706 and FS6712 apart (only the latter has the specific PCIe Switch I'm matching - the Intel PCI bridges exist in all AS67xx and FS67xx devices), but telling FS6706 and AS67xx apart - but I found a solution that worked for anyone who tested so far (FS6712 is still the only device with that PCIe Bridge and FS6706 is the only device, apart from FS6712, that has no SATA controller) :) |
Wow, awesome! Yes, they work perfectly with this patch, and each has the correct assignment. The You can make a veritable light show with a script like this 😅:
That's quite smart :) What a variety of configurations for these devices... |
Good to hear!
thanks :)
It partly makes sense: the ones with SATA drives each have a different SATA controller that supports as many drives as needed (and it's possible that the SATA controller isn't even on the mainboard, but on a separate card that also holds the SATA connectors for the bays, connected via PCIe slot, so maybe all AS67xx devices use the same mainboard - but that's just speculation on my side). BTW, nice idea with the script =) |
It's those red strips on the left and right side of the power button. They're controlled by three GPIOs that are exposed as the new LEDs red:side_inner, red:side_mid and red:side_outer One of them was previously controlled by power:front_panel, but I changed it so it's more consistent with the other two LEDs (LED pairs?) of the strips Thanks to @brunomatic for figuring out the GPIOs of these LEDs!
DanielGibson
force-pushed
the
detect-flashtor
branch
from
56176cc to
86a3478
Compare
Sorry, didn't look at that closely earlier. |
It is indeed a bit confusing, but I had tried both 46 and 66 in that case, and neither caused the LED to blink; it could be that it was working but just not doing the "blink" action. In any case, it's all moot now as your added patch makes sure they all work nicely. I don't think it's worth a separate issue, let's just get this merged as I think it covers all the bases now! |
|
@mafredri I think this is really ready for merging now ;) Of course it would've been nice of someone with an AS67xx had tested the current code, but at least it has been tested with Flashtor FS6706T and FS6712X and my FS5402T (which is correctly detected as "AS6702 or similar") and I see no reason why detection of AS67xx should fail. |
So sorry, I forgot to tell you that I've installed it last week, and all seems to work properly. Detection is OK (as AS6702 wich I have) |
This is done by checking if the "ASMedia Technology Inc. ASM2806 4-Port PCIe x2 Gen3 Packet Switch" PCIe device exists.
This should work with FS6712X (
though my code hasn't been tested by @mgc8 or @romracer yetit has been tested); unfortunately I couldn't find anyone with an FS6706T in the issues or PRs of this project.Testing an AS6706T would also be interesting, in case it also uses such a switch (my AS5402T which is mostly identical to the AS6702T does not have it - Update: AS6706T has a PCIe switch, but a different one, AS6704T doesn't have a PCIe switch).
I also implemented a
force_devicekernel module argument which lets users override the device detection.See also the discussion in #21 (comment) (and following)