śr., 22 cze 2022 o 11:24 Andrew Lunn [off-list ref] napisał(a):

On Wed, Jun 22, 2022 at 11:08:13AM +0200, Marcin Wojtas wrote:

quoted

wt., 21 cze 2022 o 13:42 Andy Shevchenko
[off-list ref] napisał(a):

quoted

On Tue, Jun 21, 2022 at 01:18:38PM +0200, Andrew Lunn wrote:

quoted

On Tue, Jun 21, 2022 at 02:09:14PM +0300, Andy Shevchenko wrote:

quoted

On Mon, Jun 20, 2022 at 09:47:31PM +0200, Andrew Lunn wrote:

...

quoted

quoted

quoted

quoted

+        Name (_CRS, ResourceTemplate ()
+        {
+            Memory32Fixed (ReadWrite,
+                0xf212a200,
+                0x00000010,

What do these magic numbers mean?

Address + Length, it's all described in the ACPI specification.

The address+plus length of what? This device is on an MDIO bus. As
such, there is no memory! It probably makes sense to somebody who
knows ACPI, but to me i have no idea what it means.

I see what you mean. Honestly I dunno what the device this description is for.
For the DSA that's behind MDIO bus? Then it's definitely makes no sense and
MDIOSerialBus() resources type is what would be good to have in ACPI
specification.

It's not device on MDIO bus, but the MDIO controller's register itself

Ah. So this is equivalent to

                CP11X_LABEL(mdio): mdio@12a200 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        compatible = "marvell,orion-mdio";
                        reg = <0x12a200 0x10>;
                        clocks = <&CP11X_LABEL(clk) 1 9>, <&CP11X_LABEL(clk) 1 5>,
                                 <&CP11X_LABEL(clk) 1 6>, <&CP11X_LABEL(clk) 1 18>;
                        status = "disabled";
                };

DT seems a lot more readable, "marvell,orion-mdio" is a good hint that
device this is. But maybe it is more readable because that is what i'm
used to.

No worries, this reaction is not uncommon (including myself), I agree
it becomes more readable, the longer you work with it :).

IMO the ACPI node of orion-mdio looks very similar. Please take a look:

        Device (SMI0)
        {
            Name (_HID, "MRVL0100")              // _HID: Hardware ID
            Name (_UID, 0x00)                          // _UID: Unique ID
            Method (_STA)                                 // _STA: Device status
            {
                Return (0xF)
            }
            Name (_CRS, ResourceTemplate ()
            {
                Memory32Fixed (ReadWrite,
                    0xf212a200,                        // Address Base
                    0x00000010,                       // Address Length
                    )
            })
        }

You can "map" the objects/methods to what you know from DT farly easily:
_HID -> compatible string
_STA -> 'status' property
_CRS & Memory32Fixed  -> 'reg' property (_CRS can also comprise IRQs
and other kind of resources, you can check [1] for more details).

Clocks are configured by firmware, so they are not referenced in the
tables and touched by the orion-mdio driver.

Please could you add a lot more comments. Given that nobody currently
actually does networking via ACPI, we have to assume everybody trying
to use it is a newbie, and more comments are better than less.

I can add more verbose description of the example and probably a
reference to https://www.kernel.org/doc/Documentation/firmware-guide/acpi/dsd/phy.rst
("DSDT entry for MDIO node").

[1] https://uefi.org/specs/ACPI/6.4/06_Device_Configuration/Device_Configuration.html#crs-current-resource-settings

Best regards,
Marcin

Thanks
        Andrew