Page MenuHomeFreeBSD

arm64: NXP add LS1088a clockgen support
ClosedPublic

Authored by bz on Jun 28 2022, 12:08 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, May 10, 4:33 AM
Unknown Object (File)
Mon, Apr 22, 7:39 AM
Unknown Object (File)
Apr 11 2024, 7:52 AM
Unknown Object (File)
Apr 11 2024, 7:52 AM
Unknown Object (File)
Apr 11 2024, 7:52 AM
Unknown Object (File)
Apr 11 2024, 7:50 AM
Unknown Object (File)
Apr 11 2024, 7:50 AM
Unknown Object (File)
Apr 10 2024, 11:54 AM

Details

Summary

Add a driver for NXP LS1088a clockgen support which passes
configuration information to QorIQ clockgen class.
The implementaiton started off from a copy of ls1028 support and was
adjusted accordingly.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 46219
Build 43108: arc lint + arc unit

Event Timeline

bz requested review of this revision.Jun 28 2022, 12:08 AM

I found one minor issue. Even though it's under HWACCEL3 ifdef, I think it should be fixed.

sys/arm64/qoriq/clk/ls1088a_clkgen.c
170–177

According to the doc, Cluster PLL2 divide-by-2 is selected when the value in HWA3CSR is equal to 6, not 2, so ls1088a_cga_pll2_div2 should be placed at ls1088a_hwaccel3_parent_names[6]. I think that ls1088a_cga_pll2_div3 should be the next element in the array. I am not 100% sure though, as it seems that there is an error in the document itself (both divide-by-2 and divide-by-3 are selected by 0110 according to the doc and that's impossible)

bz marked an inline comment as done.

Fix ls1088a_hwaccel3_parent_names using the correct offsets
in the array for pll2_div[23] as pointed out by dgr.

sys/arm64/qoriq/clk/ls1088a_clkgen.c
170–177

I see the same in my version for pll[12]_div2 and pll[12]_div3 for HWA[123]CSR in 4.4.2.4 Fields. And Reserved starting at 111 rather than at 1000.
I'll update this one; good catch!

How is '#ifdef HWACCEL3' compatible with the generic kernel and why do we need it at all?

In D35617#809144, @mmel wrote:

How is '#ifdef HWACCEL3' compatible with the generic kernel and why do we need it at all?

It's simply there to disable the code for now; while it's in the spec, I could not see anything using it. I can spell it `#ifdef __notyet__` if you prefer that.

And why didn't you leave it enabled? IMHO all HWACCEL blocks look the same, so the risk of a possible error is minimal.

Enable HWACCEL3 as well as suggested by @mmel.

This revision is now accepted and ready to land.Jul 1 2022, 2:52 PM
This revision was automatically updated to reflect the committed changes.