Messages in this thread | | | From | Marcin Wojtas <> | Date | Wed, 22 Jun 2022 12:22:23 +0200 | Subject | Re: [net-next: PATCH 09/12] Documentation: ACPI: DSD: introduce DSA description |
| |
śr., 22 cze 2022 o 11:24 Andrew Lunn <andrew@lunn.ch> napisał(a): > > On Wed, Jun 22, 2022 at 11:08:13AM +0200, Marcin Wojtas wrote: > > wt., 21 cze 2022 o 13:42 Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> napisał(a): > > > > > > On Tue, Jun 21, 2022 at 01:18:38PM +0200, Andrew Lunn wrote: > > > > On Tue, Jun 21, 2022 at 02:09:14PM +0300, Andy Shevchenko wrote: > > > > > On Mon, Jun 20, 2022 at 09:47:31PM +0200, Andrew Lunn wrote: > > > > > > ... > > > > > > > > > > + 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
| |