* [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for SDM845 SoCs
@ 2018-07-27 15:28 Sibi Sankar
2018-07-27 15:28 ` [PATCH 2/4] reset: qcom: PDC (Power Domain Controller) reset controller Sibi Sankar
` (4 more replies)
0 siblings, 5 replies; 21+ messages in thread
From: Sibi Sankar @ 2018-07-27 15:28 UTC (permalink / raw)
To: bjorn.andersson, p.zabel, robh+dt
Cc: linux-remoteproc, linux-kernel, devicetree, ohad, mark.rutland,
sricharan, akdwived, linux-arm-msm, tsoni, Sibi Sankar
Add SDM845 PDC (Power Domain Controller) reset controller binding
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
.../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++
include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++
2 files changed, 72 insertions(+)
create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h
diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
new file mode 100644
index 000000000000..85e159962e08
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
@@ -0,0 +1,52 @@
+PDC Reset Controller
+======================================
+
+This binding describes a reset-controller found on PDC-Global(Power Domain
+Controller) block for Qualcomm Technologies Inc SDM845 SoCs.
+
+Required properties:
+- compatible:
+ Usage: required
+ Value type: <string>
+ Definition: must be:
+ "qcom,sdm845-pdc-global"
+
+- reg:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: must specify the base address and size of the register
+ space.
+
+- #reset-cells:
+ Usage: required
+ Value type: <uint>
+ Definition: must be 1; cell entry represents the reset index.
+
+Example:
+
+pdc_reset: reset-controller@b2e0000 {
+ compatible = "qcom,sdm845-pdc-global";
+ reg = <0xb2e0000 0x20000>;
+ #reset-cells = <1>;
+};
+
+PDC reset clients
+======================================
+
+Device nodes that need access to reset lines should
+specify them as a reset phandle in their corresponding node as
+specified in reset.txt.
+
+For list of all valid reset indicies see
+<dt-bindings/reset/qcom,sdm845-pdc.h>
+
+Example:
+
+modem-pil@4080000 {
+ ...
+
+ resets = <&pdc_reset PDC_MODEM_SYNC_RESET>;
+ reset-names = "pdc_restart";
+
+ ...
+};
diff --git a/include/dt-bindings/reset/qcom,sdm845-pdc.h b/include/dt-bindings/reset/qcom,sdm845-pdc.h
new file mode 100644
index 000000000000..53c37f9c319a
--- /dev/null
+++ b/include/dt-bindings/reset/qcom,sdm845-pdc.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018 The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_RESET_PDC_SDM_845_H
+#define _DT_BINDINGS_RESET_PDC_SDM_845_H
+
+#define PDC_APPS_SYNC_RESET 0
+#define PDC_SP_SYNC_RESET 1
+#define PDC_AUDIO_SYNC_RESET 2
+#define PDC_SENSORS_SYNC_RESET 3
+#define PDC_AOP_SYNC_RESET 4
+#define PDC_DEBUG_SYNC_RESET 5
+#define PDC_GPU_SYNC_RESET 6
+#define PDC_DISPLAY_SYNC_RESET 7
+#define PDC_COMPUTE_SYNC_RESET 8
+#define PDC_MODEM_SYNC_RESET 9
+
+#endif
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/4] reset: qcom: PDC (Power Domain Controller) reset controller
2018-07-27 15:28 [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for SDM845 SoCs Sibi Sankar
@ 2018-07-27 15:28 ` Sibi Sankar
2018-07-31 8:51 ` Philipp Zabel
2018-08-21 22:17 ` Bjorn Andersson
2018-07-27 15:28 ` [PATCH 3/4] remoteproc: qcom: q6v5-pil: Explicitly get mss_restart line Sibi Sankar
` (3 subsequent siblings)
4 siblings, 2 replies; 21+ messages in thread
From: Sibi Sankar @ 2018-07-27 15:28 UTC (permalink / raw)
To: bjorn.andersson, p.zabel, robh+dt
Cc: linux-remoteproc, linux-kernel, devicetree, ohad, mark.rutland,
sricharan, akdwived, linux-arm-msm, tsoni, Sibi Sankar
Add reset controller for SDM845 SoC to control reset signals
provided by PDC for Modem, Compute, Display, GPU, Debug, AOP,
Sensors, Audio, SP and APPS
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
drivers/reset/Kconfig | 9 +++
drivers/reset/Makefile | 1 +
drivers/reset/reset-qcom-pdc.c | 139 +++++++++++++++++++++++++++++++++
3 files changed, 149 insertions(+)
create mode 100644 drivers/reset/reset-qcom-pdc.c
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 13d28fdbdbb5..5344e202a630 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -98,6 +98,15 @@ config RESET_QCOM_AOSS
reset signals provided by AOSS for Modem, Venus, ADSP,
GPU, Camera, Wireless, Display subsystem. Otherwise, say N.
+config RESET_QCOM_PDC
+ bool "Qcom PDC Reset Driver"
+ depends on ARCH_QCOM || COMPILE_TEST
+ help
+ This enables the PDC (Power Domain Controller) reset driver
+ for Qualcomm Technologies Inc SDM845 SoCs. Say Y if you want
+ to control reset signals provided by PDC for Modem, Compute,
+ Display, GPU, Debug, AOP, Sensors, Audio, SP and APPS.
+
config RESET_SIMPLE
bool "Simple Reset Controller Driver" if COMPILE_TEST
default ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX || ARCH_ASPEED
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 4243c38228e2..d08e8b90046a 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_RESET_MESON_AUDIO_ARB) += reset-meson-audio-arb.o
obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
obj-$(CONFIG_RESET_QCOM_AOSS) += reset-qcom-aoss.o
+obj-$(CONFIG_RESET_QCOM_PDC) += reset-qcom-pdc.o
obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
obj-$(CONFIG_RESET_STM32MP157) += reset-stm32mp1.o
obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
diff --git a/drivers/reset/reset-qcom-pdc.c b/drivers/reset/reset-qcom-pdc.c
new file mode 100644
index 000000000000..64a0041e3452
--- /dev/null
+++ b/drivers/reset/reset-qcom-pdc.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/regmap.h>
+#include <linux/of_device.h>
+#include <dt-bindings/reset/qcom,sdm845-pdc.h>
+
+struct qcom_pdc_reset_map {
+ u8 bit;
+};
+
+struct qcom_pdc_desc {
+ const struct regmap_config *config;
+ const struct qcom_pdc_reset_map *resets;
+ size_t num_resets;
+};
+
+struct qcom_pdc_reset_data {
+ struct reset_controller_dev rcdev;
+ struct regmap *regmap;
+ const struct qcom_pdc_desc *desc;
+};
+
+static const struct regmap_config sdm845_pdc_regmap_config = {
+ .name = "pdc-reset",
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x20000,
+ .fast_io = true,
+};
+
+static const struct qcom_pdc_reset_map sdm845_pdc_resets[] = {
+ [PDC_APPS_SYNC_RESET] = {0},
+ [PDC_SP_SYNC_RESET] = {1},
+ [PDC_AUDIO_SYNC_RESET] = {2},
+ [PDC_SENSORS_SYNC_RESET] = {3},
+ [PDC_AOP_SYNC_RESET] = {4},
+ [PDC_DEBUG_SYNC_RESET] = {5},
+ [PDC_GPU_SYNC_RESET] = {6},
+ [PDC_DISPLAY_SYNC_RESET] = {7},
+ [PDC_COMPUTE_SYNC_RESET] = {8},
+ [PDC_MODEM_SYNC_RESET] = {9},
+};
+
+static const struct qcom_pdc_desc sdm845_pdc_desc = {
+ .config = &sdm845_pdc_regmap_config,
+ .resets = sdm845_pdc_resets,
+ .num_resets = ARRAY_SIZE(sdm845_pdc_resets),
+};
+
+static inline struct qcom_pdc_reset_data *to_qcom_pdc_reset_data(
+ struct reset_controller_dev *rcdev)
+{
+ return container_of(rcdev, struct qcom_pdc_reset_data, rcdev);
+}
+
+static int qcom_pdc_control_assert(struct reset_controller_dev *rcdev,
+ unsigned long idx)
+{
+ struct qcom_pdc_reset_data *data = to_qcom_pdc_reset_data(rcdev);
+ const struct qcom_pdc_reset_map *map = &data->desc->resets[idx];
+
+ return regmap_update_bits(data->regmap, 0x100,
+ BIT(map->bit), BIT(map->bit));
+}
+
+static int qcom_pdc_control_deassert(struct reset_controller_dev *rcdev,
+ unsigned long idx)
+{
+ struct qcom_pdc_reset_data *data = to_qcom_pdc_reset_data(rcdev);
+ const struct qcom_pdc_reset_map *map = &data->desc->resets[idx];
+
+ return regmap_update_bits(data->regmap, 0x100, BIT(map->bit), 0);
+}
+
+static const struct reset_control_ops qcom_pdc_reset_ops = {
+ .assert = qcom_pdc_control_assert,
+ .deassert = qcom_pdc_control_deassert,
+};
+
+static int qcom_pdc_reset_probe(struct platform_device *pdev)
+{
+ struct qcom_pdc_reset_data *data;
+ struct device *dev = &pdev->dev;
+ const struct qcom_pdc_desc *desc;
+ void __iomem *base;
+ struct resource *res;
+
+ desc = of_device_get_match_data(dev);
+ if (!desc)
+ return -EINVAL;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->desc = desc;
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ data->regmap = devm_regmap_init_mmio(dev, base, desc->config);
+ if (IS_ERR(data->regmap)) {
+ dev_err(dev, "Unable to get pdc-global regmap");
+ return PTR_ERR(data->regmap);
+ }
+
+ data->rcdev.owner = THIS_MODULE;
+ data->rcdev.ops = &qcom_pdc_reset_ops;
+ data->rcdev.nr_resets = desc->num_resets;
+ data->rcdev.of_node = dev->of_node;
+
+ return devm_reset_controller_register(dev, &data->rcdev);
+}
+
+static const struct of_device_id qcom_pdc_reset_of_match[] = {
+ { .compatible = "qcom,sdm845-pdc-global", .data = &sdm845_pdc_desc },
+ {}
+};
+
+static struct platform_driver qcom_pdc_reset_driver = {
+ .probe = qcom_pdc_reset_probe,
+ .driver = {
+ .name = "qcom_pdc_reset",
+ .of_match_table = qcom_pdc_reset_of_match,
+ },
+};
+
+builtin_platform_driver(qcom_pdc_reset_driver);
+
+MODULE_DESCRIPTION("Qualcomm PDC Reset Driver");
+MODULE_LICENSE("GPL v2");
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/4] remoteproc: qcom: q6v5-pil: Explicitly get mss_restart line
2018-07-27 15:28 [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for SDM845 SoCs Sibi Sankar
2018-07-27 15:28 ` [PATCH 2/4] reset: qcom: PDC (Power Domain Controller) reset controller Sibi Sankar
@ 2018-07-27 15:28 ` Sibi Sankar
2018-07-31 8:57 ` Philipp Zabel
2018-07-27 15:28 ` [PATCH 4/4] remoteproc: qcom: q6v5-pil: Add PDC restart for modem on SDM845 SoCs Sibi Sankar
` (2 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: Sibi Sankar @ 2018-07-27 15:28 UTC (permalink / raw)
To: bjorn.andersson, p.zabel, robh+dt
Cc: linux-remoteproc, linux-kernel, devicetree, ohad, mark.rutland,
sricharan, akdwived, linux-arm-msm, tsoni, Sibi Sankar
Explicitly get mss_restart to facilitate adding PDC
restart line for modem on SDM845 SoCs
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
drivers/remoteproc/qcom_q6v5_pil.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
index b1296d614b8b..d57fdb34e3dd 100644
--- a/drivers/remoteproc/qcom_q6v5_pil.c
+++ b/drivers/remoteproc/qcom_q6v5_pil.c
@@ -1176,8 +1176,7 @@ static int q6v5_init_clocks(struct device *dev, struct clk **clks,
static int q6v5_init_reset(struct q6v5 *qproc)
{
- qproc->mss_restart = devm_reset_control_get_exclusive(qproc->dev,
- NULL);
+ qproc->mss_restart = devm_reset_control_get(qproc->dev, "mss_restart");
if (IS_ERR(qproc->mss_restart)) {
dev_err(qproc->dev, "failed to acquire mss restart\n");
return PTR_ERR(qproc->mss_restart);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/4] remoteproc: qcom: q6v5-pil: Add PDC restart for modem on SDM845 SoCs
2018-07-27 15:28 [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for SDM845 SoCs Sibi Sankar
2018-07-27 15:28 ` [PATCH 2/4] reset: qcom: PDC (Power Domain Controller) reset controller Sibi Sankar
2018-07-27 15:28 ` [PATCH 3/4] remoteproc: qcom: q6v5-pil: Explicitly get mss_restart line Sibi Sankar
@ 2018-07-27 15:28 ` Sibi Sankar
2018-07-31 8:54 ` Philipp Zabel
` (2 more replies)
2018-07-31 8:42 ` [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for " Philipp Zabel
2018-08-21 22:08 ` Bjorn Andersson
4 siblings, 3 replies; 21+ messages in thread
From: Sibi Sankar @ 2018-07-27 15:28 UTC (permalink / raw)
To: bjorn.andersson, p.zabel, robh+dt
Cc: linux-remoteproc, linux-kernel, devicetree, ohad, mark.rutland,
sricharan, akdwived, linux-arm-msm, tsoni, Sibi Sankar
In the presence of a PDC block working with subsystem RSC,
assert/deassert PDC restart in modem start/stop path.
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
.../bindings/remoteproc/qcom,q6v5.txt | 4 +++
drivers/remoteproc/qcom_q6v5_pil.c | 27 ++++++++++++++++---
2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
index 601dd9f389aa..124fb1dc6fb8 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
@@ -51,6 +51,8 @@ on the Qualcomm Hexagon core.
Usage: required
Value type: <phandle>
Definition: reference to the reset-controller for the modem sub-system
+ reference to the list of 2 reset-controllers for the modem
+ sub-system on SDM845 SoCs
reference to the list of 3 reset-controllers for the
wcss sub-system
@@ -58,6 +60,8 @@ on the Qualcomm Hexagon core.
Usage: required
Value type: <stringlist>
Definition: must be "mss_restart" for the modem sub-system
+ Definition: must be "mss_restart", "pdc_restart" for the modem
+ sub-system on SDM845 SoCs
Definition: must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
for the wcss syb-system
diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
index d57fdb34e3dd..5be794639fc3 100644
--- a/drivers/remoteproc/qcom_q6v5_pil.c
+++ b/drivers/remoteproc/qcom_q6v5_pil.c
@@ -149,6 +149,7 @@ struct q6v5 {
u32 halt_nc;
struct reset_control *mss_restart;
+ struct reset_control *pdc_restart;
struct qcom_q6v5 q6v5;
@@ -349,10 +350,17 @@ static int q6v5_load(struct rproc *rproc, const struct firmware *fw)
static int q6v5_reset_assert(struct q6v5 *qproc)
{
- if (qproc->has_alt_reset)
- return reset_control_reset(qproc->mss_restart);
- else
- return reset_control_assert(qproc->mss_restart);
+ int ret;
+
+ if (qproc->has_alt_reset) {
+ reset_control_assert(qproc->pdc_restart);
+ ret = reset_control_reset(qproc->mss_restart);
+ reset_control_deassert(qproc->pdc_restart);
+ } else {
+ ret = reset_control_assert(qproc->mss_restart);
+ }
+
+ return ret;
}
static int q6v5_reset_deassert(struct q6v5 *qproc)
@@ -360,9 +368,11 @@ static int q6v5_reset_deassert(struct q6v5 *qproc)
int ret;
if (qproc->has_alt_reset) {
+ reset_control_assert(qproc->pdc_restart);
writel(1, qproc->rmb_base + RMB_MBA_ALT_RESET);
ret = reset_control_reset(qproc->mss_restart);
writel(0, qproc->rmb_base + RMB_MBA_ALT_RESET);
+ reset_control_deassert(qproc->pdc_restart);
} else {
ret = reset_control_deassert(qproc->mss_restart);
}
@@ -1182,6 +1192,15 @@ static int q6v5_init_reset(struct q6v5 *qproc)
return PTR_ERR(qproc->mss_restart);
}
+ if (qproc->has_alt_reset) {
+ qproc->pdc_restart = devm_reset_control_get(qproc->dev,
+ "pdc_restart");
+ if (IS_ERR(qproc->pdc_restart)) {
+ dev_err(qproc->dev, "failed to acquire pdc restart\n");
+ return PTR_ERR(qproc->pdc_restart);
+ }
+ }
+
return 0;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for SDM845 SoCs
2018-07-27 15:28 [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for SDM845 SoCs Sibi Sankar
` (2 preceding siblings ...)
2018-07-27 15:28 ` [PATCH 4/4] remoteproc: qcom: q6v5-pil: Add PDC restart for modem on SDM845 SoCs Sibi Sankar
@ 2018-07-31 8:42 ` Philipp Zabel
2018-07-31 12:57 ` Sibi S
2018-08-21 22:08 ` Bjorn Andersson
4 siblings, 1 reply; 21+ messages in thread
From: Philipp Zabel @ 2018-07-31 8:42 UTC (permalink / raw)
To: Sibi Sankar, bjorn.andersson, robh+dt
Cc: linux-remoteproc, linux-kernel, devicetree, ohad, mark.rutland,
sricharan, akdwived, linux-arm-msm, tsoni
Hi Sibi,
On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote:
> Add SDM845 PDC (Power Domain Controller) reset controller binding
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> .../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++
> include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++
> 2 files changed, 72 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
> create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h
>
> diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
> new file mode 100644
> index 000000000000..85e159962e08
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
> @@ -0,0 +1,52 @@
> +PDC Reset Controller
> +======================================
> +
> +This binding describes a reset-controller found on PDC-Global(Power Domain
> +Controller) block for Qualcomm Technologies Inc SDM845 SoCs.
> +
> +Required properties:
> +- compatible:
> + Usage: required
> + Value type: <string>
> + Definition: must be:
> + "qcom,sdm845-pdc-global"
> +
> +- reg:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: must specify the base address and size of the register
> + space.
> +
> +- #reset-cells:
> + Usage: required
> + Value type: <uint>
> + Definition: must be 1; cell entry represents the reset index.
> +
> +Example:
> +
> +pdc_reset: reset-controller@b2e0000 {
Is this really just a reset controller?
The name makes it sound like a driver binding to this should also
provide pm_genpd and the binding should probably call this a power-
controller: Documentation/devicetree/bindings/power/power_domain.txt.
> + compatible = "qcom,sdm845-pdc-global";
> + reg = <0xb2e0000 0x20000>;
This looks like this is the register space of the complete PDC, not just
the reset register?
> + #reset-cells = <1>;
> +};
> +
> +PDC reset clients
> +======================================
> +
> +Device nodes that need access to reset lines should
> +specify them as a reset phandle in their corresponding node as
> +specified in reset.txt.
> +
> +For list of all valid reset indicies see
> +<dt-bindings/reset/qcom,sdm845-pdc.h>
> +
> +Example:
> +
> +modem-pil@4080000 {
> + ...
> +
> + resets = <&pdc_reset PDC_MODEM_SYNC_RESET>;
> + reset-names = "pdc_restart";
> +
> + ...
> +};
> diff --git a/include/dt-bindings/reset/qcom,sdm845-pdc.h b/include/dt-bindings/reset/qcom,sdm845-pdc.h
> new file mode 100644
> index 000000000000..53c37f9c319a
> --- /dev/null
> +++ b/include/dt-bindings/reset/qcom,sdm845-pdc.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2018 The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef _DT_BINDINGS_RESET_PDC_SDM_845_H
> +#define _DT_BINDINGS_RESET_PDC_SDM_845_H
> +
> +#define PDC_APPS_SYNC_RESET 0
> +#define PDC_SP_SYNC_RESET 1
> +#define PDC_AUDIO_SYNC_RESET 2
> +#define PDC_SENSORS_SYNC_RESET 3
> +#define PDC_AOP_SYNC_RESET 4
> +#define PDC_DEBUG_SYNC_RESET 5
> +#define PDC_GPU_SYNC_RESET 6
> +#define PDC_DISPLAY_SYNC_RESET 7
> +#define PDC_COMPUTE_SYNC_RESET 8
> +#define PDC_MODEM_SYNC_RESET 9
> +
> +#endif
regards
Philipp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/4] reset: qcom: PDC (Power Domain Controller) reset controller
2018-07-27 15:28 ` [PATCH 2/4] reset: qcom: PDC (Power Domain Controller) reset controller Sibi Sankar
@ 2018-07-31 8:51 ` Philipp Zabel
2018-07-31 13:00 ` Sibi S
2018-08-21 22:17 ` Bjorn Andersson
1 sibling, 1 reply; 21+ messages in thread
From: Philipp Zabel @ 2018-07-31 8:51 UTC (permalink / raw)
To: Sibi Sankar, bjorn.andersson, robh+dt
Cc: linux-remoteproc, linux-kernel, devicetree, ohad, mark.rutland,
sricharan, akdwived, linux-arm-msm, tsoni
On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote:
> Add reset controller for SDM845 SoC to control reset signals
> provided by PDC for Modem, Compute, Display, GPU, Debug, AOP,
> Sensors, Audio, SP and APPS
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> drivers/reset/Kconfig | 9 +++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-qcom-pdc.c | 139 +++++++++++++++++++++++++++++++++
> 3 files changed, 149 insertions(+)
> create mode 100644 drivers/reset/reset-qcom-pdc.c
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 13d28fdbdbb5..5344e202a630 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -98,6 +98,15 @@ config RESET_QCOM_AOSS
> reset signals provided by AOSS for Modem, Venus, ADSP,
> GPU, Camera, Wireless, Display subsystem. Otherwise, say N.
>
> +config RESET_QCOM_PDC
> + bool "Qcom PDC Reset Driver"
> + depends on ARCH_QCOM || COMPILE_TEST
> + help
> + This enables the PDC (Power Domain Controller) reset driver
> + for Qualcomm Technologies Inc SDM845 SoCs. Say Y if you want
> + to control reset signals provided by PDC for Modem, Compute,
> + Display, GPU, Debug, AOP, Sensors, Audio, SP and APPS.
> +
> config RESET_SIMPLE
> bool "Simple Reset Controller Driver" if COMPILE_TEST
> default ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX || ARCH_ASPEED
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 4243c38228e2..d08e8b90046a 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_RESET_MESON_AUDIO_ARB) += reset-meson-audio-arb.o
> obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
> obj-$(CONFIG_RESET_QCOM_AOSS) += reset-qcom-aoss.o
> +obj-$(CONFIG_RESET_QCOM_PDC) += reset-qcom-pdc.o
> obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
> obj-$(CONFIG_RESET_STM32MP157) += reset-stm32mp1.o
> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> diff --git a/drivers/reset/reset-qcom-pdc.c b/drivers/reset/reset-qcom-pdc.c
> new file mode 100644
> index 000000000000..64a0041e3452
> --- /dev/null
> +++ b/drivers/reset/reset-qcom-pdc.c
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/regmap.h>
> +#include <linux/of_device.h>
> +#include <dt-bindings/reset/qcom,sdm845-pdc.h>
> +
> +struct qcom_pdc_reset_map {
> + u8 bit;
> +};
> +
> +struct qcom_pdc_desc {
> + const struct regmap_config *config;
> + const struct qcom_pdc_reset_map *resets;
> + size_t num_resets;
> +};
> +
> +struct qcom_pdc_reset_data {
> + struct reset_controller_dev rcdev;
> + struct regmap *regmap;
> + const struct qcom_pdc_desc *desc;
> +};
> +
> +static const struct regmap_config sdm845_pdc_regmap_config = {
> + .name = "pdc-reset",
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .max_register = 0x20000,
> + .fast_io = true,
> +};
> +
> +static const struct qcom_pdc_reset_map sdm845_pdc_resets[] = {
> + [PDC_APPS_SYNC_RESET] = {0},
> + [PDC_SP_SYNC_RESET] = {1},
> + [PDC_AUDIO_SYNC_RESET] = {2},
> + [PDC_SENSORS_SYNC_RESET] = {3},
> + [PDC_AOP_SYNC_RESET] = {4},
> + [PDC_DEBUG_SYNC_RESET] = {5},
> + [PDC_GPU_SYNC_RESET] = {6},
> + [PDC_DISPLAY_SYNC_RESET] = {7},
> + [PDC_COMPUTE_SYNC_RESET] = {8},
> + [PDC_MODEM_SYNC_RESET] = {9},
> +};
> +
> +static const struct qcom_pdc_desc sdm845_pdc_desc = {
> + .config = &sdm845_pdc_regmap_config,
> + .resets = sdm845_pdc_resets,
> + .num_resets = ARRAY_SIZE(sdm845_pdc_resets),
> +};
> +
> +static inline struct qcom_pdc_reset_data *to_qcom_pdc_reset_data(
> + struct reset_controller_dev *rcdev)
> +{
> + return container_of(rcdev, struct qcom_pdc_reset_data, rcdev);
> +}
> +
> +static int qcom_pdc_control_assert(struct reset_controller_dev *rcdev,
> + unsigned long idx)
> +{
> + struct qcom_pdc_reset_data *data = to_qcom_pdc_reset_data(rcdev);
> + const struct qcom_pdc_reset_map *map = &data->desc->resets[idx];
> +
> + return regmap_update_bits(data->regmap, 0x100,
Does this register have a name? If so, a #define would be preferable.
Otherwise this driver looks fine to me.
> + BIT(map->bit), BIT(map->bit));
> +}
> +
> +static int qcom_pdc_control_deassert(struct reset_controller_dev *rcdev,
> + unsigned long idx)
> +{
> + struct qcom_pdc_reset_data *data = to_qcom_pdc_reset_data(rcdev);
> + const struct qcom_pdc_reset_map *map = &data->desc->resets[idx];
> +
> + return regmap_update_bits(data->regmap, 0x100, BIT(map->bit), 0);
> +}
> +
> +static const struct reset_control_ops qcom_pdc_reset_ops = {
> + .assert = qcom_pdc_control_assert,
> + .deassert = qcom_pdc_control_deassert,
> +};
> +
> +static int qcom_pdc_reset_probe(struct platform_device *pdev)
> +{
> + struct qcom_pdc_reset_data *data;
> + struct device *dev = &pdev->dev;
> + const struct qcom_pdc_desc *desc;
> + void __iomem *base;
> + struct resource *res;
> +
> + desc = of_device_get_match_data(dev);
> + if (!desc)
> + return -EINVAL;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->desc = desc;
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + data->regmap = devm_regmap_init_mmio(dev, base, desc->config);
> + if (IS_ERR(data->regmap)) {
> + dev_err(dev, "Unable to get pdc-global regmap");
> + return PTR_ERR(data->regmap);
> + }
> +
> + data->rcdev.owner = THIS_MODULE;
> + data->rcdev.ops = &qcom_pdc_reset_ops;
> + data->rcdev.nr_resets = desc->num_resets;
> + data->rcdev.of_node = dev->of_node;
> +
> + return devm_reset_controller_register(dev, &data->rcdev);
> +}
> +
> +static const struct of_device_id qcom_pdc_reset_of_match[] = {
> + { .compatible = "qcom,sdm845-pdc-global", .data = &sdm845_pdc_desc },
> + {}
> +};
> +
> +static struct platform_driver qcom_pdc_reset_driver = {
> + .probe = qcom_pdc_reset_probe,
> + .driver = {
> + .name = "qcom_pdc_reset",
> + .of_match_table = qcom_pdc_reset_of_match,
> + },
> +};
> +
> +builtin_platform_driver(qcom_pdc_reset_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm PDC Reset Driver");
> +MODULE_LICENSE("GPL v2");
regards
Philipp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] remoteproc: qcom: q6v5-pil: Add PDC restart for modem on SDM845 SoCs
2018-07-27 15:28 ` [PATCH 4/4] remoteproc: qcom: q6v5-pil: Add PDC restart for modem on SDM845 SoCs Sibi Sankar
@ 2018-07-31 8:54 ` Philipp Zabel
2018-07-31 13:13 ` Sibi S
2018-08-07 18:18 ` Rob Herring
2018-08-21 22:33 ` Bjorn Andersson
2 siblings, 1 reply; 21+ messages in thread
From: Philipp Zabel @ 2018-07-31 8:54 UTC (permalink / raw)
To: Sibi Sankar, bjorn.andersson, robh+dt
Cc: linux-remoteproc, linux-kernel, devicetree, ohad, mark.rutland,
sricharan, akdwived, linux-arm-msm, tsoni
On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote:
> In the presence of a PDC block working with subsystem RSC,
> assert/deassert PDC restart in modem start/stop path.
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> .../bindings/remoteproc/qcom,q6v5.txt | 4 +++
> drivers/remoteproc/qcom_q6v5_pil.c | 27 ++++++++++++++++---
> 2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> index 601dd9f389aa..124fb1dc6fb8 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
[...]
> @@ -1182,6 +1192,15 @@ static int q6v5_init_reset(struct q6v5 *qproc)
> return PTR_ERR(qproc->mss_restart);
> }
>
> + if (qproc->has_alt_reset) {
> + qproc->pdc_restart = devm_reset_control_get(qproc->dev,
> + "pdc_restart");
Please use devm_reset_control_get_exclusive() instead.
> + if (IS_ERR(qproc->pdc_restart)) {
> + dev_err(qproc->dev, "failed to acquire pdc restart\n");
> + return PTR_ERR(qproc->pdc_restart);
> + }
> + }
> +
> return 0;
> }
regards
Philipp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] remoteproc: qcom: q6v5-pil: Explicitly get mss_restart line
2018-07-27 15:28 ` [PATCH 3/4] remoteproc: qcom: q6v5-pil: Explicitly get mss_restart line Sibi Sankar
@ 2018-07-31 8:57 ` Philipp Zabel
2018-07-31 13:11 ` Sibi S
0 siblings, 1 reply; 21+ messages in thread
From: Philipp Zabel @ 2018-07-31 8:57 UTC (permalink / raw)
To: Sibi Sankar, bjorn.andersson, robh+dt
Cc: linux-remoteproc, linux-kernel, devicetree, ohad, mark.rutland,
sricharan, akdwived, linux-arm-msm, tsoni
On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote:
> Explicitly get mss_restart to facilitate adding PDC
> restart line for modem on SDM845 SoCs
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> drivers/remoteproc/qcom_q6v5_pil.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
> index b1296d614b8b..d57fdb34e3dd 100644
> --- a/drivers/remoteproc/qcom_q6v5_pil.c
> +++ b/drivers/remoteproc/qcom_q6v5_pil.c
> @@ -1176,8 +1176,7 @@ static int q6v5_init_clocks(struct device *dev, struct clk **clks,
>
> static int q6v5_init_reset(struct q6v5 *qproc)
> {
> - qproc->mss_restart = devm_reset_control_get_exclusive(qproc->dev,
> - NULL);
> + qproc->mss_restart = devm_reset_control_get(qproc->dev, "mss_restart");
Please keep using devm_reset_control_get_exclusive.
regards
Philipp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for SDM845 SoCs
2018-07-31 8:42 ` [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for " Philipp Zabel
@ 2018-07-31 12:57 ` Sibi S
2018-08-07 18:16 ` Rob Herring
0 siblings, 1 reply; 21+ messages in thread
From: Sibi S @ 2018-07-31 12:57 UTC (permalink / raw)
To: Philipp Zabel, bjorn.andersson, robh+dt
Cc: linux-remoteproc, linux-kernel, devicetree, ohad, mark.rutland,
sricharan, akdwived, linux-arm-msm, tsoni, ilina
Hi Philipp,
Thanks for the review!
On 07/31/2018 02:12 PM, Philipp Zabel wrote:
> Hi Sibi,
>
> On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote:
>> Add SDM845 PDC (Power Domain Controller) reset controller binding
>>
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>> .../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++
>> include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++
>> 2 files changed, 72 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
>> create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h
>>
>> diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
>> new file mode 100644
>> index 000000000000..85e159962e08
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
>> @@ -0,0 +1,52 @@
>> +PDC Reset Controller
>> +======================================
>> +
>> +This binding describes a reset-controller found on PDC-Global(Power Domain
>> +Controller) block for Qualcomm Technologies Inc SDM845 SoCs.
>> +
>> +Required properties:
>> +- compatible:
>> + Usage: required
>> + Value type: <string>
>> + Definition: must be:
>> + "qcom,sdm845-pdc-global"
>> +
>> +- reg:
>> + Usage: required
>> + Value type: <prop-encoded-array>
>> + Definition: must specify the base address and size of the register
>> + space.
>> +
>> +- #reset-cells:
>> + Usage: required
>> + Value type: <uint>
>> + Definition: must be 1; cell entry represents the reset index.
>> +
>> +Example:
>> +
>> +pdc_reset: reset-controller@b2e0000 {
>
> Is this really just a reset controller?
>
> The name makes it sound like a driver binding to this should also
> provide pm_genpd and the binding should probably call this a power-
> controller: Documentation/devicetree/bindings/power/power_domain.txt.
>
The PDC-global reg space which is a part of PDC-wrapper reg space seems
to be only used for the reset lines.
Couple of other drivers use other parts of the PDC-wrapper reg space:
https://patchwork.kernel.org/patch/10223701/ (PDC-Interrupt controller)
https://patchwork.kernel.org/patch/10255767/ (GMU-PDC incorrectly tries
to occupy the entire pdc-wrapper reg space)
since it couldn't be logically mapped into pdc-interrupt driver, it had
to be included as a separate reset driver.
>> + compatible = "qcom,sdm845-pdc-global";
>> + reg = <0xb2e0000 0x20000>;
>
> This looks like this is the register space of the complete PDC, not just
> the reset register?
>
The entire register space was chosen because it is only used for its
reset lines (had a good look at the downstream kernel and had a
conversation with Lina) and to ensure break backward compatibility for
the for the dt entry if the reg-space was used for other purposes in
the future.
>> + #reset-cells = <1>;
>> +};
>> +
>> +PDC reset clients
>> +======================================
>> +
>> +Device nodes that need access to reset lines should
>> +specify them as a reset phandle in their corresponding node as
>> +specified in reset.txt.
>> +
>> +For list of all valid reset indicies see
>> +<dt-bindings/reset/qcom,sdm845-pdc.h>
>> +
>> +Example:
>> +
>> +modem-pil@4080000 {
>> + ...
>> +
>> + resets = <&pdc_reset PDC_MODEM_SYNC_RESET>;
>> + reset-names = "pdc_restart";
>> +
>> + ...
>> +};
>> diff --git a/include/dt-bindings/reset/qcom,sdm845-pdc.h b/include/dt-bindings/reset/qcom,sdm845-pdc.h
>> new file mode 100644
>> index 000000000000..53c37f9c319a
>> --- /dev/null
>> +++ b/include/dt-bindings/reset/qcom,sdm845-pdc.h
>> @@ -0,0 +1,20 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (C) 2018 The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#ifndef _DT_BINDINGS_RESET_PDC_SDM_845_H
>> +#define _DT_BINDINGS_RESET_PDC_SDM_845_H
>> +
>> +#define PDC_APPS_SYNC_RESET 0
>> +#define PDC_SP_SYNC_RESET 1
>> +#define PDC_AUDIO_SYNC_RESET 2
>> +#define PDC_SENSORS_SYNC_RESET 3
>> +#define PDC_AOP_SYNC_RESET 4
>> +#define PDC_DEBUG_SYNC_RESET 5
>> +#define PDC_GPU_SYNC_RESET 6
>> +#define PDC_DISPLAY_SYNC_RESET 7
>> +#define PDC_COMPUTE_SYNC_RESET 8
>> +#define PDC_MODEM_SYNC_RESET 9
>> +
>> +#endif
>
> regards
> Philipp
>
--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/4] reset: qcom: PDC (Power Domain Controller) reset controller
2018-07-31 8:51 ` Philipp Zabel
@ 2018-07-31 13:00 ` Sibi S
0 siblings, 0 replies; 21+ messages in thread
From: Sibi S @ 2018-07-31 13:00 UTC (permalink / raw)
To: Philipp Zabel, bjorn.andersson, robh+dt
Cc: linux-remoteproc, linux-kernel, devicetree, ohad, mark.rutland,
sricharan, akdwived, linux-arm-msm, tsoni
Hi Philipp,
Thanks for the review!
On 07/31/2018 02:21 PM, Philipp Zabel wrote:
> On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote:
>> Add reset controller for SDM845 SoC to control reset signals
>> provided by PDC for Modem, Compute, Display, GPU, Debug, AOP,
>> Sensors, Audio, SP and APPS
>>
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>> drivers/reset/Kconfig | 9 +++
>> drivers/reset/Makefile | 1 +
>> drivers/reset/reset-qcom-pdc.c | 139 +++++++++++++++++++++++++++++++++
>> 3 files changed, 149 insertions(+)
>> create mode 100644 drivers/reset/reset-qcom-pdc.c
>>
>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>> index 13d28fdbdbb5..5344e202a630 100644
>> --- a/drivers/reset/Kconfig
>> +++ b/drivers/reset/Kconfig
>> @@ -98,6 +98,15 @@ config RESET_QCOM_AOSS
>> reset signals provided by AOSS for Modem, Venus, ADSP,
>> GPU, Camera, Wireless, Display subsystem. Otherwise, say N.
>>
>> +config RESET_QCOM_PDC
>> + bool "Qcom PDC Reset Driver"
>> + depends on ARCH_QCOM || COMPILE_TEST
>> + help
>> + This enables the PDC (Power Domain Controller) reset driver
>> + for Qualcomm Technologies Inc SDM845 SoCs. Say Y if you want
>> + to control reset signals provided by PDC for Modem, Compute,
>> + Display, GPU, Debug, AOP, Sensors, Audio, SP and APPS.
>> +
>> config RESET_SIMPLE
>> bool "Simple Reset Controller Driver" if COMPILE_TEST
>> default ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX || ARCH_ASPEED
>> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
>> index 4243c38228e2..d08e8b90046a 100644
>> --- a/drivers/reset/Makefile
>> +++ b/drivers/reset/Makefile
>> @@ -16,6 +16,7 @@ obj-$(CONFIG_RESET_MESON_AUDIO_ARB) += reset-meson-audio-arb.o
>> obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
>> obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
>> obj-$(CONFIG_RESET_QCOM_AOSS) += reset-qcom-aoss.o
>> +obj-$(CONFIG_RESET_QCOM_PDC) += reset-qcom-pdc.o
>> obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
>> obj-$(CONFIG_RESET_STM32MP157) += reset-stm32mp1.o
>> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
>> diff --git a/drivers/reset/reset-qcom-pdc.c b/drivers/reset/reset-qcom-pdc.c
>> new file mode 100644
>> index 000000000000..64a0041e3452
>> --- /dev/null
>> +++ b/drivers/reset/reset-qcom-pdc.c
>> @@ -0,0 +1,139 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset-controller.h>
>> +#include <linux/regmap.h>
>> +#include <linux/of_device.h>
>> +#include <dt-bindings/reset/qcom,sdm845-pdc.h>
>> +
>> +struct qcom_pdc_reset_map {
>> + u8 bit;
>> +};
>> +
>> +struct qcom_pdc_desc {
>> + const struct regmap_config *config;
>> + const struct qcom_pdc_reset_map *resets;
>> + size_t num_resets;
>> +};
>> +
>> +struct qcom_pdc_reset_data {
>> + struct reset_controller_dev rcdev;
>> + struct regmap *regmap;
>> + const struct qcom_pdc_desc *desc;
>> +};
>> +
>> +static const struct regmap_config sdm845_pdc_regmap_config = {
>> + .name = "pdc-reset",
>> + .reg_bits = 32,
>> + .reg_stride = 4,
>> + .val_bits = 32,
>> + .max_register = 0x20000,
>> + .fast_io = true,
>> +};
>> +
>> +static const struct qcom_pdc_reset_map sdm845_pdc_resets[] = {
>> + [PDC_APPS_SYNC_RESET] = {0},
>> + [PDC_SP_SYNC_RESET] = {1},
>> + [PDC_AUDIO_SYNC_RESET] = {2},
>> + [PDC_SENSORS_SYNC_RESET] = {3},
>> + [PDC_AOP_SYNC_RESET] = {4},
>> + [PDC_DEBUG_SYNC_RESET] = {5},
>> + [PDC_GPU_SYNC_RESET] = {6},
>> + [PDC_DISPLAY_SYNC_RESET] = {7},
>> + [PDC_COMPUTE_SYNC_RESET] = {8},
>> + [PDC_MODEM_SYNC_RESET] = {9},
>> +};
>> +
>> +static const struct qcom_pdc_desc sdm845_pdc_desc = {
>> + .config = &sdm845_pdc_regmap_config,
>> + .resets = sdm845_pdc_resets,
>> + .num_resets = ARRAY_SIZE(sdm845_pdc_resets),
>> +};
>> +
>> +static inline struct qcom_pdc_reset_data *to_qcom_pdc_reset_data(
>> + struct reset_controller_dev *rcdev)
>> +{
>> + return container_of(rcdev, struct qcom_pdc_reset_data, rcdev);
>> +}
>> +
>> +static int qcom_pdc_control_assert(struct reset_controller_dev *rcdev,
>> + unsigned long idx)
>> +{
>> + struct qcom_pdc_reset_data *data = to_qcom_pdc_reset_data(rcdev);
>> + const struct qcom_pdc_reset_map *map = &data->desc->resets[idx];
>> +
>> + return regmap_update_bits(data->regmap, 0x100,
>
> Does this register have a name? If so, a #define would be preferable.
> Otherwise this driver looks fine to me.
>
Yes will had a separate #define for it.
>> + BIT(map->bit), BIT(map->bit));
>> +}
>> +
>> +static int qcom_pdc_control_deassert(struct reset_controller_dev *rcdev,
>> + unsigned long idx)
>> +{
>> + struct qcom_pdc_reset_data *data = to_qcom_pdc_reset_data(rcdev);
>> + const struct qcom_pdc_reset_map *map = &data->desc->resets[idx];
>> +
>> + return regmap_update_bits(data->regmap, 0x100, BIT(map->bit), 0);
>> +}
>> +
>> +static const struct reset_control_ops qcom_pdc_reset_ops = {
>> + .assert = qcom_pdc_control_assert,
>> + .deassert = qcom_pdc_control_deassert,
>> +};
>> +
>> +static int qcom_pdc_reset_probe(struct platform_device *pdev)
>> +{
>> + struct qcom_pdc_reset_data *data;
>> + struct device *dev = &pdev->dev;
>> + const struct qcom_pdc_desc *desc;
>> + void __iomem *base;
>> + struct resource *res;
>> +
>> + desc = of_device_get_match_data(dev);
>> + if (!desc)
>> + return -EINVAL;
>> +
>> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + data->desc = desc;
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(base))
>> + return PTR_ERR(base);
>> +
>> + data->regmap = devm_regmap_init_mmio(dev, base, desc->config);
>> + if (IS_ERR(data->regmap)) {
>> + dev_err(dev, "Unable to get pdc-global regmap");
>> + return PTR_ERR(data->regmap);
>> + }
>> +
>> + data->rcdev.owner = THIS_MODULE;
>> + data->rcdev.ops = &qcom_pdc_reset_ops;
>> + data->rcdev.nr_resets = desc->num_resets;
>> + data->rcdev.of_node = dev->of_node;
>> +
>> + return devm_reset_controller_register(dev, &data->rcdev);
>> +}
>> +
>> +static const struct of_device_id qcom_pdc_reset_of_match[] = {
>> + { .compatible = "qcom,sdm845-pdc-global", .data = &sdm845_pdc_desc },
>> + {}
>> +};
>> +
>> +static struct platform_driver qcom_pdc_reset_driver = {
>> + .probe = qcom_pdc_reset_probe,
>> + .driver = {
>> + .name = "qcom_pdc_reset",
>> + .of_match_table = qcom_pdc_reset_of_match,
>> + },
>> +};
>> +
>> +builtin_platform_driver(qcom_pdc_reset_driver);
>> +
>> +MODULE_DESCRIPTION("Qualcomm PDC Reset Driver");
>> +MODULE_LICENSE("GPL v2");
>
> regards
> Philipp
>
--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] remoteproc: qcom: q6v5-pil: Explicitly get mss_restart line
2018-07-31 8:57 ` Philipp Zabel
@ 2018-07-31 13:11 ` Sibi S
0 siblings, 0 replies; 21+ messages in thread
From: Sibi S @ 2018-07-31 13:11 UTC (permalink / raw)
To: Philipp Zabel, bjorn.andersson, robh+dt
Cc: linux-remoteproc, linux-kernel, devicetree, ohad, mark.rutland,
sricharan, akdwived, linux-arm-msm, tsoni
Hi Philipp,
Thanks for the review!
On 07/31/2018 02:27 PM, Philipp Zabel wrote:
> On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote:
>> Explicitly get mss_restart to facilitate adding PDC
>> restart line for modem on SDM845 SoCs
>>
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>> drivers/remoteproc/qcom_q6v5_pil.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
>> index b1296d614b8b..d57fdb34e3dd 100644
>> --- a/drivers/remoteproc/qcom_q6v5_pil.c
>> +++ b/drivers/remoteproc/qcom_q6v5_pil.c
>> @@ -1176,8 +1176,7 @@ static int q6v5_init_clocks(struct device *dev, struct clk **clks,
>>
>> static int q6v5_init_reset(struct q6v5 *qproc)
>> {
>> - qproc->mss_restart = devm_reset_control_get_exclusive(qproc->dev,
>> - NULL);
>> + qproc->mss_restart = devm_reset_control_get(qproc->dev, "mss_restart");
>
> Please keep using devm_reset_control_get_exclusive.
>
got misled when I saw it being used in a recent driver :), will revert
it back.
> regards
> Philipp
>
--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] remoteproc: qcom: q6v5-pil: Add PDC restart for modem on SDM845 SoCs
2018-07-31 8:54 ` Philipp Zabel
@ 2018-07-31 13:13 ` Sibi S
0 siblings, 0 replies; 21+ messages in thread
From: Sibi S @ 2018-07-31 13:13 UTC (permalink / raw)
To: Philipp Zabel, bjorn.andersson, robh+dt
Cc: linux-remoteproc, linux-kernel, devicetree, ohad, mark.rutland,
sricharan, akdwived, linux-arm-msm, tsoni
Hi Philipp,
Thanks for the review!
On 07/31/2018 02:24 PM, Philipp Zabel wrote:
> On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote:
>> In the presence of a PDC block working with subsystem RSC,
>> assert/deassert PDC restart in modem start/stop path.
>>
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>> .../bindings/remoteproc/qcom,q6v5.txt | 4 +++
>> drivers/remoteproc/qcom_q6v5_pil.c | 27 ++++++++++++++++---
>> 2 files changed, 27 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
>> index 601dd9f389aa..124fb1dc6fb8 100644
>> --- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> [...]
>> @@ -1182,6 +1192,15 @@ static int q6v5_init_reset(struct q6v5 *qproc)
>> return PTR_ERR(qproc->mss_restart);
>> }
>>
>> + if (qproc->has_alt_reset) {
>> + qproc->pdc_restart = devm_reset_control_get(qproc->dev,
>> + "pdc_restart");
>
> Please use devm_reset_control_get_exclusive() instead.
>
will replace it in the next re-spin.
>> + if (IS_ERR(qproc->pdc_restart)) {
>> + dev_err(qproc->dev, "failed to acquire pdc restart\n");
>> + return PTR_ERR(qproc->pdc_restart);
>> + }
>> + }
>> +
>> return 0;
>> }
>
> regards
> Philipp
>
--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for SDM845 SoCs
2018-07-31 12:57 ` Sibi S
@ 2018-08-07 18:16 ` Rob Herring
2018-08-08 15:44 ` Sibi Sankar
0 siblings, 1 reply; 21+ messages in thread
From: Rob Herring @ 2018-08-07 18:16 UTC (permalink / raw)
To: Sibi S
Cc: Philipp Zabel, bjorn.andersson, linux-remoteproc, linux-kernel,
devicetree, ohad, mark.rutland, sricharan, akdwived,
linux-arm-msm, tsoni, ilina
On Tue, Jul 31, 2018 at 06:27:24PM +0530, Sibi S wrote:
> Hi Philipp,
> Thanks for the review!
>
> On 07/31/2018 02:12 PM, Philipp Zabel wrote:
> > Hi Sibi,
> >
> > On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote:
> > > Add SDM845 PDC (Power Domain Controller) reset controller binding
> > >
> > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> > > ---
> > > .../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++
> > > include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++
> > > 2 files changed, 72 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
> > > create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h
> > >
> > > diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
> > > new file mode 100644
> > > index 000000000000..85e159962e08
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
> > > @@ -0,0 +1,52 @@
> > > +PDC Reset Controller
> > > +======================================
> > > +
> > > +This binding describes a reset-controller found on PDC-Global(Power Domain
> > > +Controller) block for Qualcomm Technologies Inc SDM845 SoCs.
> > > +
> > > +Required properties:
> > > +- compatible:
> > > + Usage: required
> > > + Value type: <string>
> > > + Definition: must be:
> > > + "qcom,sdm845-pdc-global"
> > > +
> > > +- reg:
> > > + Usage: required
> > > + Value type: <prop-encoded-array>
> > > + Definition: must specify the base address and size of the register
> > > + space.
> > > +
> > > +- #reset-cells:
> > > + Usage: required
> > > + Value type: <uint>
> > > + Definition: must be 1; cell entry represents the reset index.
> > > +
> > > +Example:
> > > +
> > > +pdc_reset: reset-controller@b2e0000 {
> >
> > Is this really just a reset controller?
> >
> > The name makes it sound like a driver binding to this should also
> > provide pm_genpd and the binding should probably call this a power-
> > controller: Documentation/devicetree/bindings/power/power_domain.txt.
> >
>
> The PDC-global reg space which is a part of PDC-wrapper reg space seems
> to be only used for the reset lines.
>
> Couple of other drivers use other parts of the PDC-wrapper reg space:
> https://patchwork.kernel.org/patch/10223701/ (PDC-Interrupt controller)
> https://patchwork.kernel.org/patch/10255767/ (GMU-PDC incorrectly tries
> to occupy the entire pdc-wrapper reg space)
>
> since it couldn't be logically mapped into pdc-interrupt driver, it had
> to be included as a separate reset driver.
You can't have overlapping regions in DT (well, you can because we have
to work-around existing DTs that do, but you shouldn't).
A single node can be multiple providers such as interrupt controller and
reset controller. It's an OS problem to split that into multiple
drivers.
> > > + compatible = "qcom,sdm845-pdc-global";
> > > + reg = <0xb2e0000 0x20000>;
> >
> > This looks like this is the register space of the complete PDC, not just
> > the reset register?
> >
>
> The entire register space was chosen because it is only used for its
> reset lines (had a good look at the downstream kernel and had a conversation
> with Lina) and to ensure break backward compatibility for
> the for the dt entry if the reg-space was used for other purposes in
> the future.
Why do you want to ensure breaking backwards compatibility?
Rob
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] remoteproc: qcom: q6v5-pil: Add PDC restart for modem on SDM845 SoCs
2018-07-27 15:28 ` [PATCH 4/4] remoteproc: qcom: q6v5-pil: Add PDC restart for modem on SDM845 SoCs Sibi Sankar
2018-07-31 8:54 ` Philipp Zabel
@ 2018-08-07 18:18 ` Rob Herring
2018-08-08 15:45 ` Sibi Sankar
2018-08-21 22:33 ` Bjorn Andersson
2 siblings, 1 reply; 21+ messages in thread
From: Rob Herring @ 2018-08-07 18:18 UTC (permalink / raw)
To: Sibi Sankar
Cc: bjorn.andersson, p.zabel, linux-remoteproc, linux-kernel,
devicetree, ohad, mark.rutland, sricharan, akdwived,
linux-arm-msm, tsoni
On Fri, Jul 27, 2018 at 08:58:11PM +0530, Sibi Sankar wrote:
> In the presence of a PDC block working with subsystem RSC,
> assert/deassert PDC restart in modem start/stop path.
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> .../bindings/remoteproc/qcom,q6v5.txt | 4 +++
Please split bindings to separate patch.
> drivers/remoteproc/qcom_q6v5_pil.c | 27 ++++++++++++++++---
> 2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> index 601dd9f389aa..124fb1dc6fb8 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> @@ -51,6 +51,8 @@ on the Qualcomm Hexagon core.
> Usage: required
> Value type: <phandle>
> Definition: reference to the reset-controller for the modem sub-system
> + reference to the list of 2 reset-controllers for the modem
> + sub-system on SDM845 SoCs
> reference to the list of 3 reset-controllers for the
> wcss sub-system
>
> @@ -58,6 +60,8 @@ on the Qualcomm Hexagon core.
> Usage: required
> Value type: <stringlist>
> Definition: must be "mss_restart" for the modem sub-system
> + Definition: must be "mss_restart", "pdc_restart" for the modem
> + sub-system on SDM845 SoCs
> Definition: must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
> for the wcss syb-system
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for SDM845 SoCs
2018-08-07 18:16 ` Rob Herring
@ 2018-08-08 15:44 ` Sibi Sankar
2018-08-08 21:37 ` Jordan Crouse
0 siblings, 1 reply; 21+ messages in thread
From: Sibi Sankar @ 2018-08-08 15:44 UTC (permalink / raw)
To: Rob Herring
Cc: Philipp Zabel, bjorn.andersson, linux-remoteproc, linux-kernel,
devicetree, ohad, mark.rutland, sricharan, akdwived,
linux-arm-msm, tsoni, ilina, jcrouse
Hi Rob,
Thanks for the review
On 08/07/2018 11:46 PM, Rob Herring wrote:
> On Tue, Jul 31, 2018 at 06:27:24PM +0530, Sibi S wrote:
>> Hi Philipp,
>> Thanks for the review!
>>
>> On 07/31/2018 02:12 PM, Philipp Zabel wrote:
>>> Hi Sibi,
>>>
>>> On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote:
>>>> Add SDM845 PDC (Power Domain Controller) reset controller binding
>>>>
>>>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>>>> ---
>>>> .../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++
>>>> include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++
>>>> 2 files changed, 72 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
>>>> create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
>>>> new file mode 100644
>>>> index 000000000000..85e159962e08
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
>>>> @@ -0,0 +1,52 @@
>>>> +PDC Reset Controller
>>>> +======================================
>>>> +
>>>> +This binding describes a reset-controller found on PDC-Global(Power Domain
>>>> +Controller) block for Qualcomm Technologies Inc SDM845 SoCs.
>>>> +
>>>> +Required properties:
>>>> +- compatible:
>>>> + Usage: required
>>>> + Value type: <string>
>>>> + Definition: must be:
>>>> + "qcom,sdm845-pdc-global"
>>>> +
>>>> +- reg:
>>>> + Usage: required
>>>> + Value type: <prop-encoded-array>
>>>> + Definition: must specify the base address and size of the register
>>>> + space.
>>>> +
>>>> +- #reset-cells:
>>>> + Usage: required
>>>> + Value type: <uint>
>>>> + Definition: must be 1; cell entry represents the reset index.
>>>> +
>>>> +Example:
>>>> +
>>>> +pdc_reset: reset-controller@b2e0000 {
>>>
>>> Is this really just a reset controller?
>>>
>>> The name makes it sound like a driver binding to this should also
>>> provide pm_genpd and the binding should probably call this a power-
>>> controller: Documentation/devicetree/bindings/power/power_domain.txt.
>>>
>>
>> The PDC-global reg space which is a part of PDC-wrapper reg space seems
>> to be only used for the reset lines.
>>
>> Couple of other drivers use other parts of the PDC-wrapper reg space:
>> https://patchwork.kernel.org/patch/10223701/ (PDC-Interrupt controller)
>> https://patchwork.kernel.org/patch/10255767/ (GMU-PDC incorrectly tries
>> to occupy the entire pdc-wrapper reg space)
>>
>> since it couldn't be logically mapped into pdc-interrupt driver, it had
>> to be included as a separate reset driver.
>
> You can't have overlapping regions in DT (well, you can because we have
> to work-around existing DTs that do, but you shouldn't).
>
> A single node can be multiple providers such as interrupt controller and
> reset controller. It's an OS problem to split that into multiple
> drivers.
>
There will be no overlaps. Jordan will be changing the dt binding of
gmu_pdc so that there is no overlap I guess. What I meant to say is that
pdc-global is a separate reg-space and currently has no other
functionality other than exposing the reset lines.
>>>> + compatible = "qcom,sdm845-pdc-global";
>>>> + reg = <0xb2e0000 0x20000>;
>>>
>>> This looks like this is the register space of the complete PDC, not just
>>> the reset register?
>>>
>>
>> The entire register space was chosen because it is only used for its
>> reset lines (had a good look at the downstream kernel and had a conversation
>> with Lina) and to ensure break backward compatibility for
>> the for the dt entry if the reg-space was used for other purposes in
>> the future.
>
> Why do you want to ensure breaking backwards compatibility?
>
Similar to the AOSS reset driver which had a unused clock part, this
driver also exposes a reg space of which only reset lines are used.
> Rob
>
--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] remoteproc: qcom: q6v5-pil: Add PDC restart for modem on SDM845 SoCs
2018-08-07 18:18 ` Rob Herring
@ 2018-08-08 15:45 ` Sibi Sankar
0 siblings, 0 replies; 21+ messages in thread
From: Sibi Sankar @ 2018-08-08 15:45 UTC (permalink / raw)
To: Rob Herring
Cc: bjorn.andersson, p.zabel, linux-remoteproc, linux-kernel,
devicetree, ohad, mark.rutland, sricharan, akdwived,
linux-arm-msm, tsoni
On 08/07/2018 11:48 PM, Rob Herring wrote:
> On Fri, Jul 27, 2018 at 08:58:11PM +0530, Sibi Sankar wrote:
>> In the presence of a PDC block working with subsystem RSC,
>> assert/deassert PDC restart in modem start/stop path.
>>
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>> .../bindings/remoteproc/qcom,q6v5.txt | 4 +++
>
> Please split bindings to separate patch.
>
Okay will split them.
>> drivers/remoteproc/qcom_q6v5_pil.c | 27 ++++++++++++++++---
>> 2 files changed, 27 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
>> index 601dd9f389aa..124fb1dc6fb8 100644
>> --- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
>> @@ -51,6 +51,8 @@ on the Qualcomm Hexagon core.
>> Usage: required
>> Value type: <phandle>
>> Definition: reference to the reset-controller for the modem sub-system
>> + reference to the list of 2 reset-controllers for the modem
>> + sub-system on SDM845 SoCs
>> reference to the list of 3 reset-controllers for the
>> wcss sub-system
>>
>> @@ -58,6 +60,8 @@ on the Qualcomm Hexagon core.
>> Usage: required
>> Value type: <stringlist>
>> Definition: must be "mss_restart" for the modem sub-system
>> + Definition: must be "mss_restart", "pdc_restart" for the modem
>> + sub-system on SDM845 SoCs
>> Definition: must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
>> for the wcss syb-system
>>
--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for SDM845 SoCs
2018-08-08 15:44 ` Sibi Sankar
@ 2018-08-08 21:37 ` Jordan Crouse
2018-08-08 22:48 ` Jordan Crouse
0 siblings, 1 reply; 21+ messages in thread
From: Jordan Crouse @ 2018-08-08 21:37 UTC (permalink / raw)
To: Sibi Sankar
Cc: Rob Herring, Philipp Zabel, bjorn.andersson, linux-remoteproc,
linux-kernel, devicetree, ohad, mark.rutland, sricharan,
akdwived, linux-arm-msm, tsoni, ilina
On Wed, Aug 08, 2018 at 09:14:05PM +0530, Sibi Sankar wrote:
> Hi Rob,
> Thanks for the review
>
> On 08/07/2018 11:46 PM, Rob Herring wrote:
> >On Tue, Jul 31, 2018 at 06:27:24PM +0530, Sibi S wrote:
> >>Hi Philipp,
> >>Thanks for the review!
> >>
> >>On 07/31/2018 02:12 PM, Philipp Zabel wrote:
> >>>Hi Sibi,
> >>>
> >>>On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote:
> >>>>Add SDM845 PDC (Power Domain Controller) reset controller binding
> >>>>
> >>>>Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> >>>>---
> >>>> .../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++
> >>>> include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++
> >>>> 2 files changed, 72 insertions(+)
> >>>> create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
> >>>> create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h
> >>>>
> >>>>diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
> >>>>new file mode 100644
> >>>>index 000000000000..85e159962e08
> >>>>--- /dev/null
> >>>>+++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
> >>>>@@ -0,0 +1,52 @@
> >>>>+PDC Reset Controller
> >>>>+======================================
> >>>>+
> >>>>+This binding describes a reset-controller found on PDC-Global(Power Domain
> >>>>+Controller) block for Qualcomm Technologies Inc SDM845 SoCs.
> >>>>+
> >>>>+Required properties:
> >>>>+- compatible:
> >>>>+ Usage: required
> >>>>+ Value type: <string>
> >>>>+ Definition: must be:
> >>>>+ "qcom,sdm845-pdc-global"
> >>>>+
> >>>>+- reg:
> >>>>+ Usage: required
> >>>>+ Value type: <prop-encoded-array>
> >>>>+ Definition: must specify the base address and size of the register
> >>>>+ space.
> >>>>+
> >>>>+- #reset-cells:
> >>>>+ Usage: required
> >>>>+ Value type: <uint>
> >>>>+ Definition: must be 1; cell entry represents the reset index.
> >>>>+
> >>>>+Example:
> >>>>+
> >>>>+pdc_reset: reset-controller@b2e0000 {
> >>>
> >>>Is this really just a reset controller?
> >>>
> >>>The name makes it sound like a driver binding to this should also
> >>>provide pm_genpd and the binding should probably call this a power-
> >>>controller: Documentation/devicetree/bindings/power/power_domain.txt.
> >>>
> >>
> >>The PDC-global reg space which is a part of PDC-wrapper reg space seems
> >>to be only used for the reset lines.
> >>
> >>Couple of other drivers use other parts of the PDC-wrapper reg space:
> >>https://patchwork.kernel.org/patch/10223701/ (PDC-Interrupt controller)
> >>https://patchwork.kernel.org/patch/10255767/ (GMU-PDC incorrectly tries
> >>to occupy the entire pdc-wrapper reg space)
> >>
> >>since it couldn't be logically mapped into pdc-interrupt driver, it had
> >>to be included as a separate reset driver.
> >
> >You can't have overlapping regions in DT (well, you can because we have
> >to work-around existing DTs that do, but you shouldn't).
> >
> >A single node can be multiple providers such as interrupt controller and
> >reset controller. It's an OS problem to split that into multiple
> >drivers.
>
> There will be no overlaps. Jordan will be changing the dt binding of
> gmu_pdc so that there is no overlap I guess. What I meant to say is that
> pdc-global is a separate reg-space and currently has no other
> functionality other than exposing the reset lines.
Correct - the updated GPU DT will be:
reg = <0x506a000 0x30000>,
<0xb280000 0x10000>,
<0xb480000, 0x10000>;
reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq";
Jordan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for SDM845 SoCs
2018-08-08 21:37 ` Jordan Crouse
@ 2018-08-08 22:48 ` Jordan Crouse
0 siblings, 0 replies; 21+ messages in thread
From: Jordan Crouse @ 2018-08-08 22:48 UTC (permalink / raw)
To: Sibi Sankar, Rob Herring, Philipp Zabel, bjorn.andersson,
linux-remoteproc, linux-kernel, devicetree, ohad, mark.rutland,
sricharan, akdwived, linux-arm-msm, tsoni, ilina
On Wed, Aug 08, 2018 at 03:37:24PM -0600, Jordan Crouse wrote:
> On Wed, Aug 08, 2018 at 09:14:05PM +0530, Sibi Sankar wrote:
> > Hi Rob,
> > Thanks for the review
> >
> > On 08/07/2018 11:46 PM, Rob Herring wrote:
> > >On Tue, Jul 31, 2018 at 06:27:24PM +0530, Sibi S wrote:
> > >>Hi Philipp,
> > >>Thanks for the review!
> > >>
> > >>On 07/31/2018 02:12 PM, Philipp Zabel wrote:
> > >>>Hi Sibi,
> > >>>
> > >>>On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote:
> > >>>>Add SDM845 PDC (Power Domain Controller) reset controller binding
> > >>>>
> > >>>>Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> > >>>>---
> > >>>> .../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++
> > >>>> include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++
> > >>>> 2 files changed, 72 insertions(+)
> > >>>> create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
> > >>>> create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h
> > >>>>
> > >>>>diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
> > >>>>new file mode 100644
> > >>>>index 000000000000..85e159962e08
> > >>>>--- /dev/null
> > >>>>+++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
> > >>>>@@ -0,0 +1,52 @@
> > >>>>+PDC Reset Controller
> > >>>>+======================================
> > >>>>+
> > >>>>+This binding describes a reset-controller found on PDC-Global(Power Domain
> > >>>>+Controller) block for Qualcomm Technologies Inc SDM845 SoCs.
> > >>>>+
> > >>>>+Required properties:
> > >>>>+- compatible:
> > >>>>+ Usage: required
> > >>>>+ Value type: <string>
> > >>>>+ Definition: must be:
> > >>>>+ "qcom,sdm845-pdc-global"
> > >>>>+
> > >>>>+- reg:
> > >>>>+ Usage: required
> > >>>>+ Value type: <prop-encoded-array>
> > >>>>+ Definition: must specify the base address and size of the register
> > >>>>+ space.
> > >>>>+
> > >>>>+- #reset-cells:
> > >>>>+ Usage: required
> > >>>>+ Value type: <uint>
> > >>>>+ Definition: must be 1; cell entry represents the reset index.
> > >>>>+
> > >>>>+Example:
> > >>>>+
> > >>>>+pdc_reset: reset-controller@b2e0000 {
> > >>>
> > >>>Is this really just a reset controller?
> > >>>
> > >>>The name makes it sound like a driver binding to this should also
> > >>>provide pm_genpd and the binding should probably call this a power-
> > >>>controller: Documentation/devicetree/bindings/power/power_domain.txt.
> > >>>
> > >>
> > >>The PDC-global reg space which is a part of PDC-wrapper reg space seems
> > >>to be only used for the reset lines.
> > >>
> > >>Couple of other drivers use other parts of the PDC-wrapper reg space:
> > >>https://patchwork.kernel.org/patch/10223701/ (PDC-Interrupt controller)
> > >>https://patchwork.kernel.org/patch/10255767/ (GMU-PDC incorrectly tries
> > >>to occupy the entire pdc-wrapper reg space)
> > >>
> > >>since it couldn't be logically mapped into pdc-interrupt driver, it had
> > >>to be included as a separate reset driver.
> > >
> > >You can't have overlapping regions in DT (well, you can because we have
> > >to work-around existing DTs that do, but you shouldn't).
> > >
> > >A single node can be multiple providers such as interrupt controller and
> > >reset controller. It's an OS problem to split that into multiple
> > >drivers.
> >
> > There will be no overlaps. Jordan will be changing the dt binding of
> > gmu_pdc so that there is no overlap I guess. What I meant to say is that
> > pdc-global is a separate reg-space and currently has no other
> > functionality other than exposing the reset lines.
>
> Correct - the updated GPU DT will be:
>
> reg = <0x506a000 0x30000>,
> <0xb280000 0x10000>,
> <0xb480000, 0x10000>;
> reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq";
>
> Jordan
Code change: https://patchwork.freedesktop.org/patch/243513/
DT change: https://patchwork.freedesktop.org/patch/243539/
Jordan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for SDM845 SoCs
2018-07-27 15:28 [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for SDM845 SoCs Sibi Sankar
` (3 preceding siblings ...)
2018-07-31 8:42 ` [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for " Philipp Zabel
@ 2018-08-21 22:08 ` Bjorn Andersson
4 siblings, 0 replies; 21+ messages in thread
From: Bjorn Andersson @ 2018-08-21 22:08 UTC (permalink / raw)
To: Sibi Sankar
Cc: p.zabel, robh+dt, linux-remoteproc, linux-kernel, devicetree,
ohad, mark.rutland, sricharan, akdwived, linux-arm-msm, tsoni
On Fri 27 Jul 08:28 PDT 2018, Sibi Sankar wrote:
> Add SDM845 PDC (Power Domain Controller) reset controller binding
>
Even though this is currently describing only a reset controller I think
this binding better be talking about the "PDC Global" hardware.
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> .../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++
> include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++
> 2 files changed, 72 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
> create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h
>
> diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
Rename this qcom,pdc-global to match the compatible, and hardware name.
> new file mode 100644
> index 000000000000..85e159962e08
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt
> @@ -0,0 +1,52 @@
> +PDC Reset Controller
PDC Global
> +======================================
> +
> +This binding describes a reset-controller found on PDC-Global(Power Domain
> +Controller) block for Qualcomm Technologies Inc SDM845 SoCs.
This looks good.
> +
> +Required properties:
> +- compatible:
> + Usage: required
> + Value type: <string>
> + Definition: must be:
> + "qcom,sdm845-pdc-global"
> +
> +- reg:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: must specify the base address and size of the register
> + space.
> +
> +- #reset-cells:
> + Usage: required
> + Value type: <uint>
> + Definition: must be 1; cell entry represents the reset index.
> +
> +Example:
> +
> +pdc_reset: reset-controller@b2e0000 {
This is perfectly fine, in its current form it is a reset-controller.
> + compatible = "qcom,sdm845-pdc-global";
> + reg = <0xb2e0000 0x20000>;
> + #reset-cells = <1>;
> +};
> +
Apart from this, the binding looks good!
Regards,
Bjorn
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/4] reset: qcom: PDC (Power Domain Controller) reset controller
2018-07-27 15:28 ` [PATCH 2/4] reset: qcom: PDC (Power Domain Controller) reset controller Sibi Sankar
2018-07-31 8:51 ` Philipp Zabel
@ 2018-08-21 22:17 ` Bjorn Andersson
1 sibling, 0 replies; 21+ messages in thread
From: Bjorn Andersson @ 2018-08-21 22:17 UTC (permalink / raw)
To: Sibi Sankar
Cc: p.zabel, robh+dt, linux-remoteproc, linux-kernel, devicetree,
ohad, mark.rutland, sricharan, akdwived, linux-arm-msm, tsoni
On Fri 27 Jul 08:28 PDT 2018, Sibi Sankar wrote:
> Add reset controller for SDM845 SoC to control reset signals
> provided by PDC for Modem, Compute, Display, GPU, Debug, AOP,
> Sensors, Audio, SP and APPS
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> drivers/reset/Kconfig | 9 +++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-qcom-pdc.c | 139 +++++++++++++++++++++++++++++++++
> 3 files changed, 149 insertions(+)
> create mode 100644 drivers/reset/reset-qcom-pdc.c
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 13d28fdbdbb5..5344e202a630 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -98,6 +98,15 @@ config RESET_QCOM_AOSS
> reset signals provided by AOSS for Modem, Venus, ADSP,
> GPU, Camera, Wireless, Display subsystem. Otherwise, say N.
>
> +config RESET_QCOM_PDC
> + bool "Qcom PDC Reset Driver"
"Qualcomm"
And I do not see a strong reason for this not being tristate, the
consumers we've talked about so far can all be modules.
> + depends on ARCH_QCOM || COMPILE_TEST
> + help
> + This enables the PDC (Power Domain Controller) reset driver
> + for Qualcomm Technologies Inc SDM845 SoCs. Say Y if you want
> + to control reset signals provided by PDC for Modem, Compute,
> + Display, GPU, Debug, AOP, Sensors, Audio, SP and APPS.
> +
Implementation looks fine.
Regards,
Bjorn
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] remoteproc: qcom: q6v5-pil: Add PDC restart for modem on SDM845 SoCs
2018-07-27 15:28 ` [PATCH 4/4] remoteproc: qcom: q6v5-pil: Add PDC restart for modem on SDM845 SoCs Sibi Sankar
2018-07-31 8:54 ` Philipp Zabel
2018-08-07 18:18 ` Rob Herring
@ 2018-08-21 22:33 ` Bjorn Andersson
2 siblings, 0 replies; 21+ messages in thread
From: Bjorn Andersson @ 2018-08-21 22:33 UTC (permalink / raw)
To: Sibi Sankar
Cc: p.zabel, robh+dt, linux-remoteproc, linux-kernel, devicetree,
ohad, mark.rutland, sricharan, akdwived, linux-arm-msm, tsoni
On Fri 27 Jul 08:28 PDT 2018, Sibi Sankar wrote:
> In the presence of a PDC block working with subsystem RSC,
> assert/deassert PDC restart in modem start/stop path.
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> .../bindings/remoteproc/qcom,q6v5.txt | 4 +++
> drivers/remoteproc/qcom_q6v5_pil.c | 27 ++++++++++++++++---
> 2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> index 601dd9f389aa..124fb1dc6fb8 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> @@ -51,6 +51,8 @@ on the Qualcomm Hexagon core.
> Usage: required
> Value type: <phandle>
> Definition: reference to the reset-controller for the modem sub-system
> + reference to the list of 2 reset-controllers for the modem
> + sub-system on SDM845 SoCs
> reference to the list of 3 reset-controllers for the
> wcss sub-system
>
> @@ -58,6 +60,8 @@ on the Qualcomm Hexagon core.
> Usage: required
> Value type: <stringlist>
> Definition: must be "mss_restart" for the modem sub-system
> + Definition: must be "mss_restart", "pdc_restart" for the modem
> + sub-system on SDM845 SoCs
> Definition: must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
> for the wcss syb-system
Seems like we got an additional "Definition:" added here as we added the
wcss. Please don't add another one.
The rest looks good.
Regards,
Bjorn
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2018-08-21 22:33 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-27 15:28 [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for SDM845 SoCs Sibi Sankar
2018-07-27 15:28 ` [PATCH 2/4] reset: qcom: PDC (Power Domain Controller) reset controller Sibi Sankar
2018-07-31 8:51 ` Philipp Zabel
2018-07-31 13:00 ` Sibi S
2018-08-21 22:17 ` Bjorn Andersson
2018-07-27 15:28 ` [PATCH 3/4] remoteproc: qcom: q6v5-pil: Explicitly get mss_restart line Sibi Sankar
2018-07-31 8:57 ` Philipp Zabel
2018-07-31 13:11 ` Sibi S
2018-07-27 15:28 ` [PATCH 4/4] remoteproc: qcom: q6v5-pil: Add PDC restart for modem on SDM845 SoCs Sibi Sankar
2018-07-31 8:54 ` Philipp Zabel
2018-07-31 13:13 ` Sibi S
2018-08-07 18:18 ` Rob Herring
2018-08-08 15:45 ` Sibi Sankar
2018-08-21 22:33 ` Bjorn Andersson
2018-07-31 8:42 ` [PATCH 1/4] dt-bindings: reset: Add PDC reset binding for " Philipp Zabel
2018-07-31 12:57 ` Sibi S
2018-08-07 18:16 ` Rob Herring
2018-08-08 15:44 ` Sibi Sankar
2018-08-08 21:37 ` Jordan Crouse
2018-08-08 22:48 ` Jordan Crouse
2018-08-21 22:08 ` Bjorn Andersson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).