* [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for SDM845 SoCs
2018-08-24 13:18 [PATCH v2 0/6] Add support for PDC Global on SDM845 SoCs Sibi Sankar
@ 2018-08-24 13:18 ` Sibi Sankar
2018-08-28 0:33 ` Bjorn Andersson
` (2 more replies)
2018-08-24 13:18 ` [PATCH v2 2/6] reset: qcom: PDC Global (Power Domain Controller) reset controller Sibi Sankar
` (4 subsequent siblings)
5 siblings, 3 replies; 19+ messages in thread
From: Sibi Sankar @ 2018-08-24 13:18 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 PDC Global(Power Domain Controller) binding for SDM845 SoCs.
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
.../bindings/reset/qcom,pdc-global.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-global.txt
create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h
diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-global.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-global.txt
new file mode 100644
index 000000000000..69f9edca9503
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/qcom,pdc-global.txt
@@ -0,0 +1,52 @@
+PDC Global
+======================================
+
+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_reset";
+
+ ...
+};
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] 19+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for SDM845 SoCs
2018-08-24 13:18 ` [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for " Sibi Sankar
@ 2018-08-28 0:33 ` Bjorn Andersson
2018-08-28 0:38 ` Matthias Kaehlcke
2018-08-29 0:43 ` Rob Herring
2 siblings, 0 replies; 19+ messages in thread
From: Bjorn Andersson @ 2018-08-28 0: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 24 Aug 06:18 PDT 2018, Sibi Sankar wrote:
> Add PDC Global(Power Domain Controller) binding for SDM845 SoCs.
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
> ---
> .../bindings/reset/qcom,pdc-global.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-global.txt
> create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h
>
> diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-global.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-global.txt
> new file mode 100644
> index 000000000000..69f9edca9503
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/qcom,pdc-global.txt
> @@ -0,0 +1,52 @@
> +PDC Global
> +======================================
> +
> +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_reset";
> +
> + ...
> +};
> 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 [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for SDM845 SoCs
2018-08-24 13:18 ` [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for " Sibi Sankar
2018-08-28 0:33 ` Bjorn Andersson
@ 2018-08-28 0:38 ` Matthias Kaehlcke
2018-08-28 6:08 ` sibis
2018-08-29 0:43 ` Rob Herring
2 siblings, 1 reply; 19+ messages in thread
From: Matthias Kaehlcke @ 2018-08-28 0:38 UTC (permalink / raw)
To: Sibi Sankar
Cc: bjorn.andersson, p.zabel, robh+dt, linux-remoteproc,
linux-kernel, devicetree, ohad, mark.rutland, sricharan,
akdwived, linux-arm-msm, tsoni
Hi Sibi,
On Fri, Aug 24, 2018 at 06:48:55PM +0530, Sibi Sankar wrote:
> Add PDC Global(Power Domain Controller) binding for SDM845 SoCs.
nit: missing blank before the opening parenthesis.
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> .../bindings/reset/qcom,pdc-global.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-global.txt
> create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h
>
> diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-global.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-global.txt
> new file mode 100644
> index 000000000000..69f9edca9503
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/qcom,pdc-global.txt
> @@ -0,0 +1,52 @@
> +PDC Global
> +======================================
> +
> +This binding describes a reset-controller found on PDC-Global(Power Domain
> +Controller) block for Qualcomm Technologies Inc SDM845 SoCs.
Are there other PDC reset controllers that aren't 'global'? Otherwise
I'd suggest to use 'pdc-reset' instead of 'pdc-global', which is more
specific and in line with the name of the driver added by this series.
Or something like 'pdc-reset-global/main' if there are other
controllers?
> +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
s/indicies/indices/ (or s/indicies/lines/ ?)
Cheers
Matthias
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for SDM845 SoCs
2018-08-28 0:38 ` Matthias Kaehlcke
@ 2018-08-28 6:08 ` sibis
2018-08-28 17:11 ` Matthias Kaehlcke
0 siblings, 1 reply; 19+ messages in thread
From: sibis @ 2018-08-28 6:08 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: bjorn.andersson, p.zabel, robh+dt, linux-remoteproc,
linux-kernel, devicetree, ohad, mark.rutland, sricharan,
akdwived, linux-arm-msm, tsoni
Hi Matthias,
Thanks for the review
On 2018-08-28 06:08, Matthias Kaehlcke wrote:
> Hi Sibi,
>
> On Fri, Aug 24, 2018 at 06:48:55PM +0530, Sibi Sankar wrote:
>> Add PDC Global(Power Domain Controller) binding for SDM845 SoCs.
>
> nit: missing blank before the opening parenthesis.
>
Will fix it
>>
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>> .../bindings/reset/qcom,pdc-global.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-global.txt
>> create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h
>>
>> diff --git
>> a/Documentation/devicetree/bindings/reset/qcom,pdc-global.txt
>> b/Documentation/devicetree/bindings/reset/qcom,pdc-global.txt
>> new file mode 100644
>> index 000000000000..69f9edca9503
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/reset/qcom,pdc-global.txt
>> @@ -0,0 +1,52 @@
>> +PDC Global
>> +======================================
>> +
>> +This binding describes a reset-controller found on PDC-Global(Power
>> Domain
>> +Controller) block for Qualcomm Technologies Inc SDM845 SoCs.
>
> Are there other PDC reset controllers that aren't 'global'? Otherwise
> I'd suggest to use 'pdc-reset' instead of 'pdc-global', which is more
> specific and in line with the name of the driver added by this series.
> Or something like 'pdc-reset-global/main' if there are other
> controllers?
>
These are the only reset lines found in the pdc-global register space.
But as
explained by Bjorn, wouldn't it be better to leave it as such since
pdc-global
best describes the hardware without being limited by the current
functionality
it is being used for?
>> +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
>
> s/indicies/indices/ (or s/indicies/lines/ ?)
>
> Cheers
>
> Matthias
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for SDM845 SoCs
2018-08-28 6:08 ` sibis
@ 2018-08-28 17:11 ` Matthias Kaehlcke
0 siblings, 0 replies; 19+ messages in thread
From: Matthias Kaehlcke @ 2018-08-28 17:11 UTC (permalink / raw)
To: sibis
Cc: bjorn.andersson, p.zabel, robh+dt, linux-remoteproc,
linux-kernel, devicetree, ohad, mark.rutland, sricharan,
akdwived, linux-arm-msm, tsoni
On Tue, Aug 28, 2018 at 11:38:26AM +0530, sibis@codeaurora.org wrote:
> Hi Matthias,
> Thanks for the review
>
> On 2018-08-28 06:08, Matthias Kaehlcke wrote:
> > Hi Sibi,
> >
> > On Fri, Aug 24, 2018 at 06:48:55PM +0530, Sibi Sankar wrote:
> > > Add PDC Global(Power Domain Controller) binding for SDM845 SoCs.
> >
> > nit: missing blank before the opening parenthesis.
> >
>
> Will fix it
>
> > >
> > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> > > ---
> > > .../bindings/reset/qcom,pdc-global.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-global.txt
> > > create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/reset/qcom,pdc-global.txt
> > > b/Documentation/devicetree/bindings/reset/qcom,pdc-global.txt
> > > new file mode 100644
> > > index 000000000000..69f9edca9503
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/reset/qcom,pdc-global.txt
> > > @@ -0,0 +1,52 @@
> > > +PDC Global
> > > +======================================
> > > +
> > > +This binding describes a reset-controller found on PDC-Global(Power
> > > Domain
> > > +Controller) block for Qualcomm Technologies Inc SDM845 SoCs.
> >
> > Are there other PDC reset controllers that aren't 'global'? Otherwise
> > I'd suggest to use 'pdc-reset' instead of 'pdc-global', which is more
> > specific and in line with the name of the driver added by this series.
> > Or something like 'pdc-reset-global/main' if there are other
> > controllers?
> >
>
> These are the only reset lines found in the pdc-global register space. But
> as
> explained by Bjorn, wouldn't it be better to leave it as such since
> pdc-global
> best describes the hardware without being limited by the current
> functionality
> it is being used for?
If I understand Bjorn's reply on
https://patchwork.kernel.org/patch/10575335/ correctly it is planned
to use the 'pdc-global' compatible string in the future in some sort
of MFD driver for the 'PDC Global' IP block, which then instantiates
other drivers like the reset controller. In this case I agree that
'pdc-global' seems a reasonable choice.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for SDM845 SoCs
2018-08-24 13:18 ` [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for " Sibi Sankar
2018-08-28 0:33 ` Bjorn Andersson
2018-08-28 0:38 ` Matthias Kaehlcke
@ 2018-08-29 0:43 ` Rob Herring
2 siblings, 0 replies; 19+ messages in thread
From: Rob Herring @ 2018-08-29 0:43 UTC (permalink / raw)
To: Sibi Sankar
Cc: bjorn.andersson, p.zabel, robh+dt, linux-remoteproc,
linux-kernel, devicetree, ohad, mark.rutland, sricharan,
akdwived, linux-arm-msm, tsoni, Sibi Sankar
On Fri, 24 Aug 2018 18:48:55 +0530, Sibi Sankar wrote:
> Add PDC Global(Power Domain Controller) binding for SDM845 SoCs.
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> .../bindings/reset/qcom,pdc-global.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-global.txt
> create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 2/6] reset: qcom: PDC Global (Power Domain Controller) reset controller
2018-08-24 13:18 [PATCH v2 0/6] Add support for PDC Global on SDM845 SoCs Sibi Sankar
2018-08-24 13:18 ` [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for " Sibi Sankar
@ 2018-08-24 13:18 ` Sibi Sankar
2018-08-28 0:22 ` Matthias Kaehlcke
2018-08-24 13:18 ` [PATCH v2 3/6] dt-bindings: remoteproc: Remove additional definition tag Sibi Sankar
` (3 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Sibi Sankar @ 2018-08-24 13:18 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 SoCs to control reset signals provided
by PDC Global 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 | 142 +++++++++++++++++++++++++++++++++
3 files changed, 152 insertions(+)
create mode 100644 drivers/reset/reset-qcom-pdc.c
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 13d28fdbdbb5..c21da9fe51ec 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
+ tristate "Qualcomm 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..bb6a5e5ee0f8
--- /dev/null
+++ b/drivers/reset/reset-qcom-pdc.c
@@ -0,0 +1,142 @@
+// 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>
+
+#define RPMH_PDC_SYNC_RESET 0x100
+
+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, RPMH_PDC_SYNC_RESET,
+ 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, RPMH_PDC_SYNC_RESET,
+ 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 },
+ {}
+};
+MODULE_DEVICE_TABLE(of, qcom_pdc_reset_of_match);
+
+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,
+ },
+};
+module_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] 19+ messages in thread
* Re: [PATCH v2 2/6] reset: qcom: PDC Global (Power Domain Controller) reset controller
2018-08-24 13:18 ` [PATCH v2 2/6] reset: qcom: PDC Global (Power Domain Controller) reset controller Sibi Sankar
@ 2018-08-28 0:22 ` Matthias Kaehlcke
2018-08-28 3:02 ` Bjorn Andersson
0 siblings, 1 reply; 19+ messages in thread
From: Matthias Kaehlcke @ 2018-08-28 0:22 UTC (permalink / raw)
To: Sibi Sankar
Cc: bjorn.andersson, p.zabel, robh+dt, linux-remoteproc,
linux-kernel, devicetree, ohad, mark.rutland, sricharan,
akdwived, linux-arm-msm, tsoni
Hi Sibi,
On Fri, Aug 24, 2018 at 06:48:56PM +0530, Sibi Sankar wrote:
> Add reset controller for SDM845 SoCs to control reset signals provided
> by PDC Global 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 | 142 +++++++++++++++++++++++++++++++++
> 3 files changed, 152 insertions(+)
> create mode 100644 drivers/reset/reset-qcom-pdc.c
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 13d28fdbdbb5..c21da9fe51ec 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
> + tristate "Qualcomm 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.
What exactly does APPS mean? The AP cores, the entire SoC, something
else?
> +
> 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..bb6a5e5ee0f8
> --- /dev/null
> +++ b/drivers/reset/reset-qcom-pdc.c
> @@ -0,0 +1,142 @@
> +// 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>
Headers should be sorted in alphabetical order.
> +
> +#define RPMH_PDC_SYNC_RESET 0x100
> +
> +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;
> +};
Not sure if this structure adds much value or just a layer of
indirection:
- .config is only accessed in _probe(), sdm845_pdc_regmap_config could
be used directly
- .resets is used in _(de)assert(), sdm845_pdc_resets could be used
directly
- .num_resets is only accessed in _probe(),
ARRAY_SIZE(sdm845_pdc_resets) could be used instead
It probably makes sense if it is planned to support reset controllers
of other SoCs with this driver.
> +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, RPMH_PDC_SYNC_RESET,
> + 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, RPMH_PDC_SYNC_RESET,
> + 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");
Add missing '\n'
Say 'pdc-reset' instead of 'pdc-global'? (see also my other comment
below).
> + 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 },
Should this be 'qcom,sdm845-pdc-reset' which is more specific than
'global' and in line with the name and purpose of the driver?
Obviously this would require to adjust the bindings doc too.
Cheers
Matthias
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/6] reset: qcom: PDC Global (Power Domain Controller) reset controller
2018-08-28 0:22 ` Matthias Kaehlcke
@ 2018-08-28 3:02 ` Bjorn Andersson
2018-08-28 7:11 ` Sibi Sankar
2018-08-28 16:56 ` Matthias Kaehlcke
0 siblings, 2 replies; 19+ messages in thread
From: Bjorn Andersson @ 2018-08-28 3:02 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Sibi Sankar, p.zabel, robh+dt, linux-remoteproc, linux-kernel,
devicetree, ohad, mark.rutland, sricharan, akdwived,
linux-arm-msm, tsoni
On Mon 27 Aug 17:22 PDT 2018, Matthias Kaehlcke wrote:
> On Fri, Aug 24, 2018 at 06:48:56PM +0530, Sibi Sankar wrote:
> > diff --git a/drivers/reset/reset-qcom-pdc.c b/drivers/reset/reset-qcom-pdc.c
[..]
> > +struct qcom_pdc_desc {
> > + const struct regmap_config *config;
> > + const struct qcom_pdc_reset_map *resets;
> > + size_t num_resets;
> > +};
>
> Not sure if this structure adds much value or just a layer of
> indirection:
>
> - .config is only accessed in _probe(), sdm845_pdc_regmap_config could
> be used directly
> - .resets is used in _(de)assert(), sdm845_pdc_resets could be used
> directly
> - .num_resets is only accessed in _probe(),
> ARRAY_SIZE(sdm845_pdc_resets) could be used instead
>
> It probably makes sense if it is planned to support reset controllers
> of other SoCs with this driver.
>
I like this suggestion, once we need the configurability we can add the
type for it. It also shows that only .resets would need to be referenced
by qcom_pdc_reset_data.
> > +struct qcom_pdc_reset_data {
> > + struct reset_controller_dev rcdev;
> > + struct regmap *regmap;
> > + const struct qcom_pdc_desc *desc;
> > +};
[..]
> > +static int qcom_pdc_reset_probe(struct platform_device *pdev)
> > +{
[..]
> > + data->regmap = devm_regmap_init_mmio(dev, base, desc->config);
> > + if (IS_ERR(data->regmap)) {
> > + dev_err(dev, "Unable to get pdc-global regmap");
>
> Add missing '\n'
>
> Say 'pdc-reset' instead of 'pdc-global'? (see also my other comment
> below).
>
This regmap is created out of the single anonymous "reg", so I think the
error should be reduced to "Unable to initialize regmap\n".
[..]
> > +static const struct of_device_id qcom_pdc_reset_of_match[] = {
> > + { .compatible = "qcom,sdm845-pdc-global", .data = &sdm845_pdc_desc },
>
> Should this be 'qcom,sdm845-pdc-reset' which is more specific than
> 'global' and in line with the name and purpose of the driver?
> Obviously this would require to adjust the bindings doc too.
>
No, the binding describes the hardware block named "PDC Global",
currently implemented as a reset controller. The reason for doing this
is so that we one day can expose additional interfaces in a backwards
compatible fashion.
Regards,
Bjorn
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/6] reset: qcom: PDC Global (Power Domain Controller) reset controller
2018-08-28 3:02 ` Bjorn Andersson
@ 2018-08-28 7:11 ` Sibi Sankar
2018-08-28 16:56 ` Matthias Kaehlcke
1 sibling, 0 replies; 19+ messages in thread
From: Sibi Sankar @ 2018-08-28 7:11 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Matthias Kaehlcke, p.zabel, robh+dt, linux-remoteproc,
linux-kernel, devicetree, ohad, mark.rutland, sricharan,
akdwived, linux-arm-msm, tsoni, linux-remoteproc-owner
Hi Bjorn/Matthias,
Thanks for the review, will fix them in the next-respin.
On 2018-08-28 08:32, Bjorn Andersson wrote:
> On Mon 27 Aug 17:22 PDT 2018, Matthias Kaehlcke wrote:
>> On Fri, Aug 24, 2018 at 06:48:56PM +0530, Sibi Sankar wrote:
>> > diff --git a/drivers/reset/reset-qcom-pdc.c b/drivers/reset/reset-qcom-pdc.c
> [..]
>> > +struct qcom_pdc_desc {
>> > + const struct regmap_config *config;
>> > + const struct qcom_pdc_reset_map *resets;
>> > + size_t num_resets;
>> > +};
>>
>> Not sure if this structure adds much value or just a layer of
>> indirection:
>>
>> - .config is only accessed in _probe(), sdm845_pdc_regmap_config could
>> be used directly
>> - .resets is used in _(de)assert(), sdm845_pdc_resets could be used
>> directly
>> - .num_resets is only accessed in _probe(),
>> ARRAY_SIZE(sdm845_pdc_resets) could be used instead
>>
>> It probably makes sense if it is planned to support reset controllers
>> of other SoCs with this driver.
>>
>
> I like this suggestion, once we need the configurability we can add the
> type for it. It also shows that only .resets would need to be
> referenced
> by qcom_pdc_reset_data.
>
Will change it accordingly
>> > +struct qcom_pdc_reset_data {
>> > + struct reset_controller_dev rcdev;
>> > + struct regmap *regmap;
>> > + const struct qcom_pdc_desc *desc;
>> > +};
> [..]
>> > +static int qcom_pdc_reset_probe(struct platform_device *pdev)
>> > +{
> [..]
>> > + data->regmap = devm_regmap_init_mmio(dev, base, desc->config);
>> > + if (IS_ERR(data->regmap)) {
>> > + dev_err(dev, "Unable to get pdc-global regmap");
>>
>> Add missing '\n'
>>
>> Say 'pdc-reset' instead of 'pdc-global'? (see also my other comment
>> below).
>>
>
> This regmap is created out of the single anonymous "reg", so I think
> the
> error should be reduced to "Unable to initialize regmap\n".
>
Sure will add it but aren't we trying to regmap the entire pdc-global
register space though?
> [..]
>> > +static const struct of_device_id qcom_pdc_reset_of_match[] = {
>> > + { .compatible = "qcom,sdm845-pdc-global", .data = &sdm845_pdc_desc },
>>
>> Should this be 'qcom,sdm845-pdc-reset' which is more specific than
>> 'global' and in line with the name and purpose of the driver?
>> Obviously this would require to adjust the bindings doc too.
>>
>
> No, the binding describes the hardware block named "PDC Global",
> currently implemented as a reset controller. The reason for doing this
> is so that we one day can expose additional interfaces in a backwards
> compatible fashion.
>
> Regards,
> Bjorn
--
-- Sibi Sankar --
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/6] reset: qcom: PDC Global (Power Domain Controller) reset controller
2018-08-28 3:02 ` Bjorn Andersson
2018-08-28 7:11 ` Sibi Sankar
@ 2018-08-28 16:56 ` Matthias Kaehlcke
1 sibling, 0 replies; 19+ messages in thread
From: Matthias Kaehlcke @ 2018-08-28 16:56 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Sibi Sankar, p.zabel, robh+dt, linux-remoteproc, linux-kernel,
devicetree, ohad, mark.rutland, sricharan, akdwived,
linux-arm-msm, tsoni
On Mon, Aug 27, 2018 at 08:02:53PM -0700, Bjorn Andersson wrote:
> On Mon 27 Aug 17:22 PDT 2018, Matthias Kaehlcke wrote:
> > On Fri, Aug 24, 2018 at 06:48:56PM +0530, Sibi Sankar wrote:
> > > diff --git a/drivers/reset/reset-qcom-pdc.c b/drivers/reset/reset-qcom-pdc.c
> [..]
> > > +struct qcom_pdc_desc {
> > > + const struct regmap_config *config;
> > > + const struct qcom_pdc_reset_map *resets;
> > > + size_t num_resets;
> > > +};
> >
> > Not sure if this structure adds much value or just a layer of
> > indirection:
> >
> > - .config is only accessed in _probe(), sdm845_pdc_regmap_config could
> > be used directly
> > - .resets is used in _(de)assert(), sdm845_pdc_resets could be used
> > directly
> > - .num_resets is only accessed in _probe(),
> > ARRAY_SIZE(sdm845_pdc_resets) could be used instead
> >
> > It probably makes sense if it is planned to support reset controllers
> > of other SoCs with this driver.
> >
>
> I like this suggestion, once we need the configurability we can add the
> type for it. It also shows that only .resets would need to be referenced
> by qcom_pdc_reset_data.
>
> > > +struct qcom_pdc_reset_data {
> > > + struct reset_controller_dev rcdev;
> > > + struct regmap *regmap;
> > > + const struct qcom_pdc_desc *desc;
> > > +};
> [..]
> > > +static int qcom_pdc_reset_probe(struct platform_device *pdev)
> > > +{
> [..]
> > > + data->regmap = devm_regmap_init_mmio(dev, base, desc->config);
> > > + if (IS_ERR(data->regmap)) {
> > > + dev_err(dev, "Unable to get pdc-global regmap");
> >
> > Add missing '\n'
> >
> > Say 'pdc-reset' instead of 'pdc-global'? (see also my other comment
> > below).
> >
>
> This regmap is created out of the single anonymous "reg", so I think the
> error should be reduced to "Unable to initialize regmap\n".
>
> [..]
> > > +static const struct of_device_id qcom_pdc_reset_of_match[] = {
> > > + { .compatible = "qcom,sdm845-pdc-global", .data = &sdm845_pdc_desc },
> >
> > Should this be 'qcom,sdm845-pdc-reset' which is more specific than
> > 'global' and in line with the name and purpose of the driver?
> > Obviously this would require to adjust the bindings doc too.
> >
>
> No, the binding describes the hardware block named "PDC Global",
> currently implemented as a reset controller. The reason for doing this
> is so that we one day can expose additional interfaces in a backwards
> compatible fashion.
Sounds reasonable, thanks for the clarification.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 3/6] dt-bindings: remoteproc: Remove additional definition tag
2018-08-24 13:18 [PATCH v2 0/6] Add support for PDC Global on SDM845 SoCs Sibi Sankar
2018-08-24 13:18 ` [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for " Sibi Sankar
2018-08-24 13:18 ` [PATCH v2 2/6] reset: qcom: PDC Global (Power Domain Controller) reset controller Sibi Sankar
@ 2018-08-24 13:18 ` Sibi Sankar
2018-08-28 0:44 ` Matthias Kaehlcke
2018-08-24 13:18 ` [PATCH v2 4/6] dt-bindings: remoteproc: Add PDC reset binding for Q6V5 PIL Sibi Sankar
` (2 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Sibi Sankar @ 2018-08-24 13:18 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
Remove the additional definition tag declared for WCSS sub-system
under reset-names.
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
index 601dd9f389aa..c45e4c131fa2 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
@@ -58,7 +58,7 @@ on the Qualcomm Hexagon core.
Usage: required
Value type: <stringlist>
Definition: must be "mss_restart" for the modem sub-system
- Definition: must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
+ must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
for the wcss syb-system
- cx-supply:
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/6] dt-bindings: remoteproc: Remove additional definition tag
2018-08-24 13:18 ` [PATCH v2 3/6] dt-bindings: remoteproc: Remove additional definition tag Sibi Sankar
@ 2018-08-28 0:44 ` Matthias Kaehlcke
2018-08-29 16:38 ` Sibi Sankar
0 siblings, 1 reply; 19+ messages in thread
From: Matthias Kaehlcke @ 2018-08-28 0:44 UTC (permalink / raw)
To: Sibi Sankar
Cc: bjorn.andersson, p.zabel, robh+dt, linux-remoteproc,
linux-kernel, devicetree, ohad, mark.rutland, sricharan,
akdwived, linux-arm-msm, tsoni
Hi Sibi,
Subject: dt-bindings: remoteproc: Remove additional definition tag
nit: the subject is a bit generic, you probably should at least add
'qcom: ' to the 'path'.
On Fri, Aug 24, 2018 at 06:48:57PM +0530, Sibi Sankar wrote:
> Remove the additional definition tag declared for WCSS sub-system
> under reset-names.
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> index 601dd9f389aa..c45e4c131fa2 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> @@ -58,7 +58,7 @@ on the Qualcomm Hexagon core.
> Usage: required
> Value type: <stringlist>
> Definition: must be "mss_restart" for the modem sub-system
> - Definition: must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
> + must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
> for the wcss syb-system
Since you are already at it you might also want to fix the typo in
'wcss syb-system'.
Other than the nits:
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/6] dt-bindings: remoteproc: Remove additional definition tag
2018-08-28 0:44 ` Matthias Kaehlcke
@ 2018-08-29 16:38 ` Sibi Sankar
0 siblings, 0 replies; 19+ messages in thread
From: Sibi Sankar @ 2018-08-29 16:38 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: bjorn.andersson, p.zabel, robh+dt, linux-remoteproc,
linux-kernel, devicetree, ohad, mark.rutland, sricharan,
akdwived, linux-arm-msm, tsoni
Hi Matthias,
Thanks for the review!
On 2018-08-28 06:14, Matthias Kaehlcke wrote:
> Hi Sibi,
>
> Subject: dt-bindings: remoteproc: Remove additional definition tag
>
> nit: the subject is a bit generic, you probably should at least add
> 'qcom: ' to the 'path'.
>
Will add
> On Fri, Aug 24, 2018 at 06:48:57PM +0530, Sibi Sankar wrote:
>> Remove the additional definition tag declared for WCSS sub-system
>> under reset-names.
>>
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>> Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
>> b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
>> index 601dd9f389aa..c45e4c131fa2 100644
>> --- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
>> @@ -58,7 +58,7 @@ on the Qualcomm Hexagon core.
>> Usage: required
>> Value type: <stringlist>
>> Definition: must be "mss_restart" for the modem sub-system
>> - Definition: must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
>> + must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
>> for the wcss syb-system
>
> Since you are already at it you might also want to fix the typo in
> 'wcss syb-system'.
>
nice catch, will correct it.
I missed your query in the other thread, APPS refers to the pdc_sync
reset line for the Application processor.
> Other than the nits:
>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
--
-- Sibi Sankar --
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/6] dt-bindings: remoteproc: Add PDC reset binding for Q6V5 PIL
2018-08-24 13:18 [PATCH v2 0/6] Add support for PDC Global on SDM845 SoCs Sibi Sankar
` (2 preceding siblings ...)
2018-08-24 13:18 ` [PATCH v2 3/6] dt-bindings: remoteproc: Remove additional definition tag Sibi Sankar
@ 2018-08-24 13:18 ` Sibi Sankar
2018-08-29 0:44 ` Rob Herring
2018-08-24 13:18 ` [PATCH v2 5/6] remoteproc: qcom: q6v5-pil: Explicitly get mss_restart line Sibi Sankar
2018-08-24 13:19 ` [PATCH v2 6/6] remoteproc: qcom: q6v5-pil: Add PDC reset for modem on SDM845 SoCs Sibi Sankar
5 siblings, 1 reply; 19+ messages in thread
From: Sibi Sankar @ 2018-08-24 13:18 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 additional pdc_reset binding required for Q6V5 Modem PIL on
SDM845 SoCs.
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
index c45e4c131fa2..4c5473aeb697 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
@@ -53,6 +53,8 @@ on the Qualcomm Hexagon core.
Definition: reference to the reset-controller for the modem sub-system
reference to the list of 3 reset-controllers for the
wcss sub-system
+ reference to the list of 2 reset-controllers for the modem
+ sub-system on SDM845 SoCs
- reset-names:
Usage: required
@@ -60,6 +62,8 @@ on the Qualcomm Hexagon core.
Definition: must be "mss_restart" for the modem sub-system
must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
for the wcss syb-system
+ must be "mss_restart", "pdc_reset" for the modem
+ sub-system on SDM845 SoCs
- cx-supply:
- mss-supply:
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/6] dt-bindings: remoteproc: Add PDC reset binding for Q6V5 PIL
2018-08-24 13:18 ` [PATCH v2 4/6] dt-bindings: remoteproc: Add PDC reset binding for Q6V5 PIL Sibi Sankar
@ 2018-08-29 0:44 ` Rob Herring
0 siblings, 0 replies; 19+ messages in thread
From: Rob Herring @ 2018-08-29 0:44 UTC (permalink / raw)
To: Sibi Sankar
Cc: bjorn.andersson, p.zabel, robh+dt, linux-remoteproc,
linux-kernel, devicetree, ohad, mark.rutland, sricharan,
akdwived, linux-arm-msm, tsoni, Sibi Sankar
On Fri, 24 Aug 2018 18:48:58 +0530, Sibi Sankar wrote:
> Add additional pdc_reset binding required for Q6V5 Modem PIL on
> SDM845 SoCs.
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt | 4 ++++
> 1 file changed, 4 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 5/6] remoteproc: qcom: q6v5-pil: Explicitly get mss_restart line
2018-08-24 13:18 [PATCH v2 0/6] Add support for PDC Global on SDM845 SoCs Sibi Sankar
` (3 preceding siblings ...)
2018-08-24 13:18 ` [PATCH v2 4/6] dt-bindings: remoteproc: Add PDC reset binding for Q6V5 PIL Sibi Sankar
@ 2018-08-24 13:18 ` Sibi Sankar
2018-08-24 13:19 ` [PATCH v2 6/6] remoteproc: qcom: q6v5-pil: Add PDC reset for modem on SDM845 SoCs Sibi Sankar
5 siblings, 0 replies; 19+ messages in thread
From: Sibi Sankar @ 2018-08-24 13:18 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 reset line
for modem on SDM845 SoCs
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
drivers/remoteproc/qcom_q6v5_pil.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
index b1296d614b8b..968413edf0c8 100644
--- a/drivers/remoteproc/qcom_q6v5_pil.c
+++ b/drivers/remoteproc/qcom_q6v5_pil.c
@@ -1177,7 +1177,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);
+ "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] 19+ messages in thread
* [PATCH v2 6/6] remoteproc: qcom: q6v5-pil: Add PDC reset for modem on SDM845 SoCs
2018-08-24 13:18 [PATCH v2 0/6] Add support for PDC Global on SDM845 SoCs Sibi Sankar
` (4 preceding siblings ...)
2018-08-24 13:18 ` [PATCH v2 5/6] remoteproc: qcom: q6v5-pil: Explicitly get mss_restart line Sibi Sankar
@ 2018-08-24 13:19 ` Sibi Sankar
5 siblings, 0 replies; 19+ messages in thread
From: Sibi Sankar @ 2018-08-24 13:19 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 reset in modem start/stop path.
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
drivers/remoteproc/qcom_q6v5_pil.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
index 968413edf0c8..835fa005fe3e 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_reset;
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_reset);
+ ret = reset_control_reset(qproc->mss_restart);
+ reset_control_deassert(qproc->pdc_reset);
+ } 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_reset);
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_reset);
} else {
ret = reset_control_deassert(qproc->mss_restart);
}
@@ -1183,6 +1193,15 @@ static int q6v5_init_reset(struct q6v5 *qproc)
return PTR_ERR(qproc->mss_restart);
}
+ if (qproc->has_alt_reset) {
+ qproc->pdc_reset = devm_reset_control_get_exclusive(qproc->dev,
+ "pdc_reset");
+ if (IS_ERR(qproc->pdc_reset)) {
+ dev_err(qproc->dev, "failed to acquire pdc reset\n");
+ return PTR_ERR(qproc->pdc_reset);
+ }
+ }
+
return 0;
}
@@ -1303,12 +1322,12 @@ static int q6v5_probe(struct platform_device *pdev)
}
qproc->active_reg_count = ret;
+ qproc->has_alt_reset = desc->has_alt_reset;
ret = q6v5_init_reset(qproc);
if (ret)
goto free_rproc;
qproc->version = desc->version;
- qproc->has_alt_reset = desc->has_alt_reset;
qproc->need_mem_protection = desc->need_mem_protection;
ret = qcom_q6v5_init(&qproc->q6v5, pdev, rproc, MPSS_CRASH_REASON_SMEM,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 19+ messages in thread