Page MenuHomeFreeBSD

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

Authored by freebsd_sysctl.cz on Mon, Jun 22, 7:49 AM.
Tags
Referenced Files
Unknown Object (File)
Thu, Jul 9, 3:20 PM
Unknown Object (File)
Wed, Jul 8, 6:46 AM
Unknown Object (File)
Tue, Jul 7, 7:43 PM
Unknown Object (File)
Mon, Jul 6, 10:43 AM
Unknown Object (File)
Mon, Jul 6, 12:49 AM
Unknown Object (File)
Mon, Jul 6, 12:10 AM
Unknown Object (File)
Sun, Jul 5, 1:55 AM
Unknown Object (File)
Fri, Jul 3, 5:33 AM
Subscribers

Details

Summary

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

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.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

This looks fairly good, modulo the style issues.

sys/arm64/conf/std.mediatek
9

trailing blank lines.

sys/arm64/mediatek/mdtk_clk.c
3

Talk to be about why you chose this form. I'm not sure what it's trying to say... Applies to all the notices, but I just picked this one to ask

159

Also, just noticed this file seems to be 4 space tab stops. Is it? If so, it should be 8. We try really hard not to do 4 space tabs in new files.
Was this copied from somewhere?

sys/arm64/mediatek/mdtk_clk.h
82

All of the above show up as ragged but the following one is lined up. Are they messed up or a phab bug?

sys/arm64/mediatek/mt7622_clk_audio.c
98

This and others files seem to mix 4 and 8 space tabs. Please use hard 8 space tabs.

sys/arm64/mediatek/mt7622_clk_eth.c
70

Another style issue I've noticed a few times:
This { should be on the next line.

sys/arm64/mediatek/mt_uart.c
107

why shift1?

sys/modules/dtb/mediatek/Makefile
2

I don't think we need this .if?

8

this needs a newline at the end, here and elsewhere.

guest-seuros added inline comments.
sys/arm64/mediatek/mdtk_clk.c
3

this is the new style(9) format.

@ziaee is in charge to review them.

sys/arm64/mediatek/mdtk_clk.h
5–6

Each person on its own line.
Also while this not landing , the year must match current date : 2026.

76

These macros have confusing names.
they are hard to grep and to search .

sys/arm64/mediatek/mt7622_clk_audio.c
219

all files need to be terminated with a empty line.

You can configure git to auto add it.

sys/arm64/mediatek/mt7622_clk_infracfg.c
38

unused ?

Same remarks L:45

sys/arm64/mediatek/mt7622_clk_pciesys.c
3

Contact email is required.
Same remark about year as the other header

sys/arm64/mediatek/mt7622_clk_sgmii.c
34

this comment explains nothing..

if you want to comment it, it should be Serial Gigabit Media Independent Interface , or missing.

I prefer to have it verbose as SGMII is not something common like USB, PCIE...

48

Can we really hit this code ?

sys/arm64/mediatek/mt_uart.c
10

why this comment is placed in the middle of nowhere ?

68

not aligned.

If i touch this file, it will autolint and my diff will have lot of unrelated fixes.

use the style(9) tools.

I fixed the copyright in the source code.

mmel requested changes to this revision.Thu, Jun 25, 10:54 AM
mmel added a subscriber: mmel.

All of these clock drivers share too much common code. Firstly, why don't you subclass these from the base class in mdtk_clk.c? If that's not sufficient, why don't you use a new base class that implements all this glue and subclass it?

sys/arm64/mediatek/mt7622_clk_audio.c
1

This driver must also implement simplebus, it have many child subnodes.

145

This is nonsense.

  1. The node doesn't have a #reset-cells property.
  2. The bindings don't mention a reset function.
  3. The reset register offset (0) is clearly incorrect as it collides with the CLK_AUDIO_AFE register. #

This probably applies to all the other clock blocks too.

sys/arm64/mediatek/mt7622_clk_infracfg.c
37

Commenting on an item in the parent list is not the right choice. How do you want to hold the right parent index in this case?

sys/arm64/mediatek/mt7622_clk_topckgen.c
319–327

This doesn't look like the right implementation of a PLL clock.s

sys/arm64/mediatek/mt_uart.c
1

The MediaTek UART IP has an extended version of the baud rate generator, which is currently being used in the real world (U-Boot setup, for example). I don't see where it is implemented.

22

Unnecessary #includes.

58

Why is this code there? I don't known any interrupt bugs or quirks in this IP.

111

Again, what is the purpose of this code, and why is the code in sys/dev/uart/uart_bus_fdt.c not sufficient? See bellow.

147

By binding, 'bus' clock is not mandatory.

164

Is this not an FDT-based UART? It should use the uart_fdt_<foo> attachment.

This revision now requires changes to proceed.Thu, Jun 25, 10:54 AM

Hi! Can we have a little skeleton manpage in this commit? Here is an example of a minimum implementation that is enormously useful, just 40 lines: https://freshbsd.org/freebsd/src/commit/fd1ee28bd01429aa8c38199d5fc069e8b0b75442

sys/arm64/mediatek/mdtk_clk.c
3

this is so funny because actually @imp started it, then once it was approved formally I started pushing everyone to use it ^.^ part of why i push during reviews is that its very hard to change after the fact.