2023-09-13 13:33:58

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v6 1/4] dt-bindings: remoteproc: k3-m4f: Add K3 AM64x SoCs

On 13/09/2023 13:16, Hari Nagalla wrote:
> K3 AM64x SoC has a Cortex M4F subsystem in the MCU voltage domain.
> The remote processor's life cycle management and IPC mechanisms are
> similar across the R5F and M4F cores from remote processor driver
> point of view. However, there are subtle differences in image loading
> and starting the M4F subsystems.
>
> The YAML binding document provides the various node properties to be
> configured by the consumers of the M4F subsystem.
>
> Signed-off-by: Martyn Welch <[email protected]>
> Signed-off-by: Hari Nagalla <[email protected]>
> ---
> Changes since v1:
> - Spelling corrections
> - Corrected to pass DT checks
>
> Changes since v2:
> - Missed spelling correction to commit message
>
> Changes since v3:
> - Removed unnecessary descriptions and used generic memory region names
> - Made mboxes and memory-region optional
> - Removed unrelated items from examples
>
> Changes since v4:
> - Rebased to the latest kernel-next tree
> - Added optional sram memory region for m4f device node
>
> Changes since v5:
> - None

Hm, why none? There were errors in the binding to which you did not
respond. Did you just ignore them?

>
> .../bindings/remoteproc/ti,k3-m4f-rproc.yaml | 136 ++++++++++++++++++
> 1 file changed, 136 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
> new file mode 100644
> index 000000000000..21b7f14d9dc4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
> @@ -0,0 +1,136 @@
> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/ti,k3-m4f-rproc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI K3 M4F processor subsystems
> +
> +maintainers:
> + - Hari Nagalla <[email protected]>
> + - Mathieu Poirier <[email protected]>

Are you sure Mathieu has this device and is a maintainer of this device?

> +
> +description: |
> + Some K3 family SoCs have Arm Cortex M4F cores. AM64x is a SoC in K3
> + family with a M4F core. Typically safety oriented applications may use
> + the M4F core in isolation without an IPC. Where as some industrial and
> + home automation applications, may use the M4F core as a remote processor
> + with IPC communications.
> +
> +$ref: /schemas/arm/keystone/ti,k3-sci-common.yaml#
> +
> +properties:
> +

Drop blank line.

> + compatible:
> + enum:
> + - ti,am64-m4fss
> +
> + power-domains:
> + maxItems: 1
> +
> + "#address-cells":
> + const: 2
> +
> + "#size-cells":
> + const: 2
> +
> + reg:
> + items:
> + - description: IRAM internal memory region
> + - description: DRAM internal memory region
> +
> + reg-names:
> + items:
> + - const: iram
> + - const: dram
> +
> + resets:
> + maxItems: 1
> +
> + firmware-name:
> + $ref: /schemas/types.yaml#/definitions/string

Wrong type. This is an array. You need maxItems instead.

> + description: Name of firmware to load for the M4F core
> +
> + mboxes:
> + description: |
> + Mailbox specifier denoting the sub-mailbox, to be used for communication
> + with the remote processor. This property should match with the
> + sub-mailbox node used in the firmware image.
> + maxItems: 2

You need to describe the items instead.

> +
> + memory-region:
> + description: |
> + phandle to the reserved memory nodes to be associated with the
> + remoteproc device. The reserved memory nodes should be carveout nodes,
> + and should be defined with a "no-map" property as per the bindings in
> + Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> + Optional memory regions available for firmware specific purposes.
> + maxItems: 8
> + items:
> + - description: regions used for DMA allocations like vrings, vring buffers
> + and memory dedicated to firmware's specific purposes.
> + additionalItems: true


Best regards,
Krzysztof


2023-09-13 19:12:51

by Hari Nagalla

[permalink] [raw]
Subject: Re: [PATCH v6 1/4] dt-bindings: remoteproc: k3-m4f: Add K3 AM64x SoCs

On 9/13/23 06:32, Krzysztof Kozlowski wrote:
>> - Removed unrelated items from examples
>>
>> Changes since v4:
>> - Rebased to the latest kernel-next tree
>> - Added optional sram memory region for m4f device node
>>
>> Changes since v5:
>> - None
> Hm, why none? There were errors in the binding to which you did not
> respond. Did you just ignore them?
>
I do not see any errors in my builds. Am i missing something? Please
excuse my lack of knowledge here. Thought the bot errors were outside of
the patch submitted
(Documentation/devicetree/bindings/dma/stericsson,dma40.yaml).
Appreciate your kind inputs..

$ make -j`nproc` ARCH=arm64 V=1 CROSS_COMPILE=aarch64-none-linux-gnu-
DT_CHEKCER_FLAGS=-m dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
make --no-print-directory -C /home/a0868345/temp/linux-next \
-f /home/a0868345/temp/linux-next/Makefile dt_binding_check
make -f ./scripts/Makefile.build obj=scripts/basic
make -f ./scripts/Makefile.build obj=scripts/dtc
make -f ./scripts/Makefile.build obj=Documentation/devicetree/bindings
# LINT Documentation/devicetree/bindings
(find ./Documentation/devicetree/bindings \( -name '*.yaml' ! -name
'processed-schema*' \) | grep -F -e
"Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml" |
xargs -n200 -P$(nproc) /home/a0868345/.local/bin/yamllint -f parsable -c
./Documentation/devicetree/bindings/.yamllint >&2) || true
# DTEX
Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.example.dts
dt-extract-example
Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml >
Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.example.dts
# CHKDT Documentation/devicetree/bindings/processed-schema.json
(find ./Documentation/devicetree/bindings \( -name '*.yaml' ! -name
'processed-schema*' \) | grep -F -e
"Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml" |
xargs -n200 -P$(nproc) dt-doc-validate -u
./Documentation/devicetree/bindings) || true
# SCHEMA Documentation/devicetree/bindings/processed-schema.json
f=$(mktemp) ; find ./Documentation/devicetree/bindings \( -name
'*.yaml' ! -name 'processed-schema*' \) > $f ; dt-mk-schema -j @$f >
Documentation/devicetree/bindings/processed-schema.json ; rm -f $f
# DTC_CHK
Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.example.dtb
gcc -E
-Wp,-MMD,Documentation/devicetree/bindings/remoteproc/.ti_k3-m4f-rproc.example.dtb.d.pre.tmp
-nostdinc -I./scripts/dtc/include-prefixes -undef -D__DTS__ -x
assembler-with-cpp -o
Documentation/devicetree/bindings/remoteproc/.ti_k3-m4f-rproc.example.dtb.dts.tmp
Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.example.dts
; ./scripts/dtc/dtc -o
Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.example.dtb
-b 0 -iDocumentation/devicetree/bindings/remoteproc/
-i./scripts/dtc/include-prefixes -Wno-avoid_unnecessary_addr_size
-Wno-graph_child_address -Wno-interrupt_provider
-Wno-unique_unit_address -Wunique_unit_address_if_enabled -d
Documentation/devicetree/bindings/remoteproc/.ti_k3-m4f-rproc.example.dtb.d.dtc.tmp
Documentation/devicetree/bindings/remoteproc/.ti_k3-m4f-rproc.example.dtb.dts.tmp
; cat
Documentation/devicetree/bindings/remoteproc/.ti_k3-m4f-rproc.example.dtb.d.pre.tmp
Documentation/devicetree/bindings/remoteproc/.ti_k3-m4f-rproc.example.dtb.d.dtc.tmp
>
Documentation/devicetree/bindings/remoteproc/.ti_k3-m4f-rproc.example.dtb.d
; dt-validate -u ./Documentation/devicetree/bindings -p
./Documentation/devicetree/bindings/processed-schema.json
Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.example.dtb
|| true


>> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id:http://devicetree.org/schemas/remoteproc/ti,k3-m4f-rproc.yaml#
>> +$schema:http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: TI K3 M4F processor subsystems
>> +
>> +maintainers:
>> + - Hari Nagalla<[email protected]>
>> + - Mathieu Poirier<[email protected]>
> Are you sure Mathieu has this device and is a maintainer of this device?
>
Earlier, Mathieu suggested he can be the maintainer. Beagle play is
based on AM625 device.

I will look into the other comments for the 'ti,k3-m4f-rproc.yaml'
binding doc.

Thanks

2023-09-13 21:02:24

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v6 1/4] dt-bindings: remoteproc: k3-m4f: Add K3 AM64x SoCs

On 13/09/2023 15:59, Hari Nagalla wrote:
> On 9/13/23 06:32, Krzysztof Kozlowski wrote:
>>> - Removed unrelated items from examples
>>>
>>> Changes since v4:
>>> - Rebased to the latest kernel-next tree
>>> - Added optional sram memory region for m4f device node
>>>
>>> Changes since v5:
>>> - None
>> Hm, why none? There were errors in the binding to which you did not
>> respond. Did you just ignore them?
>>
> I do not see any errors in my builds. Am i missing something? Please
> excuse my lack of knowledge here. Thought the bot errors were outside of
> the patch submitted
> (Documentation/devicetree/bindings/dma/stericsson,dma40.yaml).
> Appreciate your kind inputs..

I would expect then some confirmation that errors can be ignored.
Instead report was left just unanswered.

>
> $ make -j`nproc` ARCH=arm64 V=1 CROSS_COMPILE=aarch64-none-linux-gnu-
> DT_CHEKCER_FLAGS=-m dt_binding_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml
> make --no-print-directory -C /home/a0868345/temp/linux-next \
> -f /home/a0868345/temp/linux-next/Makefile dt_binding_check
> make -f ./scripts/Makefile.build obj=scripts/basic
> make -f ./scripts/Makefile.build obj=scripts/dtc
> make -f ./scripts/Makefile.build obj=Documentation/devicetree/bindings
> # LINT Documentation/devicetree/bindings
> (find ./Documentation/devicetree/bindings \( -name '*.yaml' ! -name
> 'processed-schema*' \) | grep -F -e
> "Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml" |
> xargs -n200 -P$(nproc) /home/a0868345/.local/bin/yamllint -f parsable -c
> ./Documentation/devicetree/bindings/.yamllint >&2) || true
> # DTEX
> Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.example.dts
> dt-extract-example
> Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml >
> Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.example.dts
> # CHKDT Documentation/devicetree/bindings/processed-schema.json
> (find ./Documentation/devicetree/bindings \( -name '*.yaml' ! -name
> 'processed-schema*' \) | grep -F -e
> "Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.yaml" |
> xargs -n200 -P$(nproc) dt-doc-validate -u
> ./Documentation/devicetree/bindings) || true
> # SCHEMA Documentation/devicetree/bindings/processed-schema.json
> f=$(mktemp) ; find ./Documentation/devicetree/bindings \( -name
> '*.yaml' ! -name 'processed-schema*' \) > $f ; dt-mk-schema -j @$f >
> Documentation/devicetree/bindings/processed-schema.json ; rm -f $f
> # DTC_CHK
> Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.example.dtb
> gcc -E
> -Wp,-MMD,Documentation/devicetree/bindings/remoteproc/.ti_k3-m4f-rproc.example.dtb.d.pre.tmp
> -nostdinc -I./scripts/dtc/include-prefixes -undef -D__DTS__ -x
> assembler-with-cpp -o
> Documentation/devicetree/bindings/remoteproc/.ti_k3-m4f-rproc.example.dtb.dts.tmp
> Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.example.dts
> ; ./scripts/dtc/dtc -o
> Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.example.dtb
> -b 0 -iDocumentation/devicetree/bindings/remoteproc/
> -i./scripts/dtc/include-prefixes -Wno-avoid_unnecessary_addr_size
> -Wno-graph_child_address -Wno-interrupt_provider
> -Wno-unique_unit_address -Wunique_unit_address_if_enabled -d
> Documentation/devicetree/bindings/remoteproc/.ti_k3-m4f-rproc.example.dtb.d.dtc.tmp
> Documentation/devicetree/bindings/remoteproc/.ti_k3-m4f-rproc.example.dtb.dts.tmp
> ; cat
> Documentation/devicetree/bindings/remoteproc/.ti_k3-m4f-rproc.example.dtb.d.pre.tmp
> Documentation/devicetree/bindings/remoteproc/.ti_k3-m4f-rproc.example.dtb.d.dtc.tmp
> >
> Documentation/devicetree/bindings/remoteproc/.ti_k3-m4f-rproc.example.dtb.d
> ; dt-validate -u ./Documentation/devicetree/bindings -p
> ./Documentation/devicetree/bindings/processed-schema.json
> Documentation/devicetree/bindings/remoteproc/ti,k3-m4f-rproc.example.dtb
> || true
>
>
> >> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id:http://devicetree.org/schemas/remoteproc/ti,k3-m4f-rproc.yaml#
> >> +$schema:http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: TI K3 M4F processor subsystems
> >> +
> >> +maintainers:
> >> + - Hari Nagalla<[email protected]>
> >> + - Mathieu Poirier<[email protected]>
> > Are you sure Mathieu has this device and is a maintainer of this device?
> >
> Earlier, Mathieu suggested he can be the maintainer. Beagle play is
> based on AM625 device.
>

Sure, no problem, good for me.

Best regards,
Krzysztof