Page MenuHomeFreeBSD

Add Rockchip RK3399 eMMC PHY driver
ClosedPublic

Authored by ganbold on Jul 3 2019, 6:27 AM.

Details

Summary

Add driver for Rockchip RK3399 eMMC PHY.
Tested on NanoPC-T4 board.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ganbold created this revision.Jul 3 2019, 6:27 AM
ganbold updated this revision to Diff 59339.Jul 3 2019, 11:04 AM

Fix register definitions

ganbold updated this revision to Diff 59801.Jul 16 2019, 12:12 PM

Updated, once clock is enabled in sdhci_fdt, rk_emmcphy_init(void) can be called again to get the clock, etc.

manu added inline comments.Jul 16 2019, 12:44 PM
sys/arm64/rockchip/rk3399_emmcphy.c
214 ↗(On Diff #59801)

This should be the phy_init method, then sdhci can just call phy_init(sc->phy);

ganbold updated this revision to Diff 59805.Jul 16 2019, 2:31 PM

Updated accordingly.

ganbold updated this revision to Diff 59807.Jul 16 2019, 2:41 PM

Updated.

ganbold updated this revision to Diff 59813.Jul 16 2019, 3:53 PM

Disable phy when enable var is false.

ganbold updated this revision to Diff 59814.Jul 16 2019, 3:55 PM

Updated.

manu added inline comments.Jul 17 2019, 8:39 AM
sys/arm64/rockchip/rk3399_emmcphy.c
119 ↗(On Diff #59814)

This doesn't need to be a global variable now.

143 ↗(On Diff #59814)

You should validate that the phy id is correct by using phynode_get_id

145 ↗(On Diff #59814)

This is not correct, this function is suppose to disable the phy by writing to the registers.
This will just call the same function by doing that.

235 ↗(On Diff #59814)

Can you do a smaller delay value and use a loop for the timeout ?
Is there any info in the user manual on how long the DLL init is ?

ganbold updated this revision to Diff 59862.Jul 18 2019, 3:17 AM

Global variable is removed.
Phy id is validated after getting id by phynode_get_id().
When enable var is false it will disable the phy.
Looked both NetBSD and linux sources for delay value:

https://github.com/NetBSD/src/blob/trunk/sys/arch/arm/rockchip/rk_emmcphy.c#L207
https://elixir.bootlin.com/linux/v5.2-rc7/source/drivers/phy/rockchip/phy-rockchip-emmc.c#L204

Rockchip RK3399 TRM V1.3 Part2.pdf says in page 698:

After the DLL control loop reaches steady state a DLL ready signal is generated by the DLL circuits ‘phyctrl_dllrdy’. The time from ‘phyctrl_endll’ to DLL ready signal ‘phyctrl_dllrdy’ varies with the clock frequency. At 200MHz clock frequency the DLL ready delay is 2.56us, at 100MHz clock frequency the DLL ready delay is 5.112us and at 50 MHz clock frequency the DLL ready delay is 10.231us.

ganbold updated this revision to Diff 59868.Jul 18 2019, 6:26 AM

Correct the class name when creating phynode, otherwise rk_emmcphy_enable() is never called.

manu added inline comments.Jul 18 2019, 8:37 AM
sys/arm64/rockchip/rk3399_emmcphy.c
235 ↗(On Diff #59814)

Why not waiting the correct amount of time based on the frequency ?

ganbold updated this revision to Diff 59872.Jul 18 2019, 9:47 AM

Tried with smaller wait values per frequency respectively.
However it is not working. So added more comments in the code
describing why more safer value would work in any case.

manu added a comment.Jul 18 2019, 9:52 AM

Tried with smaller wait values per frequency respectively.
However it is not working. So added more comments in the code
describing why more safer value would work in any case.

Ok, fine with me, we can change that later if we need to,

manu accepted this revision.Jul 19 2019, 5:59 PM
This revision is now accepted and ready to land.Jul 19 2019, 5:59 PM
This revision was automatically updated to reflect the committed changes.