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)
Mon, Apr 22, 7:39 AM
Unknown Object (File)
Thu, Apr 11, 7:52 AM
Unknown Object (File)
Thu, Apr 11, 7:52 AM
Unknown Object (File)
Thu, Apr 11, 7:52 AM
Unknown Object (File)
Thu, Apr 11, 7:50 AM
Unknown Object (File)
Thu, Apr 11, 7:50 AM
Unknown Object (File)
Wed, Apr 10, 11:54 AM
Unknown Object (File)
Sun, Apr 7, 10:43 PM

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
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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.