linux-kernel - Re: [STLinux Kernel] [PATCH v2 2/4] remoteproc: dt: Provide bindings for ST's Remote Processor Controller driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 1 Sep 2015 09:58:25 +0100
From:	Peter Griffin <peter.griffin@...aro.org>
To:	Lee Jones <lee.jones@...aro.org>
Cc:	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	ohad@...ery.com, devicetree@...r.kernel.org, kernel@...inux.com,
	Nathan_Lynch@...tor.com
Subject: Re: [STLinux Kernel] [PATCH v2 2/4] remoteproc: dt: Provide bindings
 for ST's Remote Processor Controller driver

Hi Lee,

On Fri, 28 Aug 2015, Lee Jones wrote:

> Signed-off-by: Ludovic Barre <ludovic.barre@...com>
> Signed-off-by: Lee Jones <lee.jones@...aro.org>
> ---
>  .../devicetree/bindings/remoteproc/st-rproc.txt    | 35 ++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/st-rproc.txt

The patch documening the DT bindings should be ordered before the patch which adds
the DT node to aid reviewing.
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/st-rproc.txt b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt
> new file mode 100644
> index 0000000..fbd7d78
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt
> @@ -0,0 +1,35 @@
> +STMicroelectronics Remote Processor
> +-----------------------------------
> +
> +This binding provides support for adjunct processors found on ST SoCs.
> +
> +The remote processors can be controlled from the bootloader or the primary OS.
> +If the bootloader starts a remote processor processor the primary OS must detect
> +its state and act accordingly.
> +
> +Required properties:
> +- compatible		Should be one of:
> +				"st,st231-rproc"
> +				"st,st40-rproc"

st40-proc isn't used anywhere. The stih407 doesn't have a ST40 copro, and
looking in the vendor tree remoteproc support isn't present for stih415/6 which
are the the only upstream SoC's to have a ST40 co-pro.

So I think st40-rproc support can be removed.

> +- reg			Size and length of reserved co-processor memory
> +- resets		Reset lines (See: ../reset/reset.txt)
> +- reset-names		Must be "sw_reset" and "pwr_reset"

pwr_reset isn't used by any of the st231 co-processors. It seems to
be related to ST40 support which I don't think is required upstream.
Removing it would make the driver a fair bit smaller.

> +- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
> +- clock-names		Must be "rproc_clk"

I can't see any co-pro which uses more than one clock, so clock-names looks
superflous.

> +- clock-frequency	Clock frequency to set co-processor at if the bootloader
> +			hasn't already done so
> +- st,syscfg-boot	The register that holds the boot vector for the co-processor

I would prefer to see this binding match how most other sti drivers reference syscfg
registers which is: -

st,syscfg       = <&syscfg_core 0xf4>;

Description: phandle of sysconfig bank plus integer array containing register offsets.

It also means it is easily extendable if more than one syscfg register is
required in the future to boot a co-pro.

regards,

Peter.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/