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 X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 301CCC4361B for ; Wed, 16 Dec 2020 09:00:59 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B084722DBF for ; Wed, 16 Dec 2020 09:00:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B084722DBF Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=baikalelectronics.ru Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject: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=qt6ARc1P8JIxdSJT7OxN3pwpzvZIlVcX04BU/aaS6wI=; b=nJHG7GkRX+UeDS9vglj/h0lg7 VVvq/Mrh1s8om6lYnngOThtpQ/ems16JOfzXmxiH9rV/JaamshEk40iqgMRwyJTUcuOTHAsh3Kpit CkYL5OUWF2yFlRlhN5lRvw0BzksVL++3ssMagelatxjbGKpw70qCkyKO+3pG0gc/iWDAObSHi2Io7 YqKd3vDNa0CpDOuMGZ0oAn2DYojOywbIxujCJMiOEF8xD6UR26MJyN+CgXXWtr7qWaYGy0g5l44gC 43HEELGjHk8wy5FN62G9oGGzjXyXyGnUhFNP3yAKr+7HID3GTlsTqDCe1pLaN/oASHpz125feBXS1 Vfgf9MY0w==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kpSes-0006XG-SG; Wed, 16 Dec 2020 08:59:42 +0000 Received: from mail.baikalelectronics.com ([87.245.175.226] helo=mail.baikalelectronics.ru) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kpSep-0006Wb-OO for linux-arm-kernel@lists.infradead.org; Wed, 16 Dec 2020 08:59:41 +0000 Date: Wed, 16 Dec 2020 11:59:34 +0300 From: Serge Semin To: Rob Herring Subject: Re: [PATCH 03/25] dt-bindings: net: dwmac: Fix the TSO property declaration Message-ID: <20201216085934.tlp5axhauyshb2st@mobilestation> References: <20201214091616.13545-1-Sergey.Semin@baikalelectronics.ru> <20201214091616.13545-4-Sergey.Semin@baikalelectronics.ru> <20201215172240.GA4047815@robh.at.kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201215172240.GA4047815@robh.at.kernel.org> X-ClientProxiedBy: MAIL.baikal.int (192.168.51.25) To mail (192.168.51.25) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201216_035940_058977_587A11A6 X-CRM114-Status: GOOD ( 26.42 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Alexandre Torgue , Joao Pinto , netdev@vger.kernel.org, Lars Persson , linux-stm32@st-md-mailman.stormreply.com, Johan Hovold , Serge Semin , Alexey Malahov , Jose Abreu , Maxime Ripard , Maxime Coquelin , Jakub Kicinski , Giuseppe Cavallaro , Vyacheslav Mitrofanov , "David S. Miller" , linux-arm-kernel@lists.infradead.org, Pavel Parkhomenko Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Dec 15, 2020 at 11:22:40AM -0600, Rob Herring wrote: > On Mon, Dec 14, 2020 at 12:15:53PM +0300, Serge Semin wrote: > > Indeed the STMMAC driver doesn't take the vendor-specific compatible > > string into account to parse the "snps,tso" boolean property. It just > > makes sure the node is compatible with DW MAC 4.x, 5.x and DW xGMAC > > IP-cores. Fix the conditional statement so the TSO-property would be > > evaluated for the compatibles having the corresponding IP-core version. > > > > While at it move the whole allOf-block from the tail of the binding file > > to the head of it, as it's normally done in the most of the DT schemas. > > > > Signed-off-by: Serge Semin > > > > --- > > > > Note this won't break the bindings description, since the "snps,tso" > > property isn't parsed by the Allwinner SunX GMAC glue driver, but only > > by the generic platform DT-parser. > > But still should be valid for Allwinner? I don't know. It seems to me that even the original driver developer didn't know what DW MAC IP has been used to create the Allwinner EMAC, since in the cover letter to the original patch he said: "During the development, it appeared that in fact the hardware was a modified version of some dwmac." (See https://lwn.net/Articles/721459/) Most likely Maxime Ripard also didn't know that when he was converting the legacy bindings to the DT schema. What I do know the TSO is supported by the driver only for IP-cores with version higher than 4.00. (See the stmmac_probe_config_dt() method implementation). Version is determined by checking whether the DT device node compatible property having the "snps,dwmac-*" or "snps,dwxgmac" strings. Allwinner EMAC nodes aren't defined with those strings, so they won't have the TSO property parsed and set. > > > --- > > .../devicetree/bindings/net/snps,dwmac.yaml | 52 +++++++++---------- > > 1 file changed, 24 insertions(+), 28 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > index e084fbbf976e..0dd543c6c08e 100644 > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > @@ -37,6 +37,30 @@ select: > > required: > > - compatible > > > > +allOf: > > + - $ref: "ethernet-controller.yaml#" > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - snps,dwmac-4.00 > > + - snps,dwmac-4.10a > > + - snps,dwmac-4.20a > > + - snps,dwmac-5.10a > > + - snps,dwxgmac > > + - snps,dwxgmac-2.10 > > + > > + required: > > + - compatible > > + then: > > + properties: > > + snps,tso: > > + $ref: /schemas/types.yaml#definitions/flag > > + description: > > + Enables the TSO feature otherwise it will be managed by > > + MAC HW capability register. > > BTW, I prefer that properties are defined unconditionally, and then > restricted in conditional schemas (or ones that include this schema). Are you saying that it's ok to have all the properties unconditionally defined in some generic schema and then being un-defined (like redefined to a false-schema) in a schema including (allOf-ing) it? > > > + > > properties: > > > > # We need to include all the compatibles from schemas that will > > @@ -314,34 +338,6 @@ dependencies: > > snps,reset-active-low: ["snps,reset-gpio"] > > snps,reset-delay-us: ["snps,reset-gpio"] > > > > -allOf: > > - - $ref: "ethernet-controller.yaml#" > > - - if: > > - properties: > > - compatible: > > - contains: > > - enum: > > - - allwinner,sun7i-a20-gmac > > This does not have a fallback, so snps,tso is no longer validated. I > didn't check the rest. Until the DT node is having a compatible string with the DW MAC IP-core version the property won't be checked by the driver anyway. AFAICS noone really knows what IP was that. So most likely the allwinner emacs have been added to this conditional schema by mistake... -Sergey > > > - - allwinner,sun8i-a83t-emac > > - - allwinner,sun8i-h3-emac > > - - allwinner,sun8i-r40-emac > > - - allwinner,sun8i-v3s-emac > > - - allwinner,sun50i-a64-emac > > - - snps,dwmac-4.00 > > - - snps,dwmac-4.10a > > - - snps,dwmac-4.20a > > - - snps,dwxgmac > > - - snps,dwxgmac-2.10 > > - - st,spear600-gmac > > - > > - then: > > - properties: > > - snps,tso: > > - $ref: /schemas/types.yaml#definitions/flag > > - description: > > - Enables the TSO feature otherwise it will be managed by > > - MAC HW capability register. > > - > > additionalProperties: true > > > > examples: > > -- > > 2.29.2 > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel