linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Vesa Jääskeläinen" <dachaac@gmail.com>
To: Nava kishore Manne <nava.manne@xilinx.com>,
	p.zabel@pengutronix.de, robh+dt@kernel.org, mark.rutland@arm.com,
	michal.simek@xilinx.com, rajanv@xilinx.com, jollys@xilinx.com,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, chinnikishore369@gmail.com
Subject: Re: [PATCH v2 1/3] firmware: xilinx: Add reset API's
Date: Thu, 25 Oct 2018 23:43:24 +0300	[thread overview]
Message-ID: <e5e8a803-0563-b52b-668e-010256705e2e@gmail.com> (raw)
In-Reply-To: <20181026122424.30831-2-nava.manne@xilinx.com>

Hi,

Some comments below.

On 26/10/2018 15.24, Nava kishore Manne wrote:
> This Patch Adds reset API's to support release, assert
> and status functionalities by using firmware interface.
>
> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> ---
> Changes for v2:
> 		-None.
>
> Changes for v1:
> 		-None.
>
> Changes for RFC-V3:
> 		-None.
>
> Changes for RFC-V2:
> 		-New patch.
>
>   drivers/firmware/xilinx/zynqmp.c     |  40 +++++++++++
>   include/linux/firmware/xlnx-zynqmp.h | 136 +++++++++++++++++++++++++++++++++++
>   2 files changed, 176 insertions(+)
>
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index 84b3fd2..cd13b06 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -428,6 +428,44 @@ static int zynqmp_pm_clock_getparent(u32 clock_id, u32 *parent_id)
>   	return ret;
>   }
>   
> +/**
> + * zynqmp_pm_reset_assert - Request setting of reset (1 - assert, 0 - release)
> + * @reset:		Reset to be configured
> + * @assert_flag:	Flag stating should reset be asserted (1) or
> + *			released (0)
> + *
> + * Return: Returns status, either success or error+reason
> + */
> +static int zynqmp_pm_reset_assert(const enum zynqmp_pm_reset reset,
> +				  const enum zynqmp_pm_reset_action assert_flag)
> +{
> +	return zynqmp_pm_invoke_fn(PM_RESET_ASSERT, reset, assert_flag,
> +				   0, 0, NULL);
> +}
> +
> +/**
> + * zynqmp_pm_reset_get_status - Get status of the reset
> + * @reset:      Reset whose status should be returned
> + * @status:     Returned status
> + *
> + * Return: Returns status, either success or error+reason
> + */
> +static int zynqmp_pm_reset_get_status(const enum zynqmp_pm_reset reset,
> +				      u32 *status)
> +{
> +	u32 ret_payload[PAYLOAD_ARG_CNT];
> +	int ret;
> +
> +	if (!status)
> +		return -EINVAL;
> +
> +	ret = zynqmp_pm_invoke_fn(PM_RESET_GET_STATUS, reset, 0,
> +				  0, 0, ret_payload);
> +	*status = ret_payload[1];
> +
> +	return ret;
> +}
> +
>   static const struct zynqmp_eemi_ops eemi_ops = {
>   	.get_api_version = zynqmp_pm_get_api_version,
>   	.query_data = zynqmp_pm_query_data,
> @@ -440,6 +478,8 @@ static const struct zynqmp_eemi_ops eemi_ops = {
>   	.clock_getrate = zynqmp_pm_clock_getrate,
>   	.clock_setparent = zynqmp_pm_clock_setparent,
>   	.clock_getparent = zynqmp_pm_clock_getparent,
> +	.reset_assert = zynqmp_pm_reset_assert,
> +	.reset_get_status = zynqmp_pm_reset_get_status,
>   };
>   
>   /**
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index 015e130..29d83ca 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -34,6 +34,8 @@
>   
>   enum pm_api_id {
>   	PM_GET_API_VERSION = 1,
> +	PM_RESET_ASSERT = 17,
> +	PM_RESET_GET_STATUS,
>   	PM_QUERY_DATA = 35,
>   	PM_CLOCK_ENABLE,
>   	PM_CLOCK_DISABLE,
> @@ -73,6 +75,137 @@ enum pm_query_id {
>   	PM_QID_CLOCK_GET_ATTRIBUTES,
>   };
>   
> +enum zynqmp_pm_reset_action {
> +	PM_RESET_ACTION_RELEASE,
> +	PM_RESET_ACTION_ASSERT,
> +	PM_RESET_ACTION_PULSE,
> +};
> +
> +enum zynqmp_pm_reset {
> +	ZYNQMP_PM_RESET_START = 999,
> +	ZYNQMP_PM_RESET_PCIE_CFG,
Why not just do:

+	ZYNQMP_PM_RESET_START = 1000,
+	ZYNQMP_PM_RESET_PCIE_CFG = 1000,

This way it would not then indicate that START is actual reset line.

> +	ZYNQMP_PM_RESET_PCIE_BRIDGE,
> +	ZYNQMP_PM_RESET_PCIE_CTRL,
> +	ZYNQMP_PM_RESET_DP,
> +	ZYNQMP_PM_RESET_SWDT_CRF,
> +	ZYNQMP_PM_RESET_AFI_FM5,
> +	ZYNQMP_PM_RESET_AFI_FM4,
> +	ZYNQMP_PM_RESET_AFI_FM3,
> +	ZYNQMP_PM_RESET_AFI_FM2,
> +	ZYNQMP_PM_RESET_AFI_FM1,
> +	ZYNQMP_PM_RESET_AFI_FM0,
> +	ZYNQMP_PM_RESET_GDMA,
> +	ZYNQMP_PM_RESET_GPU_PP1,
> +	ZYNQMP_PM_RESET_GPU_PP0,
> +	ZYNQMP_PM_RESET_GPU,
> +	ZYNQMP_PM_RESET_GT,
> +	ZYNQMP_PM_RESET_SATA,
> +	ZYNQMP_PM_RESET_ACPU3_PWRON,
> +	ZYNQMP_PM_RESET_ACPU2_PWRON,
> +	ZYNQMP_PM_RESET_ACPU1_PWRON,
> +	ZYNQMP_PM_RESET_ACPU0_PWRON,
> +	ZYNQMP_PM_RESET_APU_L2,
> +	ZYNQMP_PM_RESET_ACPU3,
> +	ZYNQMP_PM_RESET_ACPU2,
> +	ZYNQMP_PM_RESET_ACPU1,
> +	ZYNQMP_PM_RESET_ACPU0,
> +	ZYNQMP_PM_RESET_DDR,
> +	ZYNQMP_PM_RESET_APM_FPD,
> +	ZYNQMP_PM_RESET_SOFT,
> +	ZYNQMP_PM_RESET_GEM0,
> +	ZYNQMP_PM_RESET_GEM1,
> +	ZYNQMP_PM_RESET_GEM2,
> +	ZYNQMP_PM_RESET_GEM3,
> +	ZYNQMP_PM_RESET_QSPI,
> +	ZYNQMP_PM_RESET_UART0,
> +	ZYNQMP_PM_RESET_UART1,
> +	ZYNQMP_PM_RESET_SPI0,
> +	ZYNQMP_PM_RESET_SPI1,
> +	ZYNQMP_PM_RESET_SDIO0,
> +	ZYNQMP_PM_RESET_SDIO1,
> +	ZYNQMP_PM_RESET_CAN0,
> +	ZYNQMP_PM_RESET_CAN1,
> +	ZYNQMP_PM_RESET_I2C0,
> +	ZYNQMP_PM_RESET_I2C1,
> +	ZYNQMP_PM_RESET_TTC0,
> +	ZYNQMP_PM_RESET_TTC1,
> +	ZYNQMP_PM_RESET_TTC2,
> +	ZYNQMP_PM_RESET_TTC3,
> +	ZYNQMP_PM_RESET_SWDT_CRL,
> +	ZYNQMP_PM_RESET_NAND,
> +	ZYNQMP_PM_RESET_ADMA,
> +	ZYNQMP_PM_RESET_GPIO,
> +	ZYNQMP_PM_RESET_IOU_CC,
> +	ZYNQMP_PM_RESET_TIMESTAMP,
> +	ZYNQMP_PM_RESET_RPU_R50,
> +	ZYNQMP_PM_RESET_RPU_R51,
> +	ZYNQMP_PM_RESET_RPU_AMBA,
> +	ZYNQMP_PM_RESET_OCM,
> +	ZYNQMP_PM_RESET_RPU_PGE,
> +	ZYNQMP_PM_RESET_USB0_CORERESET,
> +	ZYNQMP_PM_RESET_USB1_CORERESET,
> +	ZYNQMP_PM_RESET_USB0_HIBERRESET,
> +	ZYNQMP_PM_RESET_USB1_HIBERRESET,
> +	ZYNQMP_PM_RESET_USB0_APB,
> +	ZYNQMP_PM_RESET_USB1_APB,
> +	ZYNQMP_PM_RESET_IPI,
> +	ZYNQMP_PM_RESET_APM_LPD,
> +	ZYNQMP_PM_RESET_RTC,
> +	ZYNQMP_PM_RESET_SYSMON,
> +	ZYNQMP_PM_RESET_AFI_FM6,
> +	ZYNQMP_PM_RESET_LPD_SWDT,
> +	ZYNQMP_PM_RESET_FPD,
> +	ZYNQMP_PM_RESET_RPU_DBG1,
> +	ZYNQMP_PM_RESET_RPU_DBG0,
> +	ZYNQMP_PM_RESET_DBG_LPD,
> +	ZYNQMP_PM_RESET_DBG_FPD,
> +	ZYNQMP_PM_RESET_APLL,
> +	ZYNQMP_PM_RESET_DPLL,
> +	ZYNQMP_PM_RESET_VPLL,
> +	ZYNQMP_PM_RESET_IOPLL,
> +	ZYNQMP_PM_RESET_RPLL,
> +	ZYNQMP_PM_RESET_GPO3_PL_0,
> +	ZYNQMP_PM_RESET_GPO3_PL_1,
> +	ZYNQMP_PM_RESET_GPO3_PL_2,
> +	ZYNQMP_PM_RESET_GPO3_PL_3,
> +	ZYNQMP_PM_RESET_GPO3_PL_4,
> +	ZYNQMP_PM_RESET_GPO3_PL_5,
> +	ZYNQMP_PM_RESET_GPO3_PL_6,
> +	ZYNQMP_PM_RESET_GPO3_PL_7,
> +	ZYNQMP_PM_RESET_GPO3_PL_8,
> +	ZYNQMP_PM_RESET_GPO3_PL_9,
> +	ZYNQMP_PM_RESET_GPO3_PL_10,
> +	ZYNQMP_PM_RESET_GPO3_PL_11,
> +	ZYNQMP_PM_RESET_GPO3_PL_12,
> +	ZYNQMP_PM_RESET_GPO3_PL_13,
> +	ZYNQMP_PM_RESET_GPO3_PL_14,
> +	ZYNQMP_PM_RESET_GPO3_PL_15,
> +	ZYNQMP_PM_RESET_GPO3_PL_16,
> +	ZYNQMP_PM_RESET_GPO3_PL_17,
> +	ZYNQMP_PM_RESET_GPO3_PL_18,
> +	ZYNQMP_PM_RESET_GPO3_PL_19,
> +	ZNQMP_PM_RESET_GPO3_PL_20,
Typo here. Missing Y.

Thanks,
Vesa Jääskeläinen

  reply	other threads:[~2018-10-25 20:43 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-26 12:24 [PATCH v2 0/3] Add reset driver support for ZynqMP Nava kishore Manne
2018-10-26 12:24 ` [PATCH v2 1/3] firmware: xilinx: Add reset API's Nava kishore Manne
2018-10-25 20:43   ` Vesa Jääskeläinen [this message]
2018-10-26 12:24 ` [PATCH v2 2/3] dt-bindings: reset: Add bindings for ZynqMP reset driver Nava kishore Manne
2018-10-26 12:24 ` [PATCH v2 3/3] reset: reset-zynqmp: Adding support for Xilinx zynqmp reset controller Nava kishore Manne
2018-10-25 20:53   ` Vesa Jääskeläinen
2018-10-26 11:08   ` Philipp Zabel

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=e5e8a803-0563-b52b-668e-010256705e2e@gmail.com \
    --to=dachaac@gmail.com \
    --cc=chinnikishore369@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jollys@xilinx.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=michal.simek@xilinx.com \
    --cc=nava.manne@xilinx.com \
    --cc=p.zabel@pengutronix.de \
    --cc=rajanv@xilinx.com \
    --cc=robh+dt@kernel.org \
    /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 NNTP newsgroup(s).