[PATCH 0/6] add hisilicon reset

[PATCH 0/6] add hisilicon reset

[Zhangfei Gao]

Add reset-hi3660.c
Update reset-hi6220 as well
reset.c is shared by reset-hi3660.c and reset-hi6220.c
Change hi6220.dtsi accordingly

Zhangfei Gao (6):
  reset: hisilicon: add reset core
  dt-bindings: Document the hi3660 reset bindings
  reset: hisilicon: add reset-hi3660
  dt-bindings: change hi6220-reset.txt according to reset-hi6220.c
  reset: hisilicon: Use new driver reset-hi6222
  arm64: dts: hi6220: update reset node according to reset-hi6220.c

 .../bindings/reset/hisilicon,hi3660-reset.txt      |  42 ++++++
 .../bindings/reset/hisilicon,hi6220-reset.txt      |  14 +-
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi          |  20 ++-
 drivers/reset/Makefile                             |   2 +-
 drivers/reset/hisilicon/Kconfig                    |   7 +
 drivers/reset/hisilicon/Makefile                   |   4 +-
 drivers/reset/hisilicon/hi6220_reset.c             | 157 ---------------------
 drivers/reset/hisilicon/reset-hi3660.c             |  78 ++++++++++
 drivers/reset/hisilicon/reset-hi6220.c             | 123 ++++++++++++++++
 drivers/reset/hisilicon/reset.c                    | 108 ++++++++++++++
 drivers/reset/hisilicon/reset.h                    |  37 +++++
 include/dt-bindings/reset/hisi,hi3660-resets.h     |  38 +++++
 include/dt-bindings/reset/hisi,hi6220-resets.h     | 130 ++++++++---------
 13 files changed, 527 insertions(+), 233 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
 delete mode 100644 drivers/reset/hisilicon/hi6220_reset.c
 create mode 100644 drivers/reset/hisilicon/reset-hi3660.c
 create mode 100644 drivers/reset/hisilicon/reset-hi6220.c
 create mode 100644 drivers/reset/hisilicon/reset.c
 create mode 100644 drivers/reset/hisilicon/reset.h
 create mode 100644 include/dt-bindings/reset/hisi,hi3660-resets.h

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/6] reset: hisilicon: add reset core

[Zhangfei Gao]

reset.c will be shared by hisilicon chips like hi3660 and hi6220

Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/reset/Makefile           |   2 +-
 drivers/reset/hisilicon/Makefile |   1 +
 drivers/reset/hisilicon/reset.c  | 108 +++++++++++++++++++++++++++++++++++++++
 drivers/reset/hisilicon/reset.h  |  37 ++++++++++++++
 4 files changed, 147 insertions(+), 1 deletion(-)
 create mode 100644 drivers/reset/hisilicon/reset.c
 create mode 100644 drivers/reset/hisilicon/reset.h

diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index bbe7026..7e3dc4e 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -1,8 +1,8 @@
 obj-y += core.o
-obj-y += hisilicon/
 obj-$(CONFIG_ARCH_STI) += sti/
 obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
 obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
+obj-$(CONFIG_ARCH_HISI) += hisilicon/
 obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
 obj-$(CONFIG_RESET_MESON) += reset-meson.o
 obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
diff --git a/drivers/reset/hisilicon/Makefile b/drivers/reset/hisilicon/Makefile
index c932f86..df511f5 100644
--- a/drivers/reset/hisilicon/Makefile
+++ b/drivers/reset/hisilicon/Makefile
@@ -1 +1,2 @@
+obj-y	+= reset.o
 obj-$(CONFIG_COMMON_RESET_HI6220) += hi6220_reset.o
diff --git a/drivers/reset/hisilicon/reset.c b/drivers/reset/hisilicon/reset.c
new file mode 100644
index 0000000..c4971c9
--- /dev/null
+++ b/drivers/reset/hisilicon/reset.c
@@ -0,0 +1,108 @@
+/*
+ * Copyright (c) 2016-2017 Linaro Ltd.
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/types.h>
+#include <linux/of_device.h>
+#include <linux/mfd/syscon.h>
+
+#include "reset.h"
+
+struct hisi_reset_controller {
+	struct reset_controller_dev rst;
+	const struct hisi_reset_channel_data *channels;
+	struct regmap *map;
+};
+
+#define to_hisi_reset_controller(_rst) \
+	container_of(_rst, struct hisi_reset_controller, rst)
+
+static int hisi_reset_program_hw(struct reset_controller_dev *rcdev,
+				 unsigned long idx, bool assert)
+{
+	struct hisi_reset_controller *rc = to_hisi_reset_controller(rcdev);
+	const struct hisi_reset_channel_data *ch;
+
+	if (idx >= rcdev->nr_resets)
+		return -EINVAL;
+
+	ch = &rc->channels[idx];
+
+	if (assert)
+		return regmap_write(rc->map, ch->enable.reg,
+				    GENMASK(ch->enable.msb, ch->enable.lsb));
+	else
+		return regmap_write(rc->map, ch->disable.reg,
+				    GENMASK(ch->disable.msb, ch->disable.lsb));
+}
+
+static int hisi_reset_assert(struct reset_controller_dev *rcdev,
+			     unsigned long idx)
+{
+	return hisi_reset_program_hw(rcdev, idx, true);
+}
+
+static int hisi_reset_deassert(struct reset_controller_dev *rcdev,
+			       unsigned long idx)
+{
+	return hisi_reset_program_hw(rcdev, idx, false);
+}
+
+static int hisi_reset_dev(struct reset_controller_dev *rcdev,
+			  unsigned long idx)
+{
+	int err;
+
+	err = hisi_reset_assert(rcdev, idx);
+	if (err)
+		return err;
+
+	return hisi_reset_deassert(rcdev, idx);
+}
+
+static struct reset_control_ops hisi_reset_ops = {
+	.reset    = hisi_reset_dev,
+	.assert   = hisi_reset_assert,
+	.deassert = hisi_reset_deassert,
+};
+
+int hisi_reset_probe(struct platform_device *pdev)
+{
+	struct hisi_reset_controller *rc;
+	struct device_node *np = pdev->dev.of_node;
+	struct hisi_reset_controller_data *d;
+	struct device *dev = &pdev->dev;
+	const struct of_device_id *match;
+
+	match = of_match_device(dev->driver->of_match_table, dev);
+	if (!match || !match->data)
+		return -EINVAL;
+
+	d = (struct hisi_reset_controller_data *)match->data;
+	rc = devm_kzalloc(dev, sizeof(*rc), GFP_KERNEL);
+	if (!rc)
+		return -ENOMEM;
+
+	rc->map = syscon_regmap_lookup_by_phandle(np, "hisi,rst-syscon");
+	if (IS_ERR(rc->map)) {
+		dev_err(dev, "failed to get hisi,rst-syscon\n");
+		return PTR_ERR(rc->map);
+	}
+
+	rc->rst.ops = &hisi_reset_ops,
+	rc->rst.of_node = np;
+	rc->rst.nr_resets = d->nr_channels;
+	rc->channels = d->channels;
+
+	return reset_controller_register(&rc->rst);
+}
+EXPORT_SYMBOL_GPL(hisi_reset_probe);
diff --git a/drivers/reset/hisilicon/reset.h b/drivers/reset/hisilicon/reset.h
new file mode 100644
index 0000000..77259ee
--- /dev/null
+++ b/drivers/reset/hisilicon/reset.h
@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2016-2017 Linaro Ltd.
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __HISILICON_RESET_H
+#define __HISILICON_RESET_H
+
+#include <linux/device.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+
+/* reset separated register offset is 0x4 */
+#define HISI_RST_SEP(off, bit)					\
+	{ .enable	= REG_FIELD(off, bit, bit),		\
+	  .disable	= REG_FIELD(off + 0x4, bit, bit),	\
+	  .status	= REG_FIELD(off + 0x8, bit, bit), }
+
+struct hisi_reset_channel_data {
+	struct reg_field enable;
+	struct reg_field disable;
+	struct reg_field status;
+};
+
+struct hisi_reset_controller_data {
+	int nr_channels;
+	const struct hisi_reset_channel_data *channels;
+};
+
+int hisi_reset_probe(struct platform_device *pdev);
+
+#endif /* __HISILICON_RESET_H */
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] reset: hisilicon: add reset core

[Arnd Bergmann]

On Tuesday, November 22, 2016 3:49:16 PM CET Zhangfei Gao wrote:
> @@ -1,8 +1,8 @@
>  obj-y += core.o
> -obj-y += hisilicon/
>  obj-$(CONFIG_ARCH_STI) += sti/
>  obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
>  obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
> +obj-$(CONFIG_ARCH_HISI) += hisilicon/
>  obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
>  obj-$(CONFIG_RESET_MESON) += reset-meson.o
>  obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o

Please leave the obj-y line, otherwise the COMPILE_TEST variant won't work.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] reset: hisilicon: add reset core

[zhangfei]

Hi, Arnd

On 2016年11月22日 16:45, Arnd Bergmann wrote:
> On Tuesday, November 22, 2016 3:49:16 PM CET Zhangfei Gao wrote:
>> @@ -1,8 +1,8 @@
>>   obj-y += core.o
>> -obj-y += hisilicon/
>>   obj-$(CONFIG_ARCH_STI) += sti/
>>   obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
>>   obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
>> +obj-$(CONFIG_ARCH_HISI) += hisilicon/
>>   obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
>>   obj-$(CONFIG_RESET_MESON) += reset-meson.o
>>   obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> Please leave the obj-y line, otherwise the COMPILE_TEST variant won't work.

COMPILE_TEST is added in drivers/reset/hisilicon/Kconfig
like
config COMMON_RESET_HI3660
         tristate "Hi3660 Reset Driver"
         depends on ARCH_HISI || COMPILE_TEST

The reason not using "obj-y" here is that reset.c will be compiled unconditionally.

drivers/reset/hisilicon/Makefile
obj-y   += reset.o

Thanks

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] reset: hisilicon: add reset core

[Arnd Bergmann]

On Tuesday, November 22, 2016 5:22:42 PM CET zhangfei wrote:
> Hi, Arnd
> 
> On 2016年11月22日 16:45, Arnd Bergmann wrote:
> > On Tuesday, November 22, 2016 3:49:16 PM CET Zhangfei Gao wrote:
> >> @@ -1,8 +1,8 @@
> >>   obj-y += core.o
> >> -obj-y += hisilicon/
> >>   obj-$(CONFIG_ARCH_STI) += sti/
> >>   obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
> >>   obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
> >> +obj-$(CONFIG_ARCH_HISI) += hisilicon/
> >>   obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
> >>   obj-$(CONFIG_RESET_MESON) += reset-meson.o
> >>   obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> > Please leave the obj-y line, otherwise the COMPILE_TEST variant won't work.
> 
> COMPILE_TEST is added in drivers/reset/hisilicon/Kconfig
> like
> config COMMON_RESET_HI3660
>          tristate "Hi3660 Reset Driver"
>          depends on ARCH_HISI || COMPILE_TEST
> 
> The reason not using "obj-y" here is that reset.c will be compiled unconditionally.
> 
> drivers/reset/hisilicon/Makefile
> obj-y   += reset.o

Yes, that line has to change as well then, to only build it when one
of the hardware specific drivers is enabled.

	Arnd

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] reset: hisilicon: add reset core

[Philipp Zabel]

Hi Zhangfei,

Am Dienstag, den 22.11.2016, 15:49 +0800 schrieb Zhangfei Gao:
> reset.c will be shared by hisilicon chips like hi3660 and hi6220
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/reset/Makefile           |   2 +-
>  drivers/reset/hisilicon/Makefile |   1 +
>  drivers/reset/hisilicon/reset.c  | 108 +++++++++++++++++++++++++++++++++++++++
>  drivers/reset/hisilicon/reset.h  |  37 ++++++++++++++
>  4 files changed, 147 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/reset/hisilicon/reset.c
>  create mode 100644 drivers/reset/hisilicon/reset.h
> 
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index bbe7026..7e3dc4e 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -1,8 +1,8 @@
>  obj-y += core.o
> -obj-y += hisilicon/
>  obj-$(CONFIG_ARCH_STI) += sti/
>  obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
>  obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
> +obj-$(CONFIG_ARCH_HISI) += hisilicon/
>  obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
>  obj-$(CONFIG_RESET_MESON) += reset-meson.o
>  obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> diff --git a/drivers/reset/hisilicon/Makefile b/drivers/reset/hisilicon/Makefile
> index c932f86..df511f5 100644
> --- a/drivers/reset/hisilicon/Makefile
> +++ b/drivers/reset/hisilicon/Makefile
> @@ -1 +1,2 @@
> +obj-y	+= reset.o
>  obj-$(CONFIG_COMMON_RESET_HI6220) += hi6220_reset.o
> diff --git a/drivers/reset/hisilicon/reset.c b/drivers/reset/hisilicon/reset.c
> new file mode 100644
> index 0000000..c4971c9
> --- /dev/null
> +++ b/drivers/reset/hisilicon/reset.c
> @@ -0,0 +1,108 @@
> +/*
> + * Copyright (c) 2016-2017 Linaro Ltd.
> + * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/types.h>
> +#include <linux/of_device.h>
> +#include <linux/mfd/syscon.h>
> +
> +#include "reset.h"
> +
> +struct hisi_reset_controller {
> +	struct reset_controller_dev rst;
> +	const struct hisi_reset_channel_data *channels;
> +	struct regmap *map;
> +};
> +
> +#define to_hisi_reset_controller(_rst) \
> +	container_of(_rst, struct hisi_reset_controller, rst)
> +
> +static int hisi_reset_program_hw(struct reset_controller_dev *rcdev,
> +				 unsigned long idx, bool assert)
> +{
> +	struct hisi_reset_controller *rc = to_hisi_reset_controller(rcdev);
> +	const struct hisi_reset_channel_data *ch;
> +
> +	if (idx >= rcdev->nr_resets)
> +		return -EINVAL;
> +
> +	ch = &rc->channels[idx];
> +
> +	if (assert)
> +		return regmap_write(rc->map, ch->enable.reg,
> +				    GENMASK(ch->enable.msb, ch->enable.lsb));
> +	else
> +		return regmap_write(rc->map, ch->disable.reg,
> +				    GENMASK(ch->disable.msb, ch->disable.lsb));

These fields are always 1-bit wide and you are not using the
regmap_field functions to access them, so I'd suggest to remove the
struct reg_field indirection and overhead and just write

	if (assert)
		return regmap_write(rc->map, ch->enable_reg, ch->bit);
	else
		return regmap_write(rc->map, ch->disable_reg, ch->bit);

here.

> +}
> +
> +static int hisi_reset_assert(struct reset_controller_dev *rcdev,
> +			     unsigned long idx)
> +{
> +	return hisi_reset_program_hw(rcdev, idx, true);
> +}
> +
> +static int hisi_reset_deassert(struct reset_controller_dev *rcdev,
> +			       unsigned long idx)
> +{
> +	return hisi_reset_program_hw(rcdev, idx, false);
> +}
> +
> +static int hisi_reset_dev(struct reset_controller_dev *rcdev,
> +			  unsigned long idx)
> +{
> +	int err;
> +
> +	err = hisi_reset_assert(rcdev, idx);
> +	if (err)
> +		return err;
> +
> +	return hisi_reset_deassert(rcdev, idx);
> +}
> +
> +static struct reset_control_ops hisi_reset_ops = {
> +	.reset    = hisi_reset_dev,
> +	.assert   = hisi_reset_assert,
> +	.deassert = hisi_reset_deassert,
> +};
> +
> +int hisi_reset_probe(struct platform_device *pdev)
> +{
> +	struct hisi_reset_controller *rc;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct hisi_reset_controller_data *d;
> +	struct device *dev = &pdev->dev;
> +	const struct of_device_id *match;
> +
> +	match = of_match_device(dev->driver->of_match_table, dev);
> +	if (!match || !match->data)
> +		return -EINVAL;
> +
> +	d = (struct hisi_reset_controller_data *)match->data;
> +	rc = devm_kzalloc(dev, sizeof(*rc), GFP_KERNEL);
> +	if (!rc)
> +		return -ENOMEM;
> +
> +	rc->map = syscon_regmap_lookup_by_phandle(np, "hisi,rst-syscon");
> +	if (IS_ERR(rc->map)) {
> +		dev_err(dev, "failed to get hisi,rst-syscon\n");
> +		return PTR_ERR(rc->map);
> +	}
> +
> +	rc->rst.ops = &hisi_reset_ops,
> +	rc->rst.of_node = np;
> +	rc->rst.nr_resets = d->nr_channels;
> +	rc->channels = d->channels;
> +
> +	return reset_controller_register(&rc->rst);
> +}
> +EXPORT_SYMBOL_GPL(hisi_reset_probe);
> diff --git a/drivers/reset/hisilicon/reset.h b/drivers/reset/hisilicon/reset.h
> new file mode 100644
> index 0000000..77259ee
> --- /dev/null
> +++ b/drivers/reset/hisilicon/reset.h
> @@ -0,0 +1,37 @@
> +/*
> + * Copyright (c) 2016-2017 Linaro Ltd.
> + * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef __HISILICON_RESET_H
> +#define __HISILICON_RESET_H
> +
> +#include <linux/device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>
> +
> +/* reset separated register offset is 0x4 */
> +#define HISI_RST_SEP(off, bit)					\
> +	{ .enable	= REG_FIELD(off, bit, bit),		\
> +	  .disable	= REG_FIELD(off + 0x4, bit, bit),	\
> +	  .status	= REG_FIELD(off + 0x8, bit, bit), }
> +
> +struct hisi_reset_channel_data {
> +	struct reg_field enable;
> +	struct reg_field disable;
> +	struct reg_field status;
> +};

Are you expecting the bits to be at different positions in the
enable/disable/status registers? How about just

#define HISI_RST_SEP(off, _bit)		\
	{ .enable_reg	= (off),	\
	  .disable_reg	= (off) + 0x4,	\
	  .status_reg	= (off) + 0x8,	\
	  .bit		= (_bit), }

struct hisi_reset_channel_data {
	unsigned int enable_reg;
	unsigned int disable_reg;
	unsigned int status_reg;
	unsigned int bit;
};

as those struct reg_field are not accessed via regmap_field_* functions
anyway.

> +
> +struct hisi_reset_controller_data {
> +	int nr_channels;
> +	const struct hisi_reset_channel_data *channels;
> +};
> +
> +int hisi_reset_probe(struct platform_device *pdev);
> +
> +#endif /* __HISILICON_RESET_H */

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] reset: hisilicon: add reset core

[Philipp Zabel]

Hi Zhangfei,

Am Dienstag, den 22.11.2016, 15:49 +0800 schrieb Zhangfei Gao:
> reset.c will be shared by hisilicon chips like hi3660 and hi6220
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/reset/Makefile           |   2 +-
>  drivers/reset/hisilicon/Makefile |   1 +
>  drivers/reset/hisilicon/reset.c  | 108 +++++++++++++++++++++++++++++++++++++++
>  drivers/reset/hisilicon/reset.h  |  37 ++++++++++++++
>  4 files changed, 147 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/reset/hisilicon/reset.c
>  create mode 100644 drivers/reset/hisilicon/reset.h
> 
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index bbe7026..7e3dc4e 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -1,8 +1,8 @@
>  obj-y += core.o
> -obj-y += hisilicon/
>  obj-$(CONFIG_ARCH_STI) += sti/
>  obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
>  obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
> +obj-$(CONFIG_ARCH_HISI) += hisilicon/
>  obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
>  obj-$(CONFIG_RESET_MESON) += reset-meson.o
>  obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> diff --git a/drivers/reset/hisilicon/Makefile b/drivers/reset/hisilicon/Makefile
> index c932f86..df511f5 100644
> --- a/drivers/reset/hisilicon/Makefile
> +++ b/drivers/reset/hisilicon/Makefile
> @@ -1 +1,2 @@
> +obj-y	+= reset.o
>  obj-$(CONFIG_COMMON_RESET_HI6220) += hi6220_reset.o
> diff --git a/drivers/reset/hisilicon/reset.c b/drivers/reset/hisilicon/reset.c
> new file mode 100644
> index 0000000..c4971c9
> --- /dev/null
> +++ b/drivers/reset/hisilicon/reset.c
> @@ -0,0 +1,108 @@
> +/*
> + * Copyright (c) 2016-2017 Linaro Ltd.
> + * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/types.h>
> +#include <linux/of_device.h>
> +#include <linux/mfd/syscon.h>
> +
> +#include "reset.h"
> +
> +struct hisi_reset_controller {
> +	struct reset_controller_dev rst;
> +	const struct hisi_reset_channel_data *channels;
> +	struct regmap *map;
> +};
> +
> +#define to_hisi_reset_controller(_rst) \
> +	container_of(_rst, struct hisi_reset_controller, rst)
> +
> +static int hisi_reset_program_hw(struct reset_controller_dev *rcdev,
> +				 unsigned long idx, bool assert)
> +{
> +	struct hisi_reset_controller *rc = to_hisi_reset_controller(rcdev);
> +	const struct hisi_reset_channel_data *ch;
> +
> +	if (idx >= rcdev->nr_resets)
> +		return -EINVAL;
> +
> +	ch = &rc->channels[idx];
> +
> +	if (assert)
> +		return regmap_write(rc->map, ch->enable.reg,
> +				    GENMASK(ch->enable.msb, ch->enable.lsb));
> +	else
> +		return regmap_write(rc->map, ch->disable.reg,
> +				    GENMASK(ch->disable.msb, ch->disable.lsb));

These fields are always 1-bit wide and you are not using the
regmap_field functions to access them, so I'd suggest to remove the
struct reg_field indirection and overhead and just write

	if (assert)
		return regmap_write(rc->map, ch->enable_reg, ch->bit);
	else
		return regmap_write(rc->map, ch->disable_reg, ch->bit);

here.

> +}
> +
> +static int hisi_reset_assert(struct reset_controller_dev *rcdev,
> +			     unsigned long idx)
> +{
> +	return hisi_reset_program_hw(rcdev, idx, true);
> +}
> +
> +static int hisi_reset_deassert(struct reset_controller_dev *rcdev,
> +			       unsigned long idx)
> +{
> +	return hisi_reset_program_hw(rcdev, idx, false);
> +}
> +
> +static int hisi_reset_dev(struct reset_controller_dev *rcdev,
> +			  unsigned long idx)
> +{
> +	int err;
> +
> +	err = hisi_reset_assert(rcdev, idx);
> +	if (err)
> +		return err;
> +
> +	return hisi_reset_deassert(rcdev, idx);
> +}
> +
> +static struct reset_control_ops hisi_reset_ops = {
> +	.reset    = hisi_reset_dev,
> +	.assert   = hisi_reset_assert,
> +	.deassert = hisi_reset_deassert,
> +};
> +
> +int hisi_reset_probe(struct platform_device *pdev)
> +{
> +	struct hisi_reset_controller *rc;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct hisi_reset_controller_data *d;
> +	struct device *dev = &pdev->dev;
> +	const struct of_device_id *match;
> +
> +	match = of_match_device(dev->driver->of_match_table, dev);
> +	if (!match || !match->data)
> +		return -EINVAL;
> +
> +	d = (struct hisi_reset_controller_data *)match->data;
> +	rc = devm_kzalloc(dev, sizeof(*rc), GFP_KERNEL);
> +	if (!rc)
> +		return -ENOMEM;
> +
> +	rc->map = syscon_regmap_lookup_by_phandle(np, "hisi,rst-syscon");
> +	if (IS_ERR(rc->map)) {
> +		dev_err(dev, "failed to get hisi,rst-syscon\n");
> +		return PTR_ERR(rc->map);
> +	}
> +
> +	rc->rst.ops = &hisi_reset_ops,
> +	rc->rst.of_node = np;
> +	rc->rst.nr_resets = d->nr_channels;
> +	rc->channels = d->channels;
> +
> +	return reset_controller_register(&rc->rst);
> +}
> +EXPORT_SYMBOL_GPL(hisi_reset_probe);
> diff --git a/drivers/reset/hisilicon/reset.h b/drivers/reset/hisilicon/reset.h
> new file mode 100644
> index 0000000..77259ee
> --- /dev/null
> +++ b/drivers/reset/hisilicon/reset.h
> @@ -0,0 +1,37 @@
> +/*
> + * Copyright (c) 2016-2017 Linaro Ltd.
> + * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef __HISILICON_RESET_H
> +#define __HISILICON_RESET_H
> +
> +#include <linux/device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>
> +
> +/* reset separated register offset is 0x4 */
> +#define HISI_RST_SEP(off, bit)					\
> +	{ .enable	= REG_FIELD(off, bit, bit),		\
> +	  .disable	= REG_FIELD(off + 0x4, bit, bit),	\
> +	  .status	= REG_FIELD(off + 0x8, bit, bit), }
> +
> +struct hisi_reset_channel_data {
> +	struct reg_field enable;
> +	struct reg_field disable;
> +	struct reg_field status;
> +};

Are you expecting the bits to be at different positions in the
enable/disable/status registers? How about just

#define HISI_RST_SEP(off, _bit)		\
	{ .enable_reg	= (off),	\
	  .disable_reg	= (off) + 0x4,	\
	  .status_reg	= (off) + 0x8,	\
	  .bit		= (_bit), }

struct hisi_reset_channel_data {
	unsigned int enable_reg;
	unsigned int disable_reg;
	unsigned int status_reg;
	unsigned int bit;
};

as those struct reg_field are not accessed via regmap_field_* functions
anyway.

> +
> +struct hisi_reset_controller_data {
> +	int nr_channels;
> +	const struct hisi_reset_channel_data *channels;
> +};
> +
> +int hisi_reset_probe(struct platform_device *pdev);
> +
> +#endif /* __HISILICON_RESET_H */

regards
Philipp


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/6] dt-bindings: Document the hi3660 reset bindings

[Zhangfei Gao]

Add DT bindings documentation for hi3660 SoC reset controller.

Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 .../bindings/reset/hisilicon,hi3660-reset.txt      | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt

diff --git a/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
new file mode 100644
index 0000000..20e03a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
@@ -0,0 +1,42 @@
+Hisilicon System Reset Controller
+======================================
+
+Please also refer to reset.txt in this directory for common reset
+controller binding usage.
+
+The reset controller registers are part of the system-ctl block on
+hi3660 SoC.
+
+Required properties:
+- compatible: should be one of the following:
+  - "hisilicon,hi3660-reset-crgctrl : reset control for peripherals in crgctrl.
+  - "hisilicon,hi3660-reset-iomcu : reset control for peripherals in iomcu.
+- hisi,rst-syscon: phandle of the reset's syscon.
+- #reset-cells: 1, see below
+
+Example:
+	crg_ctrl: crg_ctrl@fff35000 {
+		compatible = "hisilicon,hi3660-crgctrl", "syscon";
+		reg = <0x0 0xfff35000 0x0 0x1000>;
+		#clock-cells = <1>;
+	};
+
+	crg_rst: crg_rst_controller {
+		compatible = "hisilicon,hi3660-reset-crgctrl";
+		#reset-cells = <1>;
+		hisi,rst-syscon = <&crg_ctrl>;
+	};
+
+Specifying reset lines connected to IP modules
+==============================================
+example:
+
+        ufs: ufs@..... {
+                ...
+		resets = <&crg_rst HI3660_RST_UFS>,
+			 <&crg_rst HI3660_RST_UFS_ASSERT>;
+		reset-names = "rst", "assert";
+                ...
+        };
+
+The index could be found in <dt-bindings/reset/hisi,hi3660-resets.h>.
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/6] reset: hisilicon: add reset-hi3660

[Zhangfei Gao]

Add hi3660 reset driver based on common reset.c

Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/reset/hisilicon/Kconfig                |  7 +++
 drivers/reset/hisilicon/Makefile               |  1 +
 drivers/reset/hisilicon/reset-hi3660.c         | 78 ++++++++++++++++++++++++++
 include/dt-bindings/reset/hisi,hi3660-resets.h | 38 +++++++++++++
 4 files changed, 124 insertions(+)
 create mode 100644 drivers/reset/hisilicon/reset-hi3660.c
 create mode 100644 include/dt-bindings/reset/hisi,hi3660-resets.h

diff --git a/drivers/reset/hisilicon/Kconfig b/drivers/reset/hisilicon/Kconfig
index 1ff8b0c..10134dc 100644
--- a/drivers/reset/hisilicon/Kconfig
+++ b/drivers/reset/hisilicon/Kconfig
@@ -1,3 +1,10 @@
+config COMMON_RESET_HI3660
+	tristate "Hi3660 Reset Driver"
+	depends on ARCH_HISI || COMPILE_TEST
+	default ARCH_HISI
+	help
+	  Build the Hisilicon Hi3660 reset driver.
+
 config COMMON_RESET_HI6220
 	tristate "Hi6220 Reset Driver"
 	depends on ARCH_HISI || COMPILE_TEST
diff --git a/drivers/reset/hisilicon/Makefile b/drivers/reset/hisilicon/Makefile
index df511f5..57e9893 100644
--- a/drivers/reset/hisilicon/Makefile
+++ b/drivers/reset/hisilicon/Makefile
@@ -1,2 +1,3 @@
 obj-y	+= reset.o
+obj-$(CONFIG_COMMON_RESET_HI3660) += reset-hi3660.o
 obj-$(CONFIG_COMMON_RESET_HI6220) += hi6220_reset.o
diff --git a/drivers/reset/hisilicon/reset-hi3660.c b/drivers/reset/hisilicon/reset-hi3660.c
new file mode 100644
index 0000000..7da3153
--- /dev/null
+++ b/drivers/reset/hisilicon/reset-hi3660.c
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2016-2017 Linaro Ltd.
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <dt-bindings/reset/hisi,hi3660-resets.h>
+
+#include "reset.h"
+
+static const struct hisi_reset_channel_data hi3660_iomcu_rst[] = {
+	[HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3),
+	[HI3660_RST_I2C1] = HISI_RST_SEP(0x20, 4),
+	[HI3660_RST_I2C2] = HISI_RST_SEP(0x20, 5),
+	[HI3660_RST_I2C6] = HISI_RST_SEP(0x20, 27),
+};
+
+static struct hisi_reset_controller_data hi3660_iomcu_controller = {
+	.nr_channels = ARRAY_SIZE(hi3660_iomcu_rst),
+	.channels = hi3660_iomcu_rst,
+};
+
+static const struct hisi_reset_channel_data hi3660_crgctrl_rst[] = {
+	[HI3660_RST_I2C3] = HISI_RST_SEP(0x78, 7),
+	[HI3660_RST_I2C4] = HISI_RST_SEP(0x78, 27),
+	[HI3660_RST_I2C7] = HISI_RST_SEP(0x60, 14),
+	[HI3660_RST_SD] = HISI_RST_SEP(0x90, 18),
+	[HI3660_RST_SDIO] = HISI_RST_SEP(0x90, 20),
+	[HI3660_RST_UFS] = HISI_RST_SEP(0x84, 12),
+	[HI3660_RST_UFS_ASSERT] = HISI_RST_SEP(0x84, 7),
+	[HI3660_RST_PCIE_SYS] = HISI_RST_SEP(0x84, 26),
+	[HI3660_RST_PCIE_PHY] = HISI_RST_SEP(0x84, 27),
+	[HI3660_RST_PCIE_BUS] = HISI_RST_SEP(0x84, 31),
+	[HI3660_RST_USB3OTG_PHY] = HISI_RST_SEP(0x90, 3),
+	[HI3660_RST_USB3OTG] = HISI_RST_SEP(0x90, 5),
+	[HI3660_RST_USB3OTG_32K] = HISI_RST_SEP(0x90, 6),
+	[HI3660_RST_USB3OTG_AHB] = HISI_RST_SEP(0x90, 7),
+	[HI3660_RST_USB3OTG_MUX] = HISI_RST_SEP(0x90, 8),
+};
+
+static struct hisi_reset_controller_data hi3660_crgctrl_controller = {
+	.nr_channels = ARRAY_SIZE(hi3660_crgctrl_rst),
+	.channels = hi3660_crgctrl_rst,
+};
+
+static const struct of_device_id hi3660_reset_match[] = {
+	{ .compatible = "hisilicon,hi3660-reset-crgctrl",
+	  .data = &hi3660_crgctrl_controller, },
+	{ .compatible = "hisilicon,hi3660-reset-iomcu",
+	  .data = &hi3660_iomcu_controller, },
+	{},
+};
+MODULE_DEVICE_TABLE(of, hi3660_reset_match);
+
+static struct platform_driver hi3660_reset_driver = {
+	.probe = hisi_reset_probe,
+	.driver = {
+		.name = "reset-hi3660",
+		.of_match_table = hi3660_reset_match,
+	},
+};
+
+static int __init hi3660_reset_init(void)
+{
+	return platform_driver_register(&hi3660_reset_driver);
+}
+arch_initcall(hi3660_reset_init);
+
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:hi3660-reset");
+MODULE_DESCRIPTION("HiSilicon Hi3660 Reset Driver");
diff --git a/include/dt-bindings/reset/hisi,hi3660-resets.h b/include/dt-bindings/reset/hisi,hi3660-resets.h
new file mode 100644
index 0000000..a65f382
--- /dev/null
+++ b/include/dt-bindings/reset/hisi,hi3660-resets.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2016-2017 Linaro Ltd.
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef _DT_BINDINGS_RESET_CONTROLLER_HI3660
+#define _DT_BINDINGS_RESET_CONTROLLER_HI3660
+
+/* reset in iomcu */
+#define HI3660_RST_I2C0		0
+#define HI3660_RST_I2C1		1
+#define HI3660_RST_I2C2		2
+#define HI3660_RST_I2C6		3
+
+
+/* reset in crgctrl */
+#define HI3660_RST_I2C3		0
+#define HI3660_RST_I2C4		1
+#define HI3660_RST_I2C7		2
+#define HI3660_RST_SD		3
+#define HI3660_RST_SDIO		4
+#define HI3660_RST_UFS		5
+#define HI3660_RST_UFS_ASSERT	6
+#define HI3660_RST_PCIE_SYS	7
+#define HI3660_RST_PCIE_PHY	8
+#define HI3660_RST_PCIE_BUS	9
+#define HI3660_RST_USB3OTG_PHY  10
+#define HI3660_RST_USB3OTG	11
+#define HI3660_RST_USB3OTG_32K	12
+#define HI3660_RST_USB3OTG_AHB	13
+#define HI3660_RST_USB3OTG_MUX	14
+
+#endif /*_DT_BINDINGS_RESET_CONTROLLER_HI3660*/
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] reset: hisilicon: add reset-hi3660

[Arnd Bergmann]

On Tuesday, November 22, 2016 3:49:18 PM CET Zhangfei Gao wrote:
> 
> +static const struct hisi_reset_channel_data hi3660_iomcu_rst[] = {
> +       [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3),
> +       [HI3660_RST_I2C1] = HISI_RST_SEP(0x20, 4),
> +       [HI3660_RST_I2C2] = HISI_RST_SEP(0x20, 5),
> +       [HI3660_RST_I2C6] = HISI_RST_SEP(0x20, 27),
> +};
> +
> +static struct hisi_reset_controller_data hi3660_iomcu_controller = {
> +       .nr_channels = ARRAY_SIZE(hi3660_iomcu_rst),
> +       .channels = hi3660_iomcu_rst,
> +};
> +
> +static const struct hisi_reset_channel_data hi3660_crgctrl_rst[] = {
> +       [HI3660_RST_I2C3] = HISI_RST_SEP(0x78, 7),
> +       [HI3660_RST_I2C4] = HISI_RST_SEP(0x78, 27),
> +       [HI3660_RST_I2C7] = HISI_RST_SEP(0x60, 14),
> +       [HI3660_RST_SD] = HISI_RST_SEP(0x90, 18),
> +       [HI3660_RST_SDIO] = HISI_RST_SEP(0x90, 20),
> +       [HI3660_RST_UFS] = HISI_RST_SEP(0x84, 12),
> +       [HI3660_RST_UFS_ASSERT] = HISI_RST_SEP(0x84, 7),
> +       [HI3660_RST_PCIE_SYS] = HISI_RST_SEP(0x84, 26),
> +       [HI3660_RST_PCIE_PHY] = HISI_RST_SEP(0x84, 27),
> +       [HI3660_RST_PCIE_BUS] = HISI_RST_SEP(0x84, 31),
> +       [HI3660_RST_USB3OTG_PHY] = HISI_RST_SEP(0x90, 3),
> +       [HI3660_RST_USB3OTG] = HISI_RST_SEP(0x90, 5),
> +       [HI3660_RST_USB3OTG_32K] = HISI_RST_SEP(0x90, 6),
> +       [HI3660_RST_USB3OTG_AHB] = HISI_RST_SEP(0x90, 7),
> +       [HI3660_RST_USB3OTG_MUX] = HISI_RST_SEP(0x90, 8),
> +};

I think you can avoid the trap of the ABI incompatibility if
you just define those as in the binding as tuples, using #reset-cells=2.

In particular for the first set, it seems really silly to redefine
the numbers when there is just a simple integer number.

	Arnd

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] reset: hisilicon: add reset-hi3660

[zhangfei]

Hi, Arnd

On 2016年11月22日 16:50, Arnd Bergmann wrote:
> On Tuesday, November 22, 2016 3:49:18 PM CET Zhangfei Gao wrote:
>> +static const struct hisi_reset_channel_data hi3660_iomcu_rst[] = {
>> +       [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3),
>> +       [HI3660_RST_I2C1] = HISI_RST_SEP(0x20, 4),
>> +       [HI3660_RST_I2C2] = HISI_RST_SEP(0x20, 5),
>> +       [HI3660_RST_I2C6] = HISI_RST_SEP(0x20, 27),
>> +};
>> +
>> +static struct hisi_reset_controller_data hi3660_iomcu_controller = {
>> +       .nr_channels = ARRAY_SIZE(hi3660_iomcu_rst),
>> +       .channels = hi3660_iomcu_rst,
>> +};
>> +
>> +static const struct hisi_reset_channel_data hi3660_crgctrl_rst[] = {
>> +       [HI3660_RST_I2C3] = HISI_RST_SEP(0x78, 7),
>> +       [HI3660_RST_I2C4] = HISI_RST_SEP(0x78, 27),
>> +       [HI3660_RST_I2C7] = HISI_RST_SEP(0x60, 14),
>> +       [HI3660_RST_SD] = HISI_RST_SEP(0x90, 18),
>> +       [HI3660_RST_SDIO] = HISI_RST_SEP(0x90, 20),
>> +       [HI3660_RST_UFS] = HISI_RST_SEP(0x84, 12),
>> +       [HI3660_RST_UFS_ASSERT] = HISI_RST_SEP(0x84, 7),
>> +       [HI3660_RST_PCIE_SYS] = HISI_RST_SEP(0x84, 26),
>> +       [HI3660_RST_PCIE_PHY] = HISI_RST_SEP(0x84, 27),
>> +       [HI3660_RST_PCIE_BUS] = HISI_RST_SEP(0x84, 31),
>> +       [HI3660_RST_USB3OTG_PHY] = HISI_RST_SEP(0x90, 3),
>> +       [HI3660_RST_USB3OTG] = HISI_RST_SEP(0x90, 5),
>> +       [HI3660_RST_USB3OTG_32K] = HISI_RST_SEP(0x90, 6),
>> +       [HI3660_RST_USB3OTG_AHB] = HISI_RST_SEP(0x90, 7),
>> +       [HI3660_RST_USB3OTG_MUX] = HISI_RST_SEP(0x90, 8),
>> +};
> I think you can avoid the trap of the ABI incompatibility if
> you just define those as in the binding as tuples, using #reset-cells=2.
>
> In particular for the first set, it seems really silly to redefine
> the numbers when there is just a simple integer number.

Could you clarify more, still not understand.
The number is index of the arrays, and the index will be used in dts.
The arrays lists the registers offset and bit shift.
For example:

[HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3), means register offset : 0x20, and bit shift = 3.

And Documentation/devicetree/bindings/reset/reset.txt
Required properties:
#reset-cells:   Number of cells in a reset specifier; Typically 0 for nodes
                 with a single reset output and 1 for nodes with multiple
                 reset outputs.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] reset: hisilicon: add reset-hi3660

[Arnd Bergmann]

On Tuesday, November 22, 2016 5:34:05 PM CET zhangfei wrote:
> On 2016年11月22日 16:50, Arnd Bergmann wrote:
> > On Tuesday, November 22, 2016 3:49:18 PM CET Zhangfei Gao wrote:
> >> +static const struct hisi_reset_channel_data hi3660_iomcu_rst[] = {
> >> +       [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3),
> >> +       [HI3660_RST_I2C1] = HISI_RST_SEP(0x20, 4),
> >> +       [HI3660_RST_I2C2] = HISI_RST_SEP(0x20, 5),
> >> +       [HI3660_RST_I2C6] = HISI_RST_SEP(0x20, 27),
> >> +};
> >> +
> >> +static struct hisi_reset_controller_data hi3660_iomcu_controller = {
> >> +       .nr_channels = ARRAY_SIZE(hi3660_iomcu_rst),
> >> +       .channels = hi3660_iomcu_rst,
> >> +};
> >> +
> >> +static const struct hisi_reset_channel_data hi3660_crgctrl_rst[] = {
> >> +       [HI3660_RST_I2C3] = HISI_RST_SEP(0x78, 7),
> >> +       [HI3660_RST_I2C4] = HISI_RST_SEP(0x78, 27),
> >> +       [HI3660_RST_I2C7] = HISI_RST_SEP(0x60, 14),
> >> +       [HI3660_RST_SD] = HISI_RST_SEP(0x90, 18),
> >> +       [HI3660_RST_SDIO] = HISI_RST_SEP(0x90, 20),
> >> +       [HI3660_RST_UFS] = HISI_RST_SEP(0x84, 12),
> >> +       [HI3660_RST_UFS_ASSERT] = HISI_RST_SEP(0x84, 7),
> >> +       [HI3660_RST_PCIE_SYS] = HISI_RST_SEP(0x84, 26),
> >> +       [HI3660_RST_PCIE_PHY] = HISI_RST_SEP(0x84, 27),
> >> +       [HI3660_RST_PCIE_BUS] = HISI_RST_SEP(0x84, 31),
> >> +       [HI3660_RST_USB3OTG_PHY] = HISI_RST_SEP(0x90, 3),
> >> +       [HI3660_RST_USB3OTG] = HISI_RST_SEP(0x90, 5),
> >> +       [HI3660_RST_USB3OTG_32K] = HISI_RST_SEP(0x90, 6),
> >> +       [HI3660_RST_USB3OTG_AHB] = HISI_RST_SEP(0x90, 7),
> >> +       [HI3660_RST_USB3OTG_MUX] = HISI_RST_SEP(0x90, 8),
> >> +};
> > I think you can avoid the trap of the ABI incompatibility if
> > you just define those as in the binding as tuples, using #reset-cells=2.
> >
> > In particular for the first set, it seems really silly to redefine
> > the numbers when there is just a simple integer number.
> 
> Could you clarify more, still not understand.
> The number is index of the arrays, and the index will be used in dts.
> The arrays lists the registers offset and bit shift.
> For example:
> 
> [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3), means register offset : 0x20, and bit shift = 3.
> 
> And Documentation/devicetree/bindings/reset/reset.txt
> Required properties:
> #reset-cells:   Number of cells in a reset specifier; Typically 0 for nodes
>                  with a single reset output and 1 for nodes with multiple
>                  reset outputs.

You can easily enumerate the registers that contain reset bits here,
so just use one cell for the register and another one for the index.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] reset: hisilicon: add reset-hi3660

[zhangfei]

On 2016年11月22日 17:42, Arnd Bergmann wrote:
> On Tuesday, November 22, 2016 5:34:05 PM CET zhangfei wrote:
>> On 2016年11月22日 16:50, Arnd Bergmann wrote:
>>> On Tuesday, November 22, 2016 3:49:18 PM CET Zhangfei Gao wrote:
>>>> +static const struct hisi_reset_channel_data hi3660_iomcu_rst[] = {
>>>> +       [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3),
>>>> +       [HI3660_RST_I2C1] = HISI_RST_SEP(0x20, 4),
>>>> +       [HI3660_RST_I2C2] = HISI_RST_SEP(0x20, 5),
>>>> +       [HI3660_RST_I2C6] = HISI_RST_SEP(0x20, 27),
>>>> +};
>>>> +
>>>> +static struct hisi_reset_controller_data hi3660_iomcu_controller = {
>>>> +       .nr_channels = ARRAY_SIZE(hi3660_iomcu_rst),
>>>> +       .channels = hi3660_iomcu_rst,
>>>> +};
>>>> +
>>>> +static const struct hisi_reset_channel_data hi3660_crgctrl_rst[] = {
>>>> +       [HI3660_RST_I2C3] = HISI_RST_SEP(0x78, 7),
>>>> +       [HI3660_RST_I2C4] = HISI_RST_SEP(0x78, 27),
>>>> +       [HI3660_RST_I2C7] = HISI_RST_SEP(0x60, 14),
>>>> +       [HI3660_RST_SD] = HISI_RST_SEP(0x90, 18),
>>>> +       [HI3660_RST_SDIO] = HISI_RST_SEP(0x90, 20),
>>>> +       [HI3660_RST_UFS] = HISI_RST_SEP(0x84, 12),
>>>> +       [HI3660_RST_UFS_ASSERT] = HISI_RST_SEP(0x84, 7),
>>>> +       [HI3660_RST_PCIE_SYS] = HISI_RST_SEP(0x84, 26),
>>>> +       [HI3660_RST_PCIE_PHY] = HISI_RST_SEP(0x84, 27),
>>>> +       [HI3660_RST_PCIE_BUS] = HISI_RST_SEP(0x84, 31),
>>>> +       [HI3660_RST_USB3OTG_PHY] = HISI_RST_SEP(0x90, 3),
>>>> +       [HI3660_RST_USB3OTG] = HISI_RST_SEP(0x90, 5),
>>>> +       [HI3660_RST_USB3OTG_32K] = HISI_RST_SEP(0x90, 6),
>>>> +       [HI3660_RST_USB3OTG_AHB] = HISI_RST_SEP(0x90, 7),
>>>> +       [HI3660_RST_USB3OTG_MUX] = HISI_RST_SEP(0x90, 8),
>>>> +};
>>> I think you can avoid the trap of the ABI incompatibility if
>>> you just define those as in the binding as tuples, using #reset-cells=2.
>>>
>>> In particular for the first set, it seems really silly to redefine
>>> the numbers when there is just a simple integer number.
>> Could you clarify more, still not understand.
>> The number is index of the arrays, and the index will be used in dts.
>> The arrays lists the registers offset and bit shift.
>> For example:
>>
>> [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3), means register offset : 0x20, and bit shift = 3.
>>
>> And Documentation/devicetree/bindings/reset/reset.txt
>> Required properties:
>> #reset-cells:   Number of cells in a reset specifier; Typically 0 for nodes
>>                   with a single reset output and 1 for nodes with multiple
>>                   reset outputs.
> You can easily enumerate the registers that contain reset bits here,
> so just use one cell for the register and another one for the index.

/* reset separated register offset is 0x4 */
#define HISI_RST_SEP(off, bit)                                  \
         { .enable       = REG_FIELD(off, bit, bit),             \
           .disable      = REG_FIELD(off + 0x4, bit, bit),       \
           .status       = REG_FIELD(off + 0x8, bit, bit), }

We not only provide the off and bit shift, but fulfill the members in 
the meantime.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] reset: hisilicon: add reset-hi3660

[Philipp Zabel]

Am Dienstag, den 22.11.2016, 10:42 +0100 schrieb Arnd Bergmann:
> On Tuesday, November 22, 2016 5:34:05 PM CET zhangfei wrote:
> > On 2016年11月22日 16:50, Arnd Bergmann wrote:
> > > On Tuesday, November 22, 2016 3:49:18 PM CET Zhangfei Gao wrote:
> > >> +static const struct hisi_reset_channel_data hi3660_iomcu_rst[] = {
> > >> +       [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3),
> > >> +       [HI3660_RST_I2C1] = HISI_RST_SEP(0x20, 4),
> > >> +       [HI3660_RST_I2C2] = HISI_RST_SEP(0x20, 5),
> > >> +       [HI3660_RST_I2C6] = HISI_RST_SEP(0x20, 27),
> > >> +};
> > >> +
> > >> +static struct hisi_reset_controller_data hi3660_iomcu_controller = {
> > >> +       .nr_channels = ARRAY_SIZE(hi3660_iomcu_rst),
> > >> +       .channels = hi3660_iomcu_rst,
> > >> +};
> > >> +
> > >> +static const struct hisi_reset_channel_data hi3660_crgctrl_rst[] = {
> > >> +       [HI3660_RST_I2C3] = HISI_RST_SEP(0x78, 7),
> > >> +       [HI3660_RST_I2C4] = HISI_RST_SEP(0x78, 27),
> > >> +       [HI3660_RST_I2C7] = HISI_RST_SEP(0x60, 14),
> > >> +       [HI3660_RST_SD] = HISI_RST_SEP(0x90, 18),
> > >> +       [HI3660_RST_SDIO] = HISI_RST_SEP(0x90, 20),
> > >> +       [HI3660_RST_UFS] = HISI_RST_SEP(0x84, 12),
> > >> +       [HI3660_RST_UFS_ASSERT] = HISI_RST_SEP(0x84, 7),
> > >> +       [HI3660_RST_PCIE_SYS] = HISI_RST_SEP(0x84, 26),
> > >> +       [HI3660_RST_PCIE_PHY] = HISI_RST_SEP(0x84, 27),
> > >> +       [HI3660_RST_PCIE_BUS] = HISI_RST_SEP(0x84, 31),
> > >> +       [HI3660_RST_USB3OTG_PHY] = HISI_RST_SEP(0x90, 3),
> > >> +       [HI3660_RST_USB3OTG] = HISI_RST_SEP(0x90, 5),
> > >> +       [HI3660_RST_USB3OTG_32K] = HISI_RST_SEP(0x90, 6),
> > >> +       [HI3660_RST_USB3OTG_AHB] = HISI_RST_SEP(0x90, 7),
> > >> +       [HI3660_RST_USB3OTG_MUX] = HISI_RST_SEP(0x90, 8),
> > >> +};
> > > I think you can avoid the trap of the ABI incompatibility if
> > > you just define those as in the binding as tuples, using #reset-cells=2.
> > >
> > > In particular for the first set, it seems really silly to redefine
> > > the numbers when there is just a simple integer number.
> > 
> > Could you clarify more, still not understand.
> > The number is index of the arrays, and the index will be used in dts.
> > The arrays lists the registers offset and bit shift.
> > For example:
> > 
> > [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3), means register offset : 0x20, and bit shift = 3.
> > 
> > And Documentation/devicetree/bindings/reset/reset.txt
> > Required properties:
> > #reset-cells:   Number of cells in a reset specifier; Typically 0 for nodes
> >                  with a single reset output and 1 for nodes with multiple
> >                  reset outputs.

This is just a suggestion, for reset controllers where the reset lines
can reasonably be enumerated by a single integer. If there is a good
reason to use more complicated bindings, more cells can be used.
That being said, I dislike having to spread register/bit information
throughout the device trees at the consumer/phandle sites, if the
register/bit information absolutely has to be put into the device tree,
I'd prefer a binding similar to ti-syscon, where it's all in one place.

> You can easily enumerate the registers that contain reset bits here,
> so just use one cell for the register and another one for the index.

Changing the reset cells is an incompatible change, and this is not a
straight forward register/bit mapping in hardware either. There are
currently three registers involved: enable (+0x0), disable (+0x4), and
status (+0x8). Also, what if in the future one of these reset bits have
to be handled inverted (as just happened for hi3519)?

regards
Philipp


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] reset: hisilicon: add reset-hi3660

[zhangfei]

Hi, Philipp


On 2016年11月22日 18:22, Philipp Zabel wrote:
> Am Dienstag, den 22.11.2016, 10:42 +0100 schrieb Arnd Bergmann:
>> On Tuesday, November 22, 2016 5:34:05 PM CET zhangfei wrote:
>>> On 2016年11月22日 16:50, Arnd Bergmann wrote:
>>>> On Tuesday, November 22, 2016 3:49:18 PM CET Zhangfei Gao wrote:
>>>>> +static const struct hisi_reset_channel_data hi3660_iomcu_rst[] = {
>>>>> +       [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3),
>>>>> +       [HI3660_RST_I2C1] = HISI_RST_SEP(0x20, 4),
>>>>> +       [HI3660_RST_I2C2] = HISI_RST_SEP(0x20, 5),
>>>>> +       [HI3660_RST_I2C6] = HISI_RST_SEP(0x20, 27),
>>>>> +};
>>>>> +
>>>>> +static struct hisi_reset_controller_data hi3660_iomcu_controller = {
>>>>> +       .nr_channels = ARRAY_SIZE(hi3660_iomcu_rst),
>>>>> +       .channels = hi3660_iomcu_rst,
>>>>> +};
>>>>> +
>>>>> +static const struct hisi_reset_channel_data hi3660_crgctrl_rst[] = {
>>>>> +       [HI3660_RST_I2C3] = HISI_RST_SEP(0x78, 7),
>>>>> +       [HI3660_RST_I2C4] = HISI_RST_SEP(0x78, 27),
>>>>> +       [HI3660_RST_I2C7] = HISI_RST_SEP(0x60, 14),
>>>>> +       [HI3660_RST_SD] = HISI_RST_SEP(0x90, 18),
>>>>> +       [HI3660_RST_SDIO] = HISI_RST_SEP(0x90, 20),
>>>>> +       [HI3660_RST_UFS] = HISI_RST_SEP(0x84, 12),
>>>>> +       [HI3660_RST_UFS_ASSERT] = HISI_RST_SEP(0x84, 7),
>>>>> +       [HI3660_RST_PCIE_SYS] = HISI_RST_SEP(0x84, 26),
>>>>> +       [HI3660_RST_PCIE_PHY] = HISI_RST_SEP(0x84, 27),
>>>>> +       [HI3660_RST_PCIE_BUS] = HISI_RST_SEP(0x84, 31),
>>>>> +       [HI3660_RST_USB3OTG_PHY] = HISI_RST_SEP(0x90, 3),
>>>>> +       [HI3660_RST_USB3OTG] = HISI_RST_SEP(0x90, 5),
>>>>> +       [HI3660_RST_USB3OTG_32K] = HISI_RST_SEP(0x90, 6),
>>>>> +       [HI3660_RST_USB3OTG_AHB] = HISI_RST_SEP(0x90, 7),
>>>>> +       [HI3660_RST_USB3OTG_MUX] = HISI_RST_SEP(0x90, 8),
>>>>> +};
>>>> I think you can avoid the trap of the ABI incompatibility if
>>>> you just define those as in the binding as tuples, using #reset-cells=2.
>>>>
>>>> In particular for the first set, it seems really silly to redefine
>>>> the numbers when there is just a simple integer number.
>>> Could you clarify more, still not understand.
>>> The number is index of the arrays, and the index will be used in dts.
>>> The arrays lists the registers offset and bit shift.
>>> For example:
>>>
>>> [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3), means register offset : 0x20, and bit shift = 3.
>>>
>>> And Documentation/devicetree/bindings/reset/reset.txt
>>> Required properties:
>>> #reset-cells:   Number of cells in a reset specifier; Typically 0 for nodes
>>>                   with a single reset output and 1 for nodes with multiple
>>>                   reset outputs.
> This is just a suggestion, for reset controllers where the reset lines
> can reasonably be enumerated by a single integer. If there is a good
> reason to use more complicated bindings, more cells can be used.
> That being said, I dislike having to spread register/bit information
> throughout the device trees at the consumer/phandle sites, if the
> register/bit information absolutely has to be put into the device tree,
> I'd prefer a binding similar to ti-syscon, where it's all in one place.
Thanks for the suggestion.
Will use table in dts instead of 
include/dt-bindings/reset/hisi,hi3660-resets.h
like
+               hisi,reset-bits = <0x20 0x8             /* 0: i2c0 */
+                                  0x20 0x10            /* 1: i2c1 */
+                                  0x20 0x20            /* 2: i2c2 */
+                                  0x20 0x8000000>;     /* 3: i2c6 */

To remove the potential ABI issue as pointed by Arnd.
>
>> You can easily enumerate the registers that contain reset bits here,
>> so just use one cell for the register and another one for the index.
> Changing the reset cells is an incompatible change, and this is not a
> straight forward register/bit mapping in hardware either. There are
> currently three registers involved: enable (+0x0), disable (+0x4), and
> status (+0x8). Also, what if in the future one of these reset bits have
> to be handled inverted (as just happened for hi3519)?
Discussed with Jianchen, we are only considering Kirin series now.
The inverted in hi3519 is only for some line, not the whole controller.
It is more like a bug and kirin does not have such issue.

Will send a new RFC, help take a look.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/6] dt-bindings: change hi6220-reset.txt according to reset-hi6220.c

[Zhangfei Gao]

Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 .../devicetree/bindings/reset/hisilicon,hi6220-reset.txt   | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/reset/hisilicon,hi6220-reset.txt b/Documentation/devicetree/bindings/reset/hisilicon,hi6220-reset.txt
index c25da39..6a864f3 100644
--- a/Documentation/devicetree/bindings/reset/hisilicon,hi6220-reset.txt
+++ b/Documentation/devicetree/bindings/reset/hisilicon,hi6220-reset.txt
@@ -9,10 +9,9 @@ hi6220 SoC.
 
 Required properties:
 - compatible: should be one of the following:
-  - "hisilicon,hi6220-sysctrl", "syscon" : For peripheral reset controller.
-  - "hisilicon,hi6220-mediactrl", "syscon" : For media reset controller.
-- reg: should be register base and length as documented in the
-  datasheet
+  - "hisilicon,hi6220-reset-sysctrl" : For peripheral reset controller.
+  - "hisilicon,hi6220-reset-mediactrl" : For media reset controller.
+- hisi,rst-syscon: phandle of the reset's syscon.
 - #reset-cells: 1, see below
 
 Example:
@@ -20,7 +19,12 @@ sys_ctrl: sys_ctrl@f7030000 {
 	compatible = "hisilicon,hi6220-sysctrl", "syscon";
 	reg = <0x0 0xf7030000 0x0 0x2000>;
 	#clock-cells = <1>;
+};
+
+sys_ctrl_rst: sys_rst_controller {
+	compatible = "hisilicon,hi6220-reset-sysctrl";
 	#reset-cells = <1>;
+	hisi,rst-syscon = <&sys_ctrl>;
 };
 
 Specifying reset lines connected to IP modules
@@ -29,7 +33,7 @@ example:
 
         uart1: serial@..... {
                 ...
-                resets = <&sys_ctrl PERIPH_RSTEN3_UART1>;
+                resets = <&sys_ctrl_rst PERIPH_RSTEN3_UART1>;
                 ...
         };
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] dt-bindings: change hi6220-reset.txt according to reset-hi6220.c

[Arnd Bergmann]

On Tuesday, November 22, 2016 3:49:19 PM CET Zhangfei Gao wrote:
>  Required properties:
>  - compatible: should be one of the following:
> -  - "hisilicon,hi6220-sysctrl", "syscon" : For peripheral reset controller.
> -  - "hisilicon,hi6220-mediactrl", "syscon" : For media reset controller.
> -- reg: should be register base and length as documented in the
> -  datasheet
> +  - "hisilicon,hi6220-reset-sysctrl" : For peripheral reset controller.
> +  - "hisilicon,hi6220-reset-mediactrl" : For media reset controller.
> +- hisi,rst-syscon: phandle of the reset's syscon.
>  - #reset-cells: 1, see below
> 

Please keep the old strings around for compatibility.

	Arnd

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] dt-bindings: change hi6220-reset.txt according to reset-hi6220.c

[Rob Herring]

On Tue, Nov 22, 2016 at 09:48:32AM +0100, Arnd Bergmann wrote:
> On Tuesday, November 22, 2016 3:49:19 PM CET Zhangfei Gao wrote:
> >  Required properties:
> >  - compatible: should be one of the following:
> > -  - "hisilicon,hi6220-sysctrl", "syscon" : For peripheral reset controller.
> > -  - "hisilicon,hi6220-mediactrl", "syscon" : For media reset controller.
> > -- reg: should be register base and length as documented in the
> > -  datasheet
> > +  - "hisilicon,hi6220-reset-sysctrl" : For peripheral reset controller.
> > +  - "hisilicon,hi6220-reset-mediactrl" : For media reset controller.
> > +- hisi,rst-syscon: phandle of the reset's syscon.
> >  - #reset-cells: 1, see below
> > 
> 
> Please keep the old strings around for compatibility.

Why are these even changing? The commit message should say why.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] dt-bindings: change hi6220-reset.txt according to reset-hi6220.c

[zhangfei]

Hi, Rob


On 2016年11月24日 07:06, Rob Herring wrote:
> On Tue, Nov 22, 2016 at 09:48:32AM +0100, Arnd Bergmann wrote:
>> On Tuesday, November 22, 2016 3:49:19 PM CET Zhangfei Gao wrote:
>>>   Required properties:
>>>   - compatible: should be one of the following:
>>> -  - "hisilicon,hi6220-sysctrl", "syscon" : For peripheral reset controller.
>>> -  - "hisilicon,hi6220-mediactrl", "syscon" : For media reset controller.
>>> -- reg: should be register base and length as documented in the
>>> -  datasheet
>>> +  - "hisilicon,hi6220-reset-sysctrl" : For peripheral reset controller.
>>> +  - "hisilicon,hi6220-reset-mediactrl" : For media reset controller.
>>> +- hisi,rst-syscon: phandle of the reset's syscon.
>>>   - #reset-cells: 1, see below
>>>
>> Please keep the old strings around for compatibility.
> Why are these even changing? The commit message should say why.
Have send [RFC V2:PATCH 0/2] add reset-hi3660
Not touching this file any more.
Only handle hi3660.

But hi6220 can directly use the driver.
Only need list reset info to hi6220.dts, same as hi3660.
Like
+       iomcu_rst: iomcu_rst_controller {
+               compatible = "hisilicon,hi3660-reset";
+               #reset-cells = <1>;
+               hisi,rst-syscon = <&iomcu>;
+               hisi,reset-bits = <0x20 0x8             /* 0: i2c0 */
+                                  0x20 0x10            /* 1: i2c1 */
+                                  0x20 0x20            /* 2: i2c2 */
+                                  0x20 0x8000000>;     /* 3: i2c6 */
+       };

Thanks

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

[PATCH 5/6] reset: hisilicon: Use new driver reset-hi6222

[Zhangfei Gao]

Using new reset-hi6220 with common reset.c
And keeps the same reset define

dts hi6220.c should be updated with new node sys_ctrl_rst
Solving potential issue of sys_ctrl can not be used as clock and reset
at the same time.

sys_ctrl: sys_ctrl@f7030000 {
	compatible = "hisilicon,hi6220-sysctrl", "syscon";
	reg = <0x0 0xf7030000 0x0 0x2000>;
	#clock-cells = <1>;
};

sys_ctrl_rst: sys_rst_controller {
	compatible = "hisilicon,hi6220-reset-sysctrl";
	#reset-cells = <1>;
	hisi,rst-syscon = <&sys_ctrl>;
};

uart1: serial@..... {
        ...
        resets = <&sys_ctrl_rst PERIPH_RSTEN3_UART1>;
        ...
};

Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/reset/hisilicon/Makefile               |   2 +-
 drivers/reset/hisilicon/hi6220_reset.c         | 157 -------------------------
 drivers/reset/hisilicon/reset-hi6220.c         | 123 +++++++++++++++++++
 include/dt-bindings/reset/hisi,hi6220-resets.h | 130 ++++++++++----------
 4 files changed, 190 insertions(+), 222 deletions(-)
 delete mode 100644 drivers/reset/hisilicon/hi6220_reset.c
 create mode 100644 drivers/reset/hisilicon/reset-hi6220.c

diff --git a/drivers/reset/hisilicon/Makefile b/drivers/reset/hisilicon/Makefile
index 57e9893..caddac1 100644
--- a/drivers/reset/hisilicon/Makefile
+++ b/drivers/reset/hisilicon/Makefile
@@ -1,3 +1,3 @@
 obj-y	+= reset.o
 obj-$(CONFIG_COMMON_RESET_HI3660) += reset-hi3660.o
-obj-$(CONFIG_COMMON_RESET_HI6220) += hi6220_reset.o
+obj-$(CONFIG_COMMON_RESET_HI6220) += reset-hi6220.o
diff --git a/drivers/reset/hisilicon/hi6220_reset.c b/drivers/reset/hisilicon/hi6220_reset.c
deleted file mode 100644
index 35ce53e..0000000
--- a/drivers/reset/hisilicon/hi6220_reset.c
+++ /dev/null
@@ -1,157 +0,0 @@
-/*
- * Hisilicon Hi6220 reset controller driver
- *
- * Copyright (c) 2016 Linaro Limited.
- * Copyright (c) 2015-2016 Hisilicon Limited.
- *
- * Author: Feng Chen <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#include <linux/io.h>
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/bitops.h>
-#include <linux/of.h>
-#include <linux/of_device.h>
-#include <linux/regmap.h>
-#include <linux/mfd/syscon.h>
-#include <linux/reset-controller.h>
-#include <linux/reset.h>
-#include <linux/platform_device.h>
-
-#define PERIPH_ASSERT_OFFSET      0x300
-#define PERIPH_DEASSERT_OFFSET    0x304
-#define PERIPH_MAX_INDEX          0x509
-
-#define SC_MEDIA_RSTEN            0x052C
-#define SC_MEDIA_RSTDIS           0x0530
-#define MEDIA_MAX_INDEX           8
-
-#define to_reset_data(x) container_of(x, struct hi6220_reset_data, rc_dev)
-
-enum hi6220_reset_ctrl_type {
-	PERIPHERAL,
-	MEDIA,
-};
-
-struct hi6220_reset_data {
-	struct reset_controller_dev rc_dev;
-	struct regmap *regmap;
-};
-
-static int hi6220_peripheral_assert(struct reset_controller_dev *rc_dev,
-				    unsigned long idx)
-{
-	struct hi6220_reset_data *data = to_reset_data(rc_dev);
-	struct regmap *regmap = data->regmap;
-	u32 bank = idx >> 8;
-	u32 offset = idx & 0xff;
-	u32 reg = PERIPH_ASSERT_OFFSET + bank * 0x10;
-
-	return regmap_write(regmap, reg, BIT(offset));
-}
-
-static int hi6220_peripheral_deassert(struct reset_controller_dev *rc_dev,
-				      unsigned long idx)
-{
-	struct hi6220_reset_data *data = to_reset_data(rc_dev);
-	struct regmap *regmap = data->regmap;
-	u32 bank = idx >> 8;
-	u32 offset = idx & 0xff;
-	u32 reg = PERIPH_DEASSERT_OFFSET + bank * 0x10;
-
-	return regmap_write(regmap, reg, BIT(offset));
-}
-
-static const struct reset_control_ops hi6220_peripheral_reset_ops = {
-	.assert = hi6220_peripheral_assert,
-	.deassert = hi6220_peripheral_deassert,
-};
-
-static int hi6220_media_assert(struct reset_controller_dev *rc_dev,
-			       unsigned long idx)
-{
-	struct hi6220_reset_data *data = to_reset_data(rc_dev);
-	struct regmap *regmap = data->regmap;
-
-	return regmap_write(regmap, SC_MEDIA_RSTEN, BIT(idx));
-}
-
-static int hi6220_media_deassert(struct reset_controller_dev *rc_dev,
-				 unsigned long idx)
-{
-	struct hi6220_reset_data *data = to_reset_data(rc_dev);
-	struct regmap *regmap = data->regmap;
-
-	return regmap_write(regmap, SC_MEDIA_RSTDIS, BIT(idx));
-}
-
-static const struct reset_control_ops hi6220_media_reset_ops = {
-	.assert = hi6220_media_assert,
-	.deassert = hi6220_media_deassert,
-};
-
-static int hi6220_reset_probe(struct platform_device *pdev)
-{
-	struct device_node *np = pdev->dev.of_node;
-	struct device *dev = &pdev->dev;
-	enum hi6220_reset_ctrl_type type;
-	struct hi6220_reset_data *data;
-	struct regmap *regmap;
-
-	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	type = (enum hi6220_reset_ctrl_type)of_device_get_match_data(dev);
-
-	regmap = syscon_node_to_regmap(np);
-	if (IS_ERR(regmap)) {
-		dev_err(dev, "failed to get reset controller regmap\n");
-		return PTR_ERR(regmap);
-	}
-
-	data->regmap = regmap;
-	data->rc_dev.of_node = np;
-	if (type == MEDIA) {
-		data->rc_dev.ops = &hi6220_media_reset_ops;
-		data->rc_dev.nr_resets = MEDIA_MAX_INDEX;
-	} else {
-		data->rc_dev.ops = &hi6220_peripheral_reset_ops;
-		data->rc_dev.nr_resets = PERIPH_MAX_INDEX;
-	}
-
-	return reset_controller_register(&data->rc_dev);
-}
-
-static const struct of_device_id hi6220_reset_match[] = {
-	{
-		.compatible = "hisilicon,hi6220-sysctrl",
-		.data = (void *)PERIPHERAL,
-	},
-	{
-		.compatible = "hisilicon,hi6220-mediactrl",
-		.data = (void *)MEDIA,
-	},
-	{ /* sentinel */ },
-};
-MODULE_DEVICE_TABLE(of, hi6220_reset_match);
-
-static struct platform_driver hi6220_reset_driver = {
-	.probe = hi6220_reset_probe,
-	.driver = {
-		.name = "reset-hi6220",
-		.of_match_table = hi6220_reset_match,
-	},
-};
-
-static int __init hi6220_reset_init(void)
-{
-	return platform_driver_register(&hi6220_reset_driver);
-}
-
-postcore_initcall(hi6220_reset_init);
diff --git a/drivers/reset/hisilicon/reset-hi6220.c b/drivers/reset/hisilicon/reset-hi6220.c
new file mode 100644
index 0000000..a2a64ae
--- /dev/null
+++ b/drivers/reset/hisilicon/reset-hi6220.c
@@ -0,0 +1,123 @@
+/*
+ * Copyright (c) 2016-2017 Linaro Ltd.
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <dt-bindings/reset/hisi,hi6220-resets.h>
+
+#include "reset.h"
+
+static const struct hisi_reset_channel_data hi6220_media_rst[] = {
+	[MEDIA_G3D] = HISI_RST_SEP(0x52c, 0),
+	[MEDIA_CODEC_VPU] = HISI_RST_SEP(0x52c, 2),
+	[MEDIA_CODEC_JPEG] = HISI_RST_SEP(0x52c, 3),
+	[MEDIA_ISP] = HISI_RST_SEP(0x52c, 4),
+	[MEDIA_ADE] = HISI_RST_SEP(0x52c, 5),
+	[MEDIA_MMU] = HISI_RST_SEP(0x52c, 6),
+	[MEDIA_XG2RAM1] = HISI_RST_SEP(0x52c, 7),
+};
+
+static struct hisi_reset_controller_data hi6220_media_controller = {
+	.nr_channels = ARRAY_SIZE(hi6220_media_rst),
+	.channels = hi6220_media_rst,
+};
+
+static const struct hisi_reset_channel_data hi6220_sysctrl_rst[] = {
+	[PERIPH_RSTDIS0_MMC0] = HISI_RST_SEP(0x300, 0),
+	[PERIPH_RSTDIS0_MMC1] = HISI_RST_SEP(0x300, 1),
+	[PERIPH_RSTDIS0_MMC2] = HISI_RST_SEP(0x300, 2),
+	[PERIPH_RSTDIS0_NANDC] = HISI_RST_SEP(0x300, 3),
+	[PERIPH_RSTDIS0_USBOTG_BUS] = HISI_RST_SEP(0x300, 4),
+	[PERIPH_RSTDIS0_POR_PICOPHY] = HISI_RST_SEP(0x300, 5),
+	[PERIPH_RSTDIS0_USBOTG] = HISI_RST_SEP(0x300, 6),
+	[PERIPH_RSTDIS0_USBOTG_32K] = HISI_RST_SEP(0x300, 7),
+	[PERIPH_RSTDIS1_HIFI] = HISI_RST_SEP(0x310, 0),
+	[PERIPH_RSTDIS1_DIGACODEC] = HISI_RST_SEP(0x310, 5),
+	[PERIPH_RSTEN2_IPF] = HISI_RST_SEP(0x320, 0),
+	[PERIPH_RSTEN2_SOCP] = HISI_RST_SEP(0x320, 1),
+	[PERIPH_RSTEN2_DMAC] = HISI_RST_SEP(0x320, 2),
+	[PERIPH_RSTEN2_SECENG] = HISI_RST_SEP(0x320, 3),
+	[PERIPH_RSTEN2_ABB] = HISI_RST_SEP(0x320, 4),
+	[PERIPH_RSTEN2_HPM0] = HISI_RST_SEP(0x320, 5),
+	[PERIPH_RSTEN2_HPM1] = HISI_RST_SEP(0x320, 6),
+	[PERIPH_RSTEN2_HPM2] = HISI_RST_SEP(0x320, 7),
+	[PERIPH_RSTEN2_HPM3] = HISI_RST_SEP(0x320, 8),
+	[PERIPH_RSTEN3_CSSYS] = HISI_RST_SEP(0x330, 0),
+	[PERIPH_RSTEN3_I2C0] = HISI_RST_SEP(0x330, 1),
+	[PERIPH_RSTEN3_I2C1] = HISI_RST_SEP(0x330, 2),
+	[PERIPH_RSTEN3_I2C2] = HISI_RST_SEP(0x330, 3),
+	[PERIPH_RSTEN3_I2C3] = HISI_RST_SEP(0x330, 4),
+	[PERIPH_RSTEN3_UART1] = HISI_RST_SEP(0x330, 5),
+	[PERIPH_RSTEN3_UART2] = HISI_RST_SEP(0x330, 6),
+	[PERIPH_RSTEN3_UART3] = HISI_RST_SEP(0x330, 7),
+	[PERIPH_RSTEN3_UART4] = HISI_RST_SEP(0x330, 8),
+	[PERIPH_RSTEN3_SSP] = HISI_RST_SEP(0x330, 9),
+	[PERIPH_RSTEN3_PWM] = HISI_RST_SEP(0x330, 10),
+	[PERIPH_RSTEN3_BLPWM] = HISI_RST_SEP(0x330, 11),
+	[PERIPH_RSTEN3_TSENSOR] = HISI_RST_SEP(0x330, 12),
+	[PERIPH_RSTEN3_DAPB] = HISI_RST_SEP(0x330, 18),
+	[PERIPH_RSTEN3_HKADC] = HISI_RST_SEP(0x330, 19),
+	[PERIPH_RSTEN3_CODEC_SSI] = HISI_RST_SEP(0x330, 20),
+	[PERIPH_RSTEN8_RS0] = HISI_RST_SEP(0x340, 0),
+	[PERIPH_RSTEN8_RS2] = HISI_RST_SEP(0x340, 1),
+	[PERIPH_RSTEN8_RS3] = HISI_RST_SEP(0x340, 2),
+	[PERIPH_RSTEN8_MS0] = HISI_RST_SEP(0x340, 3),
+	[PERIPH_RSTEN8_MS2] = HISI_RST_SEP(0x340, 5),
+	[PERIPH_RSTEN8_XG2RAM0] = HISI_RST_SEP(0x340, 6),
+	[PERIPH_RSTEN8_X2SRAM_TZMA] = HISI_RST_SEP(0x340, 7),
+	[PERIPH_RSTEN8_SRAM] = HISI_RST_SEP(0x340, 8),
+	[PERIPH_RSTEN8_HARQ] = HISI_RST_SEP(0x340, 10),
+	[PERIPH_RSTEN8_DDRC] = HISI_RST_SEP(0x340, 12),
+	[PERIPH_RSTEN8_DDRC_APB] = HISI_RST_SEP(0x340, 13),
+	[PERIPH_RSTEN8_DDRPACK_APB] = HISI_RST_SEP(0x340, 14),
+	[PERIPH_RSTEN8_DDRT] = HISI_RST_SEP(0x340, 17),
+	[PERIPH_RSDIST9_CARM_DAP] = HISI_RST_SEP(0x350, 0),
+	[PERIPH_RSDIST9_CARM_ATB] = HISI_RST_SEP(0x350, 1),
+	[PERIPH_RSDIST9_CARM_LBUS] = HISI_RST_SEP(0x350, 2),
+	[PERIPH_RSDIST9_CARM_POR] = HISI_RST_SEP(0x350, 3),
+	[PERIPH_RSDIST9_CARM_CORE] = HISI_RST_SEP(0x350, 4),
+	[PERIPH_RSDIST9_CARM_DBG] = HISI_RST_SEP(0x350, 5),
+	[PERIPH_RSDIST9_CARM_L2] = HISI_RST_SEP(0x350, 6),
+	[PERIPH_RSDIST9_CARM_SOCDBG] = HISI_RST_SEP(0x350, 7),
+	[PERIPH_RSDIST9_CARM_ETM] = HISI_RST_SEP(0x350, 8),
+};
+
+static struct hisi_reset_controller_data hi6220_sysctrl_controller = {
+	.nr_channels = ARRAY_SIZE(hi6220_sysctrl_rst),
+	.channels = hi6220_sysctrl_rst,
+};
+
+static const struct of_device_id hi6220_reset_match[] = {
+	{ .compatible = "hisilicon,hi6220-reset-sysctrl",
+	  .data = &hi6220_sysctrl_controller, },
+	{ .compatible = "hisilicon,hi6220-reset-mediactrl",
+	  .data = &hi6220_media_controller, },
+	{},
+};
+MODULE_DEVICE_TABLE(of, hi6220_reset_match);
+
+static struct platform_driver hi6220_reset_driver = {
+	.probe = hisi_reset_probe,
+	.driver = {
+		.name = "reset-hi6220",
+		.of_match_table = hi6220_reset_match,
+	},
+};
+
+static int __init hi6220_reset_init(void)
+{
+	return platform_driver_register(&hi6220_reset_driver);
+}
+arch_initcall(hi6220_reset_init);
+
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:hi6220-reset");
+MODULE_DESCRIPTION("HiSilicon hi6220 Reset Driver");
diff --git a/include/dt-bindings/reset/hisi,hi6220-resets.h b/include/dt-bindings/reset/hisi,hi6220-resets.h
index 322ec53..837f1a1 100644
--- a/include/dt-bindings/reset/hisi,hi6220-resets.h
+++ b/include/dt-bindings/reset/hisi,hi6220-resets.h
@@ -5,71 +5,73 @@
 #ifndef _DT_BINDINGS_RESET_CONTROLLER_HI6220
 #define _DT_BINDINGS_RESET_CONTROLLER_HI6220
 
-#define PERIPH_RSTDIS0_MMC0             0x000
-#define PERIPH_RSTDIS0_MMC1             0x001
-#define PERIPH_RSTDIS0_MMC2             0x002
-#define PERIPH_RSTDIS0_NANDC            0x003
-#define PERIPH_RSTDIS0_USBOTG_BUS       0x004
-#define PERIPH_RSTDIS0_POR_PICOPHY      0x005
-#define PERIPH_RSTDIS0_USBOTG           0x006
-#define PERIPH_RSTDIS0_USBOTG_32K       0x007
-#define PERIPH_RSTDIS1_HIFI             0x100
-#define PERIPH_RSTDIS1_DIGACODEC        0x105
-#define PERIPH_RSTEN2_IPF               0x200
-#define PERIPH_RSTEN2_SOCP              0x201
-#define PERIPH_RSTEN2_DMAC              0x202
-#define PERIPH_RSTEN2_SECENG            0x203
-#define PERIPH_RSTEN2_ABB               0x204
-#define PERIPH_RSTEN2_HPM0              0x205
-#define PERIPH_RSTEN2_HPM1              0x206
-#define PERIPH_RSTEN2_HPM2              0x207
-#define PERIPH_RSTEN2_HPM3              0x208
-#define PERIPH_RSTEN3_CSSYS             0x300
-#define PERIPH_RSTEN3_I2C0              0x301
-#define PERIPH_RSTEN3_I2C1              0x302
-#define PERIPH_RSTEN3_I2C2              0x303
-#define PERIPH_RSTEN3_I2C3              0x304
-#define PERIPH_RSTEN3_UART1             0x305
-#define PERIPH_RSTEN3_UART2             0x306
-#define PERIPH_RSTEN3_UART3             0x307
-#define PERIPH_RSTEN3_UART4             0x308
-#define PERIPH_RSTEN3_SSP               0x309
-#define PERIPH_RSTEN3_PWM               0x30a
-#define PERIPH_RSTEN3_BLPWM             0x30b
-#define PERIPH_RSTEN3_TSENSOR           0x30c
-#define PERIPH_RSTEN3_DAPB              0x312
-#define PERIPH_RSTEN3_HKADC             0x313
-#define PERIPH_RSTEN3_CODEC_SSI         0x314
-#define PERIPH_RSTEN3_PMUSSI1           0x316
-#define PERIPH_RSTEN8_RS0               0x400
-#define PERIPH_RSTEN8_RS2               0x401
-#define PERIPH_RSTEN8_RS3               0x402
-#define PERIPH_RSTEN8_MS0               0x403
-#define PERIPH_RSTEN8_MS2               0x405
-#define PERIPH_RSTEN8_XG2RAM0           0x406
-#define PERIPH_RSTEN8_X2SRAM_TZMA       0x407
-#define PERIPH_RSTEN8_SRAM              0x408
-#define PERIPH_RSTEN8_HARQ              0x40a
-#define PERIPH_RSTEN8_DDRC              0x40c
-#define PERIPH_RSTEN8_DDRC_APB          0x40d
-#define PERIPH_RSTEN8_DDRPACK_APB       0x40e
-#define PERIPH_RSTEN8_DDRT              0x411
-#define PERIPH_RSDIST9_CARM_DAP         0x500
-#define PERIPH_RSDIST9_CARM_ATB         0x501
-#define PERIPH_RSDIST9_CARM_LBUS        0x502
-#define PERIPH_RSDIST9_CARM_POR         0x503
-#define PERIPH_RSDIST9_CARM_CORE        0x504
-#define PERIPH_RSDIST9_CARM_DBG         0x505
-#define PERIPH_RSDIST9_CARM_L2          0x506
-#define PERIPH_RSDIST9_CARM_SOCDBG      0x507
-#define PERIPH_RSDIST9_CARM_ETM         0x508
 
+/* reset in sysctrl */
+#define PERIPH_RSTDIS0_MMC0             0
+#define PERIPH_RSTDIS0_MMC1             1
+#define PERIPH_RSTDIS0_MMC2             2
+#define PERIPH_RSTDIS0_NANDC            3
+#define PERIPH_RSTDIS0_USBOTG_BUS       4
+#define PERIPH_RSTDIS0_POR_PICOPHY      5
+#define PERIPH_RSTDIS0_USBOTG           6
+#define PERIPH_RSTDIS0_USBOTG_32K       7
+#define PERIPH_RSTDIS1_HIFI             8
+#define PERIPH_RSTDIS1_DIGACODEC        9
+#define PERIPH_RSTEN2_IPF               10
+#define PERIPH_RSTEN2_SOCP              11
+#define PERIPH_RSTEN2_DMAC              12
+#define PERIPH_RSTEN2_SECENG            13
+#define PERIPH_RSTEN2_ABB               14
+#define PERIPH_RSTEN2_HPM0              15
+#define PERIPH_RSTEN2_HPM1              16
+#define PERIPH_RSTEN2_HPM2              17
+#define PERIPH_RSTEN2_HPM3              18
+#define PERIPH_RSTEN3_CSSYS             19
+#define PERIPH_RSTEN3_I2C0              20
+#define PERIPH_RSTEN3_I2C1              21
+#define PERIPH_RSTEN3_I2C2              22
+#define PERIPH_RSTEN3_I2C3              23
+#define PERIPH_RSTEN3_UART1             24
+#define PERIPH_RSTEN3_UART2             25
+#define PERIPH_RSTEN3_UART3             26
+#define PERIPH_RSTEN3_UART4             27
+#define PERIPH_RSTEN3_SSP               28
+#define PERIPH_RSTEN3_PWM               29
+#define PERIPH_RSTEN3_BLPWM             30
+#define PERIPH_RSTEN3_TSENSOR           31
+#define PERIPH_RSTEN3_DAPB              32
+#define PERIPH_RSTEN3_HKADC             33
+#define PERIPH_RSTEN3_CODEC_SSI         34
+#define PERIPH_RSTEN8_RS0               35
+#define PERIPH_RSTEN8_RS2               36
+#define PERIPH_RSTEN8_RS3               37
+#define PERIPH_RSTEN8_MS0               38
+#define PERIPH_RSTEN8_MS2               39
+#define PERIPH_RSTEN8_XG2RAM0           40
+#define PERIPH_RSTEN8_X2SRAM_TZMA       41
+#define PERIPH_RSTEN8_SRAM              42
+#define PERIPH_RSTEN8_HARQ              43
+#define PERIPH_RSTEN8_DDRC              44
+#define PERIPH_RSTEN8_DDRC_APB          45
+#define PERIPH_RSTEN8_DDRPACK_APB       46
+#define PERIPH_RSTEN8_DDRT              47
+#define PERIPH_RSDIST9_CARM_DAP         48
+#define PERIPH_RSDIST9_CARM_ATB         49
+#define PERIPH_RSDIST9_CARM_LBUS        50
+#define PERIPH_RSDIST9_CARM_POR         51
+#define PERIPH_RSDIST9_CARM_CORE        52
+#define PERIPH_RSDIST9_CARM_DBG         53
+#define PERIPH_RSDIST9_CARM_L2          54
+#define PERIPH_RSDIST9_CARM_SOCDBG      55
+#define PERIPH_RSDIST9_CARM_ETM         56
+
+/* reset in media */
 #define MEDIA_G3D                       0
-#define MEDIA_CODEC_VPU                 2
-#define MEDIA_CODEC_JPEG                3
-#define MEDIA_ISP                       4
-#define MEDIA_ADE                       5
-#define MEDIA_MMU                       6
-#define MEDIA_XG2RAM1                   7
+#define MEDIA_CODEC_VPU                 1
+#define MEDIA_CODEC_JPEG                2
+#define MEDIA_ISP                       3
+#define MEDIA_ADE                       4
+#define MEDIA_MMU                       5
+#define MEDIA_XG2RAM1                   6
 
 #endif /*_DT_BINDINGS_RESET_CONTROLLER_HI6220*/
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] reset: hisilicon: Use new driver reset-hi6222

[Arnd Bergmann]

On Tuesday, November 22, 2016 3:49:20 PM CET Zhangfei Gao wrote:
> 
> -#define PERIPH_RSTDIS0_MMC0             0x000
> -#define PERIPH_RSTDIS0_MMC1             0x001
> -#define PERIPH_RSTDIS0_MMC2             0x002
> -#define PERIPH_RSTDIS0_NANDC            0x003
> -#define PERIPH_RSTDIS0_USBOTG_BUS       0x004
> -#define PERIPH_RSTDIS0_POR_PICOPHY      0x005
> -#define PERIPH_RSTDIS0_USBOTG           0x006
> -#define PERIPH_RSTDIS0_USBOTG_32K       0x007
> -#define PERIPH_RSTDIS1_HIFI             0x100
> -#define PERIPH_RSTDIS1_DIGACODEC        0x105

> +/* reset in sysctrl */
> +#define PERIPH_RSTDIS0_MMC0             0
> +#define PERIPH_RSTDIS0_MMC1             1
> +#define PERIPH_RSTDIS0_MMC2             2
> +#define PERIPH_RSTDIS0_NANDC            3
> +#define PERIPH_RSTDIS0_USBOTG_BUS       4
> +#define PERIPH_RSTDIS0_POR_PICOPHY      5
> +#define PERIPH_RSTDIS0_USBOTG           6
> +#define PERIPH_RSTDIS0_USBOTG_32K       7
> +#define PERIPH_RSTDIS1_HIFI             8

You can't redefined the binding here, this is part of the ABI.
You can however add new numbers as long as the old ones keep
working.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] reset: hisilicon: Use new driver reset-hi6222

[zhangfei]

On 2016年11月22日 16:49, Arnd Bergmann wrote:
> On Tuesday, November 22, 2016 3:49:20 PM CET Zhangfei Gao wrote:
>> -#define PERIPH_RSTDIS0_MMC0             0x000
>> -#define PERIPH_RSTDIS0_MMC1             0x001
>> -#define PERIPH_RSTDIS0_MMC2             0x002
>> -#define PERIPH_RSTDIS0_NANDC            0x003
>> -#define PERIPH_RSTDIS0_USBOTG_BUS       0x004
>> -#define PERIPH_RSTDIS0_POR_PICOPHY      0x005
>> -#define PERIPH_RSTDIS0_USBOTG           0x006
>> -#define PERIPH_RSTDIS0_USBOTG_32K       0x007
>> -#define PERIPH_RSTDIS1_HIFI             0x100
>> -#define PERIPH_RSTDIS1_DIGACODEC        0x105
>> +/* reset in sysctrl */
>> +#define PERIPH_RSTDIS0_MMC0             0
>> +#define PERIPH_RSTDIS0_MMC1             1
>> +#define PERIPH_RSTDIS0_MMC2             2
>> +#define PERIPH_RSTDIS0_NANDC            3
>> +#define PERIPH_RSTDIS0_USBOTG_BUS       4
>> +#define PERIPH_RSTDIS0_POR_PICOPHY      5
>> +#define PERIPH_RSTDIS0_USBOTG           6
>> +#define PERIPH_RSTDIS0_USBOTG_32K       7
>> +#define PERIPH_RSTDIS1_HIFI             8
> You can't redefined the binding here, this is part of the ABI.
> You can however add new numbers as long as the old ones keep
> working.
The methods are different.
The original define is offset | bit_shift, and driver has to parse 
offset and bit shift.
The new define is just index of array, which is defined in the reset-xxx.c

Thanks

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] reset: hisilicon: Use new driver reset-hi6222

[Arnd Bergmann]

On Tuesday, November 22, 2016 5:46:22 PM CET zhangfei wrote:
> On 2016年11月22日 16:49, Arnd Bergmann wrote:
> > On Tuesday, November 22, 2016 3:49:20 PM CET Zhangfei Gao wrote:
> >> -#define PERIPH_RSTDIS0_MMC0             0x000
> >> -#define PERIPH_RSTDIS0_MMC1             0x001
> >> -#define PERIPH_RSTDIS0_MMC2             0x002
> >> -#define PERIPH_RSTDIS0_NANDC            0x003
> >> -#define PERIPH_RSTDIS0_USBOTG_BUS       0x004
> >> -#define PERIPH_RSTDIS0_POR_PICOPHY      0x005
> >> -#define PERIPH_RSTDIS0_USBOTG           0x006
> >> -#define PERIPH_RSTDIS0_USBOTG_32K       0x007
> >> -#define PERIPH_RSTDIS1_HIFI             0x100
> >> -#define PERIPH_RSTDIS1_DIGACODEC        0x105
> >> +/* reset in sysctrl */
> >> +#define PERIPH_RSTDIS0_MMC0             0
> >> +#define PERIPH_RSTDIS0_MMC1             1
> >> +#define PERIPH_RSTDIS0_MMC2             2
> >> +#define PERIPH_RSTDIS0_NANDC            3
> >> +#define PERIPH_RSTDIS0_USBOTG_BUS       4
> >> +#define PERIPH_RSTDIS0_POR_PICOPHY      5
> >> +#define PERIPH_RSTDIS0_USBOTG           6
> >> +#define PERIPH_RSTDIS0_USBOTG_32K       7
> >> +#define PERIPH_RSTDIS1_HIFI             8
> > You can't redefined the binding here, this is part of the ABI.
> > You can however add new numbers as long as the old ones keep
> > working.
> The methods are different.
> The original define is offset | bit_shift, and driver has to parse 
> offset and bit shift.
> The new define is just index of array, which is defined in the reset-xxx.c

I understand that, what I mean is you have to find a way to let the new
driver still support the old binding, you can't change it.

	Arnd

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/6] arm64: dts: hi6220: update reset node according to reset-hi6220.c

[Zhangfei Gao]

Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 17839db..7918043 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -246,14 +246,24 @@
 			compatible = "hisilicon,hi6220-sysctrl", "syscon";
 			reg = <0x0 0xf7030000 0x0 0x2000>;
 			#clock-cells = <1>;
-			#reset-cells = <1>;
 		};
 
 		media_ctrl: media_ctrl@f4410000 {
 			compatible = "hisilicon,hi6220-mediactrl", "syscon";
 			reg = <0x0 0xf4410000 0x0 0x1000>;
 			#clock-cells = <1>;
+		};
+
+		sys_ctrl_rst: sys_rst_controller {
+			compatible = "hisilicon,hi6220-reset-sysctrl";
+			#reset-cells = <1>;
+			hisi,rst-syscon = <&sys_ctrl>;
+		};
+
+		media_ctrl_rst: media_rst_controller {
+			compatible = "hisilicon,hi6220-reset-mediactrl";
 			#reset-cells = <1>;
+			hisi,rst-syscon = <&media_ctrl>;
 		};
 
 		pm_ctrl: pm_ctrl@f7032000 {
@@ -771,7 +781,7 @@
 			interrupts = <0x0 0x48 0x4>;
 			clocks = <&sys_ctrl 2>, <&sys_ctrl 1>;
 			clock-names = "ciu", "biu";
-			resets = <&sys_ctrl PERIPH_RSTDIS0_MMC0>;
+			resets = <&sys_ctrl_rst PERIPH_RSTDIS0_MMC0>;
 			bus-width = <0x8>;
 			vmmc-supply = <&ldo19>;
 			pinctrl-names = "default";
@@ -794,7 +804,7 @@
 			#size-cells = <0x0>;
 			clocks = <&sys_ctrl 4>, <&sys_ctrl 3>;
 			clock-names = "ciu", "biu";
-			resets = <&sys_ctrl PERIPH_RSTDIS0_MMC1>;
+			resets = <&sys_ctrl_rst PERIPH_RSTDIS0_MMC1>;
 			vqmmc-supply = <&ldo7>;
 			vmmc-supply = <&ldo10>;
 			bus-width = <0x4>;
@@ -812,7 +822,7 @@
 			interrupts = <0x0 0x4a 0x4>;
 			clocks = <&sys_ctrl HI6220_MMC2_CIUCLK>, <&sys_ctrl HI6220_MMC2_CLK>;
 			clock-names = "ciu", "biu";
-			resets = <&sys_ctrl PERIPH_RSTDIS0_MMC2>;
+			resets = <&sys_ctrl_rst PERIPH_RSTDIS0_MMC2>;
 			bus-width = <0x4>;
 			broken-cd;
 			pinctrl-names = "default", "idle";
@@ -867,7 +877,7 @@
 			reg = <0x0 0xf4100000 0x0 0x7800>;
 			reg-names = "ade_base";
 			hisilicon,noc-syscon = <&medianoc_ade>;
-			resets = <&media_ctrl MEDIA_ADE>;
+			resets = <&media_ctrl_rst MEDIA_ADE>;
 			interrupts = <0 115 4>; /* ldi interrupt */
 
 			clocks = <&media_ctrl HI6220_ADE_CORE>,
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html