This patch adds RockChip RK356X support to existing RockChip iodomain driver.
It was submitted by sos@.
Details
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
sys/arm64/rockchip/rk_iodomain.c | ||
---|---|---|
199 | Not sure I understand the logic here, we only write to those registers for pmuio2-supply ? |
sys/arm64/rockchip/rk_iodomain.c | ||
---|---|---|
199 | yes |
This isn't correct.
There is no need for sc->conf->type here, we need to expand the struct rk_iodomain_supply to have the corresponding registers.
The if (i == 1) is wrong, we need to write to the corresponding register for each iodomain in the dts.
One should be careful doing there is multiple bits on multiple register for choosing the output voltage, we need to disable all of them and then enable the correct voltage based on the regulator value. There is three value possible, 1.8V, 2.5v and 3.3v (but not for every iodomain).
Have a look at page 208 of the TRM Part1 to have a better idea on how this is supposed to work.
VCCIO2 voltage can also be controlled by GPIO_0A7 and this is the default.
It seems that Linux doesn't do anything for this iodomain, might be a good idea to do the same here.
sys/arm64/rockchip/rk_iodomain.c | ||
---|---|---|
208 | Meh, I've missed this line earlier. Could you use a switch here, it would be easier to read I think. |
sys/arm64/rockchip/rk_iodomain.c | ||
---|---|---|
220 | sc->conf->supply[i].bit - 1 |
Look good to me. I'd love to lose all the goto's as I did I my version, but that's just me :)
I should have explained myself better, you don't NOT want the "bit - 1" anywhere its correct what I do in my original patch.
I guess the confusion is that we actually have the "bit" field with the right number in it, "the other OS" get's the bit from the index hence the - 1 there to make it fir.
OK that's much better, however it fails finding the pmuio1-supply property and I can't seem to figure out why just now, works just fine in my version, oh well, I'll investigate.
OK I figured out why it fails, the current rk_iodomain return 0 (OK) even when the property search fails, that makes the driver bind at that point and the property isn’t ready yet.
It needs to return ENXIO so the probe/attach call is repeated later when things has settled.
I took the freedom of doing the makeover again "my way" :) patch uploaded as D36180
That does the trick.
However it got me thinking that the other "goto end" maybe should be a return ENXIO as well.
Maybe we should even store the register content and restore on failure just to be sure, however "that should not happen" :)
sys/arm64/rockchip/rk_iodomain.c | ||
---|---|---|
184 | As sos@ sais we should probably return ENXIO here and don't write to syscon or call the init func. |