Message ID | 20230120224011.796097-1-michael@walle.cc |
---|---|
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp462799wrn; Fri, 20 Jan 2023 14:41:13 -0800 (PST) X-Google-Smtp-Source: AMrXdXvubDwijvU125TjyINHV3IzSM3/FRhU1PCoqu+OoGja9lq8XRkjugAkwmYYS3d/qsfjQsZi X-Received: by 2002:a05:6402:12d8:b0:49e:eaac:e783 with SMTP id k24-20020a05640212d800b0049eeaace783mr2975663edx.3.1674254473162; Fri, 20 Jan 2023 14:41:13 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1674254473; cv=none; d=google.com; s=arc-20160816; b=VnBMw3onBWeCqzExTj0WMc7yi0tOowwcxTd3JK42Ilyu3w4mz2Ze+B1ahDAD4Q5j1g 7aBWANY44n7PVuJeSpy9bj1Pk9YTYQF4Q6b8dRHzT9C8jkcNmcp5IGwUtlBhDVONxJOo S0zS1wV4Aa8F4xKETM7Q/gGryeQtNsggI1wtw0wpnpCT0IUgQR/QO51j09n5VY1SOi7s rVWVcWUlKr36rlZ2gUaDr0r181dmTdDsESo4iQg2sqF8DdsBIr8CRO4dfGvUzJsxYI1B pA2MLcNNrEOmpN+/DIqy4ZJMdkP9FD3bCWTthrk9rl2zESD68WRozp1E8vRzcG+4Ludg QE2Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=L5ySbynlRd03+mC00y1MsXYQTBq0njvvGbWSbHrwuRo=; b=qqOpl1rL7JJXnCbqJ90AXp67nIQZzhQwk5obrLlNFrr9Dt+aZvZ0+McdjpEYQ5X8jI D58cXDpbDZ7zR0Vv2rr9XV2kYbEPnRQ7KhSGbEb3Jx9gPhl6X9v0lZeDdc1N1LCmf/DV zTxp+yKpoVFjeLsaFTZE0lSTlfATU17hmSKS2zm/pgW0hS3sxeff1SL5jz5Jyjy9WJSE wVXEWAabBlEbNscGXrc6j1bugwajlPpbCVYwJMRsY9v7e8RnS6vEI0ZfDNDBwMYFLFf7 T/WBktMFSEzvOrA1c34qN+1MQOqVfE3uGY36j+VlbxINi1bK11b+urEb8fyCNXqIwEI/ NjyQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@walle.cc header.s=mail2022082101 header.b=VVJo3ZVl; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=walle.cc Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id b17-20020a056402279100b0049e369ed236si14751061ede.260.2023.01.20.14.40.49; Fri, 20 Jan 2023 14:41:13 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@walle.cc header.s=mail2022082101 header.b=VVJo3ZVl; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=walle.cc Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229678AbjATWkZ (ORCPT <rfc822;forouhar.linux@gmail.com> + 99 others); Fri, 20 Jan 2023 17:40:25 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47536 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229591AbjATWkX (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 20 Jan 2023 17:40:23 -0500 Received: from mail.3ffe.de (0001.3ffe.de [IPv6:2a01:4f8:c0c:9d57::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DCD1D7F997; Fri, 20 Jan 2023 14:40:21 -0800 (PST) Received: from mwalle01.kontron.local. (unknown [213.135.10.150]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.3ffe.de (Postfix) with ESMTPSA id A704911FB; Fri, 20 Jan 2023 23:40:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=walle.cc; s=mail2022082101; t=1674254419; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=L5ySbynlRd03+mC00y1MsXYQTBq0njvvGbWSbHrwuRo=; b=VVJo3ZVlk6cFNWqbF7TwzaJ47eFqfK7oyM7NJ6UvXE3gsmhXs7dvsvW7iX0qBVZcU/DzVi Cx6iyZiRT2oLujXfX3qd4AFkRXsmNN9AU5m7Nw9NLoU9AHenfQB4AeTGXp4SQGtnpuw8wq 8Ngc2uZAyGegjvD8IjeU+mlB3AhAz8XTZpmFkQV2fQXzLwnwpaFPkw2TNisIqDC4Y7IdIg uQh6iX+nFEOLYPanu1jeTO14SpIGtevHgGlPHj5hfejIrBoncAtUyR5NCqFB5N2O6sklHw MjBcPRR3ulFH8Yeoh+eP2hbOUSPGuC7Ntg3NN3zwsW8n3HxxKgv7yXrbzmJlbA== From: Michael Walle <michael@walle.cc> To: Yisen Zhuang <yisen.zhuang@huawei.com>, Salil Mehta <salil.mehta@huawei.com>, "David S . Miller" <davem@davemloft.net>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Florian Fainelli <f.fainelli@gmail.com>, Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>, Andrew Lunn <andrew@lunn.ch>, Heiner Kallweit <hkallweit1@gmail.com>, Russell King <linux@armlinux.org.uk>, =?utf-8?q?Marek_Beh=C3=BAn?= <kabel@kernel.org>, Xu Liang <lxu@maxlinear.com> Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Michael Walle <michael@walle.cc> Subject: [PATCH net-next 0/5] net: phy: C45-over-C22 access Date: Fri, 20 Jan 2023 23:40:06 +0100 Message-Id: <20230120224011.796097-1-michael@walle.cc> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam: Yes X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1755583058420905286?= X-GMAIL-MSGID: =?utf-8?q?1755583058420905286?= |
Series |
net: phy: C45-over-C22 access
|
|
Message
Michael Walle
Jan. 20, 2023, 10:40 p.m. UTC
After the c22 and c45 access split is finally merged. This can now be posted again. The old version can be found here: https://lore.kernel.org/netdev/20220325213518.2668832-1-michael@walle.cc/ Although all the discussion was here: https://lore.kernel.org/netdev/20220323183419.2278676-1-michael@walle.cc/ The goal here is to get the GYP215 and LAN8814 running on the Microchip LAN9668 SoC. The LAN9668 suppports one external bus and unfortunately, the LAN8814 has a bug which makes it impossible to use C45 on that bus. Fortunately, it was the intention of the GPY215 driver to be used on a C22 bus. But I think this could have never really worked, because the phy_get_c45_ids() will always do c45 accesses and thus gpy_probe() will fail. Introduce C45-over-C22 support and use it if the MDIO bus doesn't support C45. Also enable it when a PHY is promoted from C22 to C45. Changes since RFC v2: - Reased to latest net-next - new check_rc argument in mmd_phy_indirect() to retain old behavior - determine bus capabilities by bus->read and bus->read_c45 - always set phydev->c45_over_c22 if PHY is promoted Changes since RFC v1: - use __phy_mmd_indirect() in mdiobus_probe_mmd_read() - add new properties has_c45 c45_over_c22 (and remove is_c45) - drop MDIOBUS_NO_CAP handling, Andrew is preparing a series to add probe_capabilities to mark all C45 capable MDIO bus drivers Michael Walle (5): net: phy: add error checks in mmd_phy_indirect() and export it net: phy: support indirect c45 access in get_phy_c45_ids() net: phy: add support for C45-over-C22 transfers phy: net: introduce phy_promote_to_c45() net: phy: mxl-gpy: remove unneeded ops .../net/ethernet/hisilicon/hns/hns_ethtool.c | 4 +- drivers/net/phy/bcm84881.c | 2 +- drivers/net/phy/marvell10g.c | 2 +- drivers/net/phy/mxl-gpy.c | 33 +------- drivers/net/phy/phy-core.c | 48 ++++++++--- drivers/net/phy/phy.c | 6 +- drivers/net/phy/phy_device.c | 80 ++++++++++++++++--- drivers/net/phy/phylink.c | 8 +- include/linux/phy.h | 12 ++- 9 files changed, 128 insertions(+), 67 deletions(-)
Comments
On Fri, Jan 20, 2023 at 11:40:06PM +0100, Michael Walle wrote: > After the c22 and c45 access split is finally merged. This can now be > posted again. The old version can be found here: > https://lore.kernel.org/netdev/20220325213518.2668832-1-michael@walle.cc/ > Although all the discussion was here: > https://lore.kernel.org/netdev/20220323183419.2278676-1-michael@walle.cc/ > > The goal here is to get the GYP215 and LAN8814 running on the Microchip > LAN9668 SoC. The LAN9668 suppports one external bus and unfortunately, the > LAN8814 has a bug which makes it impossible to use C45 on that bus. > Fortunately, it was the intention of the GPY215 driver to be used on a C22 > bus. But I think this could have never really worked, because the > phy_get_c45_ids() will always do c45 accesses and thus gpy_probe() will > fail. > > Introduce C45-over-C22 support and use it if the MDIO bus doesn't support > C45. Also enable it when a PHY is promoted from C22 to C45. I see this breaking up into two problems. 1) Scanning the bus and finding device, be it by C22, C45, or C45 over C22. 2) Allowing drivers to access C45 register spaces, without caring if it is C45 transfers or C45 over C22. For scanning the bus we currently have: if (bus->read) { err = mdiobus_scan_bus_c22(bus); if (err) goto error; } prevent_c45_scan = mdiobus_prevent_c45_scan(bus); if (!prevent_c45_scan && bus->read_c45) { err = mdiobus_scan_bus_c45(bus); if (err) goto error; } I think we should be adding something like: else { if (bus->read) { err = mdiobus_scan_bus_c45_over_c22(bus); if (err) goto error; } } That makes the top level pretty obvious what is going on. But i think we need some more cleanup lower down. We now have a clean separation in MDIO bus drivers between C22 bus transactions and C45 transactions bus. But further up it is less clear. PHY drivers should be using phy_read_mmd()/phy_write_mmd() etc, which means access the C45 address space, but says nothing about what bus transactions to use. So that is also quite clean. The problem is in the middle. get_phy_c45_devs_in_pkg() uses mdiobus_c45_read(). Does mdiobus_c45_read() mean perform a C45 bus transaction, or access the C45 address space? I would say it means perform a C45 bus transaction. It does not take a phydev, so we are below the concept of PHYs, and so C45 over C22 does not exist at this level. So i think we need to review all calls to mdiobus_c45_read/mdiobus_c45_write() etc and see if they mean C45 bus transaction or C45 address space. Those meaning address space should be changed to phy_read_mmd()/phy_write_mmd(). get_phy_device(), get_phy_c45_devs_in_pkg(), get_phy_c45_ids(), phy_c45_probe_present() however do not deal with phydev, so cannot use phy_read_mmd()/phy_write_mmd(). They probably need the bool is_c45 replaced with an enum indicating what sort of bus transaction should be performed. Depending on that value, they can call mdiobus_c45_read() or mmd_phy_indirect() and __mdiobus_read(). I don't have time at the moment, but i would like to dig more into phydev->is_c45. has_c45 makes sense to indicate it has c45 address space. But we need to see if it is every used to indicate to use c45 transactions. But it is clear we need a new member to indicate if C45 or C45 over C22 should be performed, and this should be set by how the PHY was found in the first place. Andrew
On Mon, Jan 23, 2023 at 07:03:18PM +0100, Andrew Lunn wrote: > On Fri, Jan 20, 2023 at 11:40:06PM +0100, Michael Walle wrote: > > After the c22 and c45 access split is finally merged. This can now be > > posted again. The old version can be found here: > > https://lore.kernel.org/netdev/20220325213518.2668832-1-michael@walle.cc/ > > Although all the discussion was here: > > https://lore.kernel.org/netdev/20220323183419.2278676-1-michael@walle.cc/ > > > > The goal here is to get the GYP215 and LAN8814 running on the Microchip > > LAN9668 SoC. The LAN9668 suppports one external bus and unfortunately, the > > LAN8814 has a bug which makes it impossible to use C45 on that bus. > > Fortunately, it was the intention of the GPY215 driver to be used on a C22 > > bus. But I think this could have never really worked, because the > > phy_get_c45_ids() will always do c45 accesses and thus gpy_probe() will > > fail. > > > > Introduce C45-over-C22 support and use it if the MDIO bus doesn't support > > C45. Also enable it when a PHY is promoted from C22 to C45. > > I see this breaking up into two problems. > > 1) Scanning the bus and finding device, be it by C22, C45, or C45 over C22. > > 2) Allowing drivers to access C45 register spaces, without caring if > it is C45 transfers or C45 over C22. > > For scanning the bus we currently have: > > > if (bus->read) { > err = mdiobus_scan_bus_c22(bus); > if (err) > goto error; > } > > prevent_c45_scan = mdiobus_prevent_c45_scan(bus); > > if (!prevent_c45_scan && bus->read_c45) { > err = mdiobus_scan_bus_c45(bus); > if (err) > goto error; > } > > I think we should be adding something like: > > else { > if (bus->read) { > err = mdiobus_scan_bus_c45_over_c22(bus); > if (err) > goto error; > } > } > > That makes the top level pretty obvious what is going on. > > But i think we need some more cleanup lower down. We now have a clean > separation in MDIO bus drivers between C22 bus transactions and C45 > transactions bus. But further up it is less clear. PHY drivers should > be using phy_read_mmd()/phy_write_mmd() etc, which means access the > C45 address space, but says nothing about what bus transactions to > use. So that is also quite clean. > > The problem is in the middle. get_phy_c45_devs_in_pkg() uses > mdiobus_c45_read(). Does mdiobus_c45_read() mean perform a C45 bus > transaction, or access the C45 address space? I would say it means > perform a C45 bus transaction. It does not take a phydev, so we are > below the concept of PHYs, and so C45 over C22 does not exist at this > level. C45-over-C22 is a PHY thing, it isn't generic. We shouldn't go poking at the PHY C45-over-C22 registers unless we know for certain that the C22 device we are accessing is a PHY, otherwise we could be writing into e.g. a switch register or something else. So, the mdiobus_* API should be the raw bus API. If we want C45 bus cycles then mdiobus_c45_*() is the API that gives us that, vs C22 bus cycles through the non-C45 API. C45-over-C22 being a PHY thing is something that should be handled by phylib, and currently is. The phylib accessors there will use C45 or C45-over-C22 as appropriate. The problem comes with PHYs that maybe don't expose C22 ID registers but do have C45-over-C22. These aren't detectable without probing using the C45-over-C22 PHY protocol, but doing that gratuitously will end up writing values to e.g. switch registers and disrupting their operation. So I regard that as a very dangerous thing to be doing. Given that, it seems that such a case could not be automatically probed, and thus must be described in firmware.
> C45-over-C22 is a PHY thing, it isn't generic. We shouldn't go poking > at the PHY C45-over-C22 registers unless we know for certain that the > C22 device we are accessing is a PHY, otherwise we could be writing > into e.g. a switch register or something else. Humm, yes. Good point. > The problem comes with PHYs that maybe don't expose C22 ID registers > but do have C45-over-C22. > > Given that, it seems that such a case could not be automatically > probed, and thus must be described in firmware. We already have the compatible: - const: ethernet-phy-ieee802.3-c45 description: PHYs that implement IEEE802.3 clause 45 But it is not clear what that actually means. Does it mean it has c45 registers, or does it mean it supports C45 bus transactions? If we have that compatible, we could probe first using C45 and if that fails, or is not supported by the bus master, probe using C45 over C22. That seems safe. For Michael use case, the results of mdiobus_prevent_c45_scan(bus) needs keeping as a property of bus, so we know not to perform the C45 scan, and go direct to C45 over C22. Andrew
> - const: ethernet-phy-ieee802.3-c45 > description: PHYs that implement IEEE802.3 clause 45 > > But it is not clear what that actually means. Does it mean it has c45 > registers, or does it mean it supports C45 bus transactions? PHYs which support C45 access but don't have C45 registers aren't a thing I presume - or doesn't make any sense, right? PHYs which have C45 registers but don't support C45 access exists. e.g. the EEE registers of C22 PHYs. But they are C22 PHYs. So I'd say if you have compatible = "ethernet-phy-ieee802.3-c45", it is a PHY with C45 registers _and_ which are accessible by C45 transactions. But they might or might not support C22 access. But I think thats pretty irrelevant because you always do C45 if you can. You cannot do C45 if: (1) the mdio bus doesn't support C45 (2) you have a broken C22 phy on the mdio bus In both cases, if the PHY doesn't support C22-over-C45 you are screwed. Therefore, if you have either (1) or (2) we should always fall back to C22-over-C45. > If we have that compatible, we could probe first using C45 and if that > fails, or is not supported by the bus master, probe using C45 over > C22. That seems safe. For Michael use case, the results of > mdiobus_prevent_c45_scan(bus) needs keeping as a property of bus, so > we know not to perform the C45 scan, and go direct to C45 over C22. So you are talking about DT, in which case there is no auto probing. See phy_mask in the dt code. Only PHYs in the device tree are probed. (unless of course there is no reg property.. then there is some obscure auto scanning). So if you want a C45 PHY you'd have to have that compatible in any case. Btw. I still don't know how you can get a C45 PHY instance without a device tree, except if there is a C45 only bus or the PHY doesn't respond on C22 ids. Maybe I'm missing something here.. -michael
On Tue, Jan 24, 2023 at 01:35:48AM +0100, Michael Walle wrote: > > - const: ethernet-phy-ieee802.3-c45 > > description: PHYs that implement IEEE802.3 clause 45 > > > > But it is not clear what that actually means. Does it mean it has c45 > > registers, or does it mean it supports C45 bus transactions? > > PHYs which support C45 access but don't have C45 registers aren't > a thing I presume - or doesn't make any sense, right? > > PHYs which have C45 registers but don't support C45 access exists. > e.g. the EEE registers of C22 PHYs. But they are C22 PHYs. I wonder if there are any C22 PHYs which only allow access to EEE via C45 transactions. That would be pretty broken... To some extent, i'm still looking at everything and trying to decide if the current definitions/documentation make it clear if means C45 transfers or registers. And the documentation in DT is ambiguous, but as you point out, it probably means registers, not transactions. > So I'd say if you have compatible = "ethernet-phy-ieee802.3-c45", > it is a PHY with C45 registers _and_ which are accessible by > C45 transactions. But they might or might not support C22 access. > But I think thats pretty irrelevant because you always do C45 if > you can. You cannot do C45 if: > (1) the mdio bus doesn't support C45 > (2) you have a broken C22 phy on the mdio bus > > In both cases, if the PHY doesn't support C22-over-C45 you are > screwed. Therefore, if you have either (1) or (2) we should always > fall back to C22-over-C45. > > > If we have that compatible, we could probe first using C45 and if that > > fails, or is not supported by the bus master, probe using C45 over > > C22. That seems safe. For Michael use case, the results of > > mdiobus_prevent_c45_scan(bus) needs keeping as a property of bus, so > > we know not to perform the C45 scan, and go direct to C45 over C22. > > So you are talking about DT, in which case there is no auto probing. > See phy_mask in the dt code. Only PHYs in the device tree are probed. > (unless of course there is no reg property.. then there is some > obscure auto scanning). So if you want a C45 PHY you'd have to > have that compatible in any case. > > Btw. I still don't know how you can get a C45 PHY instance without > a device tree, except if there is a C45 only bus or the PHY doesn't > respond on C22 ids. Maybe I'm missing something here.. In the DT case, you are probably correct. But there are a number of MDIO busses which don't come from DT. Those are typically PCIe or USB devices. Those do get scanned, and my recent changes should mean they first get scanned using C22 and then C45. DSA switches also typically don't have a MDIO node in DT, it is assumed there is a 1:1 mapping between port number and address on the MDIO bus. But as you said, it would require that they don't respond to C22, or the bus master does not support C22, which does actually exist from Marvell at least. In the none DT case, we probably cannot easily do anything about C22-over-C45, because as Russell pointed out, it is potentially a destructive process doing a scan. We want some indication we do expect a PHY to be there. And "ethernet-phy-ieee802.3-c45" would do that. Andrew
>> The problem is in the middle. get_phy_c45_devs_in_pkg() uses >> mdiobus_c45_read(). Does mdiobus_c45_read() mean perform a C45 bus >> transaction, or access the C45 address space? I would say it means >> perform a C45 bus transaction. It does not take a phydev, so we are >> below the concept of PHYs, and so C45 over C22 does not exist at this >> level. > > C45-over-C22 is a PHY thing, it isn't generic. We shouldn't go poking > at the PHY C45-over-C22 registers unless we know for certain that the > C22 device we are accessing is a PHY, otherwise we could be writing > into e.g. a switch register or something else. > > So, the mdiobus_* API should be the raw bus API. If we want C45 bus > cycles then mdiobus_c45_*() is the API that gives us that, vs C22 bus > cycles through the non-C45 API. > > C45-over-C22 being a PHY thing is something that should be handled by > phylib, and currently is. The phylib accessors there will use C45 or > C45-over-C22 as appropriate. I think the crux is get_phy_device(). It is used for two different cases: (1) to scan the mdio bus (2) to add a c45 phy, i.e. in the DT/fwnode case For (1) we must not use indirect access. And for (2) we know for a fact that it must be a PHY and thus we can (and have to) fall back to c45-over-c22. Btw. for the DT case, it seems we need yet another property to indicate broken MDIO busses. -michael
> Btw. for the DT case, it seems we need yet another property > to indicate broken MDIO busses. I would prefer to avoid that. I would suggest you do what i did for the none DT case. First probe using C22 for all devices known in DT. Then call mdiobus_prevent_c45_scan() which will determine if any of the found devices are FUBAR and will break C45. Then do a second probe using C45 and/or C45 over C22 for those devices in DT with the c45 compatible. Andrew
Am 2023-01-24 22:03, schrieb Andrew Lunn: >> Btw. for the DT case, it seems we need yet another property >> to indicate broken MDIO busses. > > I would prefer to avoid that. I would suggest you do what i did for > the none DT case. First probe using C22 for all devices known in DT. > Then call mdiobus_prevent_c45_scan() which will determine if any of > the found devices are FUBAR and will break C45. Then do a second probe > using C45 and/or C45 over C22 for those devices in DT with the c45 > compatible. I tried that yesterday. Have a look at of_mdiobus_register() [1]. There the device tree is walked and each PHY with a reg property is probed. Afterwards, if there was a node without a reg property, the bus is scanned for the missing PHYs. If we would just probe c22 first, the order of the auto scanning might change, if there is a c45 phy in between two c22 phys. I was thinking to just ignore the case that the autoscan would discover a broken PHY. (1) scan c22 (2) scan c45 (maybe using c45-over-c22) (3) do the autoscan -michael [1] https://elixir.bootlin.com/linux/v6.2-rc5/source/drivers/net/mdio/of_mdio.c#L149 > > Andrew
On Tue, Jan 24, 2023 at 10:20:33PM +0100, Michael Walle wrote: > Am 2023-01-24 22:03, schrieb Andrew Lunn: > > > Btw. for the DT case, it seems we need yet another property > > > to indicate broken MDIO busses. > > > > I would prefer to avoid that. I would suggest you do what i did for > > the none DT case. First probe using C22 for all devices known in DT. > > Then call mdiobus_prevent_c45_scan() which will determine if any of > > the found devices are FUBAR and will break C45. Then do a second probe > > using C45 and/or C45 over C22 for those devices in DT with the c45 > > compatible. > > I tried that yesterday. Have a look at of_mdiobus_register() [1]. > There the device tree is walked and each PHY with a reg property > is probed. Afterwards, if there was a node without a reg property, > the bus is scanned for the missing PHYs. If we would just probe c22 > first, the order of the auto scanning might change, if there is a > c45 phy in between two c22 phys. I was thinking to just ignore the > case that the autoscan would discover a broken PHY. I think it is pretty rare to not have a reg value. The DT lint tools will complain about that, etc. So any examples are likely to be old boards. And old board are a lot less likely to have C45 PHYs. So there is a corner case left unhandled, but it seems pretty unlikely. So i agree, lets address it if anybody reports issues. But please mention it in the commit message, just i can somebody does a git bisect, etc. Andrew