Re: [PATCH 0/4] Update am437x and am335x dts to probe with ti-sysc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Monday 01 October 2018 10:02 PM, Tony Lindgren wrote:
> * Keerthy <j-keerthy@xxxxxx> [180927 05:00]:
>> Also tested on beaglebone black DS0 works fine. Although there seems to
>> be an additional warning which was not seen before:
>>
>> "l4-wkup-clkctrl:0008:0: failed to disable"
>>
>> log: https://pastebin.ubuntu.com/p/Ns2WRdVkXS/
> 
> Can you test the following patch applied on top of the
> omap-for-v4.20/dt-ti-sysc-tmp-testing-v2 testing branch?

Tony,

Apologies for the delay in responding.

AM437X-GP-EVM: https://pastebin.ubuntu.com/p/m3ybBvH7Qj/

Tested for UART/RTCWAKE/TOUCH_WAKE from DS0, apart from the expected
gpio warnings all seems fine.

AM335X-BONEBLACK: https://pastebin.ubuntu.com/p/xdsMDS5XtQ/

Tested for rtcwake/UART wake up from DS0. Again except for gpio messages
everything else seems to be fine.

Regards,
Keerthy

> 
> Based on Grygorii's i2c-omap comments, I think we should also
> do the following in the ti-sysc driver becasuse the pm_runtime
> use count can be whatever.
> 
> Regards,
> 
> Tony
> 
> 8< ----------------------
> From tony Mon Sep 17 00:00:00 2001
> From: Tony Lindgren <tony@xxxxxxxxxxx>
> Date: Mon, 1 Oct 2018 09:11:57 -0700
> Subject: [PATCH] bus: ti-sysc: Just use SET_NOIRQ_SYSTEM_SLEEP_PM_OPS
> 
> As Grygorii Strashko pointed out, the runtime PM use count of the
> children can be whatever at suspend and we should not use it. So
> let's just suspend ti-sysc at noirq level and get rid of some code.
> 
> Let's also remove the PM_SLEEP ifdef and use __maybe_unused as the
> PM code already deals with the ifdefs.
> 
> Suggested-by: Grygorii Strashko <grygorii.strashko@xxxxxx>
> Signed-off-by: Tony Lindgren <tony@xxxxxxxxxxx>
> ---
>  drivers/bus/ti-sysc.c | 113 ++----------------------------------------
>  1 file changed, 4 insertions(+), 109 deletions(-)
> 
> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -87,7 +87,6 @@ struct sysc {
>  	u32 revision;
>  	bool enabled;
>  	bool needs_resume;
> -	unsigned int noirq_suspend:1;
>  	bool child_needs_resume;
>  	struct delayed_work idle_work;
>  };
> @@ -702,75 +701,7 @@ static int __maybe_unused sysc_runtime_resume(struct device *dev)
>  	return error;
>  }
>  
> -#ifdef CONFIG_PM_SLEEP
> -static int sysc_suspend(struct device *dev)
> -{
> -	struct sysc *ddata;
> -	int error;
> -
> -	ddata = dev_get_drvdata(dev);
> -
> -	if (ddata->cfg.quirks & SYSC_QUIRK_LEGACY_IDLE)
> -		return 0;
> -
> -	if (!ddata->enabled || ddata->noirq_suspend)
> -		return 0;
> -
> -	dev_dbg(ddata->dev, "%s %s\n", __func__,
> -		ddata->name ? ddata->name : "");
> -
> -	error = pm_runtime_put_sync_suspend(dev);
> -	if (error == -EBUSY) {
> -		dev_dbg(ddata->dev, "%s busy, tagging for noirq suspend %s\n",
> -			__func__, ddata->name ? ddata->name : "");
> -
> -		ddata->noirq_suspend = true;
> -
> -		return 0;
> -	} else if (error < 0) {
> -		dev_warn(ddata->dev, "%s cannot suspend %i %s\n",
> -			 __func__, error,
> -			 ddata->name ? ddata->name : "");
> -
> -		return 0;
> -	}
> -
> -	ddata->needs_resume = true;
> -
> -	return 0;
> -}
> -
> -static int sysc_resume(struct device *dev)
> -{
> -	struct sysc *ddata;
> -	int error;
> -
> -	ddata = dev_get_drvdata(dev);
> -
> -	if (ddata->cfg.quirks & SYSC_QUIRK_LEGACY_IDLE)
> -		return 0;
> -
> -	if (!ddata->needs_resume || ddata->noirq_suspend)
> -		return 0;
> -
> -	dev_dbg(ddata->dev, "%s %s\n", __func__,
> -		ddata->name ? ddata->name : "");
> -
> -	error = pm_runtime_get_sync(dev);
> -	if (error < 0) {
> -		dev_err(ddata->dev, "%s  error %i %s\n",
> -			__func__, error,
> -			ddata->name ? ddata->name : "");
> -
> -		return error;
> -	}
> -
> -	ddata->needs_resume = false;
> -
> -	return 0;
> -}
> -
> -static int sysc_noirq_suspend(struct device *dev)
> +static int __maybe_unused sysc_noirq_suspend(struct device *dev)
>  {
>  	struct sysc *ddata;
>  	int error;
> @@ -780,26 +711,10 @@ static int sysc_noirq_suspend(struct device *dev)
>  	if (ddata->cfg.quirks & SYSC_QUIRK_LEGACY_IDLE)
>  		return 0;
>  
> -	if (!ddata->enabled || !ddata->noirq_suspend)
> -		return 0;
> -
> -	dev_dbg(ddata->dev, "%s %s\n", __func__,
> -		ddata->name ? ddata->name : "");
> -
> -	error = sysc_runtime_suspend(dev);
> -	if (error) {
> -		dev_warn(ddata->dev, "%s busy %i %s\n",
> -			 __func__, error, ddata->name ? ddata->name : "");
> -
> -		return 0;
> -	}
> -
> -	ddata->needs_resume = true;
> -
> -	return 0;
> +	return pm_runtime_force_suspend(dev);
>  }
>  
> -static int sysc_noirq_resume(struct device *dev)
> +static int __maybe_unused sysc_noirq_resume(struct device *dev)
>  {
>  	struct sysc *ddata;
>  	int error;
> @@ -809,30 +724,10 @@ static int sysc_noirq_resume(struct device *dev)
>  	if (ddata->cfg.quirks & SYSC_QUIRK_LEGACY_IDLE)
>  		return 0;
>  
> -	if (!ddata->needs_resume || !ddata->noirq_suspend)
> -		return 0;
> -
> -	dev_dbg(ddata->dev, "%s %s\n", __func__,
> -		ddata->name ? ddata->name : "");
> -
> -	error = sysc_runtime_resume(dev);
> -	if (error) {
> -		dev_warn(ddata->dev, "%s cannot resume %i %s\n",
> -			 __func__, error,
> -			 ddata->name ? ddata->name : "");
> -
> -		return error;
> -	}
> -
> -	/* Maybe also reconsider clearing noirq_suspend at some point */
> -	ddata->needs_resume = false;
> -
> -	return 0;
> +	return pm_runtime_force_resume(dev);
>  }
> -#endif
>  
>  static const struct dev_pm_ops sysc_pm_ops = {
> -	SET_SYSTEM_SLEEP_PM_OPS(sysc_suspend, sysc_resume)
>  	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(sysc_noirq_suspend, sysc_noirq_resume)
>  	SET_RUNTIME_PM_OPS(sysc_runtime_suspend,
>  			   sysc_runtime_resume,
> 



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux