diff mbox

[RFC,v4,1/3] PM / Runtime: Add an API pm_runtime_set_slave

Message ID 1423479726-23140-2-git-send-email-amit.daniel@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Amit Kachhap Feb. 9, 2015, 11:02 a.m. UTC
This API creates a pm runtime slave type device which does not itself
participates in pm runtime but depends on the master devices to power
manage them. These devices should have pm runtime callbacks.

These devices (like clock) may not implement complete pm_runtime calls
such as pm_runtime_get/pm_runtime_put due to subsystems interaction
behaviour or any other reason.

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/base/power/runtime.c | 18 ++++++++++++++++++
 include/linux/pm.h           |  1 +
 include/linux/pm_runtime.h   |  2 ++
 3 files changed, 21 insertions(+)

Comments

Alan Stern Feb. 9, 2015, 3:58 p.m. UTC | #1
On Mon, 9 Feb 2015, Amit Daniel Kachhap wrote:

> This API creates a pm runtime slave type device which does not itself
> participates in pm runtime but depends on the master devices to power
> manage them.

This makes no sense.  How can a master device manage a slave device?  
Devices are managed by drivers, not by other devices.

>  These devices should have pm runtime callbacks.
> 
> These devices (like clock) may not implement complete pm_runtime calls
> such as pm_runtime_get/pm_runtime_put due to subsystems interaction
> behaviour or any other reason.
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  drivers/base/power/runtime.c | 18 ++++++++++++++++++
>  include/linux/pm.h           |  1 +
>  include/linux/pm_runtime.h   |  2 ++
>  3 files changed, 21 insertions(+)

This patch is unacceptable because it does not update the runtime PM 
documentation file.

Besides, doesn't the no_callbacks flag already do more or less what you 
want?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Amit Kachhap Feb. 12, 2015, 9:42 a.m. UTC | #2
Hi Alan,

On Mon, Feb 9, 2015 at 9:28 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Mon, 9 Feb 2015, Amit Daniel Kachhap wrote:
>
>> This API creates a pm runtime slave type device which does not itself
>> participates in pm runtime but depends on the master devices to power
>> manage them.
>
> This makes no sense.  How can a master device manage a slave device?
> Devices are managed by drivers, not by other devices.
May be my commit is not explaining the requirements completely and the
API name may not reflect the relevance. But If you see the 3rd patch
it has one clock use-case where this new feature is used and the
current pm runtime feature is not sufficient enough to handle it. I
have one more IOMMU use case also which is not part of this patch
series.
I am not sure if this approach is final but I looked at runtime.c file
and it has couple of API's like pm_runtime_forbid/allow which
blocks/unblocks the runtime callbacks according to driver requirement.
In the similar line I added this new API.
>
>>  These devices should have pm runtime callbacks.
>>
>> These devices (like clock) may not implement complete pm_runtime calls
>> such as pm_runtime_get/pm_runtime_put due to subsystems interaction
>> behaviour or any other reason.
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  drivers/base/power/runtime.c | 18 ++++++++++++++++++
>>  include/linux/pm.h           |  1 +
>>  include/linux/pm_runtime.h   |  2 ++
>>  3 files changed, 21 insertions(+)
>
> This patch is unacceptable because it does not update the runtime PM
> documentation file.
my fault. Will update in next version.
>
> Besides, doesn't the no_callbacks flag already do more or less what you
> want?
yes to some extent. But I thought its purpose is different so I added 1 more.

Regards,
Amit D
>
> Alan Stern
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Stern Feb. 12, 2015, 3:13 p.m. UTC | #3
On Thu, 12 Feb 2015, amit daniel kachhap wrote:

> Hi Alan,
> 
> On Mon, Feb 9, 2015 at 9:28 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> > On Mon, 9 Feb 2015, Amit Daniel Kachhap wrote:
> >
> >> This API creates a pm runtime slave type device which does not itself
> >> participates in pm runtime but depends on the master devices to power
> >> manage them.
> >
> > This makes no sense.  How can a master device manage a slave device?
> > Devices are managed by drivers, not by other devices.
> May be my commit is not explaining the requirements completely and the
> API name may not reflect the relevance. But If you see the 3rd patch
> it has one clock use-case where this new feature is used and the
> current pm runtime feature is not sufficient enough to handle it. I
> have one more IOMMU use case also which is not part of this patch
> series.

Regardless, your description should say what is really happening.  The
master device doesn't power-manage the clock; some driver power-manages
it.

> I am not sure if this approach is final but I looked at runtime.c file
> and it has couple of API's like pm_runtime_forbid/allow which
> blocks/unblocks the runtime callbacks according to driver requirement.
> In the similar line I added this new API.

forbid/allow blocks/unblocks runtime PM according to the user's 
requirements, not the driver's requirements.

> > Besides, doesn't the no_callbacks flag already do more or less what you
> > want?
> yes to some extent. But I thought its purpose is different so I added 1 more.

The purpose doesn't matter.  If no_callbacks does what you want then 
you should use it instead of adding another API.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 5070c4f..e247f08 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1252,6 +1252,24 @@  void pm_runtime_no_callbacks(struct device *dev)
 EXPORT_SYMBOL_GPL(pm_runtime_no_callbacks);
 
 /**
+ * pm_runtime_set_slave - Set these devices as slave.
+ * @dev: Device to handle.
+ *
+ * Set the power.slave flag, which tells the PM core that this device is
+ * power-managed by the master device through the runtime callbacks. Since it
+ * will not manage runtime callbacks on its own, so runtime sysfs attributes is
+ * removed.
+ */
+void pm_runtime_set_slave(struct device *dev)
+{
+	spin_lock_irq(&dev->power.lock);
+	dev->power.slave = true;
+	spin_unlock_irq(&dev->power.lock);
+	if (device_is_registered(dev))
+		rpm_sysfs_remove(dev);
+}
+
+/**
  * pm_runtime_irq_safe - Leave interrupts disabled during callbacks.
  * @dev: Device to handle
  *
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 8b59763..a4090ef 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -581,6 +581,7 @@  struct dev_pm_info {
 	unsigned int		use_autosuspend:1;
 	unsigned int		timer_autosuspends:1;
 	unsigned int		memalloc_noio:1;
+	unsigned int		slave:1;
 	enum rpm_request	request;
 	enum rpm_status		runtime_status;
 	int			runtime_error;
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 30e84d4..0707a4b 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -47,6 +47,7 @@  extern void __pm_runtime_disable(struct device *dev, bool check_resume);
 extern void pm_runtime_allow(struct device *dev);
 extern void pm_runtime_forbid(struct device *dev);
 extern void pm_runtime_no_callbacks(struct device *dev);
+extern void pm_runtime_set_slave(struct device *dev);
 extern void pm_runtime_irq_safe(struct device *dev);
 extern void __pm_runtime_use_autosuspend(struct device *dev, bool use);
 extern void pm_runtime_set_autosuspend_delay(struct device *dev, int delay);
@@ -168,6 +169,7 @@  static inline bool pm_runtime_suspended_if_enabled(struct device *dev) { return
 static inline bool pm_runtime_enabled(struct device *dev) { return false; }
 
 static inline void pm_runtime_no_callbacks(struct device *dev) {}
+static inline void pm_runtime_set_slave(struct device *dev) {}
 static inline void pm_runtime_irq_safe(struct device *dev) {}
 static inline bool pm_runtime_is_irq_safe(struct device *dev) { return false; }