Rudimentary RockChip RK356X support
Needs ReviewPublic
Actions

Authored by sos on Aug 2 2022, 7:14 PM.

Details

Reviewers

ganbold
manu

Summary

First batch of changes/new files to support the rk3566/rk3568 SOC.
Kernel config for the Pine64 Quartz64 model A included for minimal build

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

sos created this revision.Aug 2 2022, 7:14 PM

Herald added a reviewer: manu. · View Herald TranscriptAug 2 2022, 7:14 PM

Herald added subscribers: emaste, andrew. · View Herald Transcript

sos requested review of this revision.Aug 2 2022, 7:14 PM

Cool. Can you incorporate following style changes?
It is based on your previous clk related patches, I hope it is mostly the same.

D31299-style.diff67 KBDownload

This revision now requires changes to proceed.Aug 3 2022, 3:21 AM

Style changes as pr ganbold, added USB2 support

Maybe we should remove dev/mmc/host/dwmmc_acpi.c from files.arm64 for now?
We can add later.

This revision now requires changes to proceed.Aug 4 2022, 2:17 AM

Can you incorporate following style changes?

D36027-style.diff292 KBDownload

Also did you test these changes on rk3328, rk3399 boards?

Would you mind splitting this review into multiple one ?

More style updates from Ganbold, misc cosmetic changes.
Support for the 3'rd USB2 port on the DWC3 controller.
Tested not to break rk3399 support (rockpro64)

@manu do you have rk3328 board to test?

s199p.wa1k9r_gmail.com added a subscriber: s199p.wa1k9r_gmail.com.Aug 5 2022, 1:58 PM

I tested these changes on rk3328(Radxa RockPi-E). gpio still working, system led blinking.

ganbold mentioned this in rG5db5bd5f3987: Add drive strength setting for RK3568 pinctrl driver..Aug 8 2022, 4:44 AM

Fix rk809 support, no other changes

sos mentioned this in D36093: Add support for the RK356X SOC ethernet controller (Designware).Aug 9 2022, 6:48 PM

manu added inline comments.Aug 10 2022, 9:41 AM

sys/arm64/rockchip/clk/rk_clk_fract.c
190	My comment from https://reviews.freebsd.org/D31299#717395 still apply here.
sys/arm64/rockchip/rk3568_iodomain.c
72	Why the need for this file ? Why not adding support in rk_iodomain.c ?
sys/arm64/rockchip/rk_dwc3.c
156	Comment is still accurate.
sys/arm64/rockchip/rk_gpio.c
75–79	Please do not move code without reason, makes reviewing harder.
88–92	Please do not change variable names, it makes reviewing much harder.
203	This comment is still true.
sys/arm64/rockchip/rk_pinctrl.c
1180	Please add proper support for rk3568 in rk_pinctrl_parse_drive and not here. Also do not use magic number like 0x01c0 or 0x0070
1206	Unneeded change.
1290	Unneeded change.
sys/arm64/rockchip/rk_tsadc.c
531	You removed a comment and no need to invert the if, makes reviewing harder.
sys/arm64/rockchip/rk_usb2phy.c
73	Set the offset to 0xe450 and remove reading the reg property, this will be more correct.
300	Doesn't seem correct.
sys/dev/iicbus/pmic/fan53555.c
68	Why this change ?
322	What is this magic 12 ?

andrew added inline comments.Aug 10 2022, 10:23 AM

sys/arm64/rockchip/clk/rk3568_cru.c
28	Not needed in files that aren't going to be merged to `stable/12`
1024	This should be named something like `rk3568_cru_probe` (and the same for other functions and global variables).
sys/arm64/rockchip/clk/rk3568_pmucru.c
28	Not needed in files that aren't going to be merged to `stable/12`
285	`rk3568_pmucru_probe`
sys/arm64/rockchip/rk_dwc3.c
156	Didn't it just get moved up?

Is there a reason the rk809/rk817 driver isn't using rk8xx.c? We could teach it about multiple voltage ranges and the register differences & wouldn't need to duplicate the code.

manu added inline comments.Aug 10 2022, 4:55 PM

sys/arm64/rockchip/clk/rk3568_pmucru.c
77	All those below aren't correct, we have proper defines in rk_cru.h A composite clock doesn't have a gate, you're abusing the system here. Same for every clock type that you added where there is a gate. Please have a look at rk3399_cru.c on how to correctly define clocks.