| Message ID | 20250115012628.1035928-1-pgwipeout@gmail.com |
|---|---|
| Headers | show |
| Series | rockchip: add a functional usb3 phy driver for rk3328 | expand |
> Wiadomość napisana przez Peter Geis <pgwipeout@gmail.com> w dniu 15 sty 2025, o godz. 02:26: > > > This is my newly reworked phy driver for the rk3328 usb3 phy. It is > based loosely on my original version, but as of now almost nothing of > the original driver remains. The main fix here is the discovery of > BIT(6) in the interrupt enable grf register fixes the usb3 disconnection > detection (mostly). On occasion an unpopulated usb3 hub will take > several seconds to disconnect. However this means all of the hack around > work to reset the usb core manually is no longer required. > > I did my best to document all registers I could find. A lot was taken > from emails between myself and Rockchip's engineers, much thanks to > William Wu <wulf@rock-chips.com> for their assistance here. The rest of > the config bits were taken from the rk3328 and rk3228h TRMs and the > downstream driver. Everything that I couldn't find a definition for is > prefixed UNK_ or UNKNOWN_. There's a lot of obviously used configuration > registers with the pipe interface that are also undocumented. > > The only major bug I have so far is my AX88179 usb3 gigabit ethernet > adapter (Pluggable brand) crashes out when large amounts of data are > transmitted. I suspect this is related to the RX and TX tuning, as > leaving it at defaults makes things worse. As I am not a USB3 engineer > and I do not have the specialized knowledge and hardware to determine > what is going wrong, I am hoping the mailing list will have an answer > here. > > Please test and review. > > Very Respectfully, > Peter Geis > > > > Peter Geis (6): > clk: rockchip: fix wrong clk_ref_usb3otg parent for rk3328 > dt-bindings: phy: rockchip: add rk3328 usb3 phy > phy: rockchip: add driver for rk3328 usb3 phy > arm64: dts: rockchip: add rk3328 usb3 phy node > arm64: dts: rockchip: enable the usb3 phy on rk3328-roc boards > arm64: dts: rockchip: enable the usb3 phy on remaining rk3328 boards > > .../bindings/phy/rockchip,inno-usb3phy.yaml | 166 ++++ > .../boot/dts/rockchip/rk3328-nanopi-r2.dtsi | 12 + > .../dts/rockchip/rk3328-orangepi-r1-plus.dtsi | 12 + > arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi | 12 + > .../boot/dts/rockchip/rk3328-rock-pi-e.dts | 12 + > .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 12 + > arch/arm64/boot/dts/rockchip/rk3328.dtsi | 39 + > drivers/clk/rockchip/clk-rk3328.c | 2 +- > drivers/phy/rockchip/Kconfig | 10 + > drivers/phy/rockchip/Makefile | 1 + > drivers/phy/rockchip/phy-rockchip-inno-usb3.c | 869 ++++++++++++++++++ > 11 files changed, 1146 insertions(+), 1 deletion(-) > create mode 100644 Documentation/devicetree/bindings/phy/rockchip,inno-usb3phy.yaml > create mode 100644 drivers/phy/rockchip/phy-rockchip-inno-usb3.c > > -- > 2.39.5 > > Peter, I applied this as test run on 6.12.9 and dmesg says: [ 16.368514] rockchip-usb3-phy ff460000.usb3-phy: could not get usb3phy ref clock [ 16.368534] rockchip-usb3-phy ff460000.usb3-phy: parse dt failed -2 [ 16.368543] rockchip-usb3-phy ff460000.usb3-phy: probe with driver rockchip-usb3-phy failed with error -2 This is on beelink a1 tvbox. Do I miss something?
On Wed, Jan 15, 2025 at 6:22 AM Piotr Oniszczuk <piotr.oniszczuk@gmail.com> wrote: > > > > > Wiadomość napisana przez Peter Geis <pgwipeout@gmail.com> w dniu 15 sty 2025, o godz. 02:26: > > > > > > This is my newly reworked phy driver for the rk3328 usb3 phy. It is > > based loosely on my original version, but as of now almost nothing of > > the original driver remains. The main fix here is the discovery of > > BIT(6) in the interrupt enable grf register fixes the usb3 disconnection > > detection (mostly). On occasion an unpopulated usb3 hub will take > > several seconds to disconnect. However this means all of the hack around > > work to reset the usb core manually is no longer required. > > > > I did my best to document all registers I could find. A lot was taken > > from emails between myself and Rockchip's engineers, much thanks to > > William Wu <wulf@rock-chips.com> for their assistance here. The rest of > > the config bits were taken from the rk3328 and rk3228h TRMs and the > > downstream driver. Everything that I couldn't find a definition for is > > prefixed UNK_ or UNKNOWN_. There's a lot of obviously used configuration > > registers with the pipe interface that are also undocumented. > > > > The only major bug I have so far is my AX88179 usb3 gigabit ethernet > > adapter (Pluggable brand) crashes out when large amounts of data are > > transmitted. I suspect this is related to the RX and TX tuning, as > > leaving it at defaults makes things worse. As I am not a USB3 engineer > > and I do not have the specialized knowledge and hardware to determine > > what is going wrong, I am hoping the mailing list will have an answer > > here. > > > > Please test and review. > > > > Very Respectfully, > > Peter Geis > > > > > > > > Peter Geis (6): > > clk: rockchip: fix wrong clk_ref_usb3otg parent for rk3328 > > dt-bindings: phy: rockchip: add rk3328 usb3 phy > > phy: rockchip: add driver for rk3328 usb3 phy > > arm64: dts: rockchip: add rk3328 usb3 phy node > > arm64: dts: rockchip: enable the usb3 phy on rk3328-roc boards > > arm64: dts: rockchip: enable the usb3 phy on remaining rk3328 boards > > > > .../bindings/phy/rockchip,inno-usb3phy.yaml | 166 ++++ > > .../boot/dts/rockchip/rk3328-nanopi-r2.dtsi | 12 + > > .../dts/rockchip/rk3328-orangepi-r1-plus.dtsi | 12 + > > arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi | 12 + > > .../boot/dts/rockchip/rk3328-rock-pi-e.dts | 12 + > > .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 12 + > > arch/arm64/boot/dts/rockchip/rk3328.dtsi | 39 + > > drivers/clk/rockchip/clk-rk3328.c | 2 +- > > drivers/phy/rockchip/Kconfig | 10 + > > drivers/phy/rockchip/Makefile | 1 + > > drivers/phy/rockchip/phy-rockchip-inno-usb3.c | 869 ++++++++++++++++++ > > 11 files changed, 1146 insertions(+), 1 deletion(-) > > create mode 100644 Documentation/devicetree/bindings/phy/rockchip,inno-usb3phy.yaml > > create mode 100644 drivers/phy/rockchip/phy-rockchip-inno-usb3.c > > > > -- > > 2.39.5 > > > > > > Peter, > I applied this as test run on 6.12.9 and dmesg says: > > [ 16.368514] rockchip-usb3-phy ff460000.usb3-phy: could not get usb3phy ref clock > [ 16.368534] rockchip-usb3-phy ff460000.usb3-phy: parse dt failed -2 > [ 16.368543] rockchip-usb3-phy ff460000.usb3-phy: probe with driver rockchip-usb3-phy failed with error -2 > > This is on beelink a1 tvbox. > > Do I miss something? > Good Morning, That is an interesting failure. The simplest answer is the `clock-names = "refclk-usb3otg", "usb3phy-otg", "usb3phy-pipe";` line was corrupted. Please check that when applied it matches the patch exactly. If you are still having problems, you can send me the compiled DTB and I'll take a look. Very Respectfully, Peter Geis >
> Wiadomość napisana przez Peter Geis <pgwipeout@gmail.com> w dniu 15 sty 2025, o godz. 13:25: > >> >> >> Do I miss something? >> > > Good Morning, > > That is an interesting failure. The simplest answer is the > `clock-names = "refclk-usb3otg", "usb3phy-otg", "usb3phy-pipe";` line > was corrupted. Please check that when applied it matches the patch > exactly. If you are still having problems, you can send me the > compiled DTB and I'll take a look. > oh - this check i done as first thing to look on :-) pls find: dtsi: https://gist.github.com/warpme/6af9e2778fb06bfb47b64f98fe79d678 and dt: https://gist.github.com/warpme/79340c54c87b2b1e52f2a146461d8c9f and compiled .dtb
On Wed, Jan 15, 2025 at 7:35 AM Piotr Oniszczuk <piotr.oniszczuk@gmail.com> wrote: > > > > > Wiadomość napisana przez Peter Geis <pgwipeout@gmail.com> w dniu 15 sty 2025, o godz. 13:25: > > > >> > >> > >> Do I miss something? > >> > > > > Good Morning, > > > > That is an interesting failure. The simplest answer is the > > `clock-names = "refclk-usb3otg", "usb3phy-otg", "usb3phy-pipe";` line > > was corrupted. Please check that when applied it matches the patch > > exactly. If you are still having problems, you can send me the > > compiled DTB and I'll take a look. > > > > oh - this check i done as first thing to look on :-) > > pls find: > > dtsi: https://gist.github.com/warpme/6af9e2778fb06bfb47b64f98fe79d678 > and dt: https://gist.github.com/warpme/79340c54c87b2b1e52f2a146461d8c9f > > > and compiled .dtb It all looks good. Do you have any CRU errors in the boot log? Can you check for the presence of clk_ref_usb3otg in /sys/kernel/debug/clk/clk_summary? >
> Wiadomość napisana przez Peter Geis <pgwipeout@gmail.com> w dniu 15 sty 2025, o godz. 14:15: > >> >> >> pls find: >> >> dtsi: https://gist.github.com/warpme/6af9e2778fb06bfb47b64f98fe79d678 >> and dt: https://gist.github.com/warpme/79340c54c87b2b1e52f2a146461d8c9f >> >> >> and compiled .dtb > > It all looks good. > Do you have any CRU errors in the boot log? > Can you check for the presence of clk_ref_usb3otg in > /sys/kernel/debug/clk/clk_summary? > >> cru errors in dmesg - seems no any. here is my dmesg: https://gist.github.com/warpme/ecf38482cc88fb68471355d011ecf974 For clk_ref_usb3otg i’m getting following: root@myth-frontend-e67a8de4c815:~ # cat /sys/kernel/debug/clk/clk_summary | grep usb3 clk_usb3otg_suspend 0 0 0 30770 0 0 50000 N deviceless no_connection_id clk_usb3otg_ref 0 0 0 24000000 0 0 50000 N deviceless no_connection_id clk_ref_usb3otg 0 0 0 24000000 0 0 50000 Y deviceless no_connection_id pclk_usb3_grf 0 0 0 75000000 0 0 50000 Y deviceless no_connection_id pclk_usb3phy_pipe 0 0 0 75000000 0 0 50000 N deviceless no_connection_id pclk_usb3phy_otg 0 0 0 75000000 0 0 50000 N deviceless no_connection_id clk_ref_usb3otg_src 0 0 0 37500000 0 0 50000 N deviceless no_connection_id aclk_usb3otg 0 0 0 150000000 0 0 50000 N deviceless no_connection_id
On Wed, Jan 15, 2025 at 8:25 AM Piotr Oniszczuk <piotr.oniszczuk@gmail.com> wrote: > > > > > Wiadomość napisana przez Peter Geis <pgwipeout@gmail.com> w dniu 15 sty 2025, o godz. 14:15: > > > >> > >> > >> pls find: > >> > >> dtsi: https://gist.github.com/warpme/6af9e2778fb06bfb47b64f98fe79d678 > >> and dt: https://gist.github.com/warpme/79340c54c87b2b1e52f2a146461d8c9f > >> > >> > >> and compiled .dtb > > > > It all looks good. > > Do you have any CRU errors in the boot log? > > Can you check for the presence of clk_ref_usb3otg in > > /sys/kernel/debug/clk/clk_summary? > > > >> > > cru errors in dmesg - seems no any. > here is my dmesg: https://gist.github.com/warpme/ecf38482cc88fb68471355d011ecf974 > > For clk_ref_usb3otg i’m getting following: > > root@myth-frontend-e67a8de4c815:~ # cat /sys/kernel/debug/clk/clk_summary | grep usb3 > clk_usb3otg_suspend 0 0 0 30770 0 0 50000 N deviceless no_connection_id > clk_usb3otg_ref 0 0 0 24000000 0 0 50000 N deviceless no_connection_id > clk_ref_usb3otg 0 0 0 24000000 0 0 50000 Y deviceless no_connection_id > pclk_usb3_grf 0 0 0 75000000 0 0 50000 Y deviceless no_connection_id > pclk_usb3phy_pipe 0 0 0 75000000 0 0 50000 N deviceless no_connection_id > pclk_usb3phy_otg 0 0 0 75000000 0 0 50000 N deviceless no_connection_id > clk_ref_usb3otg_src 0 0 0 37500000 0 0 50000 N deviceless no_connection_id > aclk_usb3otg 0 0 0 150000000 0 0 50000 N deviceless no_connection_id > I'm at a loss here, I applied the patches to a clean 6.9 tree and even built it as a module (thank you for the sentinel). I have no issues. I'm wondering if it's your .config or something about your bootloader. >
> Wiadomość napisana przez Peter Geis <pgwipeout@gmail.com> w dniu 16 sty 2025, o godz. 15:02: > >> > > I'm at a loss here, I applied the patches to a clean 6.9 tree and even >> oh maybe issue is in kernel age? 6.9 seems a bit quite old. i’m on 6.12.9….
On Thu, Jan 16, 2025 at 9:35 AM Piotr Oniszczuk <piotr.oniszczuk@gmail.com> wrote: > > > > > Wiadomość napisana przez Peter Geis <pgwipeout@gmail.com> w dniu 16 sty 2025, o godz. 15:02: > > > >> > > > > I'm at a loss here, I applied the patches to a clean 6.9 tree and even > >> > > oh maybe issue is in kernel age? > 6.9 seems a bit quite old. > i’m on 6.12.9…. The patches were prepared and tested against 6.13-rc1, but nothing has changed significantly in the kernel in regards to rk3328 clock handling in several years. I jumped back to 6.9 due to dyslexia.
On 15/01/2025 02:26, Peter Geis wrote: > > This is my newly reworked phy driver for the rk3328 usb3 phy. It is > based loosely on my original version, but as of now almost nothing of > the original driver remains. The main fix here is the discovery of > BIT(6) in the interrupt enable grf register fixes the usb3 disconnection > detection (mostly). On occasion an unpopulated usb3 hub will take > several seconds to disconnect. However this means all of the hack around > work to reset the usb core manually is no longer required. > BTW, RFC for some maintainers means "do not review, work-in-progress". For some means "review, but low priority" or "review, but for sure I have bugs here". I usually review and then someone responds: "it is not for review, it is just RFC", so to avoid my wasted time please always mention in cover letter why this is RFC. What do you expect here or why this is not ready for review as normal patch. Best regards, Krzysztof
On Sat, Jan 18, 2025 at 4:08 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 15/01/2025 02:26, Peter Geis wrote: > > > > This is my newly reworked phy driver for the rk3328 usb3 phy. It is > > based loosely on my original version, but as of now almost nothing of > > the original driver remains. The main fix here is the discovery of > > BIT(6) in the interrupt enable grf register fixes the usb3 disconnection > > detection (mostly). On occasion an unpopulated usb3 hub will take > > several seconds to disconnect. However this means all of the hack around > > work to reset the usb core manually is no longer required. > > > BTW, RFC for some maintainers means "do not review, work-in-progress". > For some means "review, but low priority" or "review, but for sure I > have bugs here". I usually review and then someone responds: "it is not > for review, it is just RFC", so to avoid my wasted time please always > mention in cover letter why this is RFC. What do you expect here or why > this is not ready for review as normal patch. Thank you, that makes sense. I marked it as RFC as I'm aware it isn't a perfect solution and there's a lot of undefined values. What I was looking for here was: - Review for code quality, so if I'm completely off track I can get to work fixing it. (Thank you so far) - Sanity testing from others struggling with the issues it fixes. - Feedback from USB engineers about the issues remaining. - Hopefully someone with access to the IP can provide insight into the magic registers. > > Best regards, > Krzysztof