arm64: mediatek: add initial MT7622/Banana Pi R64 support (revised)

[freebsd_sysctl.cz]

Summary

arm64: mediatek: add initial MT7622/Banana Pi R64 support

I talked with Adrian about this code.
I tried to correct the style and copyright details

Add initial FreeBSD/arm64 support for MediaTek MT7622-based Banana Pi
boards, primarily the Banana Pi R64.

UART support is functional and has been tested. There are still some parts that need to be cleaned up or completed, but since this target is mainly used on router boards, the current state is sufficient for now.

This change introduces the basic MediaTek platform configuration, adds a
MediaTek-specific kernel configuration, and wires the platform into the
arm64 GENERIC build. It also adds common MediaTek clock helper code and
MT7622 clock-controller support for topckgen, infracfg, audsys, ethsys,
sgmiisys, ssusbsys, pciesys, and pericfg.

The change also adds initial MT7622 pinctrl support and the required build
glue for MediaTek DTBs and platform drivers.

This is intended as the first step toward a minimally bootable MediaTek
MT7622 platform on FreeBSD. UART console bring-up has been tested on
Banana Pi R64/R3, and additional peripheral support can follow in later
reviews.

[bnovkov]

sys/arm64/mediatek/mt_clk.c

45

I don't think that this should be a panic, the error should be propagated upward and cause the clock driver to fail to attach instead.

This goes for the other panics you've added in the clock driver as well.

[freebsd_sysctl.cz]

I removed panic() functions

[freebsd_sysctl.cz]

Fix the review and remove foreign code

[adrian]

This in general looks fine. I'll go try building it in a day or two. thanks!

[mmel]

This code is fundamentally incomplete and unusable in its current state.

• All PLL clocks are crudely faked with fixed clock events instead of being dynamic, without any check.
• The UART silently accepts and programs invalid baud rates without any error reporting.
  • pinctrl implements barely ~10% of the properties defined in the bindings, rendering it effectively useless. -Several clock driver nodes (simplebus being a clear example) lack required functionality and must be rewritten from scratch.

This code is nowhere near ready and demands substantial rework. I'm sorry, but the quality of this AI slope is significantly below my acceptance limit.

[adrian]

In D58812#1350834, @mmel wrote:

This code is fundamentally incomplete and unusable in its current state.

• All PLL clocks are crudely faked with fixed clock events instead of being dynamic, without any check.
• The UART silently accepts and programs invalid baud rates without any error reporting.
  • pinctrl implements barely ~10% of the properties defined in the bindings, rendering it effectively useless. -Several clock driver nodes (simplebus being a clear example) lack required functionality and must be rewritten from scratch.

This code is nowhere near ready and demands substantial rework. I'm sorry, but the quality of this AI slope is significantly below my acceptance limit.

It doesn't look like AI to me. It looks like an MVP for bringing enough hardware up to boot.

Sure, there's a lot more code to write on top of this, but I think it's fine as a starting point to demonstrate things boot and encourage others to contribute towards the platform support.

I'd appreciate some feedback about what you mean by "simplebus being a clear example" here so I can help him figure out what to do.

[mmel]

Sorry, I was too brief. In the past, Martin gave me many versions of the clock code for pre-review, so I have a tendency to respond briefly.
The issue is the audsys/audiosys driver (at least, I didn't check the other clocks). It is not a leaf node, but it has subnodes, so it must implement the simplebus class together with the MT_CLK class. This is impossible in the current situation because both classes have their own softc.
The audsys driver version in review only derives simplebus, which means it cannot work at all. It doesn't have a method for physical access to clock related registers or locking functions (CLKDEV interface).

[adrian]

In D58812#1350916, @mmel wrote:

Sorry, I was too brief. In the past, Martin gave me many versions of the clock code for pre-review, so I have a tendency to respond briefly.
The issue is the audsys/audiosys driver (at least, I didn't check the other clocks). It is not a leaf node, but it has subnodes, so it must implement the simplebus class together with the MT_CLK class. This is impossible in the current situation because both classes have their own softc.
The audsys driver version in review only derives simplebus, which means it cannot work at all. It doesn't have a method for physical access to clock related registers or locking functions (CLKDEV interface).

Ok, when you get some time, would you mind sitting down with me and going through what changes you'd like to see? Give me some examples?
I can help Martin through them, I just want to better understand myself what we need to do here.

Thanks!

[adrian]

In D58812#1350916, @mmel wrote:

Sorry, I was too brief. In the past, Martin gave me many versions of the clock code for pre-review, so I have a tendency to respond briefly.
The issue is the audsys/audiosys driver (at least, I didn't check the other clocks). It is not a leaf node, but it has subnodes, so it must implement the simplebus class together with the MT_CLK class. This is impossible in the current situation because both classes have their own softc.
The audsys driver version in review only derives simplebus, which means it cannot work at all. It doesn't have a method for physical access to clock related registers or locking functions (CLKDEV interface).

eg I just saw https://reviews.freebsd.org/D57176 landed . Is that layout of the clock driver what you're after? (ie inheriting from clock, not simplebus; implement the locking and clock register methods.)

[mmel]

Not exactly. The D57176 has nothing to do with simplebus; all of its clock nodes in the DT are leaf nodes.

The situation is different for Mediatek. Some (at least audsys/audiosys) clock nodes in the DT are not leaf nodes, but they also work as bus nodes (so they have subnodes). This means that the given driver must subclass two classes: mt_clk_driver and simplebus. We only support this scenario if all two classes (the given driver, mt_clk_driver and simplebus) share a softc structure. This is not the case here, so the proposed code cannot work.

see:

	audsys: clock-controller@11220000 {
		compatible = "mediatek,mt7622-audsys", "syscon";
		reg = <0 0x11220000 0 0x2000>;
		#clock-cells = <1>;

		afe: audio-controller {
			compatible = "mediatek,mt7622-audio";
			....
		};
	};

versus standard

	ssusbsys: clock-controller@1a000000 {
		compatible = "mediatek,mt7622-ssusbsys";
		reg = <0 0x1a000000 0 0x1000>;
		#clock-cells = <1>;
		#reset-cells = <1>;
	};

Ideally, DEFINE_CLASS_2/3/4() would be expanded to obtain the sizes of all softc structures and a new 'device_get_softc_class(dev, class)' function would be implemented to return the appropriate softc for a given class, for classes/drivers with multiple inheritance.

However, I have not yet thoroughly investigated (in details) whether this is possible.

Today I have busy day, but I will call you tomorrow (CEST) as early as possible.