[PATCH 0/3] Extend Hisilicon reset type

[PATCH 0/3] Extend Hisilicon reset type

[Jun Nie]

Extend Hisilicon reset type to support combination of set/clear,
assert/deassert and poll/non-poll.


Jun Nie (3):
  dt-bindings: clock: Update Hisilicon reset doc
  reset: hisilicon: Extend reset operation type
  ARM: dts: Update reset for hi3519 and hi3798cv200

 .../devicetree/bindings/clock/hisi-crg.txt    | 12 ++--
 arch/arm/boot/dts/hi3519.dtsi                 |  2 +-
 .../arm64/boot/dts/hisilicon/hi3798cv200.dtsi | 47 ++++++++-------
 drivers/clk/hisilicon/reset.c                 | 58 +++++++++++++++++--
 include/dt-bindings/reset/hisilicon-resets.h  | 28 +++++++++
 5 files changed, 116 insertions(+), 31 deletions(-)
 create mode 100644 include/dt-bindings/reset/hisilicon-resets.h

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] dt-bindings: clock: Update Hisilicon reset doc

[Jun Nie]

Document the update of Hisilicon reset operation extension.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 .../devicetree/bindings/clock/hisi-crg.txt    | 12 ++++----
 include/dt-bindings/reset/hisilicon-resets.h  | 28 +++++++++++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)
 create mode 100644 include/dt-bindings/reset/hisilicon-resets.h

diff --git a/Documentation/devicetree/bindings/clock/hisi-crg.txt b/Documentation/devicetree/bindings/clock/hisi-crg.txt
index cc60b3d423f3..fd8b0a964806 100644
--- a/Documentation/devicetree/bindings/clock/hisi-crg.txt
+++ b/Documentation/devicetree/bindings/clock/hisi-crg.txt
@@ -26,19 +26,21 @@ to specify the clock which they consume.
 
 All these identifier could be found in <dt-bindings/clock/hi3519-clock.h>.
 
-- #reset-cells: should be 2.
+- #reset-cells: should be 3.
 
 A reset signal can be controlled by writing a bit register in the CRG module.
-The reset specifier consists of two cells. The first cell represents the
+The reset specifier consists of three cells. The first cell represents the
 register offset relative to the base address. The second cell represents the
-bit index in the register.
+bit index in the register. The third represent the flags to operation type.
+
+All reset flags could be found in <dt-bindings/reset/hisilicon-resets.h>
 
 Example: CRG nodes
 CRG: clock-reset-controller@12010000 {
 	compatible = "hisilicon,hi3519-crg";
 	reg = <0x12010000 0x10000>;
 	#clock-cells = <1>;
-	#reset-cells = <2>;
+	#reset-cells = <3>;
 };
 
 Example: consumer nodes
@@ -46,5 +48,5 @@ i2c0: i2c@12110000 {
 	compatible = "hisilicon,hi3519-i2c";
 	reg = <0x12110000 0x1000>;
 	clocks = <&CRG HI3519_I2C0_RST>;
-	resets = <&CRG 0xe4 0>;
+	resets = <&CRG 0xe4 0 (HISI_ASSERT_SET | HISI_DEASSERT_CLEAR)>;
 };
diff --git a/include/dt-bindings/reset/hisilicon-resets.h b/include/dt-bindings/reset/hisilicon-resets.h
new file mode 100644
index 000000000000..983e42a0c318
--- /dev/null
+++ b/include/dt-bindings/reset/hisilicon-resets.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Hisilicon Reset definitions
+ *
+ * Copyright (c) 2019 HiSilicon Technologies Co., Ltd.
+ */
+
+#ifndef __DT_BINDINGS_RESET_HISILICON_H__
+#define __DT_BINDINGS_RESET_HISILICON_H__
+
+/*
+ * The reset does not support the feature and corresponding
+ * values are not valid
+ */
+#define HISI_ASSERT_NONE		(1 << 0)
+#define HISI_DEASSERT_NONE		(1 << 1)
+
+/* When set this function is activated by polling/setting/clearing this bit */
+#define HISI_ASSERT_SET		(1 << 2)
+#define HISI_DEASSERT_SET		(1 << 3)
+#define HISI_ASSERT_CLEAR		(0 << 4)
+#define HISI_DEASSERT_CLEAR		(0 << 5)
+#define HISI_ASSERT_POLL		(0 << 6)
+#define HISI_DEASSERT_POLL		(0 << 7)
+
+#define HISI_RESET_DEFAULT		(HISI_ASSERT_SET | HISI_DEASSERT_CLEAR)
+
+#endif
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] dt-bindings: clock: Update Hisilicon reset doc

[Philipp Zabel]

Hi Jun,

I have a few questions and comments about these patches. I notice that
the changed device trees only use the default setting. Are these new
features something that is required for the present SoCs, or is this in
preparation for a new SoC?

On Mon, 2019-12-02 at 22:45 +0800, Jun Nie wrote:
> Document the update of Hisilicon reset operation extension.
> 
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  .../devicetree/bindings/clock/hisi-crg.txt    | 12 ++++----
>  include/dt-bindings/reset/hisilicon-resets.h  | 28 +++++++++++++++++++
>  2 files changed, 35 insertions(+), 5 deletions(-)
>  create mode 100644 include/dt-bindings/reset/hisilicon-resets.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/hisi-crg.txt b/Documentation/devicetree/bindings/clock/hisi-crg.txt
> index cc60b3d423f3..fd8b0a964806 100644
> --- a/Documentation/devicetree/bindings/clock/hisi-crg.txt
> +++ b/Documentation/devicetree/bindings/clock/hisi-crg.txt
> @@ -26,19 +26,21 @@ to specify the clock which they consume.
>  
>  All these identifier could be found in <dt-bindings/clock/hi3519-clock.h>.
>  
> -- #reset-cells: should be 2.
> +- #reset-cells: should be 3.
>  
>  A reset signal can be controlled by writing a bit register in the CRG module.
> -The reset specifier consists of two cells. The first cell represents the
> +The reset specifier consists of three cells. The first cell represents the
>  register offset relative to the base address. The second cell represents the
> -bit index in the register.
> +bit index in the register. The third represent the flags to operation type.
> +
> +All reset flags could be found in <dt-bindings/reset/hisilicon-resets.h>
>  
>  Example: CRG nodes
>  CRG: clock-reset-controller@12010000 {
>  	compatible = "hisilicon,hi3519-crg";
>  	reg = <0x12010000 0x10000>;
>  	#clock-cells = <1>;
> -	#reset-cells = <2>;
> +	#reset-cells = <3>;
>  };
>  
>  Example: consumer nodes
> @@ -46,5 +48,5 @@ i2c0: i2c@12110000 {
>  	compatible = "hisilicon,hi3519-i2c";
>  	reg = <0x12110000 0x1000>;
>  	clocks = <&CRG HI3519_I2C0_RST>;
> -	resets = <&CRG 0xe4 0>;
> +	resets = <&CRG 0xe4 0 (HISI_ASSERT_SET | HISI_DEASSERT_CLEAR)>;
>  };
> diff --git a/include/dt-bindings/reset/hisilicon-resets.h b/include/dt-bindings/reset/hisilicon-resets.h
> new file mode 100644
> index 000000000000..983e42a0c318
> --- /dev/null
> +++ b/include/dt-bindings/reset/hisilicon-resets.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Hisilicon Reset definitions
> + *
> + * Copyright (c) 2019 HiSilicon Technologies Co., Ltd.
> + */
> +
> +#ifndef __DT_BINDINGS_RESET_HISILICON_H__
> +#define __DT_BINDINGS_RESET_HISILICON_H__
> +
> +/*
> + * The reset does not support the feature and corresponding
> + * values are not valid
> + */
> +#define HISI_ASSERT_NONE		(1 << 0)
> +#define HISI_DEASSERT_NONE		(1 << 1)

What is the purpose of these two? Surely a reset control that does
nothing is not useful?

> +
> +/* When set this function is activated by polling/setting/clearing this bit */
> +#define HISI_ASSERT_SET		(1 << 2)
> +#define HISI_DEASSERT_SET		(1 << 3)

> +#define HISI_ASSERT_CLEAR		(0 << 4)
> +#define HISI_DEASSERT_CLEAR		(0 << 5)
> +#define HISI_ASSERT_POLL		(0 << 6)
> +#define HISI_DEASSERT_POLL		(0 << 7)

These are all zero, checking for them with an & operation in the code
always returns false.

> +
> +#define HISI_RESET_DEFAULT		(HISI_ASSERT_SET | HISI_DEASSERT_CLEAR)
> +
> +#endif

regards
Philipp


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] dt-bindings: clock: Update Hisilicon reset doc

[Jun Nie]

Philipp Zabel <p.zabel@pengutronix.de> 于2019年12月3日周二 上午1:04写道：
>
> Hi Jun,
>
> I have a few questions and comments about these patches. I notice that
> the changed device trees only use the default setting. Are these new
> features something that is required for the present SoCs, or is this in
> preparation for a new SoC?

Yes, this patch set is prepared for new SoC.

>
> On Mon, 2019-12-02 at 22:45 +0800, Jun Nie wrote:
> > Document the update of Hisilicon reset operation extension.
> >
> > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > ---
> >  .../devicetree/bindings/clock/hisi-crg.txt    | 12 ++++----
> >  include/dt-bindings/reset/hisilicon-resets.h  | 28 +++++++++++++++++++
> >  2 files changed, 35 insertions(+), 5 deletions(-)
> >  create mode 100644 include/dt-bindings/reset/hisilicon-resets.h
> >
> > diff --git a/Documentation/devicetree/bindings/clock/hisi-crg.txt b/Documentation/devicetree/bindings/clock/hisi-crg.txt
> > index cc60b3d423f3..fd8b0a964806 100644
> > --- a/Documentation/devicetree/bindings/clock/hisi-crg.txt
> > +++ b/Documentation/devicetree/bindings/clock/hisi-crg.txt
> > @@ -26,19 +26,21 @@ to specify the clock which they consume.
> >
> >  All these identifier could be found in <dt-bindings/clock/hi3519-clock.h>.
> >
> > -- #reset-cells: should be 2.
> > +- #reset-cells: should be 3.
> >
> >  A reset signal can be controlled by writing a bit register in the CRG module.
> > -The reset specifier consists of two cells. The first cell represents the
> > +The reset specifier consists of three cells. The first cell represents the
> >  register offset relative to the base address. The second cell represents the
> > -bit index in the register.
> > +bit index in the register. The third represent the flags to operation type.
> > +
> > +All reset flags could be found in <dt-bindings/reset/hisilicon-resets.h>
> >
> >  Example: CRG nodes
> >  CRG: clock-reset-controller@12010000 {
> >       compatible = "hisilicon,hi3519-crg";
> >       reg = <0x12010000 0x10000>;
> >       #clock-cells = <1>;
> > -     #reset-cells = <2>;
> > +     #reset-cells = <3>;
> >  };
> >
> >  Example: consumer nodes
> > @@ -46,5 +48,5 @@ i2c0: i2c@12110000 {
> >       compatible = "hisilicon,hi3519-i2c";
> >       reg = <0x12110000 0x1000>;
> >       clocks = <&CRG HI3519_I2C0_RST>;
> > -     resets = <&CRG 0xe4 0>;
> > +     resets = <&CRG 0xe4 0 (HISI_ASSERT_SET | HISI_DEASSERT_CLEAR)>;
> >  };
> > diff --git a/include/dt-bindings/reset/hisilicon-resets.h b/include/dt-bindings/reset/hisilicon-resets.h
> > new file mode 100644
> > index 000000000000..983e42a0c318
> > --- /dev/null
> > +++ b/include/dt-bindings/reset/hisilicon-resets.h
> > @@ -0,0 +1,28 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Hisilicon Reset definitions
> > + *
> > + * Copyright (c) 2019 HiSilicon Technologies Co., Ltd.
> > + */
> > +
> > +#ifndef __DT_BINDINGS_RESET_HISILICON_H__
> > +#define __DT_BINDINGS_RESET_HISILICON_H__
> > +
> > +/*
> > + * The reset does not support the feature and corresponding
> > + * values are not valid
> > + */
> > +#define HISI_ASSERT_NONE             (1 << 0)
> > +#define HISI_DEASSERT_NONE           (1 << 1)
>
> What is the purpose of these two? Surely a reset control that does
> nothing is not useful?
>
> > +
> > +/* When set this function is activated by polling/setting/clearing this bit */
> > +#define HISI_ASSERT_SET              (1 << 2)
> > +#define HISI_DEASSERT_SET            (1 << 3)
>
> > +#define HISI_ASSERT_CLEAR            (0 << 4)
> > +#define HISI_DEASSERT_CLEAR          (0 << 5)
> > +#define HISI_ASSERT_POLL             (0 << 6)
> > +#define HISI_DEASSERT_POLL           (0 << 7)
>
> These are all zero, checking for them with an & operation in the code
> always returns false.

Thanks for pointing out this! This is a typo in the early version
patch. I made some
mistake when preparing the patch for upstream. Will fix this issue.
>
> > +
> > +#define HISI_RESET_DEFAULT           (HISI_ASSERT_SET | HISI_DEASSERT_CLEAR)
> > +
> > +#endif
>
> regards
> Philipp
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] dt-bindings: clock: Update Hisilicon reset doc

[Philipp Zabel]

On Tue, 2019-12-03 at 11:11 +0800, Jun Nie wrote:
> Philipp Zabel <p.zabel@pengutronix.de> 于2019年12月3日周二 上午1:04写道：
> > Hi Jun,
> > 
> > I have a few questions and comments about these patches. I notice that
> > the changed device trees only use the default setting. Are these new
> > features something that is required for the present SoCs, or is this in
> > preparation for a new SoC?
> 
> Yes, this patch set is prepared for new SoC.
> 
> > On Mon, 2019-12-02 at 22:45 +0800, Jun Nie wrote:
> > > Document the update of Hisilicon reset operation extension.
> > > 
> > > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > > ---
> > >  .../devicetree/bindings/clock/hisi-crg.txt    | 12 ++++----
> > >  include/dt-bindings/reset/hisilicon-resets.h  | 28 +++++++++++++++++++
> > >  2 files changed, 35 insertions(+), 5 deletions(-)
> > >  create mode 100644 include/dt-bindings/reset/hisilicon-resets.h
> > > 
> > > diff --git a/Documentation/devicetree/bindings/clock/hisi-crg.txt b/Documentation/devicetree/bindings/clock/hisi-crg.txt
> > > index cc60b3d423f3..fd8b0a964806 100644
> > > --- a/Documentation/devicetree/bindings/clock/hisi-crg.txt
> > > +++ b/Documentation/devicetree/bindings/clock/hisi-crg.txt
> > > @@ -26,19 +26,21 @@ to specify the clock which they consume.
> > > 
> > >  All these identifier could be found in <dt-bindings/clock/hi3519-clock.h>.
> > > 
> > > -- #reset-cells: should be 2.
> > > +- #reset-cells: should be 3.

If this is only needed for a new SoC, you should introduce a new
compatible and leave #reset-cells = <2> for the old compatible. The new
compatible can require #reset-cells = <3>. Wit this, the current device
trees don't have to be changed at all.

regards
Philipp


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] reset: hisilicon: Extend reset operation type

[Jun Nie]

Extend reset operations to support combination of three type flags:
ASSERT/DEASSERT SET/CLEAR POLL.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 drivers/clk/hisilicon/reset.c | 58 ++++++++++++++++++++++++++++++++---
 1 file changed, 53 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/hisilicon/reset.c b/drivers/clk/hisilicon/reset.c
index 93cee17db8b1..de7d186b0894 100644
--- a/drivers/clk/hisilicon/reset.c
+++ b/drivers/clk/hisilicon/reset.c
@@ -2,20 +2,25 @@
 /*
  * Hisilicon Reset Controller Driver
  *
- * Copyright (c) 2015-2016 HiSilicon Technologies Co., Ltd.
+ * Copyright (c) 2015-2019 HiSilicon Technologies Co., Ltd.
  */
 
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/of_address.h>
 #include <linux/platform_device.h>
 #include <linux/reset-controller.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+
+#include <dt-bindings/reset/hisilicon-resets.h>
 #include "reset.h"
 
 #define	HISI_RESET_BIT_MASK	0x1f
 #define	HISI_RESET_OFFSET_SHIFT	8
 #define	HISI_RESET_OFFSET_MASK	0xffff00
+#define	HISI_RESET_FLAG_SHIFT	24
+#define	HISI_RESET_FLAG_MASK	0xff000000
 
 struct hisi_reset_controller {
 	spinlock_t	lock;
@@ -30,14 +35,17 @@ struct hisi_reset_controller {
 static int hisi_reset_of_xlate(struct reset_controller_dev *rcdev,
 			const struct of_phandle_args *reset_spec)
 {
+	unsigned long flags;
 	u32 offset;
 	u8 bit;
 
+	flags = (reset_spec->args[2] << HISI_RESET_FLAG_SHIFT)
+		& HISI_RESET_FLAG_MASK;
 	offset = (reset_spec->args[0] << HISI_RESET_OFFSET_SHIFT)
 		& HISI_RESET_OFFSET_MASK;
 	bit = reset_spec->args[1] & HISI_RESET_BIT_MASK;
 
-	return (offset | bit);
+	return (flags | offset | bit);
 }
 
 static int hisi_reset_assert(struct reset_controller_dev *rcdev,
@@ -48,13 +56,33 @@ static int hisi_reset_assert(struct reset_controller_dev *rcdev,
 	u32 offset, reg;
 	u8 bit;
 
+	flags = (id & HISI_RESET_FLAG_MASK) >> HISI_RESET_FLAG_SHIFT;
+	if (flags & HISI_ASSERT_NONE)
+		return -ENOTSUPP; /* assert not supported for this reset */
+
 	offset = (id & HISI_RESET_OFFSET_MASK) >> HISI_RESET_OFFSET_SHIFT;
 	bit = id & HISI_RESET_BIT_MASK;
 
+	pr_devel("%s %s to %s 0x%x:bit[%d]\n", __func__,
+		flags & HISI_ASSERT_POLL ? "poll" : "",
+		flags & HISI_ASSERT_SET ? "set":"clear", offset, bit);
+
+	if (flags & HISI_ASSERT_POLL) {
+		if (flags & HISI_ASSERT_SET)
+			return readl_poll_timeout(rstc->membase + offset,
+						  reg, reg & BIT(bit), 0, 5000);
+		else
+			return readl_poll_timeout(rstc->membase + offset,
+						  reg, !(reg & BIT(bit)),
+						  0, 5000);
+	}
+
 	spin_lock_irqsave(&rstc->lock, flags);
 
 	reg = readl(rstc->membase + offset);
-	writel(reg | BIT(bit), rstc->membase + offset);
+	/* Default is setting to assert for no flag case. */
+	reg = (flags & HISI_ASSERT_CLEAR) ? reg & ~BIT(bit) : reg | BIT(bit);
+	writel(reg, rstc->membase + offset);
 
 	spin_unlock_irqrestore(&rstc->lock, flags);
 
@@ -69,13 +97,33 @@ static int hisi_reset_deassert(struct reset_controller_dev *rcdev,
 	u32 offset, reg;
 	u8 bit;
 
+	flags = (id & HISI_RESET_FLAG_MASK) >> HISI_RESET_FLAG_SHIFT;
+	if (flags & HISI_DEASSERT_NONE)
+		return -ENOTSUPP; /* deassert not supported for this reset */
+
 	offset = (id & HISI_RESET_OFFSET_MASK) >> HISI_RESET_OFFSET_SHIFT;
 	bit = id & HISI_RESET_BIT_MASK;
 
+	pr_devel("%s %s to %s 0x%x:bit[%d]\n", __func__,
+		flags & HISI_DEASSERT_POLL ? "poll" : "",
+		flags & HISI_DEASSERT_SET ? "clear":"set", offset, bit);
+
+	if (flags & HISI_DEASSERT_POLL) {
+		if (flags & HISI_DEASSERT_SET)
+			return readl_poll_timeout(rstc->membase + offset,
+						  reg, reg & BIT(bit), 0, 5000);
+		else
+			return readl_poll_timeout(rstc->membase + offset,
+						  reg, !(reg & BIT(bit)),
+						  0, 5000);
+	}
+
 	spin_lock_irqsave(&rstc->lock, flags);
 
 	reg = readl(rstc->membase + offset);
-	writel(reg & ~BIT(bit), rstc->membase + offset);
+	/* Default is clearing to deasseart for no flag case. */
+	reg = (flags & HISI_DEASSERT_SET) ? reg | BIT(bit) : reg & ~BIT(bit);
+	writel(reg, rstc->membase + offset);
 
 	spin_unlock_irqrestore(&rstc->lock, flags);
 
@@ -103,7 +151,7 @@ struct hisi_reset_controller *hisi_reset_init(struct platform_device *pdev)
 	rstc->rcdev.owner = THIS_MODULE;
 	rstc->rcdev.ops = &hisi_reset_ops;
 	rstc->rcdev.of_node = pdev->dev.of_node;
-	rstc->rcdev.of_reset_n_cells = 2;
+	rstc->rcdev.of_reset_n_cells = 3;
 	rstc->rcdev.of_xlate = hisi_reset_of_xlate;
 	reset_controller_register(&rstc->rcdev);
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] reset: hisilicon: Extend reset operation type

[Philipp Zabel]

On Mon, 2019-12-02 at 22:45 +0800, Jun Nie wrote:
> Extend reset operations to support combination of three type flags:
> ASSERT/DEASSERT SET/CLEAR POLL.
> 
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  drivers/clk/hisilicon/reset.c | 58 ++++++++++++++++++++++++++++++++---
>  1 file changed, 53 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/hisilicon/reset.c b/drivers/clk/hisilicon/reset.c
> index 93cee17db8b1..de7d186b0894 100644
> --- a/drivers/clk/hisilicon/reset.c
> +++ b/drivers/clk/hisilicon/reset.c
> @@ -2,20 +2,25 @@
>  /*
>   * Hisilicon Reset Controller Driver
>   *
> - * Copyright (c) 2015-2016 HiSilicon Technologies Co., Ltd.
> + * Copyright (c) 2015-2019 HiSilicon Technologies Co., Ltd.
>   */
>  
>  #include <linux/io.h>
> +#include <linux/iopoll.h>
>  #include <linux/of_address.h>
>  #include <linux/platform_device.h>
>  #include <linux/reset-controller.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
> +
> +#include <dt-bindings/reset/hisilicon-resets.h>
>  #include "reset.h"
>  
>  #define	HISI_RESET_BIT_MASK	0x1f
>  #define	HISI_RESET_OFFSET_SHIFT	8
>  #define	HISI_RESET_OFFSET_MASK	0xffff00
> +#define	HISI_RESET_FLAG_SHIFT	24
> +#define	HISI_RESET_FLAG_MASK	0xff000000
>  
>  struct hisi_reset_controller {
>  	spinlock_t	lock;
> @@ -30,14 +35,17 @@ struct hisi_reset_controller {
>  static int hisi_reset_of_xlate(struct reset_controller_dev *rcdev,
>  			const struct of_phandle_args *reset_spec)
>  {
> +	unsigned long flags;
>  	u32 offset;
>  	u8 bit;
>  
> +	flags = (reset_spec->args[2] << HISI_RESET_FLAG_SHIFT)
> +		& HISI_RESET_FLAG_MASK;
>  	offset = (reset_spec->args[0] << HISI_RESET_OFFSET_SHIFT)
>  		& HISI_RESET_OFFSET_MASK;
>  	bit = reset_spec->args[1] & HISI_RESET_BIT_MASK;
>  
> -	return (offset | bit);
> +	return (flags | offset | bit);
>  }
>  
>  static int hisi_reset_assert(struct reset_controller_dev *rcdev,
> @@ -48,13 +56,33 @@ static int hisi_reset_assert(struct reset_controller_dev *rcdev,
>  	u32 offset, reg;
>  	u8 bit;
>  
> +	flags = (id & HISI_RESET_FLAG_MASK) >> HISI_RESET_FLAG_SHIFT;
> +	if (flags & HISI_ASSERT_NONE)
> +		return -ENOTSUPP; /* assert not supported for this reset */

As long as .assert and .deassert are the only implemented operations for
this reset controller, this does not make any sense to me. Are there
resets that can only be deasserted?

> +
>  	offset = (id & HISI_RESET_OFFSET_MASK) >> HISI_RESET_OFFSET_SHIFT;
>  	bit = id & HISI_RESET_BIT_MASK;
>  
> +	pr_devel("%s %s to %s 0x%x:bit[%d]\n", __func__,
> +		flags & HISI_ASSERT_POLL ? "poll" : "",
> +		flags & HISI_ASSERT_SET ? "set":"clear", offset, bit);
> +
> +	if (flags & HISI_ASSERT_POLL) {

Since HISI_ASSERT_POLL is 0, this is always false.

> +		if (flags & HISI_ASSERT_SET)
> +			return readl_poll_timeout(rstc->membase + offset,
> +						  reg, reg & BIT(bit), 0, 5000);
> +		else
> +			return readl_poll_timeout(rstc->membase + offset,
> +						  reg, !(reg & BIT(bit)),
> +						  0, 5000);

And this is effectively dead code. Do you really have hardware that
asserts or asserts a reset line in reaction to a read access?

Should HISI_ASSERT_POLL and HISI_DEASSERT_POLL be mutually exclusive?

> +	}
> +
>  	spin_lock_irqsave(&rstc->lock, flags);
>  
>  	reg = readl(rstc->membase + offset);
> -	writel(reg | BIT(bit), rstc->membase + offset);
> +	/* Default is setting to assert for no flag case. */
> +	reg = (flags & HISI_ASSERT_CLEAR) ? reg & ~BIT(bit) : reg | BIT(bit);

Consider moving this into a helper function with a boolean set/clear
parameter.

As long as HISI_ASSERT_CLEAR is 0, the first branch is dead code.

> +	writel(reg, rstc->membase + offset);
>  
>  	spin_unlock_irqrestore(&rstc->lock, flags);
>  
> @@ -69,13 +97,33 @@ static int hisi_reset_deassert(struct reset_controller_dev *rcdev,
>  	u32 offset, reg;
>  	u8 bit;
>  
> +	flags = (id & HISI_RESET_FLAG_MASK) >> HISI_RESET_FLAG_SHIFT;
> +	if (flags & HISI_DEASSERT_NONE)
> +		return -ENOTSUPP; /* deassert not supported for this reset */
> +

Are there resets that can only ever be asserted?

>  	offset = (id & HISI_RESET_OFFSET_MASK) >> HISI_RESET_OFFSET_SHIFT;
>  	bit = id & HISI_RESET_BIT_MASK;
>  
> +	pr_devel("%s %s to %s 0x%x:bit[%d]\n", __func__,
> +		flags & HISI_DEASSERT_POLL ? "poll" : "",
> +		flags & HISI_DEASSERT_SET ? "clear":"set", offset, bit);
> +
> +	if (flags & HISI_DEASSERT_POLL) {
> +		if (flags & HISI_DEASSERT_SET)
> +			return readl_poll_timeout(rstc->membase + offset,
> +						  reg, reg & BIT(bit), 0, 5000);
> +		else
> +			return readl_poll_timeout(rstc->membase + offset,
> +						  reg, !(reg & BIT(bit)),
> +						  0, 5000);

See above, this code currently can never be reached.

> +	}
> +
>  	spin_lock_irqsave(&rstc->lock, flags);
>  
>  	reg = readl(rstc->membase + offset);
> -	writel(reg & ~BIT(bit), rstc->membase + offset);
> +	/* Default is clearing to deasseart for no flag case. */
> +	reg = (flags & HISI_DEASSERT_SET) ? reg | BIT(bit) : reg & ~BIT(bit);

Same as above, the read-modify-write for set/clear could be split out
into a helper.

> +	writel(reg, rstc->membase + offset);
>  
>  	spin_unlock_irqrestore(&rstc->lock, flags);
>  
> @@ -103,7 +151,7 @@ struct hisi_reset_controller *hisi_reset_init(struct platform_device *pdev)
>  	rstc->rcdev.owner = THIS_MODULE;
>  	rstc->rcdev.ops = &hisi_reset_ops;
>  	rstc->rcdev.of_node = pdev->dev.of_node;
> -	rstc->rcdev.of_reset_n_cells = 2;
> +	rstc->rcdev.of_reset_n_cells = 3;

This breaks current device trees (before patch 3). You can make sure
device trees with #reset-cells = <2> keep working by parsing the #reset-
cells and setting this value to 2 for old DTs.

>  	rstc->rcdev.of_xlate = hisi_reset_of_xlate;
>  	reset_controller_register(&rstc->rcdev);

regards
Philipp


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] reset: hisilicon: Extend reset operation type

[Jun Nie]

Philipp Zabel <p.zabel@pengutronix.de> 于2019年12月3日周二 上午1:04写道：
>
> On Mon, 2019-12-02 at 22:45 +0800, Jun Nie wrote:
> > Extend reset operations to support combination of three type flags:
> > ASSERT/DEASSERT SET/CLEAR POLL.
> >
> > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > ---
> >  drivers/clk/hisilicon/reset.c | 58 ++++++++++++++++++++++++++++++++---
> >  1 file changed, 53 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/clk/hisilicon/reset.c b/drivers/clk/hisilicon/reset.c
> > index 93cee17db8b1..de7d186b0894 100644
> > --- a/drivers/clk/hisilicon/reset.c
> > +++ b/drivers/clk/hisilicon/reset.c
> > @@ -2,20 +2,25 @@
> >  /*
> >   * Hisilicon Reset Controller Driver
> >   *
> > - * Copyright (c) 2015-2016 HiSilicon Technologies Co., Ltd.
> > + * Copyright (c) 2015-2019 HiSilicon Technologies Co., Ltd.
> >   */
> >
> >  #include <linux/io.h>
> > +#include <linux/iopoll.h>
> >  #include <linux/of_address.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/reset-controller.h>
> >  #include <linux/slab.h>
> >  #include <linux/spinlock.h>
> > +
> > +#include <dt-bindings/reset/hisilicon-resets.h>
> >  #include "reset.h"
> >
> >  #define      HISI_RESET_BIT_MASK     0x1f
> >  #define      HISI_RESET_OFFSET_SHIFT 8
> >  #define      HISI_RESET_OFFSET_MASK  0xffff00
> > +#define      HISI_RESET_FLAG_SHIFT   24
> > +#define      HISI_RESET_FLAG_MASK    0xff000000
> >
> >  struct hisi_reset_controller {
> >       spinlock_t      lock;
> > @@ -30,14 +35,17 @@ struct hisi_reset_controller {
> >  static int hisi_reset_of_xlate(struct reset_controller_dev *rcdev,
> >                       const struct of_phandle_args *reset_spec)
> >  {
> > +     unsigned long flags;
> >       u32 offset;
> >       u8 bit;
> >
> > +     flags = (reset_spec->args[2] << HISI_RESET_FLAG_SHIFT)
> > +             & HISI_RESET_FLAG_MASK;
> >       offset = (reset_spec->args[0] << HISI_RESET_OFFSET_SHIFT)
> >               & HISI_RESET_OFFSET_MASK;
> >       bit = reset_spec->args[1] & HISI_RESET_BIT_MASK;
> >
> > -     return (offset | bit);
> > +     return (flags | offset | bit);
> >  }
> >
> >  static int hisi_reset_assert(struct reset_controller_dev *rcdev,
> > @@ -48,13 +56,33 @@ static int hisi_reset_assert(struct reset_controller_dev *rcdev,
> >       u32 offset, reg;
> >       u8 bit;
> >
> > +     flags = (id & HISI_RESET_FLAG_MASK) >> HISI_RESET_FLAG_SHIFT;
> > +     if (flags & HISI_ASSERT_NONE)
> > +             return -ENOTSUPP; /* assert not supported for this reset */
>
> As long as .assert and .deassert are the only implemented operations for
> this reset controller, this does not make any sense to me. Are there
> resets that can only be deasserted?

Some reset is asserted on power on automatically. So only .deassert is needed.
>
> > +
> >       offset = (id & HISI_RESET_OFFSET_MASK) >> HISI_RESET_OFFSET_SHIFT;
> >       bit = id & HISI_RESET_BIT_MASK;
> >
> > +     pr_devel("%s %s to %s 0x%x:bit[%d]\n", __func__,
> > +             flags & HISI_ASSERT_POLL ? "poll" : "",
> > +             flags & HISI_ASSERT_SET ? "set":"clear", offset, bit);
> > +
> > +     if (flags & HISI_ASSERT_POLL) {
>
> Since HISI_ASSERT_POLL is 0, this is always false.

Will fix the wrong value definition in next version patch. The same to
below comments.
>
> > +             if (flags & HISI_ASSERT_SET)
> > +                     return readl_poll_timeout(rstc->membase + offset,
> > +                                               reg, reg & BIT(bit), 0, 5000);
> > +             else
> > +                     return readl_poll_timeout(rstc->membase + offset,
> > +                                               reg, !(reg & BIT(bit)),
> > +                                               0, 5000);
>
> And this is effectively dead code. Do you really have hardware that
> asserts or asserts a reset line in reaction to a read access?
>
> Should HISI_ASSERT_POLL and HISI_DEASSERT_POLL be mutually exclusive?
>
> > +     }
> > +
> >       spin_lock_irqsave(&rstc->lock, flags);
> >
> >       reg = readl(rstc->membase + offset);
> > -     writel(reg | BIT(bit), rstc->membase + offset);
> > +     /* Default is setting to assert for no flag case. */
> > +     reg = (flags & HISI_ASSERT_CLEAR) ? reg & ~BIT(bit) : reg | BIT(bit);
>
> Consider moving this into a helper function with a boolean set/clear
> parameter.

Will do.
>
> As long as HISI_ASSERT_CLEAR is 0, the first branch is dead code.
>
> > +     writel(reg, rstc->membase + offset);
> >
> >       spin_unlock_irqrestore(&rstc->lock, flags);
> >
> > @@ -69,13 +97,33 @@ static int hisi_reset_deassert(struct reset_controller_dev *rcdev,
> >       u32 offset, reg;
> >       u8 bit;
> >
> > +     flags = (id & HISI_RESET_FLAG_MASK) >> HISI_RESET_FLAG_SHIFT;
> > +     if (flags & HISI_DEASSERT_NONE)
> > +             return -ENOTSUPP; /* deassert not supported for this reset */
> > +
>
> Are there resets that can only ever be asserted?

I do not know yet. Only extend this driver with all combination in logic.
>
> >       offset = (id & HISI_RESET_OFFSET_MASK) >> HISI_RESET_OFFSET_SHIFT;
> >       bit = id & HISI_RESET_BIT_MASK;
> >
> > +     pr_devel("%s %s to %s 0x%x:bit[%d]\n", __func__,
> > +             flags & HISI_DEASSERT_POLL ? "poll" : "",
> > +             flags & HISI_DEASSERT_SET ? "clear":"set", offset, bit);
> > +
> > +     if (flags & HISI_DEASSERT_POLL) {
> > +             if (flags & HISI_DEASSERT_SET)
> > +                     return readl_poll_timeout(rstc->membase + offset,
> > +                                               reg, reg & BIT(bit), 0, 5000);
> > +             else
> > +                     return readl_poll_timeout(rstc->membase + offset,
> > +                                               reg, !(reg & BIT(bit)),
> > +                                               0, 5000);
>
> See above, this code currently can never be reached.
>
> > +     }
> > +
> >       spin_lock_irqsave(&rstc->lock, flags);
> >
> >       reg = readl(rstc->membase + offset);
> > -     writel(reg & ~BIT(bit), rstc->membase + offset);
> > +     /* Default is clearing to deasseart for no flag case. */
> > +     reg = (flags & HISI_DEASSERT_SET) ? reg | BIT(bit) : reg & ~BIT(bit);
>
> Same as above, the read-modify-write for set/clear could be split out
> into a helper.
>
> > +     writel(reg, rstc->membase + offset);
> >
> >       spin_unlock_irqrestore(&rstc->lock, flags);
> >
> > @@ -103,7 +151,7 @@ struct hisi_reset_controller *hisi_reset_init(struct platform_device *pdev)
> >       rstc->rcdev.owner = THIS_MODULE;
> >       rstc->rcdev.ops = &hisi_reset_ops;
> >       rstc->rcdev.of_node = pdev->dev.of_node;
> > -     rstc->rcdev.of_reset_n_cells = 2;
> > +     rstc->rcdev.of_reset_n_cells = 3;
>
> This breaks current device trees (before patch 3). You can make sure
> device trees with #reset-cells = <2> keep working by parsing the #reset-
> cells and setting this value to 2 for old DTs.

All current dts file are modified accordingly. But existing dtb binary support
is an issue. Do you have any suggestion?

>
> >       rstc->rcdev.of_xlate = hisi_reset_of_xlate;
> >       reset_controller_register(&rstc->rcdev);
>
> regards
> Philipp
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] reset: hisilicon: Extend reset operation type

[Philipp Zabel]

Hi Jun,

On Tue, 2019-12-03 at 11:53 +0800, Jun Nie wrote:
[...]
> @@ -48,13 +56,33 @@ static int hisi_reset_assert(struct reset_controller_dev *rcdev,
> > >       u32 offset, reg;
> > >       u8 bit;
> > > 
> > > +     flags = (id & HISI_RESET_FLAG_MASK) >> HISI_RESET_FLAG_SHIFT;
> > > +     if (flags & HISI_ASSERT_NONE)
> > > +             return -ENOTSUPP; /* assert not supported for this reset */
> > 
> > As long as .assert and .deassert are the only implemented operations for
> > this reset controller, this does not make any sense to me. Are there
> > resets that can only be deasserted?
> 
> Some reset is asserted on power on automatically. So only .deassert is needed.

But if the bit was set/cleared again after being deasserted, would it
assert the reset line? Basically, I wonder if those bits are write-once
or not.

[...]
> > > +             if (flags & HISI_ASSERT_SET)
> > > +                     return readl_poll_timeout(rstc->membase + offset,
> > > +                                               reg, reg & BIT(bit), 0, 5000);
> > > +             else
> > > +                     return readl_poll_timeout(rstc->membase + offset,
> > > +                                               reg, !(reg & BIT(bit)),
> > > +                                               0, 5000);
> > 
> > And this is effectively dead code. Do you really have hardware that
> > asserts or asserts a reset line in reaction to a read access?
> > 
> > Should HISI_ASSERT_POLL and HISI_DEASSERT_POLL be mutually exclusive?

This is missing an explanation.

[...]
> > > +     writel(reg, rstc->membase + offset);
> > > 
> > >       spin_unlock_irqrestore(&rstc->lock, flags);
> > > 
> > > @@ -69,13 +97,33 @@ static int hisi_reset_deassert(struct reset_controller_dev *rcdev,
> > >       u32 offset, reg;
> > >       u8 bit;
> > > 
> > > +     flags = (id & HISI_RESET_FLAG_MASK) >> HISI_RESET_FLAG_SHIFT;
> > > +     if (flags & HISI_DEASSERT_NONE)
> > > +             return -ENOTSUPP; /* deassert not supported for this reset */
> > > +
> > 
> > Are there resets that can only ever be asserted?
> 
> I do not know yet. Only extend this driver with all combination in logic.

If this is not used, let's leave it out.

[...]
> > > @@ -103,7 +151,7 @@ struct hisi_reset_controller *hisi_reset_init(struct platform_device *pdev)
> > >       rstc->rcdev.owner = THIS_MODULE;
> > >       rstc->rcdev.ops = &hisi_reset_ops;
> > >       rstc->rcdev.of_node = pdev->dev.of_node;
> > > -     rstc->rcdev.of_reset_n_cells = 2;
> > > +     rstc->rcdev.of_reset_n_cells = 3;
> > 
> > This breaks current device trees (before patch 3). You can make sure
> > device trees with #reset-cells = <2> keep working by parsing the #reset-
> > cells and setting this value to 2 for old DTs.
> 
> All current dts file are modified accordingly. But existing dtb binary support
> is an issue. Do you have any suggestion?

Since this is for a new SoC, you should keep using of_reset_n_cells = 2
for the current SoCs and only set it to 3 for a new compatible, for
example using of_device_get_match_data().

regards
Philipp


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] reset: hisilicon: Extend reset operation type

[Jun Nie]

Philipp Zabel <p.zabel@pengutronix.de> 于2019年12月3日周二 下午10:15写道：
>
> Hi Jun,
>
> On Tue, 2019-12-03 at 11:53 +0800, Jun Nie wrote:
> [...]
> > @@ -48,13 +56,33 @@ static int hisi_reset_assert(struct reset_controller_dev *rcdev,
> > > >       u32 offset, reg;
> > > >       u8 bit;
> > > >
> > > > +     flags = (id & HISI_RESET_FLAG_MASK) >> HISI_RESET_FLAG_SHIFT;
> > > > +     if (flags & HISI_ASSERT_NONE)
> > > > +             return -ENOTSUPP; /* assert not supported for this reset */
> > >
> > > As long as .assert and .deassert are the only implemented operations for
> > > this reset controller, this does not make any sense to me. Are there
> > > resets that can only be deasserted?
> >
> > Some reset is asserted on power on automatically. So only .deassert is needed.
>
> But if the bit was set/cleared again after being deasserted, would it
> assert the reset line? Basically, I wonder if those bits are write-once
> or not.

I did not see vendor's code to clear it, seems the bit is only polled
to be set on every
device enablement.
>
> [...]
> > > > +             if (flags & HISI_ASSERT_SET)
> > > > +                     return readl_poll_timeout(rstc->membase + offset,
> > > > +                                               reg, reg & BIT(bit), 0, 5000);
> > > > +             else
> > > > +                     return readl_poll_timeout(rstc->membase + offset,
> > > > +                                               reg, !(reg & BIT(bit)),
> > > > +                                               0, 5000);
> > >
> > > And this is effectively dead code. Do you really have hardware that
> > > asserts or asserts a reset line in reaction to a read access?
> > >
> > > Should HISI_ASSERT_POLL and HISI_DEASSERT_POLL be mutually exclusive?
>
> This is missing an explanation.

They are not mutually exclusive in logic. But I did not see the need
to poll for a assert yet.
>
> [...]
> > > > +     writel(reg, rstc->membase + offset);
> > > >
> > > >       spin_unlock_irqrestore(&rstc->lock, flags);
> > > >
> > > > @@ -69,13 +97,33 @@ static int hisi_reset_deassert(struct reset_controller_dev *rcdev,
> > > >       u32 offset, reg;
> > > >       u8 bit;
> > > >
> > > > +     flags = (id & HISI_RESET_FLAG_MASK) >> HISI_RESET_FLAG_SHIFT;
> > > > +     if (flags & HISI_DEASSERT_NONE)
> > > > +             return -ENOTSUPP; /* deassert not supported for this reset */
> > > > +
> > >
> > > Are there resets that can only ever be asserted?
> >
> > I do not know yet. Only extend this driver with all combination in logic.
>
> If this is not used, let's leave it out.
>
> [...]
> > > > @@ -103,7 +151,7 @@ struct hisi_reset_controller *hisi_reset_init(struct platform_device *pdev)
> > > >       rstc->rcdev.owner = THIS_MODULE;
> > > >       rstc->rcdev.ops = &hisi_reset_ops;
> > > >       rstc->rcdev.of_node = pdev->dev.of_node;
> > > > -     rstc->rcdev.of_reset_n_cells = 2;
> > > > +     rstc->rcdev.of_reset_n_cells = 3;
> > >
> > > This breaks current device trees (before patch 3). You can make sure
> > > device trees with #reset-cells = <2> keep working by parsing the #reset-
> > > cells and setting this value to 2 for old DTs.
> >
> > All current dts file are modified accordingly. But existing dtb binary support
> > is an issue. Do you have any suggestion?
>
> Since this is for a new SoC, you should keep using of_reset_n_cells = 2
> for the current SoCs and only set it to 3 for a new compatible, for
> example using of_device_get_match_data().

Yeah, this should be the solution. I will check the detail, thanks!

Regards,
Jun

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] ARM: dts: Update reset for hi3519 and hi3798cv200

[Jun Nie]

Update reset for hi3519 and hi3798cv200 as driver is extended to
support configurable reset operation type.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 arch/arm/boot/dts/hi3519.dtsi                 |  2 +-
 .../arm64/boot/dts/hisilicon/hi3798cv200.dtsi | 47 +++++++++++--------
 2 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/arch/arm/boot/dts/hi3519.dtsi b/arch/arm/boot/dts/hi3519.dtsi
index 410409a0ed66..2335c8443d2d 100644
--- a/arch/arm/boot/dts/hi3519.dtsi
+++ b/arch/arm/boot/dts/hi3519.dtsi
@@ -37,7 +37,7 @@
 	crg: clock-reset-controller@12010000 {
 		compatible = "hisilicon,hi3519-crg";
 		#clock-cells = <1>;
-		#reset-cells = <2>;
+		#reset-cells = <3>;
 		reg = <0x12010000 0x10000>;
 	};
 
diff --git a/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi b/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi
index 13821a0ff524..0a30aaae6bf2 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi
@@ -9,8 +9,10 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/phy/phy.h>
+#include <dt-bindings/reset/hisilicon-resets.h>
 #include <dt-bindings/reset/ti-syscon.h>
 
+
 / {
 	compatible = "hisilicon,hi3798cv200";
 	interrupt-parent = <&gic>;
@@ -86,7 +88,7 @@
 			compatible = "hisilicon,hi3798cv200-crg", "syscon", "simple-mfd";
 			reg = <0x8a22000 0x1000>;
 			#clock-cells = <1>;
-			#reset-cells = <2>;
+			#reset-cells = <3>;
 
 			gmacphyrst: reset-controller {
 				compatible = "ti,syscon-reset";
@@ -103,7 +105,7 @@
 			compatible = "hisilicon,hi3798cv200-sysctrl", "syscon";
 			reg = <0x8000000 0x1000>;
 			#clock-cells = <1>;
-			#reset-cells = <2>;
+			#reset-cells = <3>;
 		};
 
 		perictrl: peripheral-controller@8a20000 {
@@ -118,20 +120,22 @@
 				compatible = "hisilicon,hi3798cv200-usb2-phy";
 				reg = <0x120 0x4>;
 				clocks = <&crg HISTB_USB2_PHY1_REF_CLK>;
-				resets = <&crg 0xbc 4>;
+				resets = <&crg 0xbc 4 HISI_RESET_DEFAULT>;
 				#address-cells = <1>;
 				#size-cells = <0>;
 
 				usb2_phy1_port0: phy@0 {
 					reg = <0>;
 					#phy-cells = <0>;
-					resets = <&crg 0xbc 8>;
+					resets = <&crg 0xbc 8
+						  HISI_RESET_DEFAULT>;
 				};
 
 				usb2_phy1_port1: phy@1 {
 					reg = <1>;
 					#phy-cells = <0>;
-					resets = <&crg 0xbc 9>;
+					resets = <&crg 0xbc 9
+						  HISI_RESET_DEFAULT>;
 				};
 			};
 
@@ -139,14 +143,15 @@
 				compatible = "hisilicon,hi3798cv200-usb2-phy";
 				reg = <0x124 0x4>;
 				clocks = <&crg HISTB_USB2_PHY2_REF_CLK>;
-				resets = <&crg 0xbc 6>;
+				resets = <&crg 0xbc 6 HISI_RESET_DEFAULT>;
 				#address-cells = <1>;
 				#size-cells = <0>;
 
 				usb2_phy2_port0: phy@0 {
 					reg = <0>;
 					#phy-cells = <0>;
-					resets = <&crg 0xbc 10>;
+					resets = <&crg 0xbc 10
+						  HISI_RESET_DEFAULT>;
 				};
 			};
 
@@ -155,7 +160,7 @@
 				reg = <0x850 0x8>;
 				#phy-cells = <1>;
 				clocks = <&crg HISTB_COMBPHY0_CLK>;
-				resets = <&crg 0x188 4>;
+				resets = <&crg 0x188 4 HISI_RESET_DEFAULT>;
 				assigned-clocks = <&crg HISTB_COMBPHY0_CLK>;
 				assigned-clock-rates = <100000000>;
 				hisilicon,fixed-mode = <PHY_TYPE_USB3>;
@@ -166,7 +171,7 @@
 				reg = <0x858 0x8>;
 				#phy-cells = <1>;
 				clocks = <&crg HISTB_COMBPHY1_CLK>;
-				resets = <&crg 0x188 12>;
+				resets = <&crg 0x188 12 HISI_RESET_DEFAULT>;
 				assigned-clocks = <&crg HISTB_COMBPHY1_CLK>;
 				assigned-clock-rates = <100000000>;
 				hisilicon,mode-select-bits = <0x0008 11 (0x3 << 11)>;
@@ -306,7 +311,7 @@
 			clocks = <&crg HISTB_SDIO0_CIU_CLK>,
 				 <&crg HISTB_SDIO0_BIU_CLK>;
 			clock-names = "ciu", "biu";
-			resets = <&crg 0x9c 4>;
+			resets = <&crg 0x9c 4 HISI_RESET_DEFAULT>;
 			reset-names = "reset";
 			status = "disabled";
 		};
@@ -320,7 +325,7 @@
 				 <&crg HISTB_MMC_SAMPLE_CLK>,
 				 <&crg HISTB_MMC_DRV_CLK>;
 			clock-names = "ciu", "biu", "ciu-sample", "ciu-drive";
-			resets = <&crg 0xa0 4>;
+			resets = <&crg 0xa0 4 HISI_RESET_DEFAULT>;
 			reset-names = "reset";
 			status = "disabled";
 		};
@@ -525,8 +530,8 @@
 			clocks = <&crg HISTB_ETH0_MAC_CLK>,
 				 <&crg HISTB_ETH0_MACIF_CLK>;
 			clock-names = "mac_core", "mac_ifc";
-			resets = <&crg 0xcc 8>,
-				 <&crg 0xcc 10>,
+			resets = <&crg 0xcc 8 HISI_RESET_DEFAULT>,
+				 <&crg 0xcc 10 HISI_RESET_DEFAULT>,
 				 <&gmacphyrst 0>;
 			reset-names = "mac_core", "mac_ifc", "phy";
 			status = "disabled";
@@ -540,8 +545,8 @@
 			clocks = <&crg HISTB_ETH1_MAC_CLK>,
 				 <&crg HISTB_ETH1_MACIF_CLK>;
 			clock-names = "mac_core", "mac_ifc";
-			resets = <&crg 0xcc 9>,
-				 <&crg 0xcc 11>,
+			resets = <&crg 0xcc 9 HISI_RESET_DEFAULT>,
+				 <&crg 0xcc 11 HISI_RESET_DEFAULT>,
 				 <&gmacphyrst 1>;
 			reset-names = "mac_core", "mac_ifc", "phy";
 			status = "disabled";
@@ -578,7 +583,9 @@
 				 <&crg HISTB_PCIE_SYS_CLK>,
 				 <&crg HISTB_PCIE_BUS_CLK>;
 			clock-names = "aux", "pipe", "sys", "bus";
-			resets = <&crg 0x18c 6>, <&crg 0x18c 5>, <&crg 0x18c 4>;
+			resets = <&crg 0x18c 6 HISI_RESET_DEFAULT>,
+				 <&crg 0x18c 5 HISI_RESET_DEFAULT>,
+				 <&crg 0x18c 4 HISI_RESET_DEFAULT>;
 			reset-names = "soft", "sys", "bus";
 			phys = <&combphy1 PHY_TYPE_PCIE>;
 			phy-names = "phy";
@@ -593,7 +600,7 @@
 				 <&crg HISTB_USB2_12M_CLK>,
 				 <&crg HISTB_USB2_48M_CLK>;
 			clock-names = "bus", "clk12", "clk48";
-			resets = <&crg 0xb8 12>;
+			resets = <&crg 0xb8 12 HISI_RESET_DEFAULT>;
 			reset-names = "bus";
 			phys = <&usb2_phy1_port0>;
 			phy-names = "usb";
@@ -608,9 +615,9 @@
 				 <&crg HISTB_USB2_PHY_CLK>,
 				 <&crg HISTB_USB2_UTMI_CLK>;
 			clock-names = "bus", "phy", "utmi";
-			resets = <&crg 0xb8 12>,
-				 <&crg 0xb8 16>,
-				 <&crg 0xb8 13>;
+			resets = <&crg 0xb8 12 HISI_RESET_DEFAULT>,
+				 <&crg 0xb8 16 HISI_RESET_DEFAULT>,
+				 <&crg 0xb8 13 HISI_RESET_DEFAULT>;
 			reset-names = "bus", "phy", "utmi";
 			phys = <&usb2_phy1_port0>;
 			phy-names = "usb";
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel