Linux-remoteproc Archive mirror
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: abdellatif.elkhlifi@arm.com,
	Bjorn Andersson <andersson@kernel.org>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Rob Herring <robh+dt@kernel.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Drew.Reed@arm.com, Adam.Johnston@arm.com,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-remoteproc@vger.kernel.org
Subject: Re: [PATCH 3/3] dt-bindings: remoteproc: Add Arm remoteproc
Date: Wed, 13 Mar 2024 19:59:00 +0000	[thread overview]
Message-ID: <8c784016-9257-4d8a-b956-a0a406746c76@arm.com> (raw)
In-Reply-To: <20240301164227.339208-4-abdellatif.elkhlifi@arm.com>

On 2024-03-01 4:42 pm, abdellatif.elkhlifi@arm.com wrote:
> From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> 
> introduce the bindings for Arm remoteproc support.
> 
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> ---
>   .../bindings/remoteproc/arm,rproc.yaml        | 69 +++++++++++++++++++
>   MAINTAINERS                                   |  1 +
>   2 files changed, 70 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/remoteproc/arm,rproc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/arm,rproc.yaml b/Documentation/devicetree/bindings/remoteproc/arm,rproc.yaml
> new file mode 100644
> index 000000000000..322197158059
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/arm,rproc.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/arm,rproc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Arm Remoteproc Devices
> +
> +maintainers:
> +  - Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> +
> +description: |
> +  Some Arm heterogeneous System-On-Chips feature remote processors that can
> +  be controlled with a reset control register and a reset status register to
> +  start or stop the processor.
> +
> +  This document defines the bindings for these remote processors.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - arm,corstone1000-extsys
> +
> +  reg:
> +    minItems: 2
> +    maxItems: 2
> +    description: |
> +      Address and size in bytes of the reset control register
> +      and the reset status register.
> +      Expects the registers to be in the order as above.
> +      Should contain an entry for each value in 'reg-names'.
> +
> +  reg-names:
> +    description: |
> +      Required names for each of the reset registers defined in
> +      the 'reg' property. Expects the names from the following
> +      list, in the specified order, each representing the corresponding
> +      reset register.
> +    items:
> +      - const: reset-control
> +      - const: reset-status
> +
> +  firmware-name:
> +    description: |
> +      Default name of the firmware to load to the remote processor.

So... is loading the firmware image achieved by somehow bitbanging it 
through the one reset register, maybe? I find it hard to believe this is 
a complete and functional binding.

Frankly at the moment I'd be inclined to say it isn't even a remoteproc 
binding (or driver) at all, it's a reset controller. Bindings are a 
contract for describing the hardware, not the current state of Linux 
driver support - if this thing still needs mailboxes, shared memory, a 
reset vector register, or whatever else to actually be useful, those 
should be in the binding from day 1 so that a) people can write and 
deploy correct DTs now, such that functionality becomes available on 
their systems as soon as driver support catches up, and b) the community 
has any hope of being able to review whether the binding is 
appropriately designed and specified for the purpose it intends to serve.

For instance right now it seems somewhat tenuous to describe two 
consecutive 32-bit registers as separate "reg" entries, but *maybe* it's 
OK if that's all there ever is. However if it's actually going to end up 
needing several more additional MMIO and/or memory regions for other 
functionality, then describing each register and location individually 
is liable to get unmanageable really fast, and a higher-level functional 
grouping (e.g. these reset-related registers together as a single 8-byte 
region) would likely be a better design.

Thanks,
Robin.

> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - firmware-name
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    extsys0: remoteproc@1a010310 {
> +            compatible = "arm,corstone1000-extsys";
> +            reg = <0x1a010310 0x4>, <0x1a010314 0x4>;
> +            reg-names = "reset-control", "reset-status";
> +            firmware-name = "es0_flashfw.elf";
> +    };
> +
> +    extsys1: remoteproc@1a010318 {
> +            compatible = "arm,corstone1000-extsys";
> +            reg = <0x1a010318 0x4>, <0x1a01031c 0x4>;
> +            reg-names = "reset-control", "reset-status";
> +            firmware-name = "es1_flashfw.elf";
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 54d6a40feea5..eddaa3841a65 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1768,6 +1768,7 @@ ARM REMOTEPROC DRIVER
>   M:	Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
>   L:	linux-remoteproc@vger.kernel.org
>   S:	Maintained
> +F:	Documentation/devicetree/bindings/remoteproc/arm,rproc.yaml
>   F:	drivers/remoteproc/arm_rproc.c
>   
>   ARM SMC WATCHDOG DRIVER

  parent reply	other threads:[~2024-03-13 19:59 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-01 16:42 [PATCH 0/3] remoteproc: introduce Arm remoteproc support abdellatif.elkhlifi
2024-03-01 16:42 ` [PATCH 1/3] remoteproc: Add Arm remoteproc driver abdellatif.elkhlifi
2024-03-04 18:30   ` Rob Herring
2024-03-04 18:42   ` Mathieu Poirier
2024-03-07 19:40     ` Abdellatif El Khlifi
2024-03-08 16:44       ` Mathieu Poirier
2024-03-11 11:44         ` Abdellatif El Khlifi
2024-03-12 16:29           ` Mathieu Poirier
2024-03-12 17:32             ` Abdellatif El Khlifi
2024-03-13 16:25               ` Mathieu Poirier
2024-03-13 17:17                 ` Abdellatif El Khlifi
2024-03-14 14:52                   ` Mathieu Poirier
2024-03-14 14:59                     ` Sudeep Holla
2024-03-14 15:16                       ` Abdellatif El Khlifi
2024-03-14 15:24                         ` Sudeep Holla
2024-03-14 16:29                       ` Mathieu Poirier
2024-03-25 17:13                         ` Abdellatif El Khlifi
2024-03-26 14:20                           ` Mathieu Poirier
2024-03-26 17:14                             ` Abdellatif El Khlifi
2024-08-22 17:09                             ` [PATCH v2 0/5] remoteproc: arm64: Introduce remoteproc support for Corstone-1000 External Systems Abdellatif El Khlifi
2024-08-22 17:09                               ` [PATCH v2 1/5] dt-bindings: remoteproc: sse710: Add the External Systems remote processors Abdellatif El Khlifi
2024-08-22 18:25                                 ` Rob Herring (Arm)
2024-08-23  6:23                                 ` Krzysztof Kozlowski
2024-09-19  9:35                                   ` Abdellatif El Khlifi
2024-09-19 10:04                                     ` Krzysztof Kozlowski
2024-09-19 14:57                                       ` Abdellatif El Khlifi
2024-09-20 12:42                                         ` Krzysztof Kozlowski
2024-09-20 14:19                                           ` Abdellatif El Khlifi
2024-09-20 14:56                                             ` Krzysztof Kozlowski
2024-09-20 16:38                                               ` Abdellatif El Khlifi
2024-09-21 18:20                                                 ` Krzysztof Kozlowski
2024-09-23 11:49                                                   ` Abdellatif El Khlifi
2024-09-23 15:29                                                     ` Krzysztof Kozlowski
2024-09-23 17:19                                                       ` Abdellatif El Khlifi
2024-09-27  7:57                                                         ` Krzysztof Kozlowski
2024-09-22 18:58                                     ` Krzysztof Kozlowski
2024-09-27 17:54                                 ` Robin Murphy
2024-10-09  9:46                                   ` Abdellatif El Khlifi
2024-08-22 17:09                               ` [PATCH v2 2/5] dt-bindings: arm: sse710: Add Host Base System Control Abdellatif El Khlifi
2024-08-23  6:25                                 ` Krzysztof Kozlowski
2024-08-22 17:09                               ` [PATCH v2 3/5] arm64: dts: corstone1000: Add MHU nodes used by the External System Abdellatif El Khlifi
2024-08-22 17:09                               ` [PATCH v2 4/5] arm64: dts: corstone1000: Add External System support Abdellatif El Khlifi
2024-08-22 17:09                               ` [PATCH v2 5/5] remoteproc: arm64: corstone1000: Add the External Systems driver Abdellatif El Khlifi
2024-09-18 15:40                                 ` Abdellatif El Khlifi
2024-09-19  8:37                                   ` Mathieu Poirier
2024-03-01 16:42 ` [PATCH 2/3] arm64: dts: Add corstone1000 external system device node abdellatif.elkhlifi
2024-03-01 19:27   ` Krzysztof Kozlowski
2024-03-08 12:21   ` Sudeep Holla
2024-03-08 14:25     ` Abdellatif El Khlifi
2024-03-01 16:42 ` [PATCH 3/3] dt-bindings: remoteproc: Add Arm remoteproc abdellatif.elkhlifi
2024-03-01 19:30   ` Krzysztof Kozlowski
2024-03-08 12:29     ` Sudeep Holla
2024-03-08 13:54       ` Abdellatif El Khlifi
2024-03-13 19:59   ` Robin Murphy [this message]
2024-03-14 13:49     ` Abdellatif El Khlifi
2024-03-14 13:56       ` Krzysztof Kozlowski
2024-03-14 15:20         ` Abdellatif El Khlifi
2024-03-14 15:19       ` Sudeep Holla
2024-03-15 14:22         ` Abdellatif El Khlifi

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=8c784016-9257-4d8a-b956-a0a406746c76@arm.com \
    --to=robin.murphy@arm.com \
    --cc=Adam.Johnston@arm.com \
    --cc=Drew.Reed@arm.com \
    --cc=abdellatif.elkhlifi@arm.com \
    --cc=andersson@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=liviu.dudau@arm.com \
    --cc=lpieralisi@kernel.org \
    --cc=mathieu.poirier@linaro.org \
    --cc=robh+dt@kernel.org \
    --cc=sudeep.holla@arm.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).