Re: [PATCH v2] tty: serial: qcom-geni-serial: Drop the icc bw votes in suspend for console

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Mon 13 Jul 22:31 PDT 2020, Rajendra Nayak wrote:

> When using the geni-serial as console, its important to be
> able to hit the lowest possible power state in suspend,
> even with no_console_suspend.
> The only thing that prevents it today on platforms like the sc7180
> is the interconnect BW votes, which we certainly don't need when
> the system is in suspend. So in the suspend handler mark them as
> ACTIVE_ONLY (0x3) and on resume switch them back to the ALWAYS tag (0x7)
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> Reviewed-by: Akash Asthana <akashast@codeaurora.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Applied

Thanks,
Bjorn

> ---
> v2: Rebased on the latest qcom for-next
> 
>  drivers/soc/qcom/qcom-geni-se.c       |  9 +++++++++
>  drivers/tty/serial/qcom_geni_serial.c | 16 +++++++++++++++-
>  include/linux/qcom-geni-se.h          |  1 +
>  3 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> index e2a0ba2..355d503 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -771,6 +771,15 @@ int geni_icc_set_bw(struct geni_se *se)
>  }
>  EXPORT_SYMBOL(geni_icc_set_bw);
>  
> +void geni_icc_set_tag(struct geni_se *se, u32 tag)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(se->icc_paths); i++)
> +		icc_set_tag(se->icc_paths[i].path, tag);
> +}
> +EXPORT_SYMBOL(geni_icc_set_tag);
> +
>  /* To do: Replace this by icc_bulk_enable once it's implemented in ICC core */
>  int geni_icc_enable(struct geni_se *se)
>  {
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 583d903..07b7b6b 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1505,16 +1505,30 @@ static int __maybe_unused qcom_geni_serial_sys_suspend(struct device *dev)
>  	struct uart_port *uport = &port->uport;
>  	struct qcom_geni_private_data *private_data = uport->private_data;
>  
> +	/*
> +	 * This is done so we can hit the lowest possible state in suspend
> +	 * even with no_console_suspend
> +	 */
> +	if (uart_console(uport)) {
> +		geni_icc_set_tag(&port->se, 0x3);
> +		geni_icc_set_bw(&port->se);
> +	}
>  	return uart_suspend_port(private_data->drv, uport);
>  }
>  
>  static int __maybe_unused qcom_geni_serial_sys_resume(struct device *dev)
>  {
> +	int ret;
>  	struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
>  	struct uart_port *uport = &port->uport;
>  	struct qcom_geni_private_data *private_data = uport->private_data;
>  
> -	return uart_resume_port(private_data->drv, uport);
> +	ret = uart_resume_port(private_data->drv, uport);
> +	if (uart_console(uport)) {
> +		geni_icc_set_tag(&port->se, 0x7);
> +		geni_icc_set_bw(&port->se);
> +	}
> +	return ret;
>  }
>  
>  static const struct dev_pm_ops qcom_geni_serial_pm_ops = {
> diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
> index afa511e..8f385fb 100644
> --- a/include/linux/qcom-geni-se.h
> +++ b/include/linux/qcom-geni-se.h
> @@ -454,6 +454,7 @@ void geni_se_rx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t len);
>  int geni_icc_get(struct geni_se *se, const char *icc_ddr);
>  
>  int geni_icc_set_bw(struct geni_se *se);
> +void geni_icc_set_tag(struct geni_se *se, u32 tag);
>  
>  int geni_icc_enable(struct geni_se *se);
>  
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>