mbox series

[v5,0/8] Add support for remoteproc modem-pil on SDM845 SoCs

Message ID 20180521172714.8551-1-sibis@codeaurora.org
Headers show
Series Add support for remoteproc modem-pil on SDM845 SoCs | expand

Message

Sibi Sankar May 21, 2018, 5:27 p.m. UTC 
This patch series add support for remoteproc Q6v5 modem-pil on Qualcomm
SDM845 SoC. The first patch adds AOSS (Always on subsystem) reset driver
to provide for mss reset line. The last couple of patches add the resets
sequence for Q6 on SDM845 and adds helper functions for arbitrary reset
assert/deassert sequences.

V5:
   corrected dt-binding and increased usleep_range for aoss reset driver
   Include the ready interrupt handling in the patch series 
   Replaced reset ops with a simpler helper function
   Re-ordered and split patches as recommended
   Inlined q6v5_request_irq function

V4:
   Removed regmap depencencies from aoss reset driver
   Separted apss shared mailbox into separate patch
   Corrected all nits and replaced with author full name

V3:
   Removed syscon dependency for the aoss reset driver
   Split dt-bindings and the aoss reset driver into separate patches
   Corrected few typos and replaced misconfigured author name

V2:
   Addressed reset-qcom-aoss review suggestions and reworked
   re-ordering of the active clk and reset sequence in
   qcom_q6v5_pil

Sibi Sankar (8):
  dt-bindings: reset: Add AOSS reset bindings for SDM845 SoCs
  reset: qcom: AOSS (always on subsystem) reset controller
  remoteproc: Move proxy unvote to handover irq handler
  remoteproc: Synchronize proxy unvote from multiple contexts
  dt-bindings: remoteproc: Add Q6v5 Modem PIL binding for SDM845
  remoteproc: qcom: Introduce reset assert/deassert helper functions
  remoteproc: qcom: Add support for mss remoteproc on SDM845
  remoteproc: qcom: Allow defining GLINK edge for mss remoteproc

 .../bindings/remoteproc/qcom,q6v5.txt         |   1 +
 .../bindings/reset/qcom,aoss-reset.txt        |  52 +++++
 drivers/remoteproc/qcom_q6v5_pil.c            | 212 ++++++++++++++++--
 drivers/reset/Kconfig                         |   9 +
 drivers/reset/Makefile                        |   1 +
 drivers/reset/reset-qcom-aoss.c               | 133 +++++++++++
 include/dt-bindings/reset/qcom,sdm845-aoss.h  |  17 ++
 7 files changed, 404 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt
 create mode 100644 drivers/reset/reset-qcom-aoss.c
 create mode 100644 include/dt-bindings/reset/qcom,sdm845-aoss.h

Comments

Bjorn Andersson May 22, 2018, 4:35 a.m. UTC | #1 
On Mon 21 May 10:27 PDT 2018, Sibi Sankar wrote:

> Introduce interrupt handler for smp2p ready interrupt to
> handle start completion. Move the proxy votes for clocks
> and regulators to the handover interrupt context.
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>

I added the "synchronization logic" of patch 4 to this one, to make it
functional and after testing on db820c I merged patch 3 through 8 into
rproc-next.

I skipped the remainder of patch 4, as I think the ready_irq should be
included and I wonder if irq management can remove the need for the
"running" variable.

I also moved the enable_irq before we actually start the core, since
it's been shown downstream that the smp2p irq might fire fast enough for
us to miss it (and the smp2p driver does not redeliver interrupts).

Regards,
Bjorn

> ---
>  drivers/remoteproc/qcom_q6v5_pil.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
> index 76a0c00aa04a..6333bdcd9448 100644
> --- a/drivers/remoteproc/qcom_q6v5_pil.c
> +++ b/drivers/remoteproc/qcom_q6v5_pil.c
> @@ -809,11 +809,6 @@ static int q6v5_start(struct rproc *rproc)
>  			"Failed to reclaim mba buffer system may become unstable\n");
>  	qproc->running = true;
>  
> -	q6v5_clk_disable(qproc->dev, qproc->proxy_clks,
> -			 qproc->proxy_clk_count);
> -	q6v5_regulator_disable(qproc, qproc->proxy_regs,
> -			       qproc->proxy_reg_count);
> -
>  	return 0;
>  
>  reclaim_mpss:
> @@ -892,6 +887,12 @@ static int q6v5_stop(struct rproc *rproc)
>  	WARN_ON(ret);
>  
>  	reset_control_assert(qproc->mss_restart);
> +
> +	q6v5_clk_disable(qproc->dev, qproc->proxy_clks,
> +			 qproc->proxy_clk_count);
> +	q6v5_regulator_disable(qproc, qproc->proxy_regs,
> +			       qproc->proxy_reg_count);
> +
>  	q6v5_clk_disable(qproc->dev, qproc->active_clks,
>  			 qproc->active_clk_count);
>  	q6v5_regulator_disable(qproc, qproc->active_regs,
> @@ -959,7 +960,7 @@ static irqreturn_t q6v5_fatal_interrupt(int irq, void *dev)
>  	return IRQ_HANDLED;
>  }
>  
> -static irqreturn_t q6v5_handover_interrupt(int irq, void *dev)
> +static irqreturn_t q6v5_ready_interrupt(int irq, void *dev)
>  {
>  	struct q6v5 *qproc = dev;
>  
> @@ -967,6 +968,18 @@ static irqreturn_t q6v5_handover_interrupt(int irq, void *dev)
>  	return IRQ_HANDLED;
>  }
>  
> +static irqreturn_t q6v5_handover_interrupt(int irq, void *dev)
> +{
> +	struct q6v5 *qproc = dev;
> +
> +	q6v5_clk_disable(qproc->dev, qproc->proxy_clks,
> +			 qproc->proxy_clk_count);
> +	q6v5_regulator_disable(qproc, qproc->proxy_regs,
> +			       qproc->proxy_reg_count);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static irqreturn_t q6v5_stop_ack_interrupt(int irq, void *dev)
>  {
>  	struct q6v5 *qproc = dev;
> @@ -1194,6 +1207,10 @@ static int q6v5_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto free_rproc;
>  
> +	ret = q6v5_request_irq(qproc, pdev, "ready", q6v5_ready_interrupt);
> +	if (ret < 0)
> +		goto free_rproc;
> +
>  	ret = q6v5_request_irq(qproc, pdev, "handover", q6v5_handover_interrupt);
>  	if (ret < 0)
>  		goto free_rproc;
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sibi Sankar May 22, 2018, 6:31 a.m. UTC | #2 
Hi Bjorn,
Thanks for the review.

On 05/22/2018 10:05 AM, Bjorn Andersson wrote:
> On Mon 21 May 10:27 PDT 2018, Sibi Sankar wrote:
> 
>> Introduce interrupt handler for smp2p ready interrupt to
>> handle start completion. Move the proxy votes for clocks
>> and regulators to the handover interrupt context.
>>
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> 
> I added the "synchronization logic" of patch 4 to this one, to make it
> functional and after testing on db820c I merged patch 3 through 8 into
> rproc-next.
> 

making the q6v5_request_irq return the irq number seems more elegant
unlike the approach I took earlier.

> I skipped the remainder of patch 4, as I think the ready_irq should be
> included and I wonder if irq management can remove the need for the
> "running" variable.
>

will include the ready_irq as well. I think we may need to keep
"running" for coredump but will check if it can substituted with
irq management.

> I also moved the enable_irq before we actually start the core, since
> it's been shown downstream that the smp2p irq might fire fast enough for
> us to miss it (and the smp2p driver does not redeliver interrupts).
> 
> Regards,
> Bjorn
> 
>> ---
>>   drivers/remoteproc/qcom_q6v5_pil.c | 29 +++++++++++++++++++++++------
>>   1 file changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
>> index 76a0c00aa04a..6333bdcd9448 100644
>> --- a/drivers/remoteproc/qcom_q6v5_pil.c
>> +++ b/drivers/remoteproc/qcom_q6v5_pil.c
>> @@ -809,11 +809,6 @@ static int q6v5_start(struct rproc *rproc)
>>   			"Failed to reclaim mba buffer system may become unstable\n");
>>   	qproc->running = true;
>>   
>> -	q6v5_clk_disable(qproc->dev, qproc->proxy_clks,
>> -			 qproc->proxy_clk_count);
>> -	q6v5_regulator_disable(qproc, qproc->proxy_regs,
>> -			       qproc->proxy_reg_count);
>> -
>>   	return 0;
>>   
>>   reclaim_mpss:
>> @@ -892,6 +887,12 @@ static int q6v5_stop(struct rproc *rproc)
>>   	WARN_ON(ret);
>>   
>>   	reset_control_assert(qproc->mss_restart);
>> +
>> +	q6v5_clk_disable(qproc->dev, qproc->proxy_clks,
>> +			 qproc->proxy_clk_count);
>> +	q6v5_regulator_disable(qproc, qproc->proxy_regs,
>> +			       qproc->proxy_reg_count);
>> +
>>   	q6v5_clk_disable(qproc->dev, qproc->active_clks,
>>   			 qproc->active_clk_count);
>>   	q6v5_regulator_disable(qproc, qproc->active_regs,
>> @@ -959,7 +960,7 @@ static irqreturn_t q6v5_fatal_interrupt(int irq, void *dev)
>>   	return IRQ_HANDLED;
>>   }
>>   
>> -static irqreturn_t q6v5_handover_interrupt(int irq, void *dev)
>> +static irqreturn_t q6v5_ready_interrupt(int irq, void *dev)
>>   {
>>   	struct q6v5 *qproc = dev;
>>   
>> @@ -967,6 +968,18 @@ static irqreturn_t q6v5_handover_interrupt(int irq, void *dev)
>>   	return IRQ_HANDLED;
>>   }
>>   
>> +static irqreturn_t q6v5_handover_interrupt(int irq, void *dev)
>> +{
>> +	struct q6v5 *qproc = dev;
>> +
>> +	q6v5_clk_disable(qproc->dev, qproc->proxy_clks,
>> +			 qproc->proxy_clk_count);
>> +	q6v5_regulator_disable(qproc, qproc->proxy_regs,
>> +			       qproc->proxy_reg_count);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>>   static irqreturn_t q6v5_stop_ack_interrupt(int irq, void *dev)
>>   {
>>   	struct q6v5 *qproc = dev;
>> @@ -1194,6 +1207,10 @@ static int q6v5_probe(struct platform_device *pdev)
>>   	if (ret < 0)
>>   		goto free_rproc;
>>   
>> +	ret = q6v5_request_irq(qproc, pdev, "ready", q6v5_ready_interrupt);
>> +	if (ret < 0)
>> +		goto free_rproc;
>> +
>>   	ret = q6v5_request_irq(qproc, pdev, "handover", q6v5_handover_interrupt);
>>   	if (ret < 0)
>>   		goto free_rproc;
>> -- 
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>>
>
Bjorn Andersson June 23, 2018, 12:46 a.m. UTC | #3 
On Mon 21 May 10:27 PDT 2018, Sibi Sankar wrote:

> Add reset controller driver for Qualcomm SDM845 SoC to
> control reset signals provided by AOSS for Modem, Venus
> ADSP, GPU, Camera, Wireless, Display subsystem
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>

With the adaptions discussed in the DT binding patch (compatible and
register offsets) you have my:

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  drivers/reset/Kconfig           |   9 +++
>  drivers/reset/Makefile          |   1 +
>  drivers/reset/reset-qcom-aoss.c | 133 ++++++++++++++++++++++++++++++++
>  3 files changed, 143 insertions(+)
>  create mode 100644 drivers/reset/reset-qcom-aoss.c
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index c0b292be1b72..756ad2b27d0f 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -82,6 +82,15 @@ config RESET_PISTACHIO
>  	help
>  	  This enables the reset driver for ImgTec Pistachio SoCs.
>  
> +config RESET_QCOM_AOSS
> +	bool "Qcom AOSS Reset Driver"
> +	depends on ARCH_QCOM || COMPILE_TEST
> +	help
> +	  This enables the AOSS (always on subsystem) reset driver
> +	  for Qualcomm SDM845 SoCs. Say Y if you want to control
> +	  reset signals provided by AOSS for Modem, Venus, ADSP,
> +	  GPU, Camera, Wireless, Display subsystem. Otherwise, say N.
> +
>  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 c1261dcfe9ad..6881e4d287f0 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
>  obj-$(CONFIG_RESET_MESON) += reset-meson.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_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-aoss.c b/drivers/reset/reset-qcom-aoss.c
> new file mode 100644
> index 000000000000..d9ca7339c434
> --- /dev/null
> +++ b/drivers/reset/reset-qcom-aoss.c
> @@ -0,0 +1,133 @@
> +// 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/delay.h>
> +#include <linux/io.h>
> +#include <linux/of_device.h>
> +#include <dt-bindings/reset/qcom,sdm845-aoss.h>
> +
> +struct qcom_aoss_reset_map {
> +	unsigned int reg;
> +};
> +
> +struct qcom_aoss_desc {
> +	const struct qcom_aoss_reset_map *resets;
> +	size_t num_resets;
> +};
> +
> +struct qcom_aoss_reset_data {
> +	struct reset_controller_dev rcdev;
> +	void __iomem *base;
> +	const struct qcom_aoss_desc *desc;
> +};
> +
> +static const struct qcom_aoss_reset_map sdm845_aoss_resets[] = {
> +	[AOSS_CC_MSS_RESTART] = {0x0},
> +	[AOSS_CC_CAMSS_RESTART] = {0x1000},
> +	[AOSS_CC_VENUS_RESTART] = {0x2000},
> +	[AOSS_CC_GPU_RESTART] = {0x3000},
> +	[AOSS_CC_DISPSS_RESTART] = {0x4000},
> +	[AOSS_CC_WCSS_RESTART] = {0x10000},
> +	[AOSS_CC_LPASS_RESTART] = {0x20000},
> +};
> +
> +static const struct qcom_aoss_desc sdm845_aoss_desc = {
> +	.resets = sdm845_aoss_resets,
> +	.num_resets = ARRAY_SIZE(sdm845_aoss_resets),
> +};
> +
> +static inline struct qcom_aoss_reset_data *to_qcom_aoss_reset_data(
> +				struct reset_controller_dev *rcdev)
> +{
> +	return container_of(rcdev, struct qcom_aoss_reset_data, rcdev);
> +}
> +
> +static int qcom_aoss_control_assert(struct reset_controller_dev *rcdev,
> +				    unsigned long idx)
> +{
> +	struct qcom_aoss_reset_data *data = to_qcom_aoss_reset_data(rcdev);
> +	const struct qcom_aoss_reset_map *map = &data->desc->resets[idx];
> +
> +	writel(1, data->base + map->reg);
> +	/* Wait 6 32kHz sleep cycles for reset */
> +	usleep_range(200, 300);
> +	return 0;
> +}
> +
> +static int qcom_aoss_control_deassert(struct reset_controller_dev *rcdev,
> +				      unsigned long idx)
> +{
> +	struct qcom_aoss_reset_data *data = to_qcom_aoss_reset_data(rcdev);
> +	const struct qcom_aoss_reset_map *map = &data->desc->resets[idx];
> +
> +	writel(0, data->base + map->reg);
> +	/* Wait 6 32kHz sleep cycles for reset */
> +	usleep_range(200, 300);
> +	return 0;
> +}
> +
> +static int qcom_aoss_control_reset(struct reset_controller_dev *rcdev,
> +					unsigned long idx)
> +{
> +	qcom_aoss_control_assert(rcdev, idx);
> +
> +	return qcom_aoss_control_deassert(rcdev, idx);
> +}
> +
> +static const struct reset_control_ops qcom_aoss_reset_ops = {
> +	.reset = qcom_aoss_control_reset,
> +	.assert = qcom_aoss_control_assert,
> +	.deassert = qcom_aoss_control_deassert,
> +};
> +
> +static int qcom_aoss_reset_probe(struct platform_device *pdev)
> +{
> +	struct qcom_aoss_reset_data *data;
> +	struct device *dev = &pdev->dev;
> +	const struct qcom_aoss_desc *desc;
> +	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);
> +	data->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(data->base))
> +		return PTR_ERR(data->base);
> +
> +	data->rcdev.owner = THIS_MODULE;
> +	data->rcdev.ops = &qcom_aoss_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_aoss_reset_of_match[] = {
> +	{ .compatible = "qcom,sdm845-aoss-reset", .data = &sdm845_aoss_desc },
> +	{}
> +};
> +
> +static struct platform_driver qcom_aoss_reset_driver = {
> +	.probe = qcom_aoss_reset_probe,
> +	.driver  = {
> +		.name = "qcom_aoss_reset",
> +		.of_match_table = qcom_aoss_reset_of_match,
> +	},
> +};
> +
> +builtin_platform_driver(qcom_aoss_reset_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm AOSS Reset Driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html