[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <41d5952b-51f8-bbc6-2e81-22d6f66320ee@ti.com>
Date: Fri, 4 Nov 2022 12:58:54 +0530
From: Md Danish Anwar <a0501179@...com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
<linux-kernel@...r.kernel.org>
CC: <davem@...emloft.net>, <edumazet@...gle.com>,
<krzysztof.kozlowski+dt@...aro.org>, <netdev@...r.kernel.org>,
<devicetree@...r.kernel.org>, <nm@...com>, <ssantosh@...nel.org>,
<s-anna@...com>, <linux-arm-kernel@...ts.infradead.org>,
<rogerq@...nel.org>, <vigneshr@...com>, <kishon@...com>,
<robh+dt@...nel.org>, <afd@...com>, <andrew@...n.ch>
Subject: Re: [PATCH v2 1/2] dt-bindings: net: Add ICSSG Ethernet Driver
bindings
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@...com>
>> ---
>> v1: https://lore.kernel.org/all/20220506052433.28087-2-p-mohan@ti.com/
>> 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@...com>
>> +
>> +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@...ts.infradead.org>
> 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@...hiver.kernel.org>; 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@...ts.infradead.org; Tue, 31 May 2022 10:08:43 +0000
> Received: by mail-ej1-x629.google.com with SMTP id f9so25684073ejc.0
> for <linux-arm-kernel@...ts.infradead.org>; 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@...aro.org>
> 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@...com>, linux-kernel@...r.kernel.org
> Cc: davem@...emloft.net, edumazet@...gle.com,
> krzysztof.kozlowski+dt@...aro.org, netdev@...r.kernel.org,
> devicetree@...r.kernel.org, nm@...com, ssantosh@...nel.org, s-anna@...com,
> linux-arm-kernel@...ts.infradead.org, rogerq@...nel.org,
> grygorii.strashko@...com, vigneshr@...com, kishon@...com,
> robh+dt@...nel.org, afd@...com, andrew@...n.ch
> References: <20220531095108.21757-1-p-mohan@...com>
> <20220531095108.21757-2-p-mohan@...com>
> From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> In-Reply-To: <20220531095108.21757-2-p-mohan@...com>
> 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@...ts.infradead.org
> 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@...ts.infradead.org?subject=unsubscribe>
> List-Archive: <http://lists.infradead.org/pipermail/linux-arm-kernel/>
> List-Post: <mailto:linux-arm-kernel@...ts.infradead.org>
> List-Help: <mailto:linux-arm-kernel-request@...ts.infradead.org?subject=help>
> List-Subscribe: <http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>,
> <mailto:linux-arm-kernel-request@...ts.infradead.org?subject=subscribe>
> Content-Type: text/plain; charset="us-ascii"
> Content-Transfer-Encoding: 7bit
> Sender: "linux-arm-kernel" <linux-arm-kernel-bounces@...ts.infradead.org>
> Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@...ts.infradead.org
>
> 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@...com>
>> ---
>> v1: https://lore.kernel.org/all/20220506052433.28087-2-p-mohan@ti.com/
>> 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@...com>
>> +
>> +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@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
Powered by blists - more mailing lists