Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: Jeremy Linton <jeremy.linton@arm.com>, Andrew Lunn <andrew@lunn.ch>
Cc: Calvin Johnson <calvin.johnson@oss.nxp.com>,
	Russell King - ARM Linux admin <linux@armlinux.org.uk>,
	Jon <jon@solid-run.com>,
	Cristi Sovaiala <cristian.sovaiala@nxp.com>,
	Ioana Ciornei <ioana.ciornei@nxp.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Madalin Bucur <madalin.bucur@oss.nxp.com>,
	netdev@vger.kernel.org, linux.cj@gmail.com,
	linux-acpi@vger.kernel.org
Subject: Re: [net-next PATCH v7 1/6] Documentation: ACPI: DSD: Document MDIO PHY
Date: Fri, 24 Jul 2020 10:39:02 -0700
Message-ID: <a95f8e07-176b-7f22-1217-466205fa22e7@gmail.com> (raw)
In-Reply-To: <97973095-5458-8ac2-890c-667f4ea6cd0e@arm.com>

On 7/24/20 10:26 AM, Jeremy Linton wrote:
> Hi,
> 
> On 7/24/20 8:39 AM, Andrew Lunn wrote:
>>> Otherwise the MDIO bus and its phy should be a
>>> child of the nic/mac using it, with standardized behaviors/etc left
>>> up to
>>> the OSPM when it comes to MDIO bus enumeration/etc.
>>
>> Hi Jeremy
>>
>> Could you be a bit more specific here please.
>>
>> DT allows
>>
>>          macb0: ethernet@fffc4000 {
>>                  compatible = "cdns,at32ap7000-macb";
>>                  reg = <0xfffc4000 0x4000>;
>>                  interrupts = <21>;
>>                  phy-mode = "rmii";
>>                  local-mac-address = [3a 0e 03 04 05 06];
>>                  clock-names = "pclk", "hclk", "tx_clk";
>>                  clocks = <&clkc 30>, <&clkc 30>, <&clkc 13>;
>>                  ethernet-phy@1 {
>>                          reg = <0x1>;
>>                          reset-gpios = <&pioE 6 1>;
>>                  };
>>          };
>>
>> So the PHY is a direct child of the MAC. The MDIO bus is not modelled
>> at all. Although this is allowed, it is deprecated, because it results
>> > in problems with advanced systems which have multiple different
>> children, and the need to differentiate them. So drivers are slowly
> 
> I don't think i'm suggesting that, because AFAIK in ACPI you would have
> to specify the DEVICE() for mdio, in order to nest a further set of
> phy's via _ADR(). I think in general what I was describing would look
> more like what you have below. But..
> 
>> migrating to always modelling the MDIO bus. In that case, the
>> phy-handle is always used to point to the PHY:
>>
>>          eth0: ethernet@522d0000 {
>>                  compatible = "socionext,synquacer-netsec";
>>                  reg = <0 0x522d0000 0x0 0x10000>, <0 0x10000000 0x0
>> 0x10000>;
>>                  interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
>>                  clocks = <&clk_netsec>;
>>                  clock-names = "phy_ref_clk";
>>                  phy-mode = "rgmii";
>>                  max-speed = <1000>;
>>                  max-frame-size = <9000>;
>>                  phy-handle = <&phy1>;
>>
>>                  mdio {
>>                          #address-cells = <1>;
>>                          #size-cells = <0>;
>>                          phy1: ethernet-phy@1 {
>>                                  compatible =
>> "ethernet-phy-ieee802.3-c22";
>>                                  reg = <1>;
>>                          };
>>                  };
>>
>> "mdio-handle" is just half of phy-handle.
>>
>> What you seem to be say is that although we have defined a generic
>> solution for ACPI which should work in all cases, it is suggested to
>> not use it? What exactly are you suggesting in its place?
> 
> When you put it that way, what i'm saying sounds crazy.
> 
> In this case what are are doing isn't as clean as what you have
> described above, its more like:
> 
> mdio: {
>   phy1: {}
>   phy2: {}
> }
> ...
> // somewhere else
> dmac1: {
>     phy-handle = <&phy1>;
> }
> 
> ... //somewhere else
> eth0: {
>    //another device talking to the mgmt controller
> }
> 
> 
> Which is special in a couple ways.
> 
> Lets rewind for a moment and say for ARM/ACPI, what we are talking about
> are "edge/server class" devices (with RAS statements/etc) where the
> expectation is that they will be running virtualized workloads using LTS
> distros, or non linux OSes. DT/etc remains an option for networking
> devices which are more "embedded", aren't SBSA, etc. So an Arm
> based/ACPI machine should be more regular and share more in the way of
> system architecture with other SBSA/SBBR/ACPI/etc machines than has been
> the case for DT machines.
> 
> A concern is then how we punch networking devices into an arbitrary VM
> in a standardized way using libvirt/whatever. If the networking device
> doesn't look like a simple self contained memory mapped resource with an
> IOMMU domain, I think everything becomes more complicated and you have
> to start going down the path of special caseing the VM firmware beyond
> how its done for self contained PCIe/SRIOV devices. The latter manage to
> pull this all off with a PCIe id, and a couple BARs fed into the VM.
> 
> So, I would hope an ACPI nic representation is closer to just a minimal
> resource list like:
> 
> eth0: {
>       compatible = "cdns,at32ap7000-macb";
>       reg = <0xfffc4000 0x4000>;
>       interrupts = <21>;
> }
> or in ACPI speak:
> Device (ETH0)
> {
>       Name (_HID, "CDNSXXX")
>       Method (_CRS, 0x0, Serialized)
>       {
>         Name (RBUF, ResourceTemplate ()
>         {
>           Memory32Fixed (ReadWrite, 0xfffc4000, 0x4000, )
>           Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive)
>           {
>             21
>           }
>         })
>         Return (RBUF)
>       }
> }
> 
> (Plus methods for pwr mgmt/etc as needed, the iommu info comes from
> another table).
> 
> Returning to the NXP part. They avoid the entirety of the above
> discussion because all this MDIO/PHY mgmt is just feeding the data into
> the mgmt controller, and the bits that are punched into the VM are
> fairly standalone.
> 
> Anyway, I think this set is generally fine, I would like to see this
> part working well with ACPI given its what we have available today. For
> the future, we also need to continue pushing everyone towards common
> hardware standards. One of the ways of doing this is having hardware
> which can be automatically enumerated/configured. Suggesting that the
> kernel has a recommended way of doing this which aids fragmentation
> isn't what we are trying to achieve with ACPI. Hence my previous comment
> that we should consider this an escape hatch rather than the last word
> in how to describe networking on ACPI/SBSA platforms.

We are at v7 of this patch series, and no authoritative ACPI Linux
maintainer appears to have reviewed this, so there is no clear sign of
this converging anywhere. This is looking a lot like busy work for
nothing. Given that the representation appears to be wildly
misunderstood and no one seems to come up with something that reaches
community agreement, what exactly is the plan here?

I am going to suggest something highly unpopular here: how about you
just load Device Tree overlays based on matching a particular board and
ship those overlays somewhere in the kernel that take care of
registering your network devices with the desired network topology?
-- 
Florian

  reply index

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-15  9:03 [net-next PATCH v7 0/6] ACPI support for dpaa2 MAC driver Calvin Johnson
2020-07-15  9:03 ` [net-next PATCH v7 1/6] Documentation: ACPI: DSD: Document MDIO PHY Calvin Johnson
2020-07-16  3:04   ` Florian Fainelli
2020-07-16  3:11     ` Andrew Lunn
2020-07-23 23:26   ` Jeremy Linton
2020-07-24 13:39     ` Andrew Lunn
2020-07-24 17:26       ` Jeremy Linton
2020-07-24 17:39         ` Florian Fainelli [this message]
2020-07-24 19:20           ` Andrew Lunn
2020-07-24 20:12             ` Andy Shevchenko
2020-07-24 20:13               ` Florian Fainelli
2020-07-24 20:20                 ` Andy Shevchenko
2020-07-25  7:36                   ` Calvin Johnson
2020-07-25 10:48                     ` Andy Shevchenko
2020-07-24 21:06               ` Russell King - ARM Linux admin
2020-07-27 17:03             ` Sudeep Holla
2020-07-24 19:14         ` Andrew Lunn
2020-07-27 17:21           ` Sudeep Holla
2020-07-28 20:34             ` Andrew Lunn
2020-07-28 20:59               ` Russell King - ARM Linux admin
2020-07-28 21:26                 ` Andy Shevchenko
2020-07-29 16:00               ` Rafael J. Wysocki
2020-07-31 15:08                 ` Andrew Lunn
2020-07-27 17:32           ` Jon Nettleton
     [not found]           ` <1595922651-sup-5323@galangal.danc.bne.opengear.com>
2020-07-28 20:45             ` Andrew Lunn
2020-07-28 20:56               ` Florian Fainelli
2020-07-28 21:28                 ` Andy Shevchenko
2020-07-28 21:40                   ` Florian Fainelli
2020-07-31 15:14                   ` Andrew Lunn
2020-09-25 13:22                     ` Grant Likely
2020-07-28 22:30             ` Jeremy Linton
2020-07-29  0:39               ` Florian Fainelli
2020-07-29  2:53                 ` Jeremy Linton
2020-07-29  3:16                   ` Florian Fainelli
2020-07-29  8:43                   ` Jon Nettleton
2020-07-29  9:39                     ` Calvin Johnson
2020-09-25 13:34   ` Grant Likely
2020-09-26  4:30     ` Calvin Johnson
2020-09-29  5:17     ` Calvin Johnson
2020-09-29 13:43       ` Andrew Lunn
2020-09-29 13:55         ` Andy Shevchenko
2020-09-29 14:32           ` Andrew Lunn
2020-09-29 14:46             ` Andy Shevchenko
2020-09-29 15:06               ` Andrew Lunn
2020-09-29 15:29               ` Arnd Bergmann
2020-09-29 14:44         ` Arnd Bergmann
2020-09-29 14:59           ` Andrew Lunn
2020-09-29 15:59             ` Grant Likely
2020-09-29 15:53         ` Grant Likely
2020-09-29 16:04           ` Calvin Johnson
2020-07-15  9:03 ` [net-next PATCH v7 2/6] net: phy: introduce device_mdiobus_register() Calvin Johnson
2020-07-16  3:05   ` Florian Fainelli
2020-07-15  9:03 ` [net-next PATCH v7 3/6] net/fsl: use device_mdiobus_register() Calvin Johnson
2020-07-16  3:05   ` Florian Fainelli
2020-07-15  9:03 ` [net-next PATCH v7 4/6] net: phy: introduce phy_find_by_mdio_handle() Calvin Johnson
2020-07-16  3:06   ` Florian Fainelli
2020-07-15  9:03 ` [net-next PATCH v7 5/6] phylink: introduce phylink_fwnode_phy_connect() Calvin Johnson
2020-07-15  9:04 ` [net-next PATCH v7 6/6] net: dpaa2-mac: Add ACPI support for DPAA2 MAC driver Calvin Johnson
2020-09-25 13:39 ` [net-next PATCH v7 0/6] ACPI support for dpaa2 " Grant Likely
2020-09-26  4:34   ` Calvin Johnson
2020-07-25 14:23 Calvin Johnson
2020-07-25 14:23 ` [net-next PATCH v7 1/6] Documentation: ACPI: DSD: Document MDIO PHY Calvin Johnson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a95f8e07-176b-7f22-1217-466205fa22e7@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=andy.shevchenko@gmail.com \
    --cc=calvin.johnson@oss.nxp.com \
    --cc=cristian.sovaiala@nxp.com \
    --cc=ioana.ciornei@nxp.com \
    --cc=jeremy.linton@arm.com \
    --cc=jon@solid-run.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux.cj@gmail.com \
    --cc=linux@armlinux.org.uk \
    --cc=madalin.bucur@oss.nxp.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git