Hi Krzysztof, I am Danish, new addition to TI team. Puranjay left TI, I'll be posting upstream patches for Programmable Real-time Unit and Industrial Communication Subsystem Gigabit (PRU_ICSSG). On 31/05/22 15:38, Krzysztof Kozlowski wrote: > On 31/05/2022 11:51, Puranjay Mohan wrote: >> Add a YAML binding document for the ICSSG Programmable real time unit >> based Ethernet driver. This driver uses the PRU and PRUSS consumer APIs >> to interface the PRUs and load/run the firmware for supporting ethernet >> functionality. >> >> Signed-off-by: Puranjay Mohan <p-mohan@xxxxxx> >> --- >> v1: https://lore.kernel.org/all/20220506052433.28087-2-p-mohan@xxxxxx/ >> v1 -> v2: >> * Addressed Rob's Comments > > Nope, they were not addressed. > >> * It includes indentation, formatting, and other minor changes. >> --- >> .../bindings/net/ti,icssg-prueth.yaml | 181 ++++++++++++++++++ >> 1 file changed, 181 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml >> >> diff --git a/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml b/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml >> new file mode 100644 >> index 000000000000..40af968e9178 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml >> @@ -0,0 +1,181 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/net/ti,icssg-prueth.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: |+ > > Missed Rob's comment. > I'll remove this in the next version of this series. >> + Texas Instruments ICSSG PRUSS Ethernet >> + >> +maintainers: >> + - Puranjay Mohan <p-mohan@xxxxxx> >> + >> +description: >> + Ethernet based on the Programmable Real-Time >> + Unit and Industrial Communication Subsystem. >> + >> +allOf: >> + - $ref: /schemas/remoteproc/ti,pru-consumer.yaml# >> + >> +properties: >> + compatible: >> + enum: >> + - ti,am654-icssg-prueth # for AM65x SoC family >> + >> + pinctrl-0: >> + maxItems: 1 >> + >> + pinctrl-names: >> + items: >> + - const: default > > You do not need these usually, they are coming from schema. > Here from what I understand, I need to delete the below block, right? pinctrl-names: items: - const: default >> + >> + sram: >> + description: >> + phandle to MSMC SRAM node >> + >> + dmas: >> + maxItems: 10 >> + description: >> + list of phandles and specifiers to UDMA. > > Please follow Rob's comment - drop description. > I'll drop desceeiption. >> + >> + dma-names: >> + items: >> + - const: tx0-0 >> + - const: tx0-1 >> + - const: tx0-2 >> + - const: tx0-3 >> + - const: tx1-0 >> + - const: tx1-1 >> + - const: tx1-2 >> + - const: tx1-3 >> + - const: rx0 >> + - const: rx1 >> + >> + ethernet-ports: >> + type: object >> + properties: >> + '#address-cells': >> + const: 1 >> + '#size-cells': >> + const: 0 >> + >> + patternProperties: >> + ^port@[0-1]$: > > How did you implement Rob's comments here? > >> + type: object >> + description: ICSSG PRUETH external ports >> + >> + $ref: ethernet-controller.yaml# >> + >> + unevaluatedProperties: false >> + additionalProperties: true > > No one proposed to add additionalProperties:true... Does it even work? > I'll remove additionalProperties:true and keep it as false, as it was in the previous version. >> + properties: >> + reg: >> + items: >> + - enum: [0, 1] >> + description: ICSSG PRUETH port number >> + >> + ti,syscon-rgmii-delay: >> + $ref: /schemas/types.yaml#/definitions/phandle-array >> + description: >> + phandle to system controller node and register offset >> + to ICSSG control register for RGMII transmit delay >> + >> + required: >> + - reg >> + >> + ti,mii-g-rt: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: | >> + phandle to MII_G_RT module's syscon regmap. >> + >> + ti,mii-rt: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: | >> + phandle to MII_RT module's syscon regmap >> + >> + interrupts: >> + minItems: 2 >> + maxItems: 2 >> + description: | >> + Interrupt specifiers to TX timestamp IRQ. >> + >> + interrupt-names: >> + items: >> + - const: tx_ts0 >> + - const: tx_ts1 >> + >> +required: >> + - compatible >> + - sram >> + - ti,mii-g-rt >> + - dmas >> + - dma-names >> + - ethernet-ports >> + - interrupts >> + - interrupt-names >> + >> +unevaluatedProperties: false >> + >> +examples: >> + - | >> + >> + /* Example k3-am654 base board SR2.0, dual-emac */ >> + pruss2_eth: pruss2_eth { >> + compatible = "ti,am654-icssg-prueth"; > > Again missed Rob's comment. > I'll keep the indentation in example as 4 for first block, 4+4 for second block and so on. > Really, you ignored four of his comments. Please respect reviewers time > but not forcing them to repeat same review comments. > > Best regards, > Krzysztof > Thanks and Regards, Md Danish Anwar. > From mboxrd@z Thu Jan 1 00:00:00 1970 > Return-Path: <linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@xxxxxxxxxxxxxxxxxxx> > 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 D8E5FC433FE > for <linux-arm-kernel@xxxxxxxxxxxxxxxxxxx>; Tue, 31 May 2022 10:10:18 +0000 (UTC) > DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; > d=lists.infradead.org; s=bombadil.20210309; h=Sender: > Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: > List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To: > Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: > Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: > List-Owner; bh=1a8R2aPtpN8VJ8nnKPmRS52Aa9f7VxdsHDMYjLy3Nec=; b=wIC+W0D4fCGLL2 > d62YYbdS41pfLBGPmRsc7zOtnryxJBMZ+3ranQeCbNLF852PAuSOHzYQlKfDiPG8G4rTileKr2zI4 > tgmrfHwiYz4zFLOrZSK/F0gUKRcXs+ivHNa5dEWipt2UaOnrPjVxJNPa2ExKalwxedTTlzXuOKvRp > lBsupH/BY2Yhwdiy0YGYEQ06wug/wAJrHw6gVZ2A54brkI/Gh0MgQA3DX2vFqtAf+FRLB5o38KzD6 > uRsYf/QKOD1ixkP2Uh+isRMsVK1GJAY1BaxNnfisYDCJ31Y4auPV1TTJ5JsqOPIl+oAc+qQOoWhUt > ZHtAzDsfrPhRteMR8nGA==; > Received: from localhost ([::1] helo=bombadil.infradead.org) > by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) > id 1nvyns-00A97O-Kp; Tue, 31 May 2022 10:08:44 +0000 > Received: from mail-ej1-x629.google.com ([2a00:1450:4864:20::629]) > by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) > id 1nvynp-00A96p-Bi > for linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; Tue, 31 May 2022 10:08:43 +0000 > Received: by mail-ej1-x629.google.com with SMTP id f9so25684073ejc.0 > for <linux-arm-kernel@xxxxxxxxxxxxxxxxxxx>; Tue, 31 May 2022 03:08:40 -0700 (PDT) > DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; > d=linaro.org; s=google; > h=message-id:date:mime-version:user-agent:subject:content-language:to > :cc:references:from:in-reply-to:content-transfer-encoding; > bh=0xEDKtRJ1gFx9yMLHsbYq7kEF2/saJCmbQuGtwp5BAo=; > b=FJlTsAsgLQm8T7cg7ztoR+hVidoRf0V5AWOmRsLnTi0T7R1GvuT2r/FFDmqkIl53A6 > DbHgnZvG+DyTKUwG/vdUmgZmVNF+w7UeZKwmdDVjGozRWvHTYOIaY1SWDxFECXfVzcy0 > LAg5f9EqBC3WEIjvhOnIkvR7P+OsgIhI/OiivIkJBdzRo9jS9E+2yx+Sm8NDltnFTUwA > IYLDY1JDsLGwfIc2HtBk3jWG83NDp0Ea+TiENVqdmzdP+uv3rQqxCeGPQQRIR8kZ+lhz > EsPkUVZzzGoPnArdmSNQGceUzqD1QuOmhiLIlqBDnkgIxv0uDEGSG52wLOF3N4QCWRdj > 7/7A== > X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; > d=1e100.net; s=20210112; > h=x-gm-message-state:message-id:date:mime-version:user-agent:subject > :content-language:to:cc:references:from:in-reply-to > :content-transfer-encoding; > bh=0xEDKtRJ1gFx9yMLHsbYq7kEF2/saJCmbQuGtwp5BAo=; > b=B8P1vdEHTMWSN4xCuq1bRYTxlQJ2WbgTX33+iZ64xwRYFE43op8cqxMLe8ItNeXhOa > ygVqXL+3FCMKHUz2qvOplSkkHDMwKumX+X/ErqhpU2u+Npqyo8EvNGH0toag3qflFryq > GsjzmWphffPt7/1Z/+K5+0dqvy92g9tDtBmNyRTn52i0RoOQ05LlbKBxkJOXEXxNaD/y > l9T9G26QAQaF7kT/gKe2Lrj+kCrTpJ/1yI05qKdrhZdka3xZa6GBIi6VRwJcynxUcxs8 > aJQL70rRY4tfncRNUghETqpjQu8clZdj57R9HaO3LEBfq8uZlFvQVTr9u13RL2twT3aa > 3UAA== > X-Gm-Message-State: AOAM531dEZwptqWX2TF8w5F1B3CdGvkaOesF74IGrBLIeJeoH/LfzFiG > 4a4rxd2QE25vDQvfgJchJ+HUkw== > X-Google-Smtp-Source: ABdhPJxvLTVkRm6gy9yj0yU6UO395ZmuZmGbmyxXWGwAn4IUJBt1uCvPDQ8Lpwmm1nn62gUGkldWYA== > X-Received: by 2002:a17:906:5d0d:b0:6fe:b420:5eab with SMTP id g13-20020a1709065d0d00b006feb4205eabmr45855204ejt.23.1653991719238; > Tue, 31 May 2022 03:08:39 -0700 (PDT) > Received: from [192.168.0.179] (xdsl-188-155-176-92.adslplus.ch. [188.155.176.92]) > by smtp.gmail.com with ESMTPSA id i16-20020a1709063c5000b006fed85c1a72sm4802036ejg.223.2022.05.31.03.08.38 > (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); > Tue, 31 May 2022 03:08:38 -0700 (PDT) > Message-ID: <4ccba38a-ccde-83cd-195b-77db7a64477c@xxxxxxxxxx> > Date: Tue, 31 May 2022 12:08:37 +0200 > MIME-Version: 1.0 > User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 > Thunderbird/91.9.1 > Subject: Re: [PATCH v2 1/2] dt-bindings: net: Add ICSSG Ethernet Driver > bindings > Content-Language: en-US > To: Puranjay Mohan <p-mohan@xxxxxx>, linux-kernel@xxxxxxxxxxxxxxx > Cc: davem@xxxxxxxxxxxxx, edumazet@xxxxxxxxxx, > krzysztof.kozlowski+dt@xxxxxxxxxx, netdev@xxxxxxxxxxxxxxx, > devicetree@xxxxxxxxxxxxxxx, nm@xxxxxx, ssantosh@xxxxxxxxxx, s-anna@xxxxxx, > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx, rogerq@xxxxxxxxxx, > grygorii.strashko@xxxxxx, vigneshr@xxxxxx, kishon@xxxxxx, > robh+dt@xxxxxxxxxx, afd@xxxxxx, andrew@xxxxxxx > References: <20220531095108.21757-1-p-mohan@xxxxxx> > <20220531095108.21757-2-p-mohan@xxxxxx> > From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > In-Reply-To: <20220531095108.21757-2-p-mohan@xxxxxx> > X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 > X-CRM114-CacheID: sfid-20220531_030841_459584_A3DCA991 > X-CRM114-Status: GOOD ( 21.47 ) > X-BeenThere: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > X-Mailman-Version: 2.1.34 > Precedence: list > List-Id: <linux-arm-kernel.lists.infradead.org> > List-Unsubscribe: <http://lists.infradead.org/mailman/options/linux-arm-kernel>, > <mailto:linux-arm-kernel-request@xxxxxxxxxxxxxxxxxxx?subject=unsubscribe> > List-Archive: <http://lists.infradead.org/pipermail/linux-arm-kernel/> > List-Post: <mailto:linux-arm-kernel@xxxxxxxxxxxxxxxxxxx> > List-Help: <mailto:linux-arm-kernel-request@xxxxxxxxxxxxxxxxxxx?subject=help> > List-Subscribe: <http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>, > <mailto:linux-arm-kernel-request@xxxxxxxxxxxxxxxxxxx?subject=subscribe> > Content-Type: text/plain; charset="us-ascii" > Content-Transfer-Encoding: 7bit > Sender: "linux-arm-kernel" <linux-arm-kernel-bounces@xxxxxxxxxxxxxxxxxxx> > Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@xxxxxxxxxxxxxxxxxxx > > On 31/05/2022 11:51, Puranjay Mohan wrote: >> Add a YAML binding document for the ICSSG Programmable real time unit >> based Ethernet driver. This driver uses the PRU and PRUSS consumer APIs >> to interface the PRUs and load/run the firmware for supporting ethernet >> functionality. >> >> Signed-off-by: Puranjay Mohan <p-mohan@xxxxxx> >> --- >> v1: https://lore.kernel.org/all/20220506052433.28087-2-p-mohan@xxxxxx/ >> v1 -> v2: >> * Addressed Rob's Comments > > Nope, they were not addressed. > >> * It includes indentation, formatting, and other minor changes. >> --- >> .../bindings/net/ti,icssg-prueth.yaml | 181 ++++++++++++++++++ >> 1 file changed, 181 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml >> >> diff --git a/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml b/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml >> new file mode 100644 >> index 000000000000..40af968e9178 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml >> @@ -0,0 +1,181 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/net/ti,icssg-prueth.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: |+ > > Missed Rob's comment. > >> + Texas Instruments ICSSG PRUSS Ethernet >> + >> +maintainers: >> + - Puranjay Mohan <p-mohan@xxxxxx> >> + >> +description: >> + Ethernet based on the Programmable Real-Time >> + Unit and Industrial Communication Subsystem. >> + >> +allOf: >> + - $ref: /schemas/remoteproc/ti,pru-consumer.yaml# >> + >> +properties: >> + compatible: >> + enum: >> + - ti,am654-icssg-prueth # for AM65x SoC family >> + >> + pinctrl-0: >> + maxItems: 1 >> + >> + pinctrl-names: >> + items: >> + - const: default > > You do not need these usually, they are coming from schema. > >> + >> + sram: >> + description: >> + phandle to MSMC SRAM node >> + >> + dmas: >> + maxItems: 10 >> + description: >> + list of phandles and specifiers to UDMA. > > Please follow Rob's comment - drop description. > >> + >> + dma-names: >> + items: >> + - const: tx0-0 >> + - const: tx0-1 >> + - const: tx0-2 >> + - const: tx0-3 >> + - const: tx1-0 >> + - const: tx1-1 >> + - const: tx1-2 >> + - const: tx1-3 >> + - const: rx0 >> + - const: rx1 >> + >> + ethernet-ports: >> + type: object >> + properties: >> + '#address-cells': >> + const: 1 >> + '#size-cells': >> + const: 0 >> + >> + patternProperties: >> + ^port@[0-1]$: > > How did you implement Rob's comments here? > >> + type: object >> + description: ICSSG PRUETH external ports >> + >> + $ref: ethernet-controller.yaml# >> + >> + unevaluatedProperties: false >> + additionalProperties: true > > No one proposed to add additionalProperties:true... Does it even work? > >> + properties: >> + reg: >> + items: >> + - enum: [0, 1] >> + description: ICSSG PRUETH port number >> + >> + ti,syscon-rgmii-delay: >> + $ref: /schemas/types.yaml#/definitions/phandle-array >> + description: >> + phandle to system controller node and register offset >> + to ICSSG control register for RGMII transmit delay >> + >> + required: >> + - reg >> + >> + ti,mii-g-rt: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: | >> + phandle to MII_G_RT module's syscon regmap. >> + >> + ti,mii-rt: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: | >> + phandle to MII_RT module's syscon regmap >> + >> + interrupts: >> + minItems: 2 >> + maxItems: 2 >> + description: | >> + Interrupt specifiers to TX timestamp IRQ. >> + >> + interrupt-names: >> + items: >> + - const: tx_ts0 >> + - const: tx_ts1 >> + >> +required: >> + - compatible >> + - sram >> + - ti,mii-g-rt >> + - dmas >> + - dma-names >> + - ethernet-ports >> + - interrupts >> + - interrupt-names >> + >> +unevaluatedProperties: false >> + >> +examples: >> + - | >> + >> + /* Example k3-am654 base board SR2.0, dual-emac */ >> + pruss2_eth: pruss2_eth { >> + compatible = "ti,am654-icssg-prueth"; > > Again missed Rob's comment. > > Really, you ignored four of his comments. Please respect reviewers time > but not forcing them to repeat same review comments. > > Best regards, > Krzysztof > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > >
