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 24FAFE7717F for ; Tue, 10 Dec 2024 10:10:58 +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:MIME-Version: Content-Transfer-Encoding:Content-Type:References:In-Reply-To:Date:Cc:To:From :Subject:Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=LPh0yVGEQ9vWISI1i/TYH0JhPJ9KKecMDH0Xpu48hcc=; b=FLtBtTu2A1cKKR+++3UlSaHTax qYwil5S3S7lnNprmj2EFI6i1XtIRaqY9BQhjiDOB29yhbZFrTV1gmBcEkt70eE9qFlx2GicMV5+z7 YAQ6CcNXgSqFIN15ULUJPS10+dRuOxXwU1/CEYayprBp/vCReWZsuO0KGcMt33yj9mj4TgIxXMWWk Q0rQxSsS4xmXh9IMFfffhUvcNO+g0xjnNLqcoi4Xr8sgIdc6Eb45BDMcDyCpTZp/R6QBwpOj11kd0 Y86SBfZhYTEloBwFuv7au/WYZ8LnMgI9niT7Dt+9pwSq9UV8IS6+iVUVtjtybyMWXlxe1dlJP3cYr +ylbFvfA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1tKxCW-0000000B5Ap-2I7P; Tue, 10 Dec 2024 10:10:44 +0000 Received: from mx1.tq-group.com ([93.104.207.81]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1tKwz8-0000000B2AX-14bh for linux-arm-kernel@lists.infradead.org; Tue, 10 Dec 2024 09:56:56 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1733824614; x=1765360614; h=message-id:subject:from:to:cc:date:in-reply-to: references:content-transfer-encoding:mime-version; bh=LPh0yVGEQ9vWISI1i/TYH0JhPJ9KKecMDH0Xpu48hcc=; b=d5qLl4fct/OS4k4YJDNLws5KWIOA115eLzRWjwYPoP3OZWjvo5whHBL7 vjFOfZyV2EQnhap3FGmBgOCOvbMkmbbDmszaI3ekJBq8cO04jvn7RB/Uf rFBgjJZu8E9hz/Wd6n0lm2VvVmGAMeRyyytcg0XbbkF0mB1cuYIupx/X6 8R7U6QR/XJiE+jcG2OK/GgoYsunztu8R7G349HwRhqJCcjFvQ3YYftxaF zOGIG3mRa+7zsOoLkxNLIQJ3a35D/WuzJmvu+8jGKni8t3Jb9Aw59U9nT A7Pqs8zFWqIk8jOix4DK91cQjvzQBE1ekzZ84k4lYuFsHR+qFMAN5ckQl A==; X-CSE-ConnectionGUID: L4jbIdFJRearAlIyQVVXcQ== X-CSE-MsgGUID: pmEQQ7tuSc27gjXA3zX56g== X-IronPort-AV: E=Sophos;i="6.12,222,1728943200"; d="scan'208";a="40507636" Received: from vmailcow01.tq-net.de ([10.150.86.48]) by mx1.tq-group.com with ESMTP; 10 Dec 2024 10:56:50 +0100 X-CheckPoint: {67581062-5-98002871-E6E29D66} X-MAIL-CPID: 3E6B7F7F1FE94745951728CD9D1DD495_3 X-Control-Analysis: str=0001.0A682F1A.67581062.0054,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0 Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id 5BA3D167FF3; Tue, 10 Dec 2024 10:56:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ew.tq-group.com; s=dkim; t=1733824605; 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=LPh0yVGEQ9vWISI1i/TYH0JhPJ9KKecMDH0Xpu48hcc=; b=lLSEVIYs5+7Y+CXHWwxGjqyRow8kmqPCwwfsqJRbU0dGB3uPArrRjNJZIt8AQsDkjzL/s8 GKke8uKjGNfjS2wkeVbymV+A15exlcdlbbXcWEYCAxm4iQAV3x5DUbCZqllq3Y6WQtzpWe ijW/PsTwD08MBNJc36SVETUAei4hL92vI2Wl/lfM9LJHE3CPe/8t1C9fQM/joAekRntcJG zkKg4f26O8tuJ7dZHeLkJBhKbkjt38lr98UbV6CvVJbrxfP4L77DjDqe6tWjYe4qARu9ot 4+XqcHL6rmyYfckuo8ljhOet1+EiN43ItM5YUOXUWzWMrQyw/2q37MCyoyW8AA== Message-ID: <309052f3f69950fe43390505cc7254aee8c8f5c6.camel@ew.tq-group.com> Subject: Re: [PATCH v2 5/5] arm64: dts: ti: Add TQ-Systems TQMa62xx SoM and MBa62xx carrier board Device Trees From: Matthias Schiffer To: Andrew Lunn Cc: Nishanth Menon , Vignesh Raghavendra , Tero Kristo , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Greg Kroah-Hartman , Kees Cook , Tony Luck , "Guilherme G. Piccoli" , Felipe Balbi , linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, linux-hardening@vger.kernel.org, Devarsh Thakkar , Hari Nagalla , linux@ew.tq-group.com Date: Tue, 10 Dec 2024 10:56:41 +0100 In-Reply-To: References: <95ff66ca2c89f69d893c2ce9eed9a0c677633c7b.1733737487.git.matthias.schiffer@ew.tq-group.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.52.3-0ubuntu1 MIME-Version: 1.0 X-Last-TLS-Session-Version: TLSv1.3 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241210_015654_815074_D59F4888 X-CRM114-Status: GOOD ( 35.48 ) 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, 2024-12-09 at 17:14 +0100, Andrew Lunn wrote: >=20 > > Not our board, but the AM62 SoC. From the datasheet: > >=20 > > "TXC is delayed internally before being driven to the RGMII[x]_TXC pin.= This > > internal delay is always enabled." So enabling the TX delay on the PHY = side > > would result in a double delay. >=20 > phy-mode describes the board. If the board does not have extra long > clock lines, phy-mode should be rgmii-id. >=20 > The fact the MAC is doing something which no other MAC does should be > hidden away in the MAC driver, as much as possible. Isn't it kind of a philosophical question whether a delay added by the SoC integration is part of the MAC or not? One could also argue that the MAC IP= core is always the same, with some SoCs adding the delay and others not. (I don'= t know if there are actually SoCs with the same IP core that don't add a dela= y; I'm just not a big fan of hiding details in the driver that could easily be described by the Device Tree, thus making the driver more generic) >=20 > The MAC driver should return -EINVAL with phy-mode rgmii, or > rmgii-rxid, because the MAC driver is physically incapable of being > used on a board which has extra long TX clock lines, which 'rmgii' or > rgmii-rxid would indicate. >=20 > Since the MAC driver is forcing the TX delay, it needs to take the > value returned from of_get_phy_mode() and mask out the TX bit before > passing it to the PHY. Hmm okay, this is what the similar ICSSG/PRUETH driver does. I've always fo= und that solution to be particularly confusing, but if that's how it's supposed= to work, I'll have to accept that. In my opinion the documentation Documentation/networking/phy.rst is not ver= y clear on this matter - the whole section "(RG)MII/electrical interface considerations" talks about whether the PHY inserts the delay or not, so my assumption was that phy-mode describes the PHY side of things and only that= . It gets even more confusing when taking into account Documentation/devicetree/bindings/net/ethernet-controller.yaml, which conta= ins comments like "RGMII with internal RX delay provided by the PHY, the MAC sh= ould not add an RX delay in this case", which sounds like there are only the cas= es "delay is added by the PHY" and "delay is added by the MAC" - the case "del= ay is part of the board design, neither MAC nor PHY add it" doesn't even appear. >=20 > Now, it could be that history has got in the way. There are boards out > there which have broken DT but work. Fixing the MAC driver to do the > correct thing will break those boards. Vendors with low quality code > which works, but not really. >=20 > ~/linux/arch/arm64/boot/dts/ti$ grep rgmii k3-am625-* > k3-am625-beagleplay.dts: phy-mode =3D "rgmii-rxid"; > k3-am625-sk.dts: phy-mode =3D "rgmii-rxid"; >=20 > Yep, these two have broken DT, they don't describe the board > correctly. >=20 > O.K. Can we fix this for you board? Yes, i think we can. If you take > rmgii-rxid, aka PHY_INTERFACE_MODE_RGMII_RXID, and mask out the TX, > you still get PHY_INTERFACE_MODE_RGMII_RXID. If you take rgmii-id, > a.k.a. PHY_INTERFACE_MODE_RGMII_ID and mask out the TX, you get > PHY_INTERFACE_MODE_RGMII_RXID, which is what you want. >=20 > Please produce a patch to the MAC driver, explaining the horrible mess > the vendor made, and how this fixes it, but should also not break > other boards. I can make this change, but "am65-cpsw-nuss" currently supports 6 different compatible strings, many of which are used for multiple SoC families. Maybe someone from TI could chime in and say whether all of these have the = fixed TXC delay, or at least the current compatible strings are already specific enough to tell whether the SoC adds a delay? >=20 > > No such defaults exist in the DP83867 driver. If any rgmii-*id mode is = used, the > > corresponding delays *must* be specified in the DTB: > >=20 > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree= /drivers/net/phy/dp83867.c#n532 >=20 > That is bad, different to pretty every other PHY driver :-( >=20 > If you want, you could patch this driver as well, make it default to > 2ns if delays are asked for. Makes sense, I'll write a patch for that as well. Best regards, Matthias >=20 > Andrew >=20 > --- > pw-bot: cr --=20 TQ-Systems GmbH | M=C3=BChlstra=C3=9Fe 2, Gut Delling | 82229 Seefeld, Germ= any Amtsgericht M=C3=BCnchen, HRB 105018 Gesch=C3=A4ftsf=C3=BChrer: Detlef Schneider, R=C3=BCdiger Stahl, Stefan Sch= neider https://www.tq-group.com/