Page MenuHomeFreeBSD

arm64: rockchip, implement the two rk808 clocks
ClosedPublic

Authored by bz on Oct 19 2020, 8:55 PM.

Details

Summary

While the xin32k clk was implemented in rk3399_cru as a fixed rate
clock, migrate it to rk805 as we will also need the 2nd clock
'rtc_clko_wifi' for SDIO and BT.
Both clocks remain fixed rate, and while the 1st one is always on
(though that is not expressed in the clk framework), the 2nd one
we can toggle on/off.

Diff Detail

Repository
rG 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

bz requested review of this revision.Oct 19 2020, 8:55 PM
sys/arm64/rockchip/rk805.c
642

You should be able to subclass the first clock def no ?

665

No need for this comment.

668

RK808 datasheet name this clock clk32kout1

668

clock-output-names could have only one clock, we should always use the name if provided.

680

RK808 datasheet name this clock clk32kout2

865

Looking at the rk805 datasheet it seems that it have the same clocks, we should always export them unconditionally.

868

We should fail here, if the xin32k clock isn't exported not a lot of thing would work (and we would panic later when we finish the clock domains I think anyway).
Not attaching would be clearer of the problem IMHO.

sys/arm64/rockchip/rk805.c
668

Can we always rely on the DTS to get this one right. Is we don't have an "xin32k" more things get cranky. That's the reason I did in the end not name it clk32kout1. We never did in rk3399_cru.c either; so there is some inconsistency somewhere?

sys/arm64/rockchip/rk805.c
668

Yes we should rely on it.
We didn't have it in rk3399_cru either as we needed to have the xin32k name used.
Note that it doesn't not really matter, there is not real situation where the rk805/rk808 node wouldn't export the clock as xin32k.

Do you plan to continue working on this ?

In D26870#674735, @manu wrote:

Do you plan to continue working on this ?

We should. I am happy to update it the next days or if you want let me know and feel free to and just update this review.

bz marked 8 inline comments as done.

Address manu's comments.

The clocks are the same for rk805 so please prefix all function rk805 instead of rk808.
Otherwise looks good.

sys/arm64/rockchip/rk805.c
605

No need for the (void) cast, not sure it's in style(9) too.

610

Same

665

s/clknode 0/clkidef.name/

677

s/clknode 1/clkidef.name/

bz marked 4 inline comments as done.

Replace the rk808 by rk805 for anything related to clocks.
Remove (void) to document that we are ignoring function resulats.
Use the decided clock name in printf.

Suggested-by: mnau

This revision is now accepted and ready to land.May 6 2021, 3:29 PM
In D26870#679430, @manu wrote:

This is now tested, the 32khz clock is correctly output on the pin when enabling the clock.

In D26870#680223, @manu wrote:
In D26870#679430, @manu wrote:

This is now tested, the 32khz clock is correctly output on the pin when enabling the clock.

Thanks for updating and testing! I am currently trying to get my rk3399s stable again as recent U-Boot + FreeBSD make them entirely unhappy. I believe it's LPDDR3 related as only nanopi4 derived ones (and a chromebook seem to use this kind of memory; and they updated the chips between 2018 and 2019 designs). I found vendor pre-compiled Android to be stable while on FreeBSD I get a lot of data corruption.

I'll update the change later today and then commit; or you could just go ahead and commit given you have the final version.

Address manu's compile comments for code changed in the last 6 months or so.

This revision now requires review to proceed.May 17 2021, 8:53 PM
This revision is now accepted and ready to land.May 18 2021, 6:25 AM