otherwise OK

spit the diff as requested by maintainer

I had mistakenly uploaded the wrong diff file

Please take this as an incomplete 'first look' review.

Fix context size + dsi_dphy_parents definition.

• New common clock base (mt_clk.c/h, replaces mdtk_clk):** shared probe/attach, a base driver class, hwreset and optional syscon. All mt7622 clock drivers (topckgen, infracfg, pericfg, audio, eth, sgmii, ssusbsys, pciesys) were converted, which removes about 500 lines of duplicated code
• Child nodes:** clock providers with child nodes attach them through simplebus_attach_impl(). A failure there no longer fails the attach, because the clocks are already registered and cannot be unregistered.
• PLL driver (mt_clk_pll.c/h):** apmixedsys PLLs are no longer fixed-rate clocks. The driver reads the firmware-programmed PCW/postdiv and uses the Linux enable/disable sequence. armpll now reports the real 1350 MHz instead of 120 MHz, and main_core_en is enabled as a parent of infra_mux1_sel.
• PLL frequency changes:** mt_clk_pll_set_freq() follows Linux mtk_pll_set_rate_regs() (PCW + postdiv, PCW_CHG latch, read-back check). Only PLLs flagged MT_PLL_SET_FREQ are changed (armpll, aud1pll, aud2pll), so a request passed up through gates and muxes cannot reprogram a system-wide PLL..
• UART (mt_uart.c):** MediaTek high-speed baud modes (HIGHS / SAMPLE_COUNT / SAMPLE_POINT), the console baud rate is read back from the hardware, #ifdef FDT guards and style cleanup

• New common clock base (mt_clk.c/h, replaces mdtk_clk):** shared probe/attach, a base driver class, hwreset and optional syscon. All mt7622 clock drivers (topckgen, infracfg, pericfg, audio, eth, sgmii, ssusbsys, pciesys) were converted, which removes about 500 lines of duplicated code
• Child nodes:** clock providers with child nodes attach them through simplebus_attach_impl(). A failure there no longer fails the attach, because the clocks are already registered and cannot be unregistered.
• PLL driver (mt_clk_pll.c/h):** apmixedsys PLLs are no longer fixed-rate clocks. The driver reads the firmware-programmed PCW/postdiv and uses the Linux enable/disable sequence. armpll now reports the real 1350 MHz instead of 120 MHz, and main_core_en is enabled as a parent of infra_mux1_sel.
• PLL frequency changes:** mt_clk_pll_set_freq() follows Linux mtk_pll_set_rate_regs() (PCW + postdiv, PCW_CHG latch, read-back check). Only PLLs flagged MT_PLL_SET_FREQ are changed (armpll, aud1pll, aud2pll), so a request passed up through gates and muxes cannot reprogram a system-wide PLL..
• UART (mt_uart.c):** MediaTek high-speed baud modes (HIGHS / SAMPLE_COUNT / SAMPLE_POINT), the console baud rate is read back from the hardware, #ifdef FDT guards and style cleanup

Please upload diff with full context.

Please upload your diff with full context next time.

The linux code will likely never be re-licenced which is why we started https://github.com/evadot/drm-subtree/ a while ago.

This is not the correct fix, aw_rsb probably shouldn't attach at BUS_PASS_SUPPORTDEV as it's a BUS where PMIC lives, it should attach at BUS_PASS_INTERRUPT like a10_twsi.
Or maybe just simply move aw_usbphy to BUS_PASS_SUPPORTDEV + BUS_PASS_ORDER_LATE.

In D58812#1379542, @freebsd_sysctl.cz wrote:

• Added simplebus_driver as a base class of all mt_clk based drivers.

Why to all? What about all the other objections?

• Added simplebus_driver as a base class of all mt_clk based drivers.
• Used simplebus_attach_impl(dev, SB_FLAG_NO_RANGES, node) instead of open-coding simplebus_init() and the child enumeration loop.
• Droped the embedded struct simplebus_softc from struct mt_clk_softc

Reworked along the lines @bz asked for, and split into a stack so each piece can
be read on its own. This revision is now only the consumer -- the SIOCGI2C
handler -- and the tunables are gone.

@dsl @adrian — I took D58258#1368055 literally and built MACLINK, then ran it on
an LX2160A (SolidRun CEX7, UEFI/ACPI; dpni0 = dpmac.17 RGMII, dpni1 = dpmac.8 and
dpni2 = dpmac.9, both 10G SFP+ with modules in).

In D58258#1367937, @dsl wrote:

In D58258#1367599, @adrian wrote:

In D58258#1367549, @dsl wrote:

@adrian well, I'm not sure that the contributor actually understands the code. It seems AI/ML generated to me and isn't aligned with the idea of mine about maclink. I'm against the changes.

What do you mean by "maclink" ? They said they used AI assistance in developing it, but it seems mostly simple enough:

• sfp is an i2c device on a bus;
• the i2c bus doesn't HAVE to be hooked up to the MAC in any way; it in theory could be hanging off of some other i2c controller in the system;
• there's information about where said bus is linked to in FDT;
• some simple shenanigans are required to be able to talk to it and fetch configuration parameters.

What doesn't quite jive with your assumptions of stuff?

MACLINK is supposed to be an abstraction which hides possibly complex topology of the devices and their interconnections which constitute a multi-gigabit link between the MAC (which is a part of a SoC usually) and a PHY/SFP+ on a PCB. This is the best explanation I've to date: https://github.com/mcusim/freebsd-src/blob/dpaa2/sys/dev/maclink/maclink.c#L31. All of the existing MACLINK bits live in https://github.com/mcusim/freebsd-src/tree/dpaa2/sys/dev/maclink, but haven't been tested yet.

Personally, I'd like a proper abstraction to be introduced first with a clear understanding how a maclink_bus can be attached by the network interface drivers and introduce new (specific?) maclink adapters which will be incapsulating all of the complex logic to discover PHYs, SFF/SFPs, PCSs, etc. and let the maclink bus (and the attaching network interface) know about high-level events, e.g. state changes, link's up/down, etc.

In D58258#1367599, @adrian wrote:

In D58258#1367549, @dsl wrote:

@adrian well, I'm not sure that the contributor actually understands the code. It seems AI/ML generated to me and isn't aligned with the idea of mine about maclink. I'm against the changes.

What do you mean by "maclink" ? They said they used AI assistance in developing it, but it seems mostly simple enough:

• sfp is an i2c device on a bus;
• the i2c bus doesn't HAVE to be hooked up to the MAC in any way; it in theory could be hanging off of some other i2c controller in the system;
• there's information about where said bus is linked to in FDT;
• some simple shenanigans are required to be able to talk to it and fetch configuration parameters.

What doesn't quite jive with your assumptions of stuff?

Thanks for the accept.

In D58258#1367668, @yarshure_gmail.com wrote:

.. My reading is that you have in mind a MAC-link layer
for dpaa2 along the lines of Linux's phylink: one place that owns link state for a
DPMAC and drives it from whatever is attached - a PHY via MDIO, a fixed-link, or an
SFP cage - so that module presence/LOS, TX_DISABLE and rate selection are handled
there rather than in each consumer. If that is roughly it, please correct the
details and I will work to it.

I believe the ACPI path was never tested on main - or we haven't flipped the defaults yet -- but for sure it'll fail to link modules due to unresolved symbols (in the future).

@dsl - on maclink: I want to make sure I am answering the right objection, because
I cannot find maclink in the tree (only enum dpaa2_mac_link_type in dpaa2_mac.h),
so I am guessing at its shape. My reading is that you have in mind a MAC-link layer
for dpaa2 along the lines of Linux's phylink: one place that owns link state for a
DPMAC and drives it from whatever is attached - a PHY via MDIO, a fixed-link, or an
SFP cage - so that module presence/LOS, TX_DISABLE and rate selection are handled
there rather than in each consumer. If that is roughly it, please correct the
details and I will work to it.

v3 addresses @adrian's two points:

In D58258#1367549, @dsl wrote:

@adrian well, I'm not sure that the contributor actually understands the code. It seems AI/ML generated to me and isn't aligned with the idea of mine about maclink. I'm against the changes.

@adrian well, I'm not sure that the contributor actually understands the code. It seems AI/ML generated to me and isn't aligned with the idea of mine about maclink. I'm against the changes.

this looks fine; please just remove teh BSD copyright text itself as the SPDX + your copyright name/email is enough. Then we should be fine for landing it!