[RFC PATCH v3 0/3] Add reset driver support for ZynqMP

[RFC PATCH v3 0/3] Add reset driver support for ZynqMP

[Nava kishore Manne]

This series of patches are created On top of the
below series of patches.
https://lkml.org/lkml/2018/8/3/687 

Nava kishore Manne (3):
  firmware: xilinx: Add reset API's
  dt-bindings: reset: Add bindings for ZynqMP reset driver
  reset: reset-zynqmp: Adding support for Xilinx zynqmp reset
    controller.

 .../firmware/xilinx/xlnx,zynqmp-firmware.txt       | 142 +++++++++++++++++++++
 drivers/firmware/xilinx/zynqmp.c                   |  40 ++++++
 drivers/reset/Makefile                             |   1 +
 drivers/reset/reset-zynqmp.c                       | 115 +++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h               | 136 ++++++++++++++++++++
 5 files changed, 434 insertions(+)
 create mode 100644 drivers/reset/reset-zynqmp.c

-- 
2.7.4

[RFC PATCH v3 1/3] firmware: xilinx: Add reset API's

[Nava kishore Manne]

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 v3:
		-None.
Changes for 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 7ccedf0..639c72f 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -447,6 +447,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,
 	.ioctl = zynqmp_pm_ioctl,
@@ -460,6 +498,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 58a7478..93decf5 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_IOCTL = 34,
 	PM_QUERY_DATA,
 	PM_CLOCK_ENABLE,
@@ -74,6 +76,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,
+	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,
+	ZYNQMP_PM_RESET_GPO3_PL_21,
+	ZYNQMP_PM_RESET_GPO3_PL_22,
+	ZYNQMP_PM_RESET_GPO3_PL_23,
+	ZYNQMP_PM_RESET_GPO3_PL_24,
+	ZYNQMP_PM_RESET_GPO3_PL_25,
+	ZYNQMP_PM_RESET_GPO3_PL_26,
+	ZYNQMP_PM_RESET_GPO3_PL_27,
+	ZYNQMP_PM_RESET_GPO3_PL_28,
+	ZYNQMP_PM_RESET_GPO3_PL_29,
+	ZYNQMP_PM_RESET_GPO3_PL_30,
+	ZYNQMP_PM_RESET_GPO3_PL_31,
+	ZYNQMP_PM_RESET_RPU_LS,
+	ZYNQMP_PM_RESET_PS_ONLY,
+	ZYNQMP_PM_RESET_PL,
+	ZYNQMP_PM_RESET_PS_PL0,
+	ZYNQMP_PM_RESET_PS_PL1,
+	ZYNQMP_PM_RESET_PS_PL2,
+	ZYNQMP_PM_RESET_PS_PL3,
+	ZYNQMP_PM_RESET_END
+};
+
 /**
  * struct zynqmp_pm_query_data - PM query data
  * @qid:	query ID
@@ -101,6 +234,9 @@ struct zynqmp_eemi_ops {
 	int (*clock_getrate)(u32 clock_id, u64 *rate);
 	int (*clock_setparent)(u32 clock_id, u32 parent_id);
 	int (*clock_getparent)(u32 clock_id, u32 *parent_id);
+	int (*reset_assert)(const enum zynqmp_pm_reset reset,
+			    const enum zynqmp_pm_reset_action assert_flag);
+	int (*reset_get_status)(const enum zynqmp_pm_reset reset, u32 *status);
 };
 
 #if IS_REACHABLE(CONFIG_ARCH_ZYNQMP)
-- 
2.7.4

Re: [RFC PATCH v3 1/3] firmware: xilinx: Add reset API's

[Philipp Zabel]

Hi,

On Wed, 2018-09-05 at 12:39 +0530, 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 v3:
> 		-None.
> Changes for 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 7ccedf0..639c72f 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -447,6 +447,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];

It doesn't really matter here, but in general I'd skip writing output
arguments in case of error.
For all I know, the result returned in ret_payload could be undefined.

regards
Philipp

RE: [RFC PATCH v3 1/3] firmware: xilinx: Add reset API's

[Nava kishore Manne]

Hi Philipp

Thanks for the quick response...
Please find my response inline.

> -----Original Message-----
> From: Philipp Zabel [mailto:p.zabel@pengutronix.de]
> Sent: Wednesday, September 5, 2018 4:00 PM
> To: Nava kishore Manne <navam@xilinx.com>; robh+dt@kernel.org;
> mark.rutland@arm.com; Michal Simek <michals@xilinx.com>; Rajan Vaja
> <RAJANV@xilinx.com>; Jolly Shah <JOLLYS@xilinx.com>;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [RFC PATCH v3 1/3] firmware: xilinx: Add reset API's
> 
> Hi,
> 
> On Wed, 2018-09-05 at 12:39 +0530, 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 v3:
> > 		-None.
> > Changes for 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 7ccedf0..639c72f 100644
> > --- a/drivers/firmware/xilinx/zynqmp.c
> > +++ b/drivers/firmware/xilinx/zynqmp.c
> > @@ -447,6 +447,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];
> 
> It doesn't really matter here, but in general I'd skip writing output arguments in
> case of error.
> For all I know, the result returned in ret_payload could be undefined.
> 

Will fix in the next version.

Regards,
Navakishore.

[RFC PATCH v3 2/3] dt-bindings: reset: Add bindings for ZynqMP reset driver

[Nava kishore Manne]

Add documentation to describe Xilinx ZynqMP reset driver
bindings.

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
---
Changes for v3:
		-Corrected Commit Msg. 
Changes for v2:
		-Moved reset node as a child to firwmare
		 node.

 .../firmware/xilinx/xlnx,zynqmp-firmware.txt       | 142 +++++++++++++++++++++
 1 file changed, 142 insertions(+)

diff --git a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
index 1b431d9..351b1bb 100644
--- a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
+++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
@@ -17,6 +17,144 @@ Required properties:
 		  - "smc" : SMC #0, following the SMCCC
 		  - "hvc" : HVC #0, following the SMCCC
 
+--------------------------------------------------------------------------
+ =  Zynq UltraScale+ MPSoC reset driver binding =
+--------------------------------------------------------------------------
+The Zynq UltraScale+ MPSoC has several different resets.
+
+See Chapter 36 of the Zynq UltraScale+ MPSoC TRM (UG) for more information
+about zynqmp resets.
+
+Please also refer to reset.txt in this directory for common reset
+controller binding usage.
+
+Required Properties:
+- compatible:	"xlnx,zynqmp-reset"
+- #reset-cells:	Specifies the number of cells needed to encode reset
+		line, should be 1
+
+Reset outputs:
+	0   :PCIE config reset.
+	1   :PCIE bridge block level reset (AXI interface).
+	2   :PCIE control block level,reset.
+	3   :Display Port block level reset (includes DPDMA).
+	4   :FPD WDT reset.
+	5   :AF_FM5 block level reset.
+	6   :AF_FM4 block level reset.
+	7   :AF_FM3 block level reset.
+	8   :AF_FM2 block level reset.
+	9   :AF_FM1 block level reset.
+	10  :AF_FM0 block level reset.
+	11  :GDMA block level reset.
+	12  :Pixel Processor (GPU_PP1) block level reset.
+	13  :Pixel Processor (GPU_PP0) block level reset.
+	14  :GPU block level reset.
+	15  :GT block level reset.
+	16  :Sata block level reset.
+	17  :ACPU3 power on reset.
+	18  :ACPU2 power on reset.
+	19  :ACPU1 power on reset.
+	20  :ACPU0 power on reset.
+	21  :APU L2 reset.
+	22  :ACPU3 reset.
+	23  :ACPU2 reset.
+	24  :ACPU1 reset.
+	25  :ACPU0 reset.
+	26  :DDR block level reset inside of the DDR Sub System.
+	27  :APM block level reset inside of the DDR Sub System.
+	28  :soft reset.
+	29  :GEM 0 reset.
+	30  :GEM 1 reset.
+	31  :GEM 2 reset.
+	32  :GEM 3 reset.
+	33  :qspi reset.
+	34  :uart0 reset.
+	35  :uart1 reset.
+	36  :spi0 reset.
+	37  :spi1 reset.
+	38  :sdio0 reset.
+	39  :sdio1 reset.
+	40  :can0 reset.
+	41  :can1 reset.
+	42  :i2c0 reset.
+	43  :i2c1 reset.
+	44  :ttc0 reset.
+	45  :ttc1 reset.
+	46  :ttc2 reset.
+	47  :ttc3 reset.
+	48  :swdt reset.
+	49  :nand reset.
+	50  :adma reset.
+	51  :gpio reset.
+	52  :iou_cc reset.
+	53  :timestamp reset.
+	54  :rpu_r50 reset.
+	55  :rpu r51 reset.
+	56  :rpu_amba reset.
+	57  :ocm reset.
+	58  :rpu_pge reset.
+	59  :usb0_core reset.
+	60  :usb1_core reset.
+	61  :usb0_hiber reset.
+	62  :usb1_hiber reset.
+	63  :usb0_apb reset.
+	64  :usb1_apb reset.
+	65  :ipi reset.
+	66  :apm reset.
+	67  :rtc reset.
+	68  :sysmon reset.
+	69  :afi_fm6 reset.
+	70  :lpd_swdt reset.
+	71  :fpd_reset.
+	72  :rpu_dbg1 reset.
+	73  :rpu_dbg0 reset.
+	74  :dbg_lpd reset.
+	75  :dbg_fpd reset.
+	76  :apll reset.
+	77  :dpll reset.
+	78  :vpll reset.
+	79  :iopll reset.
+	80  :rpll reset.
+	81  :gpio_pl_0 reset.
+	82  :gpio_pl_1 reset.
+	83  :gpio_pl_2 reset.
+	84  :gpio_pl_3 reset.
+	85  :gpio_pl_4 reset.
+	86  :gpio_pl_5 reset.
+	87  :gpio_pl_6 reset.
+	88  :gpio_pl_7 reset.
+	89  :gpio_pl_8 reset.
+	90  :gpio_pl_9 reset.
+	91  :gpio_pl_10 reset.
+	92  :gpio_pl_11 reset.
+	93  :gpio_pl_12 reset.
+	94  :gpio_pl_13 reset.
+	95  :gpio_pl_14 reset.
+	96  :gpio_pl_15 reset.
+	97  :gpio_pl_16 reset.
+	98  :gpio_pl_17 reset.
+	99  :gpio_pl_18 reset.
+	100 :gpio_pl_19 reset.
+	101 :gpio_pl_20 reset.
+	102 :gpio_pl_21 reset.
+	103 :gpio_pl_22 reset.
+	104 :gpio_pl_23 reset.
+	105 :gpio_pl_24 reset.
+	106 :gpio_pl_25 reset.
+	107 :gpio_pl_26 reset.
+	108 :gpio_pl_27 reset.
+	109 :gpio_pl_28 reset.
+	110 :gpio_pl_29 reset.
+	111 :gpio_pl_30 reset.
+	112 :gpio_pl_31 reset.
+	113 :rpu_ls reset.
+	114 :ps_only reset.
+	115 :pl reset.
+	116 :ps_pl0 reset
+	117 :ps_pl1 reset
+	118 :ps_pl2 reset
+	119 :ps_pl3 reset
+
 -------
 Example
 -------
@@ -25,5 +163,9 @@ firmware {
 	zynqmp_firmware: zynqmp-firmware {
 		compatible = "xlnx,zynqmp-firmware";
 		method = "smc";
+		reset-controller:reset-controller@0 {
+			compatible = "xlnx,zynqmp-reset";
+			#reset-cells = <1>;
+		};
 	};
 };
-- 
2.7.4

Re: [RFC PATCH v3 2/3] dt-bindings: reset: Add bindings for ZynqMP reset driver

[Philipp Zabel]

Hi,

On Wed, 2018-09-05 at 12:39 +0530, Nava kishore Manne wrote:
> Add documentation to describe Xilinx ZynqMP reset driver
> bindings.
> 
> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> ---
> Changes for v3:
> 		-Corrected Commit Msg. 
> Changes for v2:
> 		-Moved reset node as a child to firwmare
> 		 node.
> 
>  .../firmware/xilinx/xlnx,zynqmp-firmware.txt       | 142 +++++++++++++++++++++
>  1 file changed, 142 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
> index 1b431d9..351b1bb 100644
> --- a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
> +++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
[...]
>  -------
>  Example
>  -------
> @@ -25,5 +163,9 @@ firmware {
>  	zynqmp_firmware: zynqmp-firmware {
>  		compatible = "xlnx,zynqmp-firmware";
>  		method = "smc";
> +		reset-controller:reset-controller@0 {

I think the label should use underscore instead of hyphen, and the unit
address part should be removed from the node name. There is no unit
address, as there is no reg property inside the node and the parent node
doesn't have #address-cells:

		reset_controller: reset-controller {

> +			compatible = "xlnx,zynqmp-reset";
> +			#reset-cells = <1>;
> +		};
>  	};
>  };

regards
Philipp

RE: [RFC PATCH v3 2/3] dt-bindings: reset: Add bindings for ZynqMP reset driver

[Nava kishore Manne]

Hi Philipp

Thanks for the quick response..
Please find my commnets inline.

> -----Original Message-----
> From: Philipp Zabel [mailto:p.zabel@pengutronix.de]
> Sent: Wednesday, September 5, 2018 3:40 PM
> To: Nava kishore Manne <navam@xilinx.com>; robh+dt@kernel.org;
> mark.rutland@arm.com; Michal Simek <michals@xilinx.com>; Rajan Vaja
> <RAJANV@xilinx.com>; Jolly Shah <JOLLYS@xilinx.com>;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [RFC PATCH v3 2/3] dt-bindings: reset: Add bindings for ZynqMP
> reset driver
> 
> Hi,
> 
> On Wed, 2018-09-05 at 12:39 +0530, Nava kishore Manne wrote:
> > Add documentation to describe Xilinx ZynqMP reset driver bindings.
> >
> > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> > ---
> > Changes for v3:
> > 		-Corrected Commit Msg.
> > Changes for v2:
> > 		-Moved reset node as a child to firwmare
> > 		 node.
> >
> >  .../firmware/xilinx/xlnx,zynqmp-firmware.txt       | 142
> +++++++++++++++++++++
> >  1 file changed, 142 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmwa
> > re.txt
> > b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmwa
> > re.txt
> > index 1b431d9..351b1bb 100644
> > ---
> > a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmwa
> > re.txt
> > +++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-fi
> > +++ rmware.txt
> [...]
> >  -------
> >  Example
> >  -------
> > @@ -25,5 +163,9 @@ firmware {
> >  	zynqmp_firmware: zynqmp-firmware {
> >  		compatible = "xlnx,zynqmp-firmware";
> >  		method = "smc";
> > +		reset-controller:reset-controller@0 {
> 
> I think the label should use underscore instead of hyphen, and the unit address
> part should be removed from the node name. There is no unit address, as there
> is no reg property inside the node and the parent node doesn't have #address-
> cells:
> 

Will fix in the next version.

Regards,
Navakishore.

Re: [RFC PATCH v3 2/3] dt-bindings: reset: Add bindings for ZynqMP reset driver

[Rob Herring]

On Wed, Sep 05, 2018 at 12:39:01PM +0530, Nava kishore Manne wrote:
> Add documentation to describe Xilinx ZynqMP reset driver
> bindings.
> 
> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> ---
> Changes for v3:
> 		-Corrected Commit Msg. 
> Changes for v2:
> 		-Moved reset node as a child to firwmare
> 		 node.

Why? There's no reason for that. The parent can be multiple providers. 
You only need child nodes if the sub functions have their own DT 
resources.

> 
>  .../firmware/xilinx/xlnx,zynqmp-firmware.txt       | 142 +++++++++++++++++++++
>  1 file changed, 142 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
> index 1b431d9..351b1bb 100644
> --- a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
> +++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
> @@ -17,6 +17,144 @@ Required properties:
>  		  - "smc" : SMC #0, following the SMCCC
>  		  - "hvc" : HVC #0, following the SMCCC
>  
> +--------------------------------------------------------------------------
> + =  Zynq UltraScale+ MPSoC reset driver binding =
> +--------------------------------------------------------------------------
> +The Zynq UltraScale+ MPSoC has several different resets.
> +
> +See Chapter 36 of the Zynq UltraScale+ MPSoC TRM (UG) for more information
> +about zynqmp resets.
> +
> +Please also refer to reset.txt in this directory for common reset
> +controller binding usage.
> +
> +Required Properties:
> +- compatible:	"xlnx,zynqmp-reset"
> +- #reset-cells:	Specifies the number of cells needed to encode reset
> +		line, should be 1
> +
> +Reset outputs:
> +	0   :PCIE config reset.
> +	1   :PCIE bridge block level reset (AXI interface).
> +	2   :PCIE control block level,reset.
> +	3   :Display Port block level reset (includes DPDMA).
> +	4   :FPD WDT reset.
> +	5   :AF_FM5 block level reset.
> +	6   :AF_FM4 block level reset.
> +	7   :AF_FM3 block level reset.
> +	8   :AF_FM2 block level reset.
> +	9   :AF_FM1 block level reset.
> +	10  :AF_FM0 block level reset.
> +	11  :GDMA block level reset.
> +	12  :Pixel Processor (GPU_PP1) block level reset.
> +	13  :Pixel Processor (GPU_PP0) block level reset.
> +	14  :GPU block level reset.
> +	15  :GT block level reset.
> +	16  :Sata block level reset.
> +	17  :ACPU3 power on reset.
> +	18  :ACPU2 power on reset.
> +	19  :ACPU1 power on reset.
> +	20  :ACPU0 power on reset.
> +	21  :APU L2 reset.
> +	22  :ACPU3 reset.
> +	23  :ACPU2 reset.
> +	24  :ACPU1 reset.
> +	25  :ACPU0 reset.
> +	26  :DDR block level reset inside of the DDR Sub System.
> +	27  :APM block level reset inside of the DDR Sub System.
> +	28  :soft reset.
> +	29  :GEM 0 reset.
> +	30  :GEM 1 reset.
> +	31  :GEM 2 reset.
> +	32  :GEM 3 reset.
> +	33  :qspi reset.
> +	34  :uart0 reset.
> +	35  :uart1 reset.
> +	36  :spi0 reset.
> +	37  :spi1 reset.
> +	38  :sdio0 reset.
> +	39  :sdio1 reset.
> +	40  :can0 reset.
> +	41  :can1 reset.
> +	42  :i2c0 reset.
> +	43  :i2c1 reset.
> +	44  :ttc0 reset.
> +	45  :ttc1 reset.
> +	46  :ttc2 reset.
> +	47  :ttc3 reset.
> +	48  :swdt reset.
> +	49  :nand reset.
> +	50  :adma reset.
> +	51  :gpio reset.
> +	52  :iou_cc reset.
> +	53  :timestamp reset.
> +	54  :rpu_r50 reset.
> +	55  :rpu r51 reset.
> +	56  :rpu_amba reset.
> +	57  :ocm reset.
> +	58  :rpu_pge reset.
> +	59  :usb0_core reset.
> +	60  :usb1_core reset.
> +	61  :usb0_hiber reset.
> +	62  :usb1_hiber reset.
> +	63  :usb0_apb reset.
> +	64  :usb1_apb reset.
> +	65  :ipi reset.
> +	66  :apm reset.
> +	67  :rtc reset.
> +	68  :sysmon reset.
> +	69  :afi_fm6 reset.
> +	70  :lpd_swdt reset.
> +	71  :fpd_reset.
> +	72  :rpu_dbg1 reset.
> +	73  :rpu_dbg0 reset.
> +	74  :dbg_lpd reset.
> +	75  :dbg_fpd reset.
> +	76  :apll reset.
> +	77  :dpll reset.
> +	78  :vpll reset.
> +	79  :iopll reset.
> +	80  :rpll reset.
> +	81  :gpio_pl_0 reset.
> +	82  :gpio_pl_1 reset.
> +	83  :gpio_pl_2 reset.
> +	84  :gpio_pl_3 reset.
> +	85  :gpio_pl_4 reset.
> +	86  :gpio_pl_5 reset.
> +	87  :gpio_pl_6 reset.
> +	88  :gpio_pl_7 reset.
> +	89  :gpio_pl_8 reset.
> +	90  :gpio_pl_9 reset.
> +	91  :gpio_pl_10 reset.
> +	92  :gpio_pl_11 reset.
> +	93  :gpio_pl_12 reset.
> +	94  :gpio_pl_13 reset.
> +	95  :gpio_pl_14 reset.
> +	96  :gpio_pl_15 reset.
> +	97  :gpio_pl_16 reset.
> +	98  :gpio_pl_17 reset.
> +	99  :gpio_pl_18 reset.
> +	100 :gpio_pl_19 reset.
> +	101 :gpio_pl_20 reset.
> +	102 :gpio_pl_21 reset.
> +	103 :gpio_pl_22 reset.
> +	104 :gpio_pl_23 reset.
> +	105 :gpio_pl_24 reset.
> +	106 :gpio_pl_25 reset.
> +	107 :gpio_pl_26 reset.
> +	108 :gpio_pl_27 reset.
> +	109 :gpio_pl_28 reset.
> +	110 :gpio_pl_29 reset.
> +	111 :gpio_pl_30 reset.
> +	112 :gpio_pl_31 reset.
> +	113 :rpu_ls reset.
> +	114 :ps_only reset.
> +	115 :pl reset.
> +	116 :ps_pl0 reset
> +	117 :ps_pl1 reset
> +	118 :ps_pl2 reset
> +	119 :ps_pl3 reset
> +
>  -------
>  Example
>  -------
> @@ -25,5 +163,9 @@ firmware {
>  	zynqmp_firmware: zynqmp-firmware {
>  		compatible = "xlnx,zynqmp-firmware";
>  		method = "smc";
> +		reset-controller:reset-controller@0 {
> +			compatible = "xlnx,zynqmp-reset";
> +			#reset-cells = <1>;
> +		};
>  	};
>  };
> -- 
> 2.7.4
> 

RE: [RFC PATCH v3 2/3] dt-bindings: reset: Add bindings for ZynqMP reset driver

[Nava kishore Manne]

Hi Rob,

Thanks for providing the comments...
Please find my response inline.

> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Tuesday, September 11, 2018 1:42 AM
> To: Nava kishore Manne <navam@xilinx.com>
> Cc: mark.rutland@arm.com; Michal Simek <michals@xilinx.com>;
> p.zabel@pengutronix.de; Rajan Vaja <RAJANV@xilinx.com>; Jolly Shah
> <JOLLYS@xilinx.com>; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [RFC PATCH v3 2/3] dt-bindings: reset: Add bindings for ZynqMP
> reset driver
> 
> On Wed, Sep 05, 2018 at 12:39:01PM +0530, Nava kishore Manne wrote:
> > Add documentation to describe Xilinx ZynqMP reset driver bindings.
> >
> > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> > ---
> > Changes for v3:
> > 		-Corrected Commit Msg.
> > Changes for v2:
> > 		-Moved reset node as a child to firwmare
> > 		 node.
> 
> Why? There's no reason for that. The parent can be multiple providers.
> You only need child nodes if the sub functions have their own DT resources.
> 

This driver having a dependency on firmware. So I have moved this node as a child to firmware
As per other drivers which is using this firmware interface 
https://lkml.org/lkml/2018/8/14/432 
I didn't get your comment could you please elaborate more??? 
Did you mean I should move the changes in the file to Documentation/devicetree/bindings/reset/ ???

Regards,
Navakishore.

[RFC PATCH v3 3/3] reset: reset-zynqmp: Adding support for Xilinx zynqmp reset controller.

[Nava kishore Manne]

Add a reset controller driver for Xilinx Zynq UltraScale+ MPSoC.
The zynqmp reset-controller has the ability to reset lines
connected to different blocks and peripheral in the Soc.

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
---
Changes for v3:
		-None.
Changes for v2:
		-Moved eemi_ops into a priv struct as suggested
		 by philipp.

 drivers/reset/Makefile       |   1 +
 drivers/reset/reset-zynqmp.c | 115 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 116 insertions(+)
 create mode 100644 drivers/reset/reset-zynqmp.c

diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index c1261dc..27e4a33 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -21,4 +21,5 @@ obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
 obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
 obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
 obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
+obj-$(CONFIG_ARCH_ZYNQMP) += reset-zynqmp.o
 
diff --git a/drivers/reset/reset-zynqmp.c b/drivers/reset/reset-zynqmp.c
new file mode 100644
index 0000000..f908492
--- /dev/null
+++ b/drivers/reset/reset-zynqmp.c
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Xilinx, Inc.
+ *
+ */
+
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/firmware/xlnx-zynqmp.h>
+
+#define ZYNQMP_NR_RESETS (ZYNQMP_PM_RESET_END - ZYNQMP_PM_RESET_START - 2)
+#define ZYNQMP_RESET_ID (ZYNQMP_PM_RESET_START + 1)
+
+struct zynqmp_reset_data {
+	struct reset_controller_dev rcdev;
+	const struct zynqmp_eemi_ops *eemi_ops;
+};
+
+static inline struct zynqmp_reset_data *
+to_zynqmp_reset_data(struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct zynqmp_reset_data, rcdev);
+}
+
+static int zynqmp_reset_assert(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
+
+	return priv->eemi_ops->reset_assert(ZYNQMP_RESET_ID + id,
+					    PM_RESET_ACTION_ASSERT);
+}
+
+static int zynqmp_reset_deassert(struct reset_controller_dev *rcdev,
+				 unsigned long id)
+{
+	struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
+
+	return priv->eemi_ops->reset_assert(ZYNQMP_RESET_ID + id,
+					    PM_RESET_ACTION_RELEASE);
+}
+
+static int zynqmp_reset_status(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
+	int val, err;
+
+	err = priv->eemi_ops->reset_get_status(ZYNQMP_RESET_ID + id, &val);
+	if (!err)
+		return -EINVAL;
+
+	return val;
+}
+
+static int zynqmp_reset_reset(struct reset_controller_dev *rcdev,
+			      unsigned long id)
+{
+	struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
+
+	return priv->eemi_ops->reset_assert(ZYNQMP_RESET_ID + id,
+					    PM_RESET_ACTION_PULSE);
+}
+
+static struct reset_control_ops zynqmp_reset_ops = {
+	.reset = zynqmp_reset_reset,
+	.assert = zynqmp_reset_assert,
+	.deassert = zynqmp_reset_deassert,
+	.status = zynqmp_reset_status,
+};
+
+static int zynqmp_reset_probe(struct platform_device *pdev)
+{
+	struct zynqmp_reset_data *priv;
+
+	priv = devm_kzalloc(&pdev->dev,
+			    sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, priv);
+
+	priv->eemi_ops = zynqmp_pm_get_eemi_ops();
+	if (!priv->eemi_ops)
+		return -ENXIO;
+
+	priv->rcdev.ops = &zynqmp_reset_ops;
+	priv->rcdev.owner = THIS_MODULE;
+	priv->rcdev.of_node = pdev->dev.of_node;
+	priv->rcdev.nr_resets = ZYNQMP_NR_RESETS;
+
+	return devm_reset_controller_register(&pdev->dev, &priv->rcdev);
+}
+
+static const struct of_device_id zynqmp_reset_dt_ids[] = {
+	{ .compatible = "xlnx,zynqmp-reset", },
+	{ },
+};
+
+static struct platform_driver zynqmp_reset_driver = {
+	.probe	= zynqmp_reset_probe,
+	.driver = {
+		.name		= KBUILD_MODNAME,
+		.of_match_table	= zynqmp_reset_dt_ids,
+	},
+};
+
+static int __init zynqmp_reset_init(void)
+{
+	return platform_driver_register(&zynqmp_reset_driver);
+}
+
+arch_initcall(zynqmp_reset_init);
-- 
2.7.4

Re: [RFC PATCH v3 3/3] reset: reset-zynqmp: Adding support for Xilinx zynqmp reset controller.

[Philipp Zabel]

Hi,

thank you for the patch. I have a few comments below:

On Wed, 2018-09-05 at 12:39 +0530, Nava kishore Manne wrote:
> Add a reset controller driver for Xilinx Zynq UltraScale+ MPSoC.
> The zynqmp reset-controller has the ability to reset lines
> connected to different blocks and peripheral in the Soc.
> 
> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> ---
> Changes for v3:
> 		-None.
> Changes for v2:
> 		-Moved eemi_ops into a priv struct as suggested
> 		 by philipp.
> 
>  drivers/reset/Makefile       |   1 +
>  drivers/reset/reset-zynqmp.c | 115 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 116 insertions(+)
>  create mode 100644 drivers/reset/reset-zynqmp.c
> 
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index c1261dc..27e4a33 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -21,4 +21,5 @@ obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
>  obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
>  obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
>  obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
> +obj-$(CONFIG_ARCH_ZYNQMP) += reset-zynqmp.o
>  
> diff --git a/drivers/reset/reset-zynqmp.c b/drivers/reset/reset-zynqmp.c
> new file mode 100644
> index 0000000..f908492
> --- /dev/null
> +++ b/drivers/reset/reset-zynqmp.c
> @@ -0,0 +1,115 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2018 Xilinx, Inc.
> + *
> + */
> +
> +#include <linux/io.h>

I think including io.h is not necessary.

[...]
> +static int zynqmp_reset_status(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
> +	int val, err;
> +
> +	err = priv->eemi_ops->reset_get_status(ZYNQMP_RESET_ID + id, &val);
> +	if (!err)
> +		return -EINVAL;

This looks like it should be

	if (err)
		return err;

instead.

[...]
> +static struct reset_control_ops zynqmp_reset_ops = {

static const struct reset_control_ops zynqmp_reset_ops = {

> +	.reset = zynqmp_reset_reset,
> +	.assert = zynqmp_reset_assert,
> +	.deassert = zynqmp_reset_deassert,
> +	.status = zynqmp_reset_status,
> +};
> +
> +static int zynqmp_reset_probe(struct platform_device *pdev)
> +{
> +	struct zynqmp_reset_data *priv;
> +
> +	priv = devm_kzalloc(&pdev->dev,
> +			    sizeof(*priv), GFP_KERNEL);

This should fit on one line.

regards
Philipp

RE: [RFC PATCH v3 3/3] reset: reset-zynqmp: Adding support for Xilinx zynqmp reset controller.

[Nava kishore Manne]

Hi Philipp

Thanks for the quick response...
Please find my response inline.

> -----Original Message-----
> From: Philipp Zabel [mailto:p.zabel@pengutronix.de]
> Sent: Wednesday, September 5, 2018 4:00 PM
> To: Nava kishore Manne <navam@xilinx.com>; robh+dt@kernel.org;
> mark.rutland@arm.com; Michal Simek <michals@xilinx.com>; Rajan Vaja
> <RAJANV@xilinx.com>; Jolly Shah <JOLLYS@xilinx.com>;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [RFC PATCH v3 3/3] reset: reset-zynqmp: Adding support for Xilinx
> zynqmp reset controller.
> 
> Hi,
> 
> thank you for the patch. I have a few comments below:
> 
> On Wed, 2018-09-05 at 12:39 +0530, Nava kishore Manne wrote:
> > Add a reset controller driver for Xilinx Zynq UltraScale+ MPSoC.
> > The zynqmp reset-controller has the ability to reset lines connected
> > to different blocks and peripheral in the Soc.
> >
> > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> > ---
> > Changes for v3:
> > 		-None.
> > Changes for v2:
> > 		-Moved eemi_ops into a priv struct as suggested
> > 		 by philipp.
> >
> >  drivers/reset/Makefile       |   1 +
> >  drivers/reset/reset-zynqmp.c | 115
> > +++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 116 insertions(+)
> >  create mode 100644 drivers/reset/reset-zynqmp.c
> >
> > diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile index
> > c1261dc..27e4a33 100644
> > --- a/drivers/reset/Makefile
> > +++ b/drivers/reset/Makefile
> > @@ -21,4 +21,5 @@ obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
> >  obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
> >  obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
> >  obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
> > +obj-$(CONFIG_ARCH_ZYNQMP) += reset-zynqmp.o
> >
> > diff --git a/drivers/reset/reset-zynqmp.c
> > b/drivers/reset/reset-zynqmp.c new file mode 100644 index
> > 0000000..f908492
> > --- /dev/null
> > +++ b/drivers/reset/reset-zynqmp.c
> > @@ -0,0 +1,115 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2018 Xilinx, Inc.
> > + *
> > + */
> > +
> > +#include <linux/io.h>
> 
> I think including io.h is not necessary.
> 
> [...]

Will fix in the next version.

> > +static int zynqmp_reset_status(struct reset_controller_dev *rcdev,
> > +			       unsigned long id)
> > +{
> > +	struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
> > +	int val, err;
> > +
> > +	err = priv->eemi_ops->reset_get_status(ZYNQMP_RESET_ID + id, &val);
> > +	if (!err)
> > +		return -EINVAL;
> 
> This looks like it should be
> 
> 	if (err)
> 		return err;
> 
> instead.
> 
> [...]
Will fix in the next version.

> > +static struct reset_control_ops zynqmp_reset_ops = {
> 
> static const struct reset_control_ops zynqmp_reset_ops = {
> 
Will fix in the next version.

> > +	.reset = zynqmp_reset_reset,
> > +	.assert = zynqmp_reset_assert,
> > +	.deassert = zynqmp_reset_deassert,
> > +	.status = zynqmp_reset_status,
> > +};
> > +
> > +static int zynqmp_reset_probe(struct platform_device *pdev) {
> > +	struct zynqmp_reset_data *priv;
> > +
> > +	priv = devm_kzalloc(&pdev->dev,
> > +			    sizeof(*priv), GFP_KERNEL);
> 
> This should fit on one line.
> 
Will fix in the next version.

Regards,
Navakishore.