mbox series

[v11,0/6] Add Xilinx RPU subsystem support

Message ID 20221114233940.2096237-1-tanmay.shah@amd.com
Headers show
Series Add Xilinx RPU subsystem support | expand

Message

Tanmay Shah Nov. 14, 2022, 11:39 p.m. UTC
This patch series adds bindings document for RPU subsystem found on Xilinx
ZynqMP platforms. It also adds device nodes and driver to enable RPU
subsystem in split mode and lockstep mode.

Xilinx ZynqMP platform contains Remote Processing Unit(RPU). RPU subsystem
contains two arm cortex r5f cores. RPU subsystem can be configured in
split mode, lockstep mode and single-cpu mode.

RPU subsystem also contains 4 Tightly Coupled Memory(TCM) banks.
In lockstep mode, all 4 banks are combined and total of 256KB memory is
made available to r5 core0. In split mode, both cores can access two
TCM banks i.e. 128 KB.

RPU can also fetch data and execute instructions from DDR memory along with
TCM memory.
---

Changes in v11:
  - rename binding filename to match with compatible string
  - change $id: value accordingly as well
  - Rebase on latest rproc-next branch and resolve merge conflicts
  - remove redundant < 0 check for function of_get_available_child_count()
  - return 'ret' variable rather than masking the real error code when
    parsing "xlnx,cluster-mode" property fails
  - remove redundant use of devm_free()
  - call  of_reserved_mem_device_release() to release reserved memory
    in case driver probe fails or driver is removed.

Changes in v10:
  - bindings: rename example node to remoteproc
  - dts: Rename node name to remoteproc
  - switch to AMD email ID 
  - fix Kconfig unmet dependecy error reported by kernel test robot
  - fix r5_rproc object mem leak in function zynqmp_r5_add_rproc_core
  - add explanation of hardcoded TCM nodes
  - remove redundant ToDo comment
  - remove redundant check of tcm_bank_count and rmem_count
  - remove explicit free reserved_mem in zynqmp_r5_get_mem_region_node
  - fix leaked reference of child_dev during zynqmp_r5_cluster_init
    Also fix possible crash in exit path release_r5_cores 
  - do not remove mem-region and tcm carveouts explicitly in case of failure.
    It will be deleted as part of rproc_del. This also simplifies logic to
    use rproc_add_carveout
  - fix documentation all over the driver

Changes in v9:
  - bindings: remove power-domains property description
  - bindings: fix nitpicks in description of properties
  - dts: remove unused labels
  - replace devm_rproc_alloc with rproc_alloc
  - %s/until/while/r
  - %s/i > -1/i >=0/r
  - fix type of tcm_mode from int to enum rpu_tcm_comb
  - release &child_pdev->dev references
  - remove zynqmp_r5_core_exit()
  - undefined memory-region property isn't failure
  - remove tcm bank count check from ops
  - fix tcm bank turn-off sequence
  - fix parse_fw function documentation
  - do not use rproc_mem_entry_init on vdev0buffers
  - check tcm banks shouldn't be 0
  - declare variabls in reverse xmas tree order
  - remove extra line

Changes in v8:
  - add 'items:' for sram property

Changes in v7:
  - Add minItems in sram property

Changes in v6:
  - Add maxItems to sram and memory-region property

Changes in v5:
  - Add constraints of the possible values of xlnx,cluster-mode property
  - fix description of power-domains property for r5 core
  - Remove reg, address-cells and size-cells properties as it is not required
  - Fix description of mboxes property
  - Add description of each memory-region and remove old .txt binding link
    reference in the description
  - Remove optional reg property from r5fss node
  - Move r5fss node out of axi node

Changes in v4:
  - Add memory-region, mboxes and mbox-names properties in dt-bindings example
  - Add reserved memory region node and use it in Xilinx dt RPU subsystem node
  - Remove redundant header files
  - use dev_err_probe() to report errors during probe
  - Fix missing check on error code returned by zynqmp_r5_add_rproc_core()
  - Fix memory leaks all over the driver when resource allocation fails for any core
  - make cluster mode check only at one place
  - remove redundant initialization of variable
  - remove redundant use of of_node_put() 
  - Fix Comment format problem
  - Assign offset of zynqmp_tcm_banks instead of duplicating it
  - Add tcm and memory regions rproc carveouts during prepare instead of parse_fw
  - Remove rproc_mem_entry object from r5_core
  - Use put_device() and rproc_del() APIs to fix memory leaks
  - Replace pr_* with dev_*. This was missed in v3, fix now.
  - Use "GPL" instead of "GPL v2" in MODULE_LICENSE macro. This was reported by checkpatch script.

Changes in v3:
  - Fix checkpatch script indentation warning
  - Remove unused variable from xilinx remoteproc driver
  - use C style comments, i.e /*...*/
  - Remove redundant debug information which can be derived using /proc/device-tree
  - Fix multiline comment format
  - s/"final fot TCM"/"final for TCM"
  - Function devm_kzalloc() does not return an code on error, just NULL.
    Remove redundant error check for this function throughout the driver.
  - Fix RPU mode configuration and add documentation accordingly
  - Get rid of the indentations to match function documentation style with rest of the driver
  - Fix memory leak by only using r5_rproc->priv and not replace it with new instance
  - Use 'i' for the outer loop and 'j' for the inner one as per convention
  - Remove redundant error and NULL checks throughout the driver
  - Use devm_kcalloc() when more than one element is required
  - Add memory-regions carveouts during driver probe instead of parse_fw call
    This removes redundant copy of reserved_mem object in r5_core structure.
  - Fix memory leak by using of_node_put()
  - Fix indentation of tcm_mem_map function args
  - Remove redundant init of variables
  - Initialize tcm bank size variable for lockstep mode
  - Replace u32 with phys_addr_t for variable stroing memory bank address
  - Add documentation of TCM behavior in lockstep mode
  - Use dev_get_drvdata instead of platform driver API
  - Remove info level messages
  - Fix checkpatch.pl warnings
  - Add documentation for the Xilinx r5f platform to understand driver design

Changes in v2:
  - Remove proprietary copyright footer from cover letter

Ben Levinsky (3):
  firmware: xilinx: Add ZynqMP firmware ioctl enums for RPU
    configuration.
  firmware: xilinx: Add shutdown/wakeup APIs
  firmware: xilinx: Add RPU configuration APIs

Tanmay Shah (3):
  dt-bindings: remoteproc: Add Xilinx RPU subsystem bindings
  arm64: dts: xilinx: zynqmp: Add RPU subsystem device node
  drivers: remoteproc: Add Xilinx r5 remoteproc driver

 .../remoteproc/xlnx,zynqmp-r5fss.yaml         |  135 +++
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi        |   33 +
 drivers/firmware/xilinx/zynqmp.c              |   97 ++
 drivers/remoteproc/Kconfig                    |   13 +
 drivers/remoteproc/Makefile                   |    1 +
 drivers/remoteproc/xlnx_r5_remoteproc.c       | 1067 +++++++++++++++++
 include/dt-bindings/power/xlnx-zynqmp-power.h |    6 +
 include/linux/firmware/xlnx-zynqmp.h          |   60 +
 8 files changed, 1412 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
 create mode 100644 drivers/remoteproc/xlnx_r5_remoteproc.c


base-commit: 6eed169c7fefd9cdbbccb5ba7a98470cc0c09c63

Comments

Mathieu Poirier Nov. 16, 2022, 6:38 p.m. UTC | #1
Hi Tanmay,

I think this patchset is ready to be merged.  Two things are missing though:

Patches 1 and 2: We are missing a tag from one of the fellow in the DT brigade.
They handle a lot of patches so let's give them time.

Patches 4 and 5: Michal's ack.  He already reviewed that code in the previous
iteration but it wasn't added to this set, probably because you did some
modification to get the patches to apply.  In that case it is usually fine to
carry the tag since there isn't any modification to the code you are
introducing.  But you aired on the side of caution and that is also fine.
Please reach out to him again for another review.

Thanks,
Mathieu


On Mon, Nov 14, 2022 at 03:39:34PM -0800, Tanmay Shah wrote:
> This patch series adds bindings document for RPU subsystem found on Xilinx
> ZynqMP platforms. It also adds device nodes and driver to enable RPU
> subsystem in split mode and lockstep mode.
> 
> Xilinx ZynqMP platform contains Remote Processing Unit(RPU). RPU subsystem
> contains two arm cortex r5f cores. RPU subsystem can be configured in
> split mode, lockstep mode and single-cpu mode.
> 
> RPU subsystem also contains 4 Tightly Coupled Memory(TCM) banks.
> In lockstep mode, all 4 banks are combined and total of 256KB memory is
> made available to r5 core0. In split mode, both cores can access two
> TCM banks i.e. 128 KB.
> 
> RPU can also fetch data and execute instructions from DDR memory along with
> TCM memory.
> ---
> 
> Changes in v11:
>   - rename binding filename to match with compatible string
>   - change $id: value accordingly as well
>   - Rebase on latest rproc-next branch and resolve merge conflicts
>   - remove redundant < 0 check for function of_get_available_child_count()
>   - return 'ret' variable rather than masking the real error code when
>     parsing "xlnx,cluster-mode" property fails
>   - remove redundant use of devm_free()
>   - call  of_reserved_mem_device_release() to release reserved memory
>     in case driver probe fails or driver is removed.
> 
> Changes in v10:
>   - bindings: rename example node to remoteproc
>   - dts: Rename node name to remoteproc
>   - switch to AMD email ID 
>   - fix Kconfig unmet dependecy error reported by kernel test robot
>   - fix r5_rproc object mem leak in function zynqmp_r5_add_rproc_core
>   - add explanation of hardcoded TCM nodes
>   - remove redundant ToDo comment
>   - remove redundant check of tcm_bank_count and rmem_count
>   - remove explicit free reserved_mem in zynqmp_r5_get_mem_region_node
>   - fix leaked reference of child_dev during zynqmp_r5_cluster_init
>     Also fix possible crash in exit path release_r5_cores 
>   - do not remove mem-region and tcm carveouts explicitly in case of failure.
>     It will be deleted as part of rproc_del. This also simplifies logic to
>     use rproc_add_carveout
>   - fix documentation all over the driver
> 
> Changes in v9:
>   - bindings: remove power-domains property description
>   - bindings: fix nitpicks in description of properties
>   - dts: remove unused labels
>   - replace devm_rproc_alloc with rproc_alloc
>   - %s/until/while/r
>   - %s/i > -1/i >=0/r
>   - fix type of tcm_mode from int to enum rpu_tcm_comb
>   - release &child_pdev->dev references
>   - remove zynqmp_r5_core_exit()
>   - undefined memory-region property isn't failure
>   - remove tcm bank count check from ops
>   - fix tcm bank turn-off sequence
>   - fix parse_fw function documentation
>   - do not use rproc_mem_entry_init on vdev0buffers
>   - check tcm banks shouldn't be 0
>   - declare variabls in reverse xmas tree order
>   - remove extra line
> 
> Changes in v8:
>   - add 'items:' for sram property
> 
> Changes in v7:
>   - Add minItems in sram property
> 
> Changes in v6:
>   - Add maxItems to sram and memory-region property
> 
> Changes in v5:
>   - Add constraints of the possible values of xlnx,cluster-mode property
>   - fix description of power-domains property for r5 core
>   - Remove reg, address-cells and size-cells properties as it is not required
>   - Fix description of mboxes property
>   - Add description of each memory-region and remove old .txt binding link
>     reference in the description
>   - Remove optional reg property from r5fss node
>   - Move r5fss node out of axi node
> 
> Changes in v4:
>   - Add memory-region, mboxes and mbox-names properties in dt-bindings example
>   - Add reserved memory region node and use it in Xilinx dt RPU subsystem node
>   - Remove redundant header files
>   - use dev_err_probe() to report errors during probe
>   - Fix missing check on error code returned by zynqmp_r5_add_rproc_core()
>   - Fix memory leaks all over the driver when resource allocation fails for any core
>   - make cluster mode check only at one place
>   - remove redundant initialization of variable
>   - remove redundant use of of_node_put() 
>   - Fix Comment format problem
>   - Assign offset of zynqmp_tcm_banks instead of duplicating it
>   - Add tcm and memory regions rproc carveouts during prepare instead of parse_fw
>   - Remove rproc_mem_entry object from r5_core
>   - Use put_device() and rproc_del() APIs to fix memory leaks
>   - Replace pr_* with dev_*. This was missed in v3, fix now.
>   - Use "GPL" instead of "GPL v2" in MODULE_LICENSE macro. This was reported by checkpatch script.
> 
> Changes in v3:
>   - Fix checkpatch script indentation warning
>   - Remove unused variable from xilinx remoteproc driver
>   - use C style comments, i.e /*...*/
>   - Remove redundant debug information which can be derived using /proc/device-tree
>   - Fix multiline comment format
>   - s/"final fot TCM"/"final for TCM"
>   - Function devm_kzalloc() does not return an code on error, just NULL.
>     Remove redundant error check for this function throughout the driver.
>   - Fix RPU mode configuration and add documentation accordingly
>   - Get rid of the indentations to match function documentation style with rest of the driver
>   - Fix memory leak by only using r5_rproc->priv and not replace it with new instance
>   - Use 'i' for the outer loop and 'j' for the inner one as per convention
>   - Remove redundant error and NULL checks throughout the driver
>   - Use devm_kcalloc() when more than one element is required
>   - Add memory-regions carveouts during driver probe instead of parse_fw call
>     This removes redundant copy of reserved_mem object in r5_core structure.
>   - Fix memory leak by using of_node_put()
>   - Fix indentation of tcm_mem_map function args
>   - Remove redundant init of variables
>   - Initialize tcm bank size variable for lockstep mode
>   - Replace u32 with phys_addr_t for variable stroing memory bank address
>   - Add documentation of TCM behavior in lockstep mode
>   - Use dev_get_drvdata instead of platform driver API
>   - Remove info level messages
>   - Fix checkpatch.pl warnings
>   - Add documentation for the Xilinx r5f platform to understand driver design
> 
> Changes in v2:
>   - Remove proprietary copyright footer from cover letter
> 
> Ben Levinsky (3):
>   firmware: xilinx: Add ZynqMP firmware ioctl enums for RPU
>     configuration.
>   firmware: xilinx: Add shutdown/wakeup APIs
>   firmware: xilinx: Add RPU configuration APIs
> 
> Tanmay Shah (3):
>   dt-bindings: remoteproc: Add Xilinx RPU subsystem bindings
>   arm64: dts: xilinx: zynqmp: Add RPU subsystem device node
>   drivers: remoteproc: Add Xilinx r5 remoteproc driver
> 
>  .../remoteproc/xlnx,zynqmp-r5fss.yaml         |  135 +++
>  arch/arm64/boot/dts/xilinx/zynqmp.dtsi        |   33 +
>  drivers/firmware/xilinx/zynqmp.c              |   97 ++
>  drivers/remoteproc/Kconfig                    |   13 +
>  drivers/remoteproc/Makefile                   |    1 +
>  drivers/remoteproc/xlnx_r5_remoteproc.c       | 1067 +++++++++++++++++
>  include/dt-bindings/power/xlnx-zynqmp-power.h |    6 +
>  include/linux/firmware/xlnx-zynqmp.h          |   60 +
>  8 files changed, 1412 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
>  create mode 100644 drivers/remoteproc/xlnx_r5_remoteproc.c
> 
> 
> base-commit: 6eed169c7fefd9cdbbccb5ba7a98470cc0c09c63
> -- 
> 2.25.1
>
Tanmay Shah Nov. 16, 2022, 8:46 p.m. UTC | #2
Hi Mathieu,

Thanks for reviews.

On 11/16/22 10:38 AM, Mathieu Poirier wrote:
> Hi Tanmay,
>
> I think this patchset is ready to be merged.  Two things are missing though:
>
> Patches 1 and 2: We are missing a tag from one of the fellow in the DT brigade.
> They handle a lot of patches so let's give them time.
Sure.
>
> Patches 4 and 5: Michal's ack.  He already reviewed that code in the previous
> iteration but it wasn't added to this set, probably because you did some
> modification to get the patches to apply.  In that case it is usually fine to
> carry the tag since there isn't any modification to the code you are
> introducing.  But you aired on the side of caution and that is also fine.
> Please reach out to him again for another review.

Yes, I rebased on latest rproc-next and resolved merge conflict in those 
patches.

As it wasn't clean rebase, I removed Michal's Ack in case he prefer to 
review again.

I will reach out to him for reviews.

Thanks,

Tanmay

>
> Thanks,
> Mathieu
>
>
> On Mon, Nov 14, 2022 at 03:39:34PM -0800, Tanmay Shah wrote:
>> This patch series adds bindings document for RPU subsystem found on Xilinx
>> ZynqMP platforms. It also adds device nodes and driver to enable RPU
>> subsystem in split mode and lockstep mode.
>>
>> Xilinx ZynqMP platform contains Remote Processing Unit(RPU). RPU subsystem
>> contains two arm cortex r5f cores. RPU subsystem can be configured in
>> split mode, lockstep mode and single-cpu mode.
>>
>> RPU subsystem also contains 4 Tightly Coupled Memory(TCM) banks.
>> In lockstep mode, all 4 banks are combined and total of 256KB memory is
>> made available to r5 core0. In split mode, both cores can access two
>> TCM banks i.e. 128 KB.
>>
>> RPU can also fetch data and execute instructions from DDR memory along with
>> TCM memory.
>> ---
>>
>> Changes in v11:
>>    - rename binding filename to match with compatible string
>>    - change $id: value accordingly as well
>>    - Rebase on latest rproc-next branch and resolve merge conflicts
>>    - remove redundant < 0 check for function of_get_available_child_count()
>>    - return 'ret' variable rather than masking the real error code when
>>      parsing "xlnx,cluster-mode" property fails
>>    - remove redundant use of devm_free()
>>    - call  of_reserved_mem_device_release() to release reserved memory
>>      in case driver probe fails or driver is removed.
>>
>> Changes in v10:
>>    - bindings: rename example node to remoteproc
>>    - dts: Rename node name to remoteproc
>>    - switch to AMD email ID
>>    - fix Kconfig unmet dependecy error reported by kernel test robot
>>    - fix r5_rproc object mem leak in function zynqmp_r5_add_rproc_core
>>    - add explanation of hardcoded TCM nodes
>>    - remove redundant ToDo comment
>>    - remove redundant check of tcm_bank_count and rmem_count
>>    - remove explicit free reserved_mem in zynqmp_r5_get_mem_region_node
>>    - fix leaked reference of child_dev during zynqmp_r5_cluster_init
>>      Also fix possible crash in exit path release_r5_cores
>>    - do not remove mem-region and tcm carveouts explicitly in case of failure.
>>      It will be deleted as part of rproc_del. This also simplifies logic to
>>      use rproc_add_carveout
>>    - fix documentation all over the driver
>>
>> Changes in v9:
>>    - bindings: remove power-domains property description
>>    - bindings: fix nitpicks in description of properties
>>    - dts: remove unused labels
>>    - replace devm_rproc_alloc with rproc_alloc
>>    - %s/until/while/r
>>    - %s/i > -1/i >=0/r
>>    - fix type of tcm_mode from int to enum rpu_tcm_comb
>>    - release &child_pdev->dev references
>>    - remove zynqmp_r5_core_exit()
>>    - undefined memory-region property isn't failure
>>    - remove tcm bank count check from ops
>>    - fix tcm bank turn-off sequence
>>    - fix parse_fw function documentation
>>    - do not use rproc_mem_entry_init on vdev0buffers
>>    - check tcm banks shouldn't be 0
>>    - declare variabls in reverse xmas tree order
>>    - remove extra line
>>
>> Changes in v8:
>>    - add 'items:' for sram property
>>
>> Changes in v7:
>>    - Add minItems in sram property
>>
>> Changes in v6:
>>    - Add maxItems to sram and memory-region property
>>
>> Changes in v5:
>>    - Add constraints of the possible values of xlnx,cluster-mode property
>>    - fix description of power-domains property for r5 core
>>    - Remove reg, address-cells and size-cells properties as it is not required
>>    - Fix description of mboxes property
>>    - Add description of each memory-region and remove old .txt binding link
>>      reference in the description
>>    - Remove optional reg property from r5fss node
>>    - Move r5fss node out of axi node
>>
>> Changes in v4:
>>    - Add memory-region, mboxes and mbox-names properties in dt-bindings example
>>    - Add reserved memory region node and use it in Xilinx dt RPU subsystem node
>>    - Remove redundant header files
>>    - use dev_err_probe() to report errors during probe
>>    - Fix missing check on error code returned by zynqmp_r5_add_rproc_core()
>>    - Fix memory leaks all over the driver when resource allocation fails for any core
>>    - make cluster mode check only at one place
>>    - remove redundant initialization of variable
>>    - remove redundant use of of_node_put()
>>    - Fix Comment format problem
>>    - Assign offset of zynqmp_tcm_banks instead of duplicating it
>>    - Add tcm and memory regions rproc carveouts during prepare instead of parse_fw
>>    - Remove rproc_mem_entry object from r5_core
>>    - Use put_device() and rproc_del() APIs to fix memory leaks
>>    - Replace pr_* with dev_*. This was missed in v3, fix now.
>>    - Use "GPL" instead of "GPL v2" in MODULE_LICENSE macro. This was reported by checkpatch script.
>>
>> Changes in v3:
>>    - Fix checkpatch script indentation warning
>>    - Remove unused variable from xilinx remoteproc driver
>>    - use C style comments, i.e /*...*/
>>    - Remove redundant debug information which can be derived using /proc/device-tree
>>    - Fix multiline comment format
>>    - s/"final fot TCM"/"final for TCM"
>>    - Function devm_kzalloc() does not return an code on error, just NULL.
>>      Remove redundant error check for this function throughout the driver.
>>    - Fix RPU mode configuration and add documentation accordingly
>>    - Get rid of the indentations to match function documentation style with rest of the driver
>>    - Fix memory leak by only using r5_rproc->priv and not replace it with new instance
>>    - Use 'i' for the outer loop and 'j' for the inner one as per convention
>>    - Remove redundant error and NULL checks throughout the driver
>>    - Use devm_kcalloc() when more than one element is required
>>    - Add memory-regions carveouts during driver probe instead of parse_fw call
>>      This removes redundant copy of reserved_mem object in r5_core structure.
>>    - Fix memory leak by using of_node_put()
>>    - Fix indentation of tcm_mem_map function args
>>    - Remove redundant init of variables
>>    - Initialize tcm bank size variable for lockstep mode
>>    - Replace u32 with phys_addr_t for variable stroing memory bank address
>>    - Add documentation of TCM behavior in lockstep mode
>>    - Use dev_get_drvdata instead of platform driver API
>>    - Remove info level messages
>>    - Fix checkpatch.pl warnings
>>    - Add documentation for the Xilinx r5f platform to understand driver design
>>
>> Changes in v2:
>>    - Remove proprietary copyright footer from cover letter
>>
>> Ben Levinsky (3):
>>    firmware: xilinx: Add ZynqMP firmware ioctl enums for RPU
>>      configuration.
>>    firmware: xilinx: Add shutdown/wakeup APIs
>>    firmware: xilinx: Add RPU configuration APIs
>>
>> Tanmay Shah (3):
>>    dt-bindings: remoteproc: Add Xilinx RPU subsystem bindings
>>    arm64: dts: xilinx: zynqmp: Add RPU subsystem device node
>>    drivers: remoteproc: Add Xilinx r5 remoteproc driver
>>
>>   .../remoteproc/xlnx,zynqmp-r5fss.yaml         |  135 +++
>>   arch/arm64/boot/dts/xilinx/zynqmp.dtsi        |   33 +
>>   drivers/firmware/xilinx/zynqmp.c              |   97 ++
>>   drivers/remoteproc/Kconfig                    |   13 +
>>   drivers/remoteproc/Makefile                   |    1 +
>>   drivers/remoteproc/xlnx_r5_remoteproc.c       | 1067 +++++++++++++++++
>>   include/dt-bindings/power/xlnx-zynqmp-power.h |    6 +
>>   include/linux/firmware/xlnx-zynqmp.h          |   60 +
>>   8 files changed, 1412 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
>>   create mode 100644 drivers/remoteproc/xlnx_r5_remoteproc.c
>>
>>
>> base-commit: 6eed169c7fefd9cdbbccb5ba7a98470cc0c09c63
>> -- 
>> 2.25.1
>>
Rob Herring Nov. 20, 2022, 4:22 p.m. UTC | #3
On Wed, Nov 16, 2022 at 12:38 PM Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
>
> Hi Tanmay,
>
> I think this patchset is ready to be merged.  Two things are missing though:
>
> Patches 1 and 2: We are missing a tag from one of the fellow in the DT brigade.
> They handle a lot of patches so let's give them time.

Patch 2 should go via the sub-arch tree. I don;t review .dts files regularly.

Rob
Rob Herring Nov. 20, 2022, 4:26 p.m. UTC | #4
On Sun, Nov 20, 2022 at 10:22 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Wed, Nov 16, 2022 at 12:38 PM Mathieu Poirier
> <mathieu.poirier@linaro.org> wrote:
> >
> > Hi Tanmay,
> >
> > I think this patchset is ready to be merged.  Two things are missing though:
> >
> > Patches 1 and 2: We are missing a tag from one of the fellow in the DT brigade.
> > They handle a lot of patches so let's give them time.
>
> Patch 2 should go via the sub-arch tree. I don;t review .dts files regularly.

Except I guess there's a header dependency here. Then the dts needs an
ack from Michal if it doesn't already or it can go next cycle.

Rob
Michal Simek Nov. 25, 2022, 9:22 a.m. UTC | #5
On 11/15/22 00:39, Tanmay Shah wrote:
> RPU subsystem can be configured in cluster-mode or split mode.
> Also each r5 core has separate power domains.
> 
> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> ---
> 
> Changes in v11:
>    - None
> 
> Changes in v10:
>    - Rename node name to remoteproc
> 
> Changes in v9:
>    - remove unused labels
> 
> Changes in v8:
>    - None
> 
> Changes in v7:
>    - None
> 
> Changes in v6:
>    - None
> 
> Changes in v5:
>    - Remove optional reg property from r5fss node
>    - Move r5fss node out of axi node
> 
> Changes in v4:
>    - Add reserved memory region node and use it in RPU subsystem node
> 
> Changes in v3:
>    - Fix checkpatch.pl style warning
> 
>   arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 33 ++++++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> index a549265e55f6..c0f60833c0ae 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> @@ -100,6 +100,22 @@ opp03 {
>   		};
>   	};
>   
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		rproc_0_fw_image: memory@3ed00000 {
> +			no-map;
> +			reg = <0x0 0x3ed00000 0x0 0x40000>;
> +		};
> +
> +		rproc_1_fw_image: memory@3ef00000 {
> +			no-map;
> +			reg = <0x0 0x3ef00000 0x0 0x40000>;
> +		};
> +	};
> +
>   	zynqmp_ipi: zynqmp_ipi {
>   		compatible = "xlnx,zynqmp-ipi-mailbox";
>   		interrupt-parent = <&gic>;
> @@ -203,6 +219,23 @@ fpga_full: fpga-full {
>   		ranges;
>   	};
>   
> +	remoteproc {
> +		compatible = "xlnx,zynqmp-r5fss";
> +		xlnx,cluster-mode = <1>;
> +
> +		r5f-0 {
> +			compatible = "xlnx,zynqmp-r5f";
> +			power-domains = <&zynqmp_firmware PD_RPU_0>;
> +			memory-region = <&rproc_0_fw_image>;
> +		};
> +
> +		r5f-1 {
> +			compatible = "xlnx,zynqmp-r5f";
> +			power-domains = <&zynqmp_firmware PD_RPU_1>;
> +			memory-region = <&rproc_1_fw_image>;
> +		};
> +	};
> +
>   	amba: axi {
>   		compatible = "simple-bus";
>   		#address-cells = <2>;

Matthieu: If you want to take this via your tree here is mine.

Acked-by: Michal Simek <michal.simek@amd.com>

In another case I will queue it for next release when dt binding is applied.

Thanks,
Michal
Michal Simek Nov. 25, 2022, 9:24 a.m. UTC | #6
On 11/15/22 00:39, Tanmay Shah wrote:
> From: Ben Levinsky <ben.levinsky@amd.com>
> 
> Add shutdown/wakeup a resource eemi operations to shutdown
> or bringup a resource.
> 
> Note alignment of args matches convention of other fn's in this file.
> The reason being that the long fn name results in aligned args that
> otherwise go over 80 chars so shift right to avoid this
> 
> Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> ---
> 
> Changes in v11:
>    - Rebase on latest rproc-next branch and resolve merge conflicts
> 
> Changes in v10:
>    - None
> 
> Changes in v9:
>    - None
> 
> Changes in v8:
>    - None
> 
> Changes in v7:
>    - None
> 
> Changes in v6:
>    - None
> 
> Changes in v5:
>    - None
> 
> Changes in v4:
>    - None
> 
> Changes in v3:
>    - None
> 
>   drivers/firmware/xilinx/zynqmp.c     | 35 ++++++++++++++++++++++++++++
>   include/linux/firmware/xlnx-zynqmp.h | 23 ++++++++++++++++++
>   2 files changed, 58 insertions(+)
> 
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index ff5cabe70a2b..1865e43ed7e7 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -1159,6 +1159,41 @@ int zynqmp_pm_release_node(const u32 node)
>   }
>   EXPORT_SYMBOL_GPL(zynqmp_pm_release_node);
>   
> +/**
> + * zynqmp_pm_force_pwrdwn - PM call to request for another PU or subsystem to
> + *             be powered down forcefully
> + * @node:  Node ID of the targeted PU or subsystem
> + * @ack:   Flag to specify whether acknowledge is requested
> + *
> + * Return: status, either success or error+reason
> + */
> +int zynqmp_pm_force_pwrdwn(const u32 node,
> +			   const enum zynqmp_pm_request_ack ack)
> +{
> +	return zynqmp_pm_invoke_fn(PM_FORCE_POWERDOWN, node, ack, 0, 0, NULL);
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_force_pwrdwn);
> +
> +/**
> + * zynqmp_pm_request_wake - PM call to wake up selected master or subsystem
> + * @node:  Node ID of the master or subsystem
> + * @set_addr:  Specifies whether the address argument is relevant
> + * @address:   Address from which to resume when woken up
> + * @ack:   Flag to specify whether acknowledge requested
> + *
> + * Return: status, either success or error+reason
> + */
> +int zynqmp_pm_request_wake(const u32 node,
> +			   const bool set_addr,
> +			   const u64 address,
> +			   const enum zynqmp_pm_request_ack ack)
> +{
> +	/* set_addr flag is encoded into 1st bit of address */
> +	return zynqmp_pm_invoke_fn(PM_REQUEST_WAKEUP, node, address | set_addr,
> +				   address >> 32, ack, NULL);
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_request_wake);
> +
>   /**
>    * zynqmp_pm_set_requirement() - PM call to set requirement for PM slaves
>    * @node:		Node ID of the slave
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index bdbf855b5eef..ad3f2bd0c470 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -12,6 +12,7 @@
>   
>   #ifndef __FIRMWARE_ZYNQMP_H__
>   #define __FIRMWARE_ZYNQMP_H__
> +#include <linux/types.h>
>   
>   #include <linux/err.h>
>   
> @@ -87,6 +88,8 @@ enum pm_api_cb_id {
>   enum pm_api_id {
>   	PM_GET_API_VERSION = 1,
>   	PM_REGISTER_NOTIFIER = 5,
> +	PM_FORCE_POWERDOWN = 8,
> +	PM_REQUEST_WAKEUP = 10,
>   	PM_SYSTEM_SHUTDOWN = 12,
>   	PM_REQUEST_NODE = 13,
>   	PM_RELEASE_NODE = 14,
> @@ -521,6 +524,12 @@ int zynqmp_pm_is_function_supported(const u32 api_id, const u32 id);
>   int zynqmp_pm_set_feature_config(enum pm_feature_config_id id, u32 value);
>   int zynqmp_pm_get_feature_config(enum pm_feature_config_id id, u32 *payload);
>   int zynqmp_pm_register_sgi(u32 sgi_num, u32 reset);
> +int zynqmp_pm_force_pwrdwn(const u32 target,
> +			   const enum zynqmp_pm_request_ack ack);
> +int zynqmp_pm_request_wake(const u32 node,
> +			   const bool set_addr,
> +			   const u64 address,
> +			   const enum zynqmp_pm_request_ack ack);
>   int zynqmp_pm_set_sd_config(u32 node, enum pm_sd_config_type config, u32 value);
>   int zynqmp_pm_set_gem_config(u32 node, enum pm_gem_config_type config,
>   			     u32 value);
> @@ -795,6 +804,20 @@ static inline int zynqmp_pm_register_sgi(u32 sgi_num, u32 reset)
>   	return -ENODEV;
>   }
>   
> +static inline int zynqmp_pm_force_pwrdwn(const u32 target,
> +					 const enum zynqmp_pm_request_ack ack)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline int zynqmp_pm_request_wake(const u32 node,
> +					 const bool set_addr,
> +					 const u64 address,
> +					 const enum zynqmp_pm_request_ack ack)
> +{
> +	return -ENODEV;
> +}
> +
>   static inline int zynqmp_pm_set_sd_config(u32 node,
>   					  enum pm_sd_config_type config,
>   					  u32 value)

Acked-by: Michal Simek <michal.simek@amd.com>

Thanks,
Michal
Michal Simek Nov. 25, 2022, 9:25 a.m. UTC | #7
On 11/15/22 00:39, Tanmay Shah wrote:
> From: Ben Levinsky <ben.levinsky@amd.com>
> 
> This patch adds APIs to access to configure RPU and its
> processor-specific memory.
> 
> That is query the run-time mode of RPU as either split or lockstep as well
> as API to set this mode. In addition add APIs to access configuration of
> the RPUs' tightly coupled memory (TCM).
> 
> Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> ---
> 
> Changes in v11:
>    - Rebase on latest rproc-next branch and resolve merge conflicts
> 
> Changes in v10:
>    - None
> 
> Changes in v9:
>    - None
> 
> Changes in v8:
>    - None
> 
> Changes in v7:
>    - None
> 
> Changes in v6:
>    - None
> 
> Changes in v5:
>    - None
> 
> Changes in v4:
>    - None
> 
> Changes in v3:
>    - Add missing function argument documentation
> 
>   drivers/firmware/xilinx/zynqmp.c     | 62 ++++++++++++++++++++++++++++
>   include/linux/firmware/xlnx-zynqmp.h | 18 ++++++++
>   2 files changed, 80 insertions(+)
> 
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index 1865e43ed7e7..e4981e7f3500 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -1159,6 +1159,68 @@ int zynqmp_pm_release_node(const u32 node)
>   }
>   EXPORT_SYMBOL_GPL(zynqmp_pm_release_node);
>   
> +/**
> + * zynqmp_pm_get_rpu_mode() - Get RPU mode
> + * @node_id:	Node ID of the device
> + * @rpu_mode:	return by reference value
> + *		either split or lockstep
> + *
> + * Return:	return 0 on success or error+reason.
> + *		if success, then  rpu_mode will be set
> + *		to current rpu mode.
> + */
> +int zynqmp_pm_get_rpu_mode(u32 node_id, enum rpu_oper_mode *rpu_mode)
> +{
> +	u32 ret_payload[PAYLOAD_ARG_CNT];
> +	int ret;
> +
> +	ret = zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
> +				  IOCTL_GET_RPU_OPER_MODE, 0, 0, ret_payload);
> +
> +	/* only set rpu_mode if no error */
> +	if (ret == XST_PM_SUCCESS)
> +		*rpu_mode = ret_payload[0];
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_get_rpu_mode);
> +
> +/**
> + * zynqmp_pm_set_rpu_mode() - Set RPU mode
> + * @node_id:	Node ID of the device
> + * @rpu_mode:	Argument 1 to requested IOCTL call. either split or lockstep
> + *
> + *		This function is used to set RPU mode to split or
> + *		lockstep
> + *
> + * Return:	Returns status, either success or error+reason
> + */
> +int zynqmp_pm_set_rpu_mode(u32 node_id, enum rpu_oper_mode rpu_mode)
> +{
> +	return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
> +				   IOCTL_SET_RPU_OPER_MODE, (u32)rpu_mode,
> +				   0, NULL);
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_set_rpu_mode);
> +
> +/**
> + * zynqmp_pm_set_tcm_config - configure TCM
> + * @node_id:	Firmware specific TCM subsystem ID
> + * @tcm_mode:	Argument 1 to requested IOCTL call
> + *              either PM_RPU_TCM_COMB or PM_RPU_TCM_SPLIT
> + *
> + * This function is used to set RPU mode to split or combined
> + *
> + * Return: status: 0 for success, else failure
> + */
> +int zynqmp_pm_set_tcm_config(u32 node_id, enum rpu_tcm_comb tcm_mode)
> +{
> +	return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
> +				   IOCTL_TCM_COMB_CONFIG, (u32)tcm_mode, 0,
> +				   NULL);
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_set_tcm_config);
> +
>   /**
>    * zynqmp_pm_force_pwrdwn - PM call to request for another PU or subsystem to
>    *             be powered down forcefully
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index ad3f2bd0c470..cf92e739fa3b 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -530,6 +530,9 @@ int zynqmp_pm_request_wake(const u32 node,
>   			   const bool set_addr,
>   			   const u64 address,
>   			   const enum zynqmp_pm_request_ack ack);
> +int zynqmp_pm_get_rpu_mode(u32 node_id, enum rpu_oper_mode *rpu_mode);
> +int zynqmp_pm_set_rpu_mode(u32 node_id, u32 arg1);
> +int zynqmp_pm_set_tcm_config(u32 node_id, u32 arg1);
>   int zynqmp_pm_set_sd_config(u32 node, enum pm_sd_config_type config, u32 value);
>   int zynqmp_pm_set_gem_config(u32 node, enum pm_gem_config_type config,
>   			     u32 value);
> @@ -818,6 +821,21 @@ static inline int zynqmp_pm_request_wake(const u32 node,
>   	return -ENODEV;
>   }
>   
> +static inline int zynqmp_pm_get_rpu_mode(u32 node_id, enum rpu_oper_mode *rpu_mode)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline int zynqmp_pm_set_rpu_mode(u32 node_id, u32 arg1)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline int zynqmp_pm_set_tcm_config(u32 node_id, u32 arg1)
> +{
> +	return -ENODEV;
> +}
> +
>   static inline int zynqmp_pm_set_sd_config(u32 node,
>   					  enum pm_sd_config_type config,
>   					  u32 value)

Acked-by: Michal Simek <michal.simek@amd.com>

Thanks,
Michal
Mathieu Poirier Nov. 25, 2022, 4:13 p.m. UTC | #8
On Fri, Nov 25, 2022 at 10:22:47AM +0100, Michal Simek wrote:
> 
> 
> On 11/15/22 00:39, Tanmay Shah wrote:
> > RPU subsystem can be configured in cluster-mode or split mode.
> > Also each r5 core has separate power domains.
> > 
> > Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> > ---
> > 
> > Changes in v11:
> >    - None
> > 
> > Changes in v10:
> >    - Rename node name to remoteproc
> > 
> > Changes in v9:
> >    - remove unused labels
> > 
> > Changes in v8:
> >    - None
> > 
> > Changes in v7:
> >    - None
> > 
> > Changes in v6:
> >    - None
> > 
> > Changes in v5:
> >    - Remove optional reg property from r5fss node
> >    - Move r5fss node out of axi node
> > 
> > Changes in v4:
> >    - Add reserved memory region node and use it in RPU subsystem node
> > 
> > Changes in v3:
> >    - Fix checkpatch.pl style warning
> > 
> >   arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 33 ++++++++++++++++++++++++++
> >   1 file changed, 33 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> > index a549265e55f6..c0f60833c0ae 100644
> > --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> > +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> > @@ -100,6 +100,22 @@ opp03 {
> >   		};
> >   	};
> > +	reserved-memory {
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
> > +		ranges;
> > +
> > +		rproc_0_fw_image: memory@3ed00000 {
> > +			no-map;
> > +			reg = <0x0 0x3ed00000 0x0 0x40000>;
> > +		};
> > +
> > +		rproc_1_fw_image: memory@3ef00000 {
> > +			no-map;
> > +			reg = <0x0 0x3ef00000 0x0 0x40000>;
> > +		};
> > +	};
> > +
> >   	zynqmp_ipi: zynqmp_ipi {
> >   		compatible = "xlnx,zynqmp-ipi-mailbox";
> >   		interrupt-parent = <&gic>;
> > @@ -203,6 +219,23 @@ fpga_full: fpga-full {
> >   		ranges;
> >   	};
> > +	remoteproc {
> > +		compatible = "xlnx,zynqmp-r5fss";
> > +		xlnx,cluster-mode = <1>;
> > +
> > +		r5f-0 {
> > +			compatible = "xlnx,zynqmp-r5f";
> > +			power-domains = <&zynqmp_firmware PD_RPU_0>;
> > +			memory-region = <&rproc_0_fw_image>;
> > +		};
> > +
> > +		r5f-1 {
> > +			compatible = "xlnx,zynqmp-r5f";
> > +			power-domains = <&zynqmp_firmware PD_RPU_1>;
> > +			memory-region = <&rproc_1_fw_image>;
> > +		};
> > +	};
> > +
> >   	amba: axi {
> >   		compatible = "simple-bus";
> >   		#address-cells = <2>;
> 
> Matthieu: If you want to take this via your tree here is mine.
> 
> Acked-by: Michal Simek <michal.simek@amd.com>

I have applied the whole set.

Thanks,
Mathieu

> 
> In another case I will queue it for next release when dt binding is applied.
> 
> Thanks,
> Michal