realtek-poe: Add PSE ID quirk for Zyxel GS1900-24HP-v1 #6
Conversation
|
Tested CI#15 on my GS1900-24HPv1:
A WS-AP3825i plugged into port 1 powered on as soon as the device did. Note that issuing Tested-by: Martin Kennedy hurricos@gmail.com |
|
The |
|
To add to my tested-by: I am noticing (though I have not A/B'd to verify this changeset / ddd72d2 is responsible) that "lan1": {
"priority": 2,
"mode": "PoE",
"status": "Searching"
},
"lan2": {
"priority": 2,
"mode": "PoE",
"status": "Delivering power",
"consumption": 5.100000
},
"lan3": {
"priority": 2,
"mode": "PoE",
"status": "Searching"
},
"lan4": {
"priority": 2,
"mode": "PoE",
"status": "Searching"
},
"lan5": {
"priority": 2,
"mode": "PoE",
"status": "Disabled"
},
"lan6": {
"priority": 2,
"mode": "PoE",
"status": "Searching"
},In fact, ports 9-24 all show as disabled (as does 5). I think this might be a race-condition of some sort -- I haven't traced it to see what it does, but To add to this: I moved the PoE sink to port 23, found it come up; issued I haven't tried applying just the functional / non-refactoring changes to test whether this is a regression from one of the commits in #4, and I bet the new functional commits would not apply without your cleanup. Just giving you a heads up that I can tweak |
|
No need to tweak |
I thought I needed to tweak it, as
|
|
I'm not surprised. realtek-poe is propped up by 2x4s and held together with duct tape. First milestone is to get it to power on the ports. That makes 99% of users happy. Then we can worry about the undefined behavior and deadlocks that I found. |
|
While still using CI14 realtek-poe package I can confirm that just a Reconnecting PoE devices on other 9-24 ports will power those devices on although the Doing a EDIT: Also doing a reboot while ports 9-24 were active before with PoE, just quickly powers off and powers on the PoE devices during boot but eventually PoE power goes and stays down for port 9-24. However port status now shows a mix of I can also confirm @Hurricos that there is strange behaviour concerning ports 5,6,7, they sometimes ran into a |
|
Maybe an interresting but logical find is that disabling |
What does GET_STR() expand to? For simple cases, it's trivial.
Operator precedence in C can result in unexpected behavior when
"a" or "b" are complex expressions:
GET_STR(i + 2, array)
==> (i + 2 < ARRAY_SIZE() ? array[i + 2] : NULL);
GET_STR(1, array + i)
==> (1 < ARRAY_SIZE() ? array + i[1] : NULL)
The former incorrectly checks the array size against the second term,
but dereferences a higher index. It can result in out-of-bounds reads.
The latter results in nonsense code
Macro expansions can be fixed by enclosing expresions in parentheses.
This ensures expressions get evaluated in the expected order. Add
parentheses in GET_STR(), where appropriate.
Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Only status data for the first 8 ports was requested. Most devices have 8 ports, and the reply packet has room for 8 ports. It was the perfect match! People quickly figured out realtek-poe works for unicorn switches with more than 8 PoE ports. And so they did. And they increased MAX_PORT without regards to the out-of-bounds access problem they created. And realtek-poe refused to crash! Command 0x2a has proven to be unreliable for devices with more than 8 ports. It returns garbage for ports > 8, even on switchea with more than 8 PoE ports. Command 0x28 is more reliable. It only sends 4 port statuses per packet, but returns correct data. If the port number exceeds the total ports on the device, it correctly returns 0xff. Replace port status query with command 0x28. Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
mrnuke
force-pushed
the
mrnuke-more-power
branch
2 times, most recently
from
35b1bdf
to
2eb1831
Compare
|
Would be useful to have |
The "set global power budget" command contains a PSE controller in its argument list. This was hardcoded to 0. On some switches, commands to PSE 0 do nothing. In those cases, the set budget command won't stick. The power budget remains at its default value, usually 0. Because of this, ports remain stuck in the "Requesting power" state. To solve this, send the budget command to the correct PSE ID or set of IDs. Use a mask to encode which PSE IDs neet the set budget command. When the "compatible" devicetree string matches Zyxel GS1900-24HP-v1, set the mask to 0xff, to command PS ID's 0->7. The vendor firmware also uses ID's 0->7. This enables power delivery to PoE sinks. Tested-by: Martin Kennedy hurricos@gmail.com Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
mrnuke
force-pushed
the
mrnuke-more-power
branch
from
2eb1831
to
a19d224
Compare
Here's a .tar.gz of it: https://paste.c-net.org/LeelooLucid
Each of these was run with about 15s between then. |
One more thing -- that run didn't show the |
|
Seems to be the same root cause as #9 (comment) |
OK. Looks like it's a separate issue unrelated to this PR. Some next steps are outlined here. I am now merging this. Thank you @mrnuke! |
The "set global power budget" command contains a PSE controller in its
argument list. This was hardcoded to 0. On some switches, commands to
PSE 0 do nothing. In those cases, the set budget command won't stick.
The power budget remains at its default value, usually 0. Because of
this, ports remain stuck in the "Requesting power" state.
To solve this, send the budget command to the correct PSE ID or set of
IDs. Use a mask to encode which PSE IDs neet the set budget command.
When the "compatible" devicetree string matches Zyxel GS1900-24HP-v1,
set the mask to 0xff, to command PS ID's 0->7. The vendor firmware
also uses ID's 0->7. This enables power delivery to PoE sinks.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com