Re: [net-next: PATCH 2/3] net: mvpp2: enable using phylink with ACPI

From: Marcin Wojtas <mw@semihalf.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	netdev <netdev@vger.kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Russell King - ARM Linux <linux@armlinux.org.uk>,
	Grzegorz Jaszczyk <jaz@semihalf.com>,
	Grzegorz Bernacki <gjb@semihalf.com>,
	upstream@semihalf.com,
	Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>,
	Jon Nettleton <jon@solid-run.com>, Jon Masters <jcm@redhat.com>,
	rjw@rjwysocki.net, lenb@kernel.org
Subject: Re: [net-next: PATCH 2/3] net: mvpp2: enable using phylink with ACPI
Date: Mon, 14 Jun 2021 01:46:06 +0200	[thread overview]
Message-ID: <CAPv3WKetRLOkkOz3Cj_D5pf824VGoz+sQ6wNukTS2PKoAcdFyw@mail.gmail.com> (raw)
In-Reply-To: <YMZ6E99Q/zuFh4b1@lunn.ch>

<Adding ACPI Maintainers>

Hi Andrew,

niedz., 13 cze 2021 o 23:35 Andrew Lunn <andrew@lunn.ch> napisał(a):
>
> > True. I picked the port type properties that are interpreted by
> > phylink. Basically, I think that everything that's described in:
> > devicetree/bindings/net/ethernet-controller.yaml
> > is valid for the ACPI as well
>
> So you are saying ACPI is just DT stuff into tables? Then why bother
> with ACPI? Just use DT.

Any user is free to use whatever they like, however apparently there
must have been valid reasons, why ARM is choosing ACPI as the
preferred way of describing the hardware over DT. In such
circumstances, we all work to improve adoption and its usability for
existing devices.

Regarding the properties in _DSD package, please refer to
https://www.kernel.org/doc/html/latest/firmware-guide/acpi/DSD-properties-rules.html,
especially to two fragments:
"The _DSD (Device Specific Data) configuration object, introduced in
ACPI 5.1, allows any type of device configuration data to be provided
via the ACPI namespace. In principle, the format of the data may be
arbitrary [...]"
"It often is useful to make _DSD return property sets that follow
Device Tree bindings."
Therefore what I understand is that (within some constraints) simple
reusing existing sets of nodes' properties, should not violate ACPI
spec. In this patchset no new extension/interfaces/method is
introduced.

>
> Right, O.K. Please document anything which phylink already supports:
>
> hylink.c:               ret = fwnode_property_read_u32(fixed_node, "speed", &speed);
> phylink.c:              if (fwnode_property_read_bool(fixed_node, "full-duplex"))
> phylink.c:              if (fwnode_property_read_bool(fixed_node, "pause"))
> phylink.c:              if (fwnode_property_read_bool(fixed_node, "asym-pause"))
> phylink.c:              ret = fwnode_property_read_u32_array(fwnode, "fixed-link",
> phylink.c:              ret = fwnode_property_read_u32_array(fwnode, "fixed-link",
> phylink.c:      if (dn || fwnode_property_present(fwnode, "fixed-link"))
> phylink.c:      if ((fwnode_property_read_string(fwnode, "managed", &managed) == 0 &&
>
> If you are adding new properties, please do that In a separate patch,
> which needs an ACPI maintainer to ACK it before it gets merged.
>

Ok, I can extend the documentation.

Best regards,
Marcin