Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933584AbdLRJkf (ORCPT ); Mon, 18 Dec 2017 04:40:35 -0500 Received: from mail-io0-f194.google.com ([209.85.223.194]:42551 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933244AbdLRJkc (ORCPT ); Mon, 18 Dec 2017 04:40:32 -0500 X-Google-Smtp-Source: ACJfBos0KAaZodSMC56uh+jRGV+eNlmSZnXkcNDEVOw02CvAtBGOnGsLWsGdmMMKuKxTjKX3SRWlV5t3yDTzsIXoWT8= MIME-Version: 1.0 In-Reply-To: <1513588684-15647-1-git-send-email-mw@semihalf.com> References: <1513588684-15647-1-git-send-email-mw@semihalf.com> From: Ard Biesheuvel Date: Mon, 18 Dec 2017 10:40:31 +0100 Message-ID: Subject: Re: [net-next: PATCH 0/8] Armada 7k/8k PP2 ACPI support To: Marcin Wojtas , Graeme Gregory , "linux-acpi@vger.kernel.org" Cc: "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "" , "David S. Miller" , Russell King - ARM Linux , "Rafael J. Wysocki" , Andrew Lunn , Florian Fainelli , =?UTF-8?Q?Antoine_T=C3=A9nart?= , Thomas Petazzoni , Gregory CLEMENT , ezequiel.garcia@free-electrons.com, Nadav Haklai , Neta Zur Hershkovits , jaz@semihalf.com, Tomasz Nowicki Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2467 Lines: 55 On 18 December 2017 at 10:17, Marcin Wojtas wrote: > Hi, > > This patchset introduces ACPI support in mvpp2 and mvmdio drivers. > First three patches introduce fwnode helpers for obtaining PHY > information from nodes and also MDIO fwnode API for registering > the bus with its PHY/devices. > > Following patches update code of the mvmdio and mvpp2 drivers > to support ACPI tables handling. The latter is done in 4 steps, > as can be seen in the commits. For the details, please > refer to the commit messages. > > Drivers operation was tested on top of the net-next branch > with both DT and ACPI. Although for compatibility reasons > with older platforms, the mvmdio driver keeps using > of_ MDIO registering, new fwnode_ one proved to fully work > with DT as well (tested on MacchiatoBin board). > > mvpp2/mvmdio driver can work with the ACPI representation, as exposed > on a public branch: > https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/marvell-armada-wip > It was compiled together with the most recent Tianocore EDK2 revision. > Please refer to the firmware build instruction on MacchiatoBin board: > http://wiki.macchiatobin.net/tiki-index.php?page=Build+from+source+-+UEFI+EDK+II > > Above support configures 1G to use its PHY normally. 10G can work now > only with the link interrupt mode. Somehow reading of the > string property in fwnode_mdiobus_child_is_phy works only with > DT and cannot cope with 10G PHY nodes as in: > https://pastebin.com/3JnYpU0A > > Above root cause will be further checked. In the meantime I will > appreciate any comments or remarks for the kernel patches. > Hi Marcin, I have added linux-acpi and Graeme to cc. I think it makes sense to discuss the way you describe the device topology before looking at the patches in more detail. In particular, I would like to request feedback on the use of [redundant] 'reg' properties and the use of _DSD + compatible to describe PHYs. Usually, we try to avoid this, given that it is essentially a ACPI encapsulated DT dialect that is difficult to support in drivers unless they are based on DT to begin with. Also, non-standard _DSD properties require a vendor prefix, it is not freeform. For reference, the ACPI description is here (starting at line 175) https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/blob/72d5ac23b20dd74d479daa5e40ba443264b31261/Platforms/Marvell/Armada/AcpiTables/Armada80x0McBin/Dsdt.asl -- Ard.