Re: [PATCH v8 3/5] firmware: xilinx: Add RPU configuration APIs
Hey Ben,
On Mon, Aug 10, 2020 at 08:32:11PM -0700, Ben Levinsky wrote:
> This patch adds APIs to provide access and a configuration interface
> to the current power state of a sub-system on Zynqmp sub-system.
>
> Signed-off-by: Ben Levinsky
> ---
> v3:
> - add xilinx-related platform mgmt fn's instead of wrapping around
> function pointer in xilinx eemi ops struct
> v4:
> - add default values for enums
> ---
> drivers/firmware/xilinx/zynqmp.c | 99
> include/linux/firmware/xlnx-zynqmp.h | 34 ++
> 2 files changed, 133 insertions(+)
>
> diff --git a/drivers/firmware/xilinx/zynqmp.c
> b/drivers/firmware/xilinx/zynqmp.c
> index f1a0bd35deeb..fdd61d258e55 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -846,6 +846,61 @@ 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
> + * @arg1:Argument 1 to requested IOCTL call
> + * @arg2:Argument 2 to requested IOCTL call
> + * @out: Returned output value
> + *
> + * Return: RPU mode
> + */
> +int zynqmp_pm_get_rpu_mode(u32 node_id,
> +u32 arg1, u32 arg2, u32 *out)
> +{
> + return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
> +IOCTL_GET_RPU_OPER_MODE, 0, 0, out);
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_get_rpu_mode);
> +
> +/**
> + * zynqmp_pm_set_rpu_mode() - Set RPU mode
> + * @node_id: Node ID of the device
> + * @ioctl_id:ID of the requested IOCTL
> + * @arg2:Argument 2 to requested IOCTL call
> + * @out: Returned output value
> + *
> + * This function is used to set RPU mode.
> + *
> + * Return: Returns status, either success or error+reason
> + */
> +int zynqmp_pm_set_rpu_mode(u32 node_id,
> +u32 arg1, u32 arg2, u32 *out)
> +{
> + return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
> +IOCTL_SET_RPU_OPER_MODE, 0, 0, out);
This should pass through arg1 instead of 0 for the argument after
IOCTL_SET_RPU_OPER_MODE. As this is, the RPU will always be configured
in lockstep mode regardless of a request to switch to split mode.
Also, the kerneldoc for these functions could use some love (ioctl_id no
longer exists, arg1 isn't documented, arg2 is documented by very
non-specifically).
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_set_rpu_mode);
> +
> +/**
> + * zynqmp_pm_tcm_comb_config - configure TCM
> + * @node_id: Node ID of the device
> + * @arg1:Argument 1 to requested IOCTL call
> + * @arg2:Argument 2 to requested IOCTL call
> + * @out: Returned output value
> + *
> + * This function is used to set RPU mode.
> + *
> + * Return: Returns status, either success or error+reason
> + */
> +int zynqmp_pm_set_tcm_config(u32 node_id,
> + u32 arg1, u32 arg2, u32 *out)
> +{
> + return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
> +IOCTL_TCM_COMB_CONFIG, 0, 0, out);
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_set_tcm_config);
> +
> /**
> * zynqmp_pm_force_powerdown - PM call to request for another PU or
> subsystem to
> * be powered down forcefully
> @@ -881,6 +936,50 @@ int zynqmp_pm_request_wakeup(const u32 node,
> }
> EXPORT_SYMBOL_GPL(zynqmp_pm_request_wakeup);
>
> +/**
> + * zynqmp_pm_get_node_status - PM call to request a node's current power
> state
> + * @node: ID of the component or sub-system in question
> + * @status:Current operating state of the requested node
> + * @requirements: Current requirements asserted on the node,
> + * used for slave nodes only.
> + * @usage: Usage information, used for slave nodes only:
> + * PM_USAGE_NO_MASTER - No master is currently using
> + * the node
> + * PM_USAGE_CURRENT_MASTER - Only requesting master is
> + * currently using the node
> + * PM_USAGE_OTHER_MASTER - Only other masters are
> + * currently using the node
> + * PM_USAGE_BOTH_MASTERS - Both the current and at least
> + * one other master is currently
> + * using the node
> + *
> + * Return: status, either success or error+reason
> + */
> +int zynqmp_pm_get_node_status(const u32 node,
> + u32 *status, u32 *requirements,
> + u32 *usage)
> +
> +{
> + u32 ret_payload[PAYLOAD_ARG_CNT];
> + int ret;
> +
> + if (!status)
> + return -EINVAL;
> +
> + ret = zynqmp_pm_invoke_fn(PM_GET_NODE_STATUS, node, 0, 0, 0,
> + ret_payload);
> + if (ret_payload[0] == XST_PM_SUCCESS) {
> + *status = ret_payload[1];
> + if (requirements)
> + *requirements = ret_payload[2];
> + if (usage)
> + *usage =
RE: [PATCH v8 3/5] firmware: xilinx: Add RPU configuration APIs
> -Original Message-
> From: Stefano Stabellini
> Sent: Thursday, August 13, 2020 1:36 PM
> To: Ben Levinsky
> Cc: Stefano Stabellini ; Michal Simek
> ; devicet...@vger.kernel.org;
> mathieu.poir...@linaro.org; Ed T. Mooring ; linux-
> remotep...@vger.kernel.org; linux-kernel@vger.kernel.org;
> robh...@kernel.org; Jiaying Liang ; linux-arm-
> ker...@lists.infradead.org
> Subject: Re: [PATCH v8 3/5] firmware: xilinx: Add RPU configuration APIs
>
> On Mon, 10 Aug 2020, Ben Levinsky wrote:
> > This patch adds APIs to provide access and a configuration interface
> > to the current power state of a sub-system on Zynqmp sub-system.
>
> This doesn't read correctly
>
[Ben Levinsky] ok will update commit message as follows in v9:
firmware: xilinx: Add RPU configuration APIs
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
> > ---
> > v3:
> > - add xilinx-related platform mgmt fn's instead of wrapping around
> > function pointer in xilinx eemi ops struct
> > v4:
> > - add default values for enums
> > ---
> > drivers/firmware/xilinx/zynqmp.c | 99
> > include/linux/firmware/xlnx-zynqmp.h | 34 ++
> > 2 files changed, 133 insertions(+)
> >
> > diff --git a/drivers/firmware/xilinx/zynqmp.c
> b/drivers/firmware/xilinx/zynqmp.c
> > index f1a0bd35deeb..fdd61d258e55 100644
> > --- a/drivers/firmware/xilinx/zynqmp.c
> > +++ b/drivers/firmware/xilinx/zynqmp.c
> > @@ -846,6 +846,61 @@ 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
> > + * @arg1: Argument 1 to requested IOCTL call
> > + * @arg2: Argument 2 to requested IOCTL call
> > + * @out: Returned output value
> > + *
> > + * Return: RPU mode
>
> What kind of output are we expecting? An enum? Which one?
>
[Ben Levinsky] will update the return comment. Return: RPU mode. This is enum
rpu_oper_mode
>
> > + */
> > +int zynqmp_pm_get_rpu_mode(u32 node_id,
> > + u32 arg1, u32 arg2, u32 *out)
> > +{
> > + return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
> > + IOCTL_GET_RPU_OPER_MODE, 0, 0, out);
> > +}
> > +EXPORT_SYMBOL_GPL(zynqmp_pm_get_rpu_mode);
> > +
> > +/**
> > + * zynqmp_pm_set_rpu_mode() - Set RPU mode
> > + * @node_id: Node ID of the device
> > + * @ioctl_id: ID of the requested IOCTL
> > + * @arg2: Argument 2 to requested IOCTL call
> > + * @out: Returned output value
> > + *
> > + * This function is used to set RPU mode.
> > + *
> > + * Return: Returns status, either success or error+reason
> > + */
> > +int zynqmp_pm_set_rpu_mode(u32 node_id,
> > + u32 arg1, u32 arg2, u32 *out)
> > +{
> > + return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
> > + IOCTL_SET_RPU_OPER_MODE, 0, 0, out);
> > +}
> > +EXPORT_SYMBOL_GPL(zynqmp_pm_set_rpu_mode);
> > +
> > +/**
> > + * zynqmp_pm_tcm_comb_config - configure TCM
> > + * @node_id: Node ID of the device
> > + * @arg1: Argument 1 to requested IOCTL call
> > + * @arg2: Argument 2 to requested IOCTL call
> > + * @out: Returned output value
>
> Same question on the type of the output value
>
[Ben Levinsky] will update in v9: Returns status: 0 for success, else failure
>
> > + * This function is used to set RPU mode.
> > + *
> > + * Return: Returns status, either success or error+reason
> > + */
> > +int zynqmp_pm_set_tcm_config(u32 node_id,
> > +u32 arg1, u32 arg2, u32 *out)
> > +{
> > + return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
> > + IOCTL_TCM_COMB_CONFIG, 0, 0, out);
> > +}
> > +EXPORT_SYMBOL_GPL(zynqmp_pm_set_tcm_config);
> > +
> > /**
> > * zynqmp_pm_force_powerdown - PM call to request for another PU or
> subsystem to
> > * be powered down forcefully
> > @@ -881,6 +936,50 @@ int zynqmp_pm_request_wakeup(const u32 node,
> > }
> > EXPORT_SYMBOL_GPL(zynqmp_pm_request_wakeup);
> >
> > +/**
> > + * zynqmp_pm_get_node_status -
Re: [PATCH v8 3/5] firmware: xilinx: Add RPU configuration APIs
On Mon, 10 Aug 2020, Ben Levinsky wrote:
> This patch adds APIs to provide access and a configuration interface
> to the current power state of a sub-system on Zynqmp sub-system.
This doesn't read correctly
> Signed-off-by: Ben Levinsky
> ---
> v3:
> - add xilinx-related platform mgmt fn's instead of wrapping around
> function pointer in xilinx eemi ops struct
> v4:
> - add default values for enums
> ---
> drivers/firmware/xilinx/zynqmp.c | 99
> include/linux/firmware/xlnx-zynqmp.h | 34 ++
> 2 files changed, 133 insertions(+)
>
> diff --git a/drivers/firmware/xilinx/zynqmp.c
> b/drivers/firmware/xilinx/zynqmp.c
> index f1a0bd35deeb..fdd61d258e55 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -846,6 +846,61 @@ 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
> + * @arg1:Argument 1 to requested IOCTL call
> + * @arg2:Argument 2 to requested IOCTL call
> + * @out: Returned output value
> + *
> + * Return: RPU mode
What kind of output are we expecting? An enum? Which one?
> + */
> +int zynqmp_pm_get_rpu_mode(u32 node_id,
> +u32 arg1, u32 arg2, u32 *out)
> +{
> + return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
> +IOCTL_GET_RPU_OPER_MODE, 0, 0, out);
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_get_rpu_mode);
> +
> +/**
> + * zynqmp_pm_set_rpu_mode() - Set RPU mode
> + * @node_id: Node ID of the device
> + * @ioctl_id:ID of the requested IOCTL
> + * @arg2:Argument 2 to requested IOCTL call
> + * @out: Returned output value
> + *
> + * This function is used to set RPU mode.
> + *
> + * Return: Returns status, either success or error+reason
> + */
> +int zynqmp_pm_set_rpu_mode(u32 node_id,
> +u32 arg1, u32 arg2, u32 *out)
> +{
> + return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
> +IOCTL_SET_RPU_OPER_MODE, 0, 0, out);
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_set_rpu_mode);
> +
> +/**
> + * zynqmp_pm_tcm_comb_config - configure TCM
> + * @node_id: Node ID of the device
> + * @arg1:Argument 1 to requested IOCTL call
> + * @arg2:Argument 2 to requested IOCTL call
> + * @out: Returned output value
Same question on the type of the output value
> + * This function is used to set RPU mode.
> + *
> + * Return: Returns status, either success or error+reason
> + */
> +int zynqmp_pm_set_tcm_config(u32 node_id,
> + u32 arg1, u32 arg2, u32 *out)
> +{
> + return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
> +IOCTL_TCM_COMB_CONFIG, 0, 0, out);
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_set_tcm_config);
> +
> /**
> * zynqmp_pm_force_powerdown - PM call to request for another PU or
> subsystem to
> * be powered down forcefully
> @@ -881,6 +936,50 @@ int zynqmp_pm_request_wakeup(const u32 node,
> }
> EXPORT_SYMBOL_GPL(zynqmp_pm_request_wakeup);
>
> +/**
> + * zynqmp_pm_get_node_status - PM call to request a node's current power
> state
> + * @node: ID of the component or sub-system in question
> + * @status:Current operating state of the requested node
> + * @requirements: Current requirements asserted on the node,
> + * used for slave nodes only.
> + * @usage: Usage information, used for slave nodes only:
> + * PM_USAGE_NO_MASTER - No master is currently using
> + * the node
> + * PM_USAGE_CURRENT_MASTER - Only requesting master is
> + * currently using the node
> + * PM_USAGE_OTHER_MASTER - Only other masters are
> + * currently using the node
> + * PM_USAGE_BOTH_MASTERS - Both the current and at least
> + * one other master is currently
> + * using the node
> + *
> + * Return: status, either success or error+reason
Same question on status
> + */
> +int zynqmp_pm_get_node_status(const u32 node,
> + u32 *status, u32 *requirements,
> + u32 *usage)
> +
> +{
> + u32 ret_payload[PAYLOAD_ARG_CNT];
> + int ret;
> +
> + if (!status)
> + return -EINVAL;
> +
> + ret = zynqmp_pm_invoke_fn(PM_GET_NODE_STATUS, node, 0, 0, 0,
> + ret_payload);
> + if (ret_payload[0] == XST_PM_SUCCESS) {
> + *status = ret_payload[1];
> + if (requirements)
> + *requirements = ret_payload[2];
> + if (usage)
> + *usage = ret_payload[3];
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_get_node_status);
> +
> /**
> * zynqmp_pm_set_requirement() - PM call to set requirement for PM slaves
> * @node:Node ID of the slave
>
