* [PATCH 1/6] reset: hisilicon: add reset core
2016-11-22 7:49 [PATCH 0/6] add hisilicon reset Zhangfei Gao
@ 2016-11-22 7:49 ` Zhangfei Gao
2016-11-22 8:45 ` Arnd Bergmann
` (2 more replies)
2016-11-22 7:49 ` [PATCH 2/6] dt-bindings: Document the hi3660 reset bindings Zhangfei Gao
` (4 subsequent siblings)
5 siblings, 3 replies; 24+ messages in thread
From: Zhangfei Gao @ 2016-11-22 7:49 UTC (permalink / raw)
To: linux-arm-kernel
reset.c will be shared by hisilicon chips like hi3660 and hi6220
Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 1/6] reset: hisilicon: add reset core
2016-11-22 7:49 ` [PATCH 1/6] reset: hisilicon: add reset core Zhangfei Gao
@ 2016-11-22 8:45 ` Arnd Bergmann
2016-11-22 9:22 ` zhangfei
2016-11-22 10:22 ` Philipp Zabel
2016-11-22 10:22 ` Philipp Zabel
2 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2016-11-22 8:45 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/6] reset: hisilicon: add reset core
2016-11-22 8:45 ` Arnd Bergmann
@ 2016-11-22 9:22 ` zhangfei
2016-11-22 9:41 ` Arnd Bergmann
0 siblings, 1 reply; 24+ messages in thread
From: zhangfei @ 2016-11-22 9:22 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 1/6] reset: hisilicon: add reset core
2016-11-22 9:22 ` zhangfei
@ 2016-11-22 9:41 ` Arnd Bergmann
0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2016-11-22 9:41 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/6] reset: hisilicon: add reset core
2016-11-22 7:49 ` [PATCH 1/6] reset: hisilicon: add reset core Zhangfei Gao
2016-11-22 8:45 ` Arnd Bergmann
@ 2016-11-22 10:22 ` Philipp Zabel
2016-11-22 10:22 ` Philipp Zabel
2 siblings, 0 replies; 24+ messages in thread
From: Philipp Zabel @ 2016-11-22 10:22 UTC (permalink / raw)
To: linux-arm-kernel
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@linaro.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
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 1/6] reset: hisilicon: add reset core
2016-11-22 7:49 ` [PATCH 1/6] reset: hisilicon: add reset core Zhangfei Gao
2016-11-22 8:45 ` Arnd Bergmann
2016-11-22 10:22 ` Philipp Zabel
@ 2016-11-22 10:22 ` Philipp Zabel
2 siblings, 0 replies; 24+ messages in thread
From: Philipp Zabel @ 2016-11-22 10:22 UTC (permalink / raw)
To: linux-arm-kernel
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@linaro.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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/6] dt-bindings: Document the hi3660 reset bindings
2016-11-22 7:49 [PATCH 0/6] add hisilicon reset Zhangfei Gao
2016-11-22 7:49 ` [PATCH 1/6] reset: hisilicon: add reset core Zhangfei Gao
@ 2016-11-22 7:49 ` Zhangfei Gao
2016-11-22 7:49 ` [PATCH 3/6] reset: hisilicon: add reset-hi3660 Zhangfei Gao
` (3 subsequent siblings)
5 siblings, 0 replies; 24+ messages in thread
From: Zhangfei Gao @ 2016-11-22 7:49 UTC (permalink / raw)
To: linux-arm-kernel
Add DT bindings documentation for hi3660 SoC reset controller.
Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.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 at 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 at ..... {
+ ...
+ 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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 3/6] reset: hisilicon: add reset-hi3660
2016-11-22 7:49 [PATCH 0/6] add hisilicon reset Zhangfei Gao
2016-11-22 7:49 ` [PATCH 1/6] reset: hisilicon: add reset core Zhangfei Gao
2016-11-22 7:49 ` [PATCH 2/6] dt-bindings: Document the hi3660 reset bindings Zhangfei Gao
@ 2016-11-22 7:49 ` Zhangfei Gao
2016-11-22 8:50 ` Arnd Bergmann
2016-11-22 7:49 ` [PATCH 4/6] dt-bindings: change hi6220-reset.txt according to reset-hi6220.c Zhangfei Gao
` (2 subsequent siblings)
5 siblings, 1 reply; 24+ messages in thread
From: Zhangfei Gao @ 2016-11-22 7:49 UTC (permalink / raw)
To: linux-arm-kernel
Add hi3660 reset driver based on common reset.c
Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 3/6] reset: hisilicon: add reset-hi3660
2016-11-22 7:49 ` [PATCH 3/6] reset: hisilicon: add reset-hi3660 Zhangfei Gao
@ 2016-11-22 8:50 ` Arnd Bergmann
2016-11-22 9:34 ` zhangfei
0 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2016-11-22 8:50 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 3/6] reset: hisilicon: add reset-hi3660
2016-11-22 8:50 ` Arnd Bergmann
@ 2016-11-22 9:34 ` zhangfei
2016-11-22 9:42 ` Arnd Bergmann
0 siblings, 1 reply; 24+ messages in thread
From: zhangfei @ 2016-11-22 9:34 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 3/6] reset: hisilicon: add reset-hi3660
2016-11-22 9:34 ` zhangfei
@ 2016-11-22 9:42 ` Arnd Bergmann
2016-11-22 10:02 ` zhangfei
2016-11-22 10:22 ` Philipp Zabel
0 siblings, 2 replies; 24+ messages in thread
From: Arnd Bergmann @ 2016-11-22 9:42 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 3/6] reset: hisilicon: add reset-hi3660
2016-11-22 9:42 ` Arnd Bergmann
@ 2016-11-22 10:02 ` zhangfei
2016-11-22 10:22 ` Philipp Zabel
1 sibling, 0 replies; 24+ messages in thread
From: zhangfei @ 2016-11-22 10:02 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 3/6] reset: hisilicon: add reset-hi3660
2016-11-22 9:42 ` Arnd Bergmann
2016-11-22 10:02 ` zhangfei
@ 2016-11-22 10:22 ` Philipp Zabel
2016-11-23 8:02 ` zhangfei
1 sibling, 1 reply; 24+ messages in thread
From: Philipp Zabel @ 2016-11-22 10:22 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 3/6] reset: hisilicon: add reset-hi3660
2016-11-22 10:22 ` Philipp Zabel
@ 2016-11-23 8:02 ` zhangfei
0 siblings, 0 replies; 24+ messages in thread
From: zhangfei @ 2016-11-23 8:02 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/6] dt-bindings: change hi6220-reset.txt according to reset-hi6220.c
2016-11-22 7:49 [PATCH 0/6] add hisilicon reset Zhangfei Gao
` (2 preceding siblings ...)
2016-11-22 7:49 ` [PATCH 3/6] reset: hisilicon: add reset-hi3660 Zhangfei Gao
@ 2016-11-22 7:49 ` Zhangfei Gao
2016-11-22 8:48 ` Arnd Bergmann
2016-11-22 7:49 ` [PATCH 5/6] reset: hisilicon: Use new driver reset-hi6222 Zhangfei Gao
2016-11-22 7:49 ` [PATCH 6/6] arm64: dts: hi6220: update reset node according to reset-hi6220.c Zhangfei Gao
5 siblings, 1 reply; 24+ messages in thread
From: Zhangfei Gao @ 2016-11-22 7:49 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.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 at 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 at ..... {
...
- resets = <&sys_ctrl PERIPH_RSTEN3_UART1>;
+ resets = <&sys_ctrl_rst PERIPH_RSTEN3_UART1>;
...
};
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 4/6] dt-bindings: change hi6220-reset.txt according to reset-hi6220.c
2016-11-22 7:49 ` [PATCH 4/6] dt-bindings: change hi6220-reset.txt according to reset-hi6220.c Zhangfei Gao
@ 2016-11-22 8:48 ` Arnd Bergmann
2016-11-23 23:06 ` Rob Herring
0 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2016-11-22 8:48 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/6] dt-bindings: change hi6220-reset.txt according to reset-hi6220.c
2016-11-22 8:48 ` Arnd Bergmann
@ 2016-11-23 23:06 ` Rob Herring
2016-11-24 0:41 ` zhangfei
0 siblings, 1 reply; 24+ messages in thread
From: Rob Herring @ 2016-11-23 23:06 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/6] dt-bindings: change hi6220-reset.txt according to reset-hi6220.c
2016-11-23 23:06 ` Rob Herring
@ 2016-11-24 0:41 ` zhangfei
0 siblings, 0 replies; 24+ messages in thread
From: zhangfei @ 2016-11-24 0:41 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 5/6] reset: hisilicon: Use new driver reset-hi6222
2016-11-22 7:49 [PATCH 0/6] add hisilicon reset Zhangfei Gao
` (3 preceding siblings ...)
2016-11-22 7:49 ` [PATCH 4/6] dt-bindings: change hi6220-reset.txt according to reset-hi6220.c Zhangfei Gao
@ 2016-11-22 7:49 ` Zhangfei Gao
2016-11-22 8:49 ` Arnd Bergmann
2016-11-22 7:49 ` [PATCH 6/6] arm64: dts: hi6220: update reset node according to reset-hi6220.c Zhangfei Gao
5 siblings, 1 reply; 24+ messages in thread
From: Zhangfei Gao @ 2016-11-22 7:49 UTC (permalink / raw)
To: linux-arm-kernel
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 at 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 at ..... {
...
resets = <&sys_ctrl_rst PERIPH_RSTEN3_UART1>;
...
};
Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.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@hisilicon.com>
- *
- * 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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 5/6] reset: hisilicon: Use new driver reset-hi6222
2016-11-22 7:49 ` [PATCH 5/6] reset: hisilicon: Use new driver reset-hi6222 Zhangfei Gao
@ 2016-11-22 8:49 ` Arnd Bergmann
2016-11-22 9:46 ` zhangfei
0 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2016-11-22 8:49 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 5/6] reset: hisilicon: Use new driver reset-hi6222
2016-11-22 8:49 ` Arnd Bergmann
@ 2016-11-22 9:46 ` zhangfei
2016-11-22 9:55 ` Arnd Bergmann
0 siblings, 1 reply; 24+ messages in thread
From: zhangfei @ 2016-11-22 9:46 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 5/6] reset: hisilicon: Use new driver reset-hi6222
2016-11-22 9:46 ` zhangfei
@ 2016-11-22 9:55 ` Arnd Bergmann
0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2016-11-22 9:55 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 6/6] arm64: dts: hi6220: update reset node according to reset-hi6220.c
2016-11-22 7:49 [PATCH 0/6] add hisilicon reset Zhangfei Gao
` (4 preceding siblings ...)
2016-11-22 7:49 ` [PATCH 5/6] reset: hisilicon: Use new driver reset-hi6222 Zhangfei Gao
@ 2016-11-22 7:49 ` Zhangfei Gao
5 siblings, 0 replies; 24+ messages in thread
From: Zhangfei Gao @ 2016-11-22 7:49 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.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 at 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 at 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
^ permalink raw reply related [flat|nested] 24+ messages in thread