All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: Dave Gerlach <d-gerlach@ti.com>
Cc: Tero Kristo <t-kristo@ti.com>,
	linux-omap@vger.kernel.org, Franklin S Cooper <fcooper@ti.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] ARM: OMAP2+: omap_device: Sync omap_device and pm_runtime after probe defer
Date: Tue, 4 Apr 2017 08:52:23 -0700	[thread overview]
Message-ID: <20170404155222.GP10760@atomide.com> (raw)
In-Reply-To: <20170330195818.17669-1-d-gerlach@ti.com>

* Dave Gerlach <d-gerlach@ti.com> [170330 13:01]:
> Starting from commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM
> states at probe error and driver unbind") pm_runtime core now changes
> device runtime_status back to after RPM_SUSPENDED after a probe defer.
> Certain OMAP devices make use of "ti,no-idle-on-init" flag which causes
> omap_device_enable to be called during the BUS_NOTIFY_ADD_DEVICE event
> during probe, along with pm_runtime_set_active.
> 
> This call to pm_runtime_set_active typically will prevent a call to
> pm_runtime_get in a driver probe function from re-enabling the
> omap_device. However, in the case of a probe defer that happens before
> the driver probe function is able to run, such as a missing pinctrl
> states defer, pm_runtime_reinit will set the device as RPM_SUSPENDED and
> then once driver probe is actually able to run, pm_runtime_get will see
> the device as suspended and call through to the omap_device layer,
> attempting to enable the already enabled omap_device and causing errors
> like this:
> 
> omap-gpmc 50000000.gpmc: omap_device: omap_device_enable() called from invalid state 1
> omap-gpmc 50000000.gpmc: use pm_runtime_put_sync_suspend() in driver?
> 
> We can avoid this error by making sure the pm_runtime status of a device
> matches the omap_device state before a probe attempt. By extending the
> omap_device bus notifier to act on the BUS_NOTIFY_BIND_DRIVER event we
> can check if a device is enabled in omap_device but with a pm_runtime
> status of RPM_SUSPENDED and once again mark the device as RPM_ACTIVE to
> avoid a second incorrect call to omap_device_enable.
> 
> Tested-by: Franklin S Cooper Jr. <fcooper@ti.com>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>

Applying into omap-for-v4.11/fixes thanks. Will let this one sit
in Linux next for a while.

Regards,

Tony

> ---
>  arch/arm/mach-omap2/omap_device.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
> index e920dd83e443..f989145480c8 100644
> --- a/arch/arm/mach-omap2/omap_device.c
> +++ b/arch/arm/mach-omap2/omap_device.c
> @@ -222,6 +222,14 @@ static int _omap_device_notifier_call(struct notifier_block *nb,
>  				dev_err(dev, "failed to idle\n");
>  		}
>  		break;
> +	case BUS_NOTIFY_BIND_DRIVER:
> +		od = to_omap_device(pdev);
> +		if (od && (od->_state == OMAP_DEVICE_STATE_ENABLED) &&
> +		    pm_runtime_status_suspended(dev)) {
> +			od->_driver_status = BUS_NOTIFY_BIND_DRIVER;
> +			pm_runtime_set_active(dev);
> +		}
> +		break;
>  	case BUS_NOTIFY_ADD_DEVICE:
>  		if (pdev->dev.of_node)
>  			omap_device_build_from_dt(pdev);
> -- 
> 2.11.0
> 

WARNING: multiple messages have this Message-ID (diff)
From: tony@atomide.com (Tony Lindgren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: OMAP2+: omap_device: Sync omap_device and pm_runtime after probe defer
Date: Tue, 4 Apr 2017 08:52:23 -0700	[thread overview]
Message-ID: <20170404155222.GP10760@atomide.com> (raw)
In-Reply-To: <20170330195818.17669-1-d-gerlach@ti.com>

* Dave Gerlach <d-gerlach@ti.com> [170330 13:01]:
> Starting from commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM
> states at probe error and driver unbind") pm_runtime core now changes
> device runtime_status back to after RPM_SUSPENDED after a probe defer.
> Certain OMAP devices make use of "ti,no-idle-on-init" flag which causes
> omap_device_enable to be called during the BUS_NOTIFY_ADD_DEVICE event
> during probe, along with pm_runtime_set_active.
> 
> This call to pm_runtime_set_active typically will prevent a call to
> pm_runtime_get in a driver probe function from re-enabling the
> omap_device. However, in the case of a probe defer that happens before
> the driver probe function is able to run, such as a missing pinctrl
> states defer, pm_runtime_reinit will set the device as RPM_SUSPENDED and
> then once driver probe is actually able to run, pm_runtime_get will see
> the device as suspended and call through to the omap_device layer,
> attempting to enable the already enabled omap_device and causing errors
> like this:
> 
> omap-gpmc 50000000.gpmc: omap_device: omap_device_enable() called from invalid state 1
> omap-gpmc 50000000.gpmc: use pm_runtime_put_sync_suspend() in driver?
> 
> We can avoid this error by making sure the pm_runtime status of a device
> matches the omap_device state before a probe attempt. By extending the
> omap_device bus notifier to act on the BUS_NOTIFY_BIND_DRIVER event we
> can check if a device is enabled in omap_device but with a pm_runtime
> status of RPM_SUSPENDED and once again mark the device as RPM_ACTIVE to
> avoid a second incorrect call to omap_device_enable.
> 
> Tested-by: Franklin S Cooper Jr. <fcooper@ti.com>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>

Applying into omap-for-v4.11/fixes thanks. Will let this one sit
in Linux next for a while.

Regards,

Tony

> ---
>  arch/arm/mach-omap2/omap_device.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
> index e920dd83e443..f989145480c8 100644
> --- a/arch/arm/mach-omap2/omap_device.c
> +++ b/arch/arm/mach-omap2/omap_device.c
> @@ -222,6 +222,14 @@ static int _omap_device_notifier_call(struct notifier_block *nb,
>  				dev_err(dev, "failed to idle\n");
>  		}
>  		break;
> +	case BUS_NOTIFY_BIND_DRIVER:
> +		od = to_omap_device(pdev);
> +		if (od && (od->_state == OMAP_DEVICE_STATE_ENABLED) &&
> +		    pm_runtime_status_suspended(dev)) {
> +			od->_driver_status = BUS_NOTIFY_BIND_DRIVER;
> +			pm_runtime_set_active(dev);
> +		}
> +		break;
>  	case BUS_NOTIFY_ADD_DEVICE:
>  		if (pdev->dev.of_node)
>  			omap_device_build_from_dt(pdev);
> -- 
> 2.11.0
> 

  reply	other threads:[~2017-04-04 15:52 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-30 19:58 [PATCH] ARM: OMAP2+: omap_device: Sync omap_device and pm_runtime after probe defer Dave Gerlach
2017-03-30 19:58 ` Dave Gerlach
2017-04-04 15:52 ` Tony Lindgren [this message]
2017-04-04 15:52   ` Tony Lindgren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170404155222.GP10760@atomide.com \
    --to=tony@atomide.com \
    --cc=d-gerlach@ti.com \
    --cc=fcooper@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=t-kristo@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.