All of lore.kernel.org
 help / color / mirror / Atom feed
From: Conor Dooley <conor@kernel.org>
To: linux-aspeed@lists.ozlabs.org
Subject: [PATCH 30 2/7] Add binding for Aspeed SOC
Date: Wed, 31 Jan 2024 00:32:42 +0000	[thread overview]
Message-ID: <20240131-felt-tip-mountain-e29606b63acf@spud> (raw)
In-Reply-To: <LV8PR11MB8463D0017B82135324127DB98B7D2@LV8PR11MB8463.namprd11.prod.outlook.com>

Hey,

On Tue, Jan 30, 2024 at 11:30:10PM +0000, Corona, Ernesto wrote:
> Aspeed AST2400, AST2500 and AST2600 JTAG controller driver.
> 
> Signed-off-by: Oleksandr Shamray <oleksandrs@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> Signed-off-by: Ernesto Corona <ernesto.corona@intel.com>
> Signed-off-by: Omar Castro <omar.eduardo.castro@linux.intel.com>
> Acked-by: Rob Herring <robh@kernel.org>

Where did this ack come from? The conversion to a yaml binding was in
v29 (according to your changelog) but I don't see the ack from Rob
there.
I think a conversion from (text?) to yaml would be sufficient of a
change to drop his tag.

> v28->v29
> - Change documentation to the new dt-bindings yaml format.

>  .../devicetree/bindings/jtag/aspeed-jtag.yaml | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/jtag/aspeed-jtag.yaml
> 
> diff --git a/Documentation/devicetree/bindings/jtag/aspeed-jtag.yaml b/Documentation/devicetree/bindings/jtag/aspeed-jtag.yaml
> new file mode 100644
> index 000000000000..1a412e83b81b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/jtag/aspeed-jtag.yaml

Filename matching a compatible please.

> @@ -0,0 +1,85 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/jtag/aspeed-jtag.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Aspeed JTAG driver for ast2400, ast2500 and ast2600 SoC
> +
> +description:
> +  Driver adds support of Aspeed 24/25/2600 series SOC JTAG controller.
> +  Driver implements the following jtag ops
> +    freq_get
> +    freq_set
> +    status_get
> +    status_set
> +    xfer
> +    mode_set
> +    bitbang
> +    enable
> +    disable

None of the driver stuff, nor where you tested this, is relevant to the
description in the binding nor the title of the binding. Just describe
the hardware.

> +
> +  It has been tested on Mellanox system with BMC equipped with
> +  Aspeed 2520 SoC for programming CPLD devices.
> +
> +  It has also been tested on Intel system using Aspeed 25xx SoC
> +  for JTAG communication.
> +
> +  Tested on Intel system using Aspeed 26xx SoC for JTAG communication.
> +
> +maintainers:
> +  - Oleksandr Shamray <oleksandrs@mellanox.com>
> +  - Jiri Pirko <jiri@nvidia.com>
> +  - Ernesto Corona<ernesto.corona@intel.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - aspeed,ast2400-jtag
> +              - aspeed,ast2500-jtag
> +              - aspeed,ast2600-jtag

You don't need the "oneOf" or "items" here, it's enough to do:
  compatible:
    enum:
      - allwinner,sun5i-a13-mbus
      - allwinner,sun8i-a33-mbus

> +
> +
> +  reg:
> +    items:
> +      - description: JTAG Master controller register range
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/aspeed-clock.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +      jtag: jtag at 1e6e4000 {

The labels for both of these examples are also not needed
as they're not used anywhere.

Thanks,
Conor.

> +          compatible = "aspeed,ast2500-jtag";
> +          reg = <0x1e6e4000 0x1c>;
> +          clocks = <&syscon ASPEED_CLK_APB>;
> +          resets = <&syscon ASPEED_RESET_JTAG_MASTER>;
> +          interrupts = <43>;
> +      };
> +  - |
> +    #include <dt-bindings/clock/aspeed-clock.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +      jtag1: jtag at 1e6e4100 {
> +          compatible = "aspeed,ast2600-jtag";
> +          reg = <0x1e6e4100 0x40>;
> +          clocks = <&syscon ASPEED_CLK_APB1>;
> +          resets = <&syscon ASPEED_RESET_JTAG_MASTER2>;
> +          interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
> +      };
> +
> +...
> -- 
> 2.25.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20240131/cbc87905/attachment.sig>

WARNING: multiple messages have this Message-ID (diff)
From: Conor Dooley <conor@kernel.org>
To: "Corona, Ernesto" <ernesto.corona@intel.com>
Cc: "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>,
	"'oleksandrs@mellanox.com'" <oleksandrs@mellanox.com>,
	"'jiri@nvidia.com'" <jiri@nvidia.com>,
	"Castro, Omar Eduardo" <omar.eduardo.castro@intel.com>,
	"'omar.eduardo.castro@linux.intel.com'"
	<omar.eduardo.castro@linux.intel.com>,
	"'robh@kernel.org'" <robh@kernel.org>,
	"'corbet@lwn.net'" <corbet@lwn.net>,
	"'mchehab+samsung@kernel.org'" <mchehab+samsung@kernel.org>,
	"'alexandre.belloni@bootlin.com'" <alexandre.belloni@bootlin.com>,
	"'tytso@mit.edu'" <tytso@mit.edu>,
	"'arnd@arndb.de'" <arnd@arndb.de>,
	"'ebiggers@google.com'" <ebiggers@google.com>,
	"'mark.rutland@arm.com'" <mark.rutland@arm.com>,
	"'joel@jms.id.au'" <joel@jms.id.au>,
	"'andrew@aj.id.au'" <andrew@aj.id.au>,
	"Filary, Steven A" <steven.a.filary@intel.com>,
	"'vadimp@mellanox.com'" <vadimp@mellanox.com>,
	"'amithash@fb.com'" <amithash@fb.com>,
	"'patrickw3@fb.com'" <patrickw3@fb.com>,
	"Chen, Luke" <luke_chen@aspeedtech.com>,
	"'billy_tsai@aspeedtech.com'" <billy_tsai@aspeedtech.com>,
	"'rgrs@protonmail.com'" <rgrs@protonmail.com>
Subject: Re: [PATCH 30 2/7] Add binding for Aspeed SOC
Date: Wed, 31 Jan 2024 00:32:42 +0000	[thread overview]
Message-ID: <20240131-felt-tip-mountain-e29606b63acf@spud> (raw)
In-Reply-To: <LV8PR11MB8463D0017B82135324127DB98B7D2@LV8PR11MB8463.namprd11.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 4108 bytes --]

Hey,

On Tue, Jan 30, 2024 at 11:30:10PM +0000, Corona, Ernesto wrote:
> Aspeed AST2400, AST2500 and AST2600 JTAG controller driver.
> 
> Signed-off-by: Oleksandr Shamray <oleksandrs@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> Signed-off-by: Ernesto Corona <ernesto.corona@intel.com>
> Signed-off-by: Omar Castro <omar.eduardo.castro@linux.intel.com>
> Acked-by: Rob Herring <robh@kernel.org>

Where did this ack come from? The conversion to a yaml binding was in
v29 (according to your changelog) but I don't see the ack from Rob
there.
I think a conversion from (text?) to yaml would be sufficient of a
change to drop his tag.

> v28->v29
> - Change documentation to the new dt-bindings yaml format.

>  .../devicetree/bindings/jtag/aspeed-jtag.yaml | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/jtag/aspeed-jtag.yaml
> 
> diff --git a/Documentation/devicetree/bindings/jtag/aspeed-jtag.yaml b/Documentation/devicetree/bindings/jtag/aspeed-jtag.yaml
> new file mode 100644
> index 000000000000..1a412e83b81b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/jtag/aspeed-jtag.yaml

Filename matching a compatible please.

> @@ -0,0 +1,85 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/jtag/aspeed-jtag.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Aspeed JTAG driver for ast2400, ast2500 and ast2600 SoC
> +
> +description:
> +  Driver adds support of Aspeed 24/25/2600 series SOC JTAG controller.
> +  Driver implements the following jtag ops
> +    freq_get
> +    freq_set
> +    status_get
> +    status_set
> +    xfer
> +    mode_set
> +    bitbang
> +    enable
> +    disable

None of the driver stuff, nor where you tested this, is relevant to the
description in the binding nor the title of the binding. Just describe
the hardware.

> +
> +  It has been tested on Mellanox system with BMC equipped with
> +  Aspeed 2520 SoC for programming CPLD devices.
> +
> +  It has also been tested on Intel system using Aspeed 25xx SoC
> +  for JTAG communication.
> +
> +  Tested on Intel system using Aspeed 26xx SoC for JTAG communication.
> +
> +maintainers:
> +  - Oleksandr Shamray <oleksandrs@mellanox.com>
> +  - Jiri Pirko <jiri@nvidia.com>
> +  - Ernesto Corona<ernesto.corona@intel.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - aspeed,ast2400-jtag
> +              - aspeed,ast2500-jtag
> +              - aspeed,ast2600-jtag

You don't need the "oneOf" or "items" here, it's enough to do:
  compatible:
    enum:
      - allwinner,sun5i-a13-mbus
      - allwinner,sun8i-a33-mbus

> +
> +
> +  reg:
> +    items:
> +      - description: JTAG Master controller register range
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/aspeed-clock.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +      jtag: jtag@1e6e4000 {

The labels for both of these examples are also not needed
as they're not used anywhere.

Thanks,
Conor.

> +          compatible = "aspeed,ast2500-jtag";
> +          reg = <0x1e6e4000 0x1c>;
> +          clocks = <&syscon ASPEED_CLK_APB>;
> +          resets = <&syscon ASPEED_RESET_JTAG_MASTER>;
> +          interrupts = <43>;
> +      };
> +  - |
> +    #include <dt-bindings/clock/aspeed-clock.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +      jtag1: jtag@1e6e4100 {
> +          compatible = "aspeed,ast2600-jtag";
> +          reg = <0x1e6e4100 0x40>;
> +          clocks = <&syscon ASPEED_CLK_APB1>;
> +          resets = <&syscon ASPEED_RESET_JTAG_MASTER2>;
> +          interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
> +      };
> +
> +...
> -- 
> 2.25.1

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Conor Dooley <conor@kernel.org>
To: "Corona, Ernesto" <ernesto.corona@intel.com>
Cc: "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>,
	"'oleksandrs@mellanox.com'" <oleksandrs@mellanox.com>,
	"'jiri@nvidia.com'" <jiri@nvidia.com>,
	"Castro, Omar Eduardo" <omar.eduardo.castro@intel.com>,
	"'omar.eduardo.castro@linux.intel.com'"
	<omar.eduardo.castro@linux.intel.com>,
	"'robh@kernel.org'" <robh@kernel.org>,
	"'corbet@lwn.net'" <corbet@lwn.net>,
	"'mchehab+samsung@kernel.org'" <mchehab+samsung@kernel.org>,
	"'alexandre.belloni@bootlin.com'" <alexandre.belloni@bootlin.com>,
	"'tytso@mit.edu'" <tytso@mit.edu>,
	"'arnd@arndb.de'" <arnd@arndb.de>,
	"'ebiggers@google.com'" <ebiggers@google.com>,
	"'mark.rutland@arm.com'" <mark.rutland@arm.com>,
	"'joel@jms.id.au'" <joel@jms.id.au>,
	"'andrew@aj.id.au'" <andrew@aj.id.au>,
	"Filary, Steven A" <steven.a.filary@intel.com>,
	"'vadimp@mellanox.com'" <vadimp@mellanox.com>,
	"'amithash@fb.com'" <amithash@fb.com>,
	"'patrickw3@fb.com'" <patrickw3@fb.com>,
	"Chen, Luke" <luke_chen@aspeedtech.com>,
	"'billy_tsai@aspeedtech.com'" <billy_tsai@aspeedtech.com>,
	"'rgrs@protonmail.com'" <rgrs@protonmail.com>
Subject: Re: [PATCH 30 2/7] Add binding for Aspeed SOC
Date: Wed, 31 Jan 2024 00:32:42 +0000	[thread overview]
Message-ID: <20240131-felt-tip-mountain-e29606b63acf@spud> (raw)
In-Reply-To: <LV8PR11MB8463D0017B82135324127DB98B7D2@LV8PR11MB8463.namprd11.prod.outlook.com>


[-- Attachment #1.1: Type: text/plain, Size: 4108 bytes --]

Hey,

On Tue, Jan 30, 2024 at 11:30:10PM +0000, Corona, Ernesto wrote:
> Aspeed AST2400, AST2500 and AST2600 JTAG controller driver.
> 
> Signed-off-by: Oleksandr Shamray <oleksandrs@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> Signed-off-by: Ernesto Corona <ernesto.corona@intel.com>
> Signed-off-by: Omar Castro <omar.eduardo.castro@linux.intel.com>
> Acked-by: Rob Herring <robh@kernel.org>

Where did this ack come from? The conversion to a yaml binding was in
v29 (according to your changelog) but I don't see the ack from Rob
there.
I think a conversion from (text?) to yaml would be sufficient of a
change to drop his tag.

> v28->v29
> - Change documentation to the new dt-bindings yaml format.

>  .../devicetree/bindings/jtag/aspeed-jtag.yaml | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/jtag/aspeed-jtag.yaml
> 
> diff --git a/Documentation/devicetree/bindings/jtag/aspeed-jtag.yaml b/Documentation/devicetree/bindings/jtag/aspeed-jtag.yaml
> new file mode 100644
> index 000000000000..1a412e83b81b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/jtag/aspeed-jtag.yaml

Filename matching a compatible please.

> @@ -0,0 +1,85 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/jtag/aspeed-jtag.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Aspeed JTAG driver for ast2400, ast2500 and ast2600 SoC
> +
> +description:
> +  Driver adds support of Aspeed 24/25/2600 series SOC JTAG controller.
> +  Driver implements the following jtag ops
> +    freq_get
> +    freq_set
> +    status_get
> +    status_set
> +    xfer
> +    mode_set
> +    bitbang
> +    enable
> +    disable

None of the driver stuff, nor where you tested this, is relevant to the
description in the binding nor the title of the binding. Just describe
the hardware.

> +
> +  It has been tested on Mellanox system with BMC equipped with
> +  Aspeed 2520 SoC for programming CPLD devices.
> +
> +  It has also been tested on Intel system using Aspeed 25xx SoC
> +  for JTAG communication.
> +
> +  Tested on Intel system using Aspeed 26xx SoC for JTAG communication.
> +
> +maintainers:
> +  - Oleksandr Shamray <oleksandrs@mellanox.com>
> +  - Jiri Pirko <jiri@nvidia.com>
> +  - Ernesto Corona<ernesto.corona@intel.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - aspeed,ast2400-jtag
> +              - aspeed,ast2500-jtag
> +              - aspeed,ast2600-jtag

You don't need the "oneOf" or "items" here, it's enough to do:
  compatible:
    enum:
      - allwinner,sun5i-a13-mbus
      - allwinner,sun8i-a33-mbus

> +
> +
> +  reg:
> +    items:
> +      - description: JTAG Master controller register range
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/aspeed-clock.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +      jtag: jtag@1e6e4000 {

The labels for both of these examples are also not needed
as they're not used anywhere.

Thanks,
Conor.

> +          compatible = "aspeed,ast2500-jtag";
> +          reg = <0x1e6e4000 0x1c>;
> +          clocks = <&syscon ASPEED_CLK_APB>;
> +          resets = <&syscon ASPEED_RESET_JTAG_MASTER>;
> +          interrupts = <43>;
> +      };
> +  - |
> +    #include <dt-bindings/clock/aspeed-clock.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +      jtag1: jtag@1e6e4100 {
> +          compatible = "aspeed,ast2600-jtag";
> +          reg = <0x1e6e4100 0x40>;
> +          clocks = <&syscon ASPEED_CLK_APB1>;
> +          resets = <&syscon ASPEED_RESET_JTAG_MASTER2>;
> +          interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
> +      };
> +
> +...
> -- 
> 2.25.1

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2024-01-31  0:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-30 23:30 [PATCH 30 2/7] Add binding for Aspeed SOC Corona, Ernesto
2024-01-30 23:30 ` Corona, Ernesto
2024-01-30 23:30 ` Corona, Ernesto
2024-01-31  0:32 ` Conor Dooley [this message]
2024-01-31  0:32   ` Conor Dooley
2024-01-31  0:32   ` Conor Dooley
2024-01-31  8:18 ` Krzysztof Kozlowski
2024-01-31  8:18   ` Krzysztof Kozlowski
2024-01-31  8:18   ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240131-felt-tip-mountain-e29606b63acf@spud \
    --to=conor@kernel.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.