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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D9D2FCE7A81 for ; Fri, 22 Sep 2023 21:58:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230034AbjIVV63 (ORCPT ); Fri, 22 Sep 2023 17:58:29 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33570 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229924AbjIVV61 (ORCPT ); Fri, 22 Sep 2023 17:58:27 -0400 Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8C4F8197; Fri, 22 Sep 2023 14:58:20 -0700 (PDT) Received: by mail.gandi.net (Postfix) with ESMTPSA id 3E790240002; Fri, 22 Sep 2023 21:57:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=arinc9.com; s=gm1; t=1695419898; 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=/YVTZcfReib7t/7pkUyxikgIMHTHiQw2lUN/ib+i8mE=; b=Ja/5fpeT5o5akQ0s5PhqFJUDY6DQ7QeszgvhChp7+woMQdUgGJW0GPwQA0aCTTG1OGy9Hb +1p6OYpCKg7g/jxXASbU6FMY0XJsjosnVSVh1i+y1bWLDL2msdeQe7ZdQcxP++36sYwLcn tu11yHNRFvgPiyIdH14mXaKjcyzntmp0K2kg7QiHYToaPwIcWvVSqBH3jFdn8CdJyFjfpj tdeVDShAPsGMOZmtAe0mhghv1EuT8kYzBSy2d6Hhvwx7EMpIaemtl9DdmnIBVGTTnTagzY P+X0gsJTPfggsQk9zBywEZD5TRUEFOcRSr2PWM1Vn94oEDspzq6M+iHqMcF9Xw== Message-ID: <6c1bb7df-34cd-4db9-95b6-959c87b68588@arinc9.com> Date: Sat, 23 Sep 2023 00:57:52 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net-next v2 00/10] define and enforce phylink bindings Content-Language: en-US To: "Russell King (Oracle)" , Rob Herring , Krzysztof Kozlowski Cc: "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Conor Dooley , George McCollister , Andrew Lunn , Florian Fainelli , Vladimir Oltean , Kurt Kanzenbach , Matthias Brugger , AngeloGioacchino Del Regno , Woojung Huh , UNGLinuxDriver@microchip.com, Linus Walleij , =?UTF-8?Q?Alvin_=C5=A0ipraga?= , =?UTF-8?B?Q2zDqW1lbnQgTMOpZ2Vy?= , Marcin Wojtas , Lars Povlsen , Steen Hegelund , Daniel Machon , Radhey Shyam Pandey , Daniel Golle , Landen Chao , DENG Qingfang , Sean Wang , Geert Uytterhoeven , Magnus Damm , Maxime Chevallier , Nicolas Ferre , Claudiu Beznea , Marek Vasut , Claudiu Manoil , Alexandre Belloni , John Crispin , Madalin Bucur , Ioana Ciornei , Lorenzo Bianconi , Felix Fietkau , Horatiu Vultur , Oleksij Rempel , Alexandre Torgue , Giuseppe Cavallaro , Jose Abreu , Grygorii Strashko , Sekhar Nori , Shyam Pandey , mithat.guner@xeront.com, erkin.bozoglu@xeront.com, netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, linux-renesas-soc@vger.kernel.org References: <20230916110902.234273-1-arinc.unal@arinc9.com> From: =?UTF-8?B?QXLEsW7DpyDDnE5BTA==?= In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-GND-Sasl: arinc.unal@arinc9.com Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 22/09/2023 15:40, Russell King (Oracle) wrote: > On Sat, Sep 16, 2023 at 02:08:52PM +0300, Arınç ÜNAL wrote: >> Hello there. >> >> This patch series defines phylink bindings and enforces them for the >> ethernet controllers that need them. >> >> Some schemas had to be changed to properly enforce phylink bindings for all >> of the affected ethernet controllers. Some of the documents of these >> ethernet controllers were non json-schema, which had to be converted. >> >> I will convert the remaining documents to json-schema while this patch >> series receives reviews. > > I can't say that I'm comfortable with this. We appear to be defining > bindings based on software implementation, and a desire for the DT > tooling to enforce what the software implementation wants. Isn't this > against the aims of device tree and device tree binding documentation? > Seems to me like feature-creep. > > The bindings that phylink parses are already documented in the > ethernet controller yaml document. Specifically: > > - phylink does not parse the phy-mode property, that is left to the > implementation to pass to phylink, which can implement it any > which way they choose (and even default to something.) > > - phylink does not require a phy property - phylink does expect a PHY > to be attached, but how that PHY is attached is up to the ethernet > controller driver. It may call one of the phylink functions that > parses the phy property, or it may manually supply the phy device to > phylink. Either way, phylink does not itself require a PHY property. > > - phylink does not require a sfp property - this obviously is optional. > > So, all in all, ethernet-controller already describes it, and to create > a DT binding document that pretends that phylink requires any of this > stuff is, in my mind, wrong. > > DSA requires certain properties by dint of the parsing and setup of > phylink being in generic code - this is not because phylink requires > certain properties, but phylink does require certain information in > order to function correctly. > > The issue here is _how_ phylink gets that information, and as I state > above, it _can_ come from DT, but it can also be given that information > manually. > > As an example, there are plenty of drivers in the tree which try to > parse a phy node, and if that's not present, they try to see if a PHY > exists at a default# bus address. > > We seem to be digging outselves a hole here, where "phylink must have > these properties". No, that is wrong. I agree. My patch description here failed to explain the actual issue, which is missing hardware descriptions. Here's what I understand. An ethernet-controller is a MAC. For the MAC to work properly with its link partner, at least one of these must be described: - pointer to a PHY to retrieve link information from the PHY - pointer to a PCS to retrieve link information from the PCS - pointer to an SFP to retrieve link information from the SFP - static link information Andrew under the discussion of patch 7 said that enforcing this may expose bugs on MAC drivers that never looked at the devicetree to control the MAC's link which would cause regressions, implying we should hold back on enforcing it. I've agreed not to enforce it, not because it is incorrect description of ethernet controller hardware - I think it is correct - but because I won't be the one to deal with the regressions when this dt-bindings change goes through. I won't also enforce it selectively, as saying "these drivers use phylink_fwnode_phy_connect() therefore there won't be any bad surprises on the hardware they control so let's enforce it only for them" is nonsense in the context of describing hardware. I will focus on documenting the missing MDIO bus descriptions on certain ethernet switches and converting ethernet switch documents (maybe ethernet controllers too) to json-schema. There's the incorrect link descriptions on dsa-port.yaml as confirmed by Vladimir on the discussion of v1 series so I'll fix that. I've also got some ethernet controller rules that I think won't break any driver so I will submit them as well. Arınç