From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D207FCED274 for ; Tue, 8 Oct 2024 07:29:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Message-ID:Subject:Cc:To: From:Date:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Y2Eov6yFpaMhBa+zRN8cKs/RYWnxfaxocRnMjKu5r/s=; b=sv2pyWC5zJ1HsWDy2B5XK4mvBl dD3MTfoYosMA/19UUoICMdaCO76uG3EgvCcRjOOgJK/OLBvoU8VOTqoNXCD3hpAn6WIRUE0ekL5J3 sPV5HLeUKozSTnXfb0RYTJRuKczro+FN7vZ/qC116JStLQRUdlLfSxPZciChF96JRyHt/a9ooX7RD CpBb2yK3CJ/oS8ADCdY50MrtcD+PuGyf7qfMYvIEyCeEU3+O7W3p49sWMdqxeemSLxUtdWskPM+PP gIPMsICtMGTQxraUzTZxqKBipowc8OgYseUiy+wpT74OPl6R4BEkhynNKoQfReiA0xgwaO5AnLd5n sEDzjhRg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1sy4es-00000004nBk-06A9; Tue, 08 Oct 2024 07:29:26 +0000 Received: from relay4-d.mail.gandi.net ([2001:4b98:dc4:8::224]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1sy4ba-00000004mnc-1AKb for linux-arm-kernel@lists.infradead.org; Tue, 08 Oct 2024 07:26:05 +0000 Received: by mail.gandi.net (Postfix) with ESMTPSA id 12AFEE0004; Tue, 8 Oct 2024 07:25:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1728372359; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Y2Eov6yFpaMhBa+zRN8cKs/RYWnxfaxocRnMjKu5r/s=; b=N9OPW9ghUApwMwLVnBcXpiSVmIgOeRi3NcixPpyq4UcWVz5fFLn4G4h9bokE7Y6jyFjjBd hSqsmYbjS7UV/w8MAhFrSxDEYtH32oLyhnZV3Bk2CT1dRCXBUKFzKbQKbXM4ujMGYGqWwC y24XT12GTiJV7hp786Lms6QgpfX4Mq4YXVo7w4BP3NObjCerBqFZiSMWO2n4rjnUHRTs09 tHPj2IGrvMCbPFlG3VzxGzhFx/ICzGbmChQTsZWXTxbyulPn7PfWBz0X1JvxdYnz3gl7l5 lIk3tOl/KBhGeCBkPgWpAVNMbuupDN8LGJmfgMcNniwMgKBbFzW4nzZZM+WNMg== Date: Tue, 8 Oct 2024 09:25:57 +0200 From: Maxime Chevallier To: Andrew Lunn Cc: "Russell King (Oracle)" , davem@davemloft.net, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, thomas.petazzoni@bootlin.com, Jakub Kicinski , Eric Dumazet , Paolo Abeni , linux-arm-kernel@lists.infradead.org, Christophe Leroy , Herve Codina , Florian Fainelli , Heiner Kallweit , Vladimir Oltean , Marek =?UTF-8?B?QmVow7pu?= , =?UTF-8?B?S8O2cnk=?= Maincent , Oleksij Rempel Subject: Re: [PATCH net-next v2 7/9] net: phy: introduce ethtool_phy_ops to get and set phy configuration Message-ID: <20241008092557.50db7539@device-21.home> In-Reply-To: References: <20241004161601.2932901-1-maxime.chevallier@bootlin.com> <20241004161601.2932901-8-maxime.chevallier@bootlin.com> <4d4c0c85-ec27-4707-9613-2146aa68bf8c@lunn.ch> <20241007123751.3df87430@device-21.home> <6bdaf8de-8f7e-42db-8c29-1e8a48c4ddda@lunn.ch> <20241007154839.4b9c6a02@device-21.home> Organization: Bootlin X-Mailer: Claws Mail 4.3.0 (GTK 3.24.43; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-GND-Sasl: maxime.chevallier@bootlin.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241008_002602_650626_B7ACB05B X-CRM114-Status: GOOD ( 56.65 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, 7 Oct 2024 18:37:29 +0200 Andrew Lunn wrote: > > That's a legit point. I mentioned in the cover for V1 that this in > > itself doesn't really bring anything useful. The only point being that > > it makes it easy to test if a PHY has a working isolation mode, but > > given that we'll assume that it doesn't by default, that whole point > > is moot. > > > > I would therefore understand if you consider that having a kAPI for > > that isn't very interesting and that I shall include this work as part > > of the multi-PHY support. > > kAPI add a lot of Maintenance burden. So we should not add them unless > they are justified. to me, there is not a good justification for this. That's fine by me. > > > Sure thing. There are multiple devices out-there that may have multiple > > PHYs accessible from the MAC, through muxers (I'm trying to be generic > > enough to address all cases, gpio muxers, mmio-controlled muxers, etc.), > > but let me describe the HW I'm working on that's a bit more problematic. > > > > The first such platform I have has an fs_enet MAC, a pair of LXT973 > > PHYs for which the isolate mode doesn't work, and no on-board circuitry to > > perform the isolation. Here, we have to power one PHY down when unused : > > > > /--- LXT973 > > fs_enet -- MII--| > > \--- LXT973 > > So you have at least regulators under Linux control? Is that what you > mean by power down? Pulling the plug and putting it back again is > somewhat different to isolation. All its state is going to be lost, > meaning phylib needs to completely initialise it again. Or can you > hide this using PM? Just suspend/resume it? Ah no, I wasn't referring to regulators but rather the BMCR PDOWN bit to just shut the PHY down, as in suspend. Indeed the state is lost. The way I'm supporting this is : - If one PHY has the link, it keeps it until link-down - When link-down, I round-robin between the 2 phys: - Attach the PHY to the netdev - See if it can establish link and negotiate with LP - If there's nothing after a given period ( 2 seconds default ), then I detach the PHY, attach the other one, and start again, until one of them has link. That's very limited indeed, we have no way of saying "first that has link wins". > > The second board has a fs_enet MAC and a pair of KSZ8041 PHYs connected > > in MII. > > > > The third one has a pair of KSZ8041 PHYs connected to a > > ucc_geth MAC in RMII. > > > > On both these boards, we isolate the PHYs when unused, and we also > > drive a GPIO to toggle some on-board circuitry to disconnect the MII > > lines as well for the unused PHY. I'd have to run some tests to see if > > this circuitry could be enough, without relying at all on PHY > > isolation : > > > > /--- KSZ8041 > > | > > MAC ------ MUX > > | | > > to SoC <-gpio--/ \--- KSZ8041 > > > > > > One point is, if you look at the first case (no mux), we need to know > > if the PHYs are able to isolate or not in order to use the proper > > switching strategy (isolate or power-down). > > That explains the hardware, but what are the use cases? How did the > hardware designer envision this hardware being used? The use-case is link redundancy, if one PHY loses the link, we hope that we still have link on the other one and switchover. This is one of the things I discussed at netdev 0x17. > If you need to power the PHY off, you cannot have dynamic behaviour > where the first to have link wins. But if you can have the media side > functional, you can do some dynamic behaviours. True. > Although, is it wise > for the link to come up, yet to be functionally dead because it has no > MAC connected? Good point. What would you think ? I already deal with the identified issue which is that both PHYs are link-up with LP, both connected to the same switch. When we switch between the active PHYs, we send a gratuitous ARP on the new PHY to refresh the switch's FDB. Do you see that as being an issue, having the LP see link-up when the link cannot actually convey data ? Besides the energy detect feature you mention, I don't see what other options we can have unfortunately :( > There are some Marvell Switches which support both internal Copper > PHYs and a SERDES port. The hardware allows first to get link to have > a functional MAC. But in Linux we have not supported that, and we > leave the unused part down so it does not get link. My plan is to support these as well. For the end-user, it makes no difference wether the HW internally has 2 PHYs each with one port, or 1 phy with 2 ports. So to me, if we want to support phy_mux, we should also support the case you mention above. I have some code to support this, but that's the part where I'm still getting things ironed-out, this is pretty tricky to represent that properly, especially in DT. > > Maybe we actually want energy detect, not link, to decide which PHY > should get the MAC? But i have no real idea what you can do with > energy detect, and it would also mean building out the read_status() > call to report additional things, etc. Note that I'm trying to support a bigger set of use-cases besides the pure 2-PHY setup. One being that we have a MUX within the SoC on the SERDES lanes, allowing to steer the MII interface between a PHY and an SFP bus (Turris Omnia has such a setup). Is it possible to have an equivalent "energy detect" on all kinds of SFPs ? As a note, I do see that both Russell and you may think you're being "drip-fed" (I learned that term today) information, that's not my intent at all, I wasn't expecting this discussion now, sorry about that. I was saying to Russell that I would start a new thread, but we already have a discussion going here, let me know if we shall continue the discussion here or on a new thread. Thanks, Maxime