Add support for PDC Global on SDM845 SoCs
Subsystems (4)
v2 cover Add support for PDC Global on SDM845 SoCs
This patch series add support for PDC Global (Power Domain Controller) on SDM845 SoCs and adds pdc reset lines assert/deassert to remoteproc Q6v5 modem-pil. The first two patches adds PDC Global reset driver to control reset signals of Modem, Compute, Display, GPU, Debug, AOP, Sensors, Audio, SP and APPS. The last four patches (cleans up)/adds pdc reset lines to q6v5 bindings and asserts/deasserts in modem start/stop path.
v2:
Incorporated Philipp/Bjorn/Rob suggestions Renamed reset binding to pdc-global.txt replaced offset with #define of register name replaced with devm_reset_control_get_exclusive() Separted dt binding from the drivers
The last 4 remoteproc patches can be picked up after the pdc global reset driver lands on linux-next.
Sibi Sankar (6):
dt-bindings: reset: Add PDC Global binding for SDM845 SoCs
reset: qcom: PDC Global (Power Domain Controller) reset controller
dt-bindings: remoteproc: Remove additional definition tag
dt-bindings: remoteproc: Add PDC reset binding for Q6V5 PIL
remoteproc: qcom: q6v5-pil: Explicitly get mss_restart line
remoteproc: qcom: q6v5-pil: Add PDC reset for modem on SDM845 SoCs
.../bindings/remoteproc/qcom,q6v5.txt | 6 +- .../bindings/reset/qcom,pdc-global.txt | 52 +++++++ drivers/remoteproc/qcom_q6v5_pil.c | 31 +++- drivers/reset/Kconfig | 9 ++ drivers/reset/Makefile | 1 + drivers/reset/reset-qcom-pdc.c | 142 ++++++++++++++++++ include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++ 7 files changed, 254 insertions(+), 7 deletions(-) create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-global.txt create mode 100644 drivers/reset/reset-qcom-pdc.c create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v2 1/6 dt-bindings: reset: Add PDC Global binding for SDM845 SoCs
Add PDC Global(Power Domain Controller) binding for SDM845 SoCs.
--- .../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
Documentation/devicetree/bindings/reset/qcom,pdc-global.txt new file +52 -0
@@ -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"; + + ... +};
include/dt-bindings/reset/qcom,sdm845-pdc.h new file +20 -0
@@ -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
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for SDM845 SoCs
Quoted 3 lines · Sibi Sankar
Add PDC Global(Power Domain Controller) binding for SDM845 SoCs.
Signed-off-by Sibi Sankar <[email protected]>
Regards,
Bjorn
Quoted 95 lines
--- .../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.hDocumentation/devicetree/bindings/reset/qcom,pdc-global.txt new file +52
-0@@ -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"; + + ... +};include/dt-bindings/reset/qcom,sdm845-pdc.h new file +20
-0@@ -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 Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for SDM845 SoCs
In reply to Sibi SankarHi Sibi,
Quoted 1 line · Sibi Sankar
Add PDC Global(Power Domain Controller) binding for SDM845 SoCs.
nit: missing blank before the opening parenthesis.
Quoted 15 lines
Signed-off-by Sibi Sankar <[email protected]>--- .../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.hDocumentation/devicetree/bindings/reset/qcom,pdc-global.txt new file +0
-0
Documentation/devicetree/bindings/reset/qcom,pdc-global.txt line 5
Quoted 5 lines
Documentation/devicetree/bindings/reset/qcom,pdc-global.txt new file +5
-0@@ -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?
Quoted 24 lines
Documentation/devicetree/bindings/reset/qcom,pdc-global.txt new file +24
-0+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>;
Documentation/devicetree/bindings/reset/qcom,pdc-global.txt line 40
Quoted 10 lines
Documentation/devicetree/bindings/reset/qcom,pdc-global.txt new file +10
-0+}; + +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
Re: [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for SDM845 SoCs
Hi Matthias,
Thanks for the review
Quoted 7 lines · Matthias Kaehlcke
Hi Sibi,
Quoted 1 line · Sibi Sankar
Add PDC Global(Power Domain Controller) binding for SDM845 SoCs.
nit: missing blank before the opening parenthesis.
Will fix it
Quoted 21 lines
Quoted 21 lines
Signed-off-by Sibi Sankar <[email protected]>--- .../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.hDocumentation/devicetree/bindings/reset/qcom,pdc-global.txt new file +4
-0@@ -0,0 +1,52 @@ +PDC Global +====================================== + +This binding describes a reset-controller found on PDC-Global(Power
Quoted 9 lines
Quoted 2 lines
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?
Quoted 40 lines
Quoted 24 lines
Documentation/devicetree/bindings/reset/qcom,pdc-global.txt new file +24
-0+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>;Documentation/devicetree/bindings/reset/qcom,pdc-global.txt line 40
Quoted 10 lines
Documentation/devicetree/bindings/reset/qcom,pdc-global.txt new file +10
-0+}; + +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 sees/indicies/indices/ (or s/indicies/lines/ ?)
Cheers
Matthias
Re: [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for SDM845 SoCs
Quoted 47 lines · On Tue, Aug 28, 2018 at 11:38:26AM +0530, [email protected] wrote:
Hi Matthias,
Thanks for the reviewQuoted 7 lines · Matthias Kaehlcke
Hi Sibi,
Quoted 1 line · Sibi Sankar
Add PDC Global(Power Domain Controller) binding for SDM845 SoCs.
nit: missing blank before the opening parenthesis.
Will fix it
Quoted 23 lines
Quoted 23 lines
Signed-off-by Sibi Sankar <[email protected]>--- .../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.hdiff --git
a/Documentation/devicetree/bindings/reset/qcom,pdc-global.txtb/Documentation/devicetree/bindings/reset/qcom,pdc-global.txtnew file mode 100644
index 000000000000..69f9edca9503Documentation/devicetree/bindings/reset/qcom,pdc-global.txt new file +4
-0@@ -0,0 +1,52 @@ +PDC Global +====================================== + +This binding describes a reset-controller found on PDC-Global(PowerQuoted 9 lines
Quoted 2 lines
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?
Quoted 7 lines
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.
Re: [PATCH v2 1/6] dt-bindings: reset: Add PDC Global binding for SDM845 SoCs
In reply to Sibi SankarQuoted 10 lines · Sibi Sankar
Add PDC Global(Power Domain Controller) binding for SDM845 SoCs.
Signed-off-by Sibi Sankar <[email protected]>--- .../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
PATCH v2 2/6 reset: qcom: PDC Global (Power Domain Controller) reset controller
In reply to Sibi SankarAdd 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
--- 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
drivers/reset/Kconfig +9 -0
@@ -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
drivers/reset/Makefile +1 -0
@@ -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
drivers/reset/reset-qcom-pdc.c new file +142 -0
142 lines not on this page · raw message
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2 2/6] reset: qcom: PDC Global (Power Domain Controller) reset controller
Hi Sibi,
Quoted 18 lines · 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 <[email protected]>--- 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.cdrivers/reset/Kconfig +0
-0@@ -98,6 +98,15 @@ config RESET_QCOM_AOSS reset signals provided by AOSS for Modem, Venus, ADSP,
drivers/reset/Kconfig config RESET_QCOM_AOSS · line 108
Quoted 10 lines
drivers/reset/Kconfig +8
-0GPU, 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?
Quoted 23 lines
drivers/reset/Kconfig +1
-0+ config RESET_SIMPLE bool "Simple Reset Controller Driver" if COMPILE_TEST default ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX || ARCH_ASPEEDdrivers/reset/Makefile +1
-0@@ -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.odrivers/reset/reset-qcom-pdc.c new file +1
-0@@ -0,0 +1,142 @@ +// SPDX-License-Identifier: GPL-2.0
drivers/reset/reset-qcom-pdc.c line 11
Quoted 10 lines
drivers/reset/reset-qcom-pdc.c new file +10
-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.
drivers/reset/reset-qcom-pdc.c line 23
Quoted 12 lines
drivers/reset/reset-qcom-pdc.c new file +12
-0+ +#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.
Quoted 80 lines
drivers/reset/reset-qcom-pdc.c new file +80
-0+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 ;
drivers/reset/reset-qcom-pdc.c line 114
Quoted 10 lines
drivers/reset/reset-qcom-pdc.c new file +10
-0+ + 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).
drivers/reset/reset-qcom-pdc.c line 127
Quoted 13 lines
drivers/reset/reset-qcom-pdc.c new file +13
-0+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
Re: [PATCH v2 2/6] reset: qcom: PDC Global (Power Domain Controller) reset controller
Quoted 2 lines · Matthias Kaehlcke
Quoted 1 line · Sibi Sankar
drivers/reset/reset-qcom-pdc.c +0
-0
[..]
Quoted 16 lines
drivers/reset/reset-qcom-pdc.c line 23
Quoted 5 lines
drivers/reset/reset-qcom-pdc.c +5
-0+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_configcould be used directly
- .resets is used in _(de)assert(),sdm845_pdc_resetscould be used directly
- .num_resets is only accessed in_probe(),ARRAY_SIZE(sdm845_pdc_resets)could be used instead
Quoted 3 lines
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.
Quoted 5 lines
Quoted 5 lines
drivers/reset/reset-qcom-pdc.c +5
-0+struct qcom_pdc_reset_data { +struct reset_controller_dev rcdev; +struct regmap *regmap; +const struct qcom_pdc_desc *desc; +};
[..]
Quoted 2 lines
Quoted 2 lines
drivers/reset/reset-qcom-pdc.c +2
-0+static int qcom_pdc_reset_probe (struct platform_device *pdev) +{
[..]
Quoted 9 lines
drivers/reset/reset-qcom-pdc.c line 114
Quoted 3 lines
drivers/reset/reset-qcom-pdc.c +3
-0+ 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".
[..]
Quoted 7 lines
drivers/reset/reset-qcom-pdc.c line 127
Quoted 2 lines
drivers/reset/reset-qcom-pdc.c +2
-0+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
Re: [PATCH v2 2/6] reset: qcom: PDC Global (Power Domain Controller) reset controller
Hi Bjorn/Matthias,
Thanks for the review, will fix them in the next-respin.
Quoted 20 lines · Bjorn Andersson
Quoted 2 lines · Matthias Kaehlcke
Quoted 1 line · Sibi Sankar
drivers/reset/reset-qcom-pdc.c +0
-0[..]
Quoted 16 lines
drivers/reset/reset-qcom-pdc.c line 23
Quoted 5 lines
drivers/reset/reset-qcom-pdc.c +5
-0+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_configcould be used directly
- .resets is used in _(de)assert(),sdm845_pdc_resetscould be used directly
- .num_resets is only accessed in_probe(),ARRAY_SIZE(sdm845_pdc_resets)could be used instead
Quoted 8 lines
Quoted 3 lines
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
Quoted 15 lines
Quoted 5 lines
Quoted 5 lines
drivers/reset/reset-qcom-pdc.c +5
-0+struct qcom_pdc_reset_data { +struct reset_controller_dev rcdev; +struct regmap *regmap; +const struct qcom_pdc_desc *desc; +};[..]
Quoted 2 lines
Quoted 2 lines
drivers/reset/reset-qcom-pdc.c +2
-0+static int qcom_pdc_reset_probe (struct platform_device *pdev) +{[..]
Quoted 6 lines
drivers/reset/reset-qcom-pdc.c line 114
Quoted 3 lines
drivers/reset/reset-qcom-pdc.c +3
-0+ 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'
Quoted 7 lines
Quoted 3 lines
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?
Quoted 16 lines
[..]
Quoted 7 lines
drivers/reset/reset-qcom-pdc.c line 127
Quoted 2 lines
drivers/reset/reset-qcom-pdc.c +2
-0+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
Signature · 3 lines
-- Sibi Sankar -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH v2 2/6] reset: qcom: PDC Global (Power Domain Controller) reset controller
In reply to Bjorn AnderssonQuoted 59 lines · Bjorn Andersson
Quoted 2 lines · Matthias Kaehlcke
Quoted 1 line · Sibi Sankar
drivers/reset/reset-qcom-pdc.c +0
-0[..]
Quoted 16 lines
drivers/reset/reset-qcom-pdc.c line 23
Quoted 5 lines
drivers/reset/reset-qcom-pdc.c +5
-0+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_configcould be used directly
- .resets is used in _(de)assert(),sdm845_pdc_resetscould be used directly
- .num_resets is only accessed in_probe(),ARRAY_SIZE(sdm845_pdc_resets)could be used insteadQuoted 3 lines
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.Quoted 5 lines
Quoted 5 lines
drivers/reset/reset-qcom-pdc.c +5
-0+struct qcom_pdc_reset_data { +struct reset_controller_dev rcdev; +struct regmap *regmap; +const struct qcom_pdc_desc *desc; +};[..]
Quoted 2 lines
Quoted 2 lines
drivers/reset/reset-qcom-pdc.c +2
-0+static int qcom_pdc_reset_probe (struct platform_device *pdev) +{[..]
Quoted 9 lines
drivers/reset/reset-qcom-pdc.c line 114
Quoted 3 lines
drivers/reset/reset-qcom-pdc.c +3
-0+ 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".
[..]
Quoted 7 lines
drivers/reset/reset-qcom-pdc.c line 127
Quoted 2 lines
drivers/reset/reset-qcom-pdc.c +2
-0+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.
Quoted 4 lines
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.
PATCH v2 3/6 dt-bindings: remoteproc: Remove additional definition tag
In reply to Sibi SankarRemove the additional definition tag declared for WCSS sub-system under reset-names.
--- Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +1 -1
@@ -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:
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2 3/6] dt-bindings: remoteproc: Remove additional definition tag
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'.
Quoted 13 lines · Sibi Sankar
Remove the additional definition tag declared for WCSS sub-system under reset-names.
Signed-off-by Sibi Sankar <[email protected]>--- Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +0
-0
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt on the Qualcomm Hexagon core. · line 62
Quoted 6 lines
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +1
-1@@ -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:
Re: [PATCH v2 3/6] dt-bindings: remoteproc: Remove additional definition tag
Hi Matthias,
Thanks for the review!
Quoted 7 lines · Matthias Kaehlcke
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
Quoted 9 lines · Sibi Sankar
Quoted 8 lines
Remove the additional definition tag declared for WCSS sub-system under reset-names.
Signed-off-by Sibi Sankar <[email protected]>--- Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 15 lines
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt on the Qualcomm Hexagon core. · line 62
Quoted 11 lines
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +1
-1@@ -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-systemSince 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.
Quoted 3 lines
Other than the nits:
Reviewed-by Matthias Kaehlcke <[email protected]>
Signature · 3 lines
-- Sibi Sankar -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH v2 4/6 dt-bindings: remoteproc: Add PDC reset binding for Q6V5 PIL
In reply to Sibi SankarAdd additional pdc_reset binding required for Q6V5 Modem PIL on SDM845 SoCs.
--- Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt | 4 ++++ 1 file changed, 4 insertions(+)
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +4 -0
@@ -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:
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2 4/6] dt-bindings: remoteproc: Add PDC reset binding for Q6V5 PIL
Quoted 8 lines · Sibi Sankar
Add additional
pdc_resetbinding required for Q6V5 Modem PIL on SDM845 SoCs.Signed-off-by Sibi Sankar <[email protected]>--- Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt | 4 ++++ 1 file changed, 4 insertions(+)
PATCH v2 5/6 remoteproc: qcom: q6v5-pil: Explicitly get mss_restart line
In reply to Sibi SankarExplicitly get mss_restart to facilitate adding PDC reset line for modem on SDM845 SoCs
--- drivers/remoteproc/qcom_q6v5_pil.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/remoteproc/qcom_q6v5_pil.c +1 -1
@@ -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);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v2 6/6 remoteproc: qcom: q6v5-pil: Add PDC reset for modem on SDM845 SoCs
In reply to Sibi SankarIn the presence of a PDC block working with subsystem RSC, assert/deassert PDC reset in modem start/stop path.
--- drivers/remoteproc/qcom_q6v5_pil.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-)
drivers/remoteproc/qcom_q6v5_pil.c +24 -5
@@ -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,
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project