qcom: Add runtime PM support

Add runtime PM support to handle (core and iface) clocks for devices without a controllable power domain. Once the drivers for these devices are converted to use runtime PM apis, all clock handling (for core and iface) from these drivers can then be removed.

Signed-off-by Rajendra Nayak <[email protected]>

---
 drivers/clk/qcom/gdsc.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

drivers/clk/qcom/gdsc.c +10 -0

index 480ebf6..92b0f6d 100644
@@ -15,6 +15,7 @@
 #include <linux/err.h>
 #include <linux/jiffies.h>
 #include <linux/pm_clock.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
 #include "gdsc.h"
 

@@ -226,3 +227,22 @@ void gdsc_unregister(struct device *dev)
 {
        of_genpd_del_provider(dev->of_node);
 }
+
+static struct dev_pm_domain default_qcom_pm_domain = {
+       .ops = {
+               USE_PM_CLK_RUNTIME_OPS
+               USE_PLATFORM_PM_SLEEP_OPS
+       },
+};
+
+static struct pm_clk_notifier_block qcom_pm_notifier = {

Quoted 39 lines · Rajendra Nayak

Add runtime PM support to handle (core and iface) clocks for devices without a controllable power domain. Once the drivers for these devices are converted to use runtime PM apis, all clock handling (for core and iface) from these drivers can then be removed.

Signed-off-by Rajendra Nayak <[email protected]>

---
  drivers/clk/qcom/gdsc.c | 20 ++++++++++++++++++++
  1 file changed, 20 insertions(+)

drivers/clk/qcom/gdsc.c +14 -0

index 480ebf6..92b0f6d 100644
@@ -15,6 +15,7 @@
  #include <linux/err.h>
  #include <linux/jiffies.h>
  #include <linux/pm_clock.h>
+#include <linux/platform_device.h>
  #include <linux/slab.h>
  #include "gdsc.h"
 

@@ -226,3 +227,22 @@ void gdsc_unregister(struct device *dev)
  {
         of_genpd_del_provider(dev->of_node);
  }
+
+static struct dev_pm_domain default_qcom_pm_domain = {
+       .ops = {
+               USE_PM_CLK_RUNTIME_OPS
+               USE_PLATFORM_PM_SLEEP_OPS
+       },
+};
+
+static struct pm_clk_notifier_block qcom_pm_notifier = {
+       .pm_domain      = &default_qcom_pm_domain,
+       .con_ids        = { "core", "iface" },
+};
+

drivers/clk/qcom/gdsc.c void gdsc_unregister(struct device *dev) · line 248

Quoted 6 lines

drivers/clk/qcom/gdsc.c +6 -0

+static int __init qcom_pm_runtime_init(void)
+{
+       pm_clk_add_notifier(&platform_bus_type, &qcom_pm_notifier);
+       return 0;
+}
+core_initcall(qcom_pm_runtime_init);

First, I don't follow how this code is related to GDSC.

Second, I want to see less users of pm_clk_add_notifier() since it's not the proper/long-term way of how to assign PM domain pointers to a device. Instead that shall be done at device registration point. In your case while parsing the DT nodes and adding devices onto to their buses.

Yes, I understand that it will requires quite some work to adopt to this behaviour - but that how it shall be done.

Finally, an important note, you don't need to use PM domains for these devices at all and thus you don't need to fix what I propose. Instead you only have to implement the runtime PM callbacks for each driver and manage the clocks from there. That is probably also a easier solution.

Quoted 5 lines

Second, I want to see less users of pm_clk_add_notifier() since it's not the proper/long-term way of how to assign PM domain pointers to a device. Instead that shall be done at device registration point. In your case while parsing the DT nodes and adding devices onto to their buses.

but these are devices which do not really have a controllable power domain, they just have controllable clocks.

Quoted 8 lines

Yes, I understand that it will requires quite some work to adopt to this behaviour - but that how it shall be done.

Finally, an important note, you don't need to use PM domains for these devices at all and thus you don't need to fix what I propose. Instead you only have to implement the runtime PM callbacks for each driver and manage the clocks from there. That is probably also a easier solution.

But that would mean I repeat the same code in all drivers to do a clk_get/prepare/enable/disable/unprepare of the same "core" and "iface" clocks. I thought we have clock_ops.c just to avoid that (atleast up until we have a better way of doing it)
And there are atleast a few architecture which have used it to avoid the duplication across all drivers (omap1/davinci/sh/keystone)

Quoted 18 lines · Rajendra Nayak

Quoted 5 lines

Second, I want to see less users of pm_clk_add_notifier() since it's not the proper/long-term way of how to assign PM domain pointers to a device. Instead that shall be done at device registration point. In your case while parsing the DT nodes and adding devices onto to their buses.

but these are devices which do not really have a controllable power domain, they just have controllable clocks.

Quoted 8 lines

Yes, I understand that it will requires quite some work to adopt to this behaviour - but that how it shall be done.

Finally, an important note, you don't need to use PM domains for these devices at all and thus you don't need to fix what I propose. Instead you only have to implement the runtime PM callbacks for each driver and manage the clocks from there. That is probably also a easier solution.

Quoted 6 lines

But that would mean I repeat the same code in all drivers to do a clk_get/prepare/enable/disable/unprepare of the same "core" and "iface" clocks. I thought we have clock_ops.c just to avoid that (atleast up until we have a better way of doing it)
And there are atleast a few architecture which have used it to avoid the duplication across all drivers (omap1/davinci/sh/keystone)

At least for arm/shmobile, we're planning to move away from this, cfr.
http://www.spinics.net/lists/linux-sh/msg41114.html

Quoted 26 lines · Geert Uytterhoeven

Quoted 18 lines · Rajendra Nayak

Quoted 5 lines

Second, I want to see less users of pm_clk_add_notifier() since it's not the proper/long-term way of how to assign PM domain pointers to a device. Instead that shall be done at device registration point. In your case while parsing the DT nodes and adding devices onto to their buses.

but these are devices which do not really have a controllable power domain, they just have controllable clocks.

Quoted 8 lines

Yes, I understand that it will requires quite some work to adopt to this behaviour - but that how it shall be done.

Finally, an important note, you don't need to use PM domains for these devices at all and thus you don't need to fix what I propose. Instead you only have to implement the runtime PM callbacks for each driver and manage the clocks from there. That is probably also a easier solution.

Quoted 6 lines

But that would mean I repeat the same code in all drivers to do a clk_get/prepare/enable/disable/unprepare of the same "core" and "iface" clocks. I thought we have clock_ops.c just to avoid that (atleast up until we have a better way of doing it)
And there are atleast a few architecture which have used it to avoid the duplication across all drivers (omap1/davinci/sh/keystone)

Quoted 2 lines

At least for arm/shmobile, we're planning to move away from this, cfr.
http://www.spinics.net/lists/linux-sh/msg41114.html

Just to clarify for Rajendra's sake...

SH is moving away from the pm_clk_add_notifier(), but not duplicating the clk_get/prepare/enable/disable/unprepare across all the drivers.

Rather, they're using a genpd to model the clock domain, and taking advantage of the pm_domain speciic attach callback to attach the PM clocks.

This should work for qcom also assuming that these device nodes don't also need to belong to a different PM domain.

Quoted 10 lines

Quoted 10 lines

Quoted 6 lines

But that would mean I repeat the same code in all drivers to do a clk_get/prepare/enable/disable/unprepare of the same "core" and "iface" clocks. I thought we have clock_ops.c just to avoid that (atleast up until we have a better way of doing it)
And there are atleast a few architecture which have used it to avoid the duplication across all drivers (omap1/davinci/sh/keystone)

At least for arm/shmobile, we're planning to move away from this, cfr.
http://www.spinics.net/lists/linux-sh/msg41114.html

Quoted 11 lines

Just to clarify for Rajendra's sake...

SH is moving away from the pm_clk_add_notifier(), but not duplicating the clk_get/prepare/enable/disable/unprepare across all the drivers.

Rather, they're using a genpd to model the clock domain, and taking advantage of the pm_domain speciic attach callback to attach the PM clocks.

This should work for qcom also assuming that these device nodes don't also need to belong to a different PM domain.

Thanks Kevin, I did look up the patches that Geert pointed me to, and figured I can do something similar for qcom as well like you said.

There are 2 types of devices that I will need to handle, one which have clocks and also a power switch to turn the power domain on/off (camera, graphics, display), and others which only have clocks and no power switch to control the power domain (serial, sdhc, i2c, spi).

I was already using genpd attach/detach to handle clocks for the devices with a power switch and genpd on/off to turn the PD on and off.

I guess I can also control the rest of the devices the same way, just that the genpd on/off for them would do nothing.
That way I don't have to use pm_clk_add_notifier() and can also associate the power domain (with no on/off control) to devices through DT (and there isn;t any duplication of code in the drivers)

Quoted 5 lines

I guess I can also control the rest of the devices the same way, just that the genpd on/off for them would do nothing.
That way I don't have to use pm_clk_add_notifier() and can also associate the power domain (with no on/off control) to devices through DT (and there isn;t any duplication of code in the drivers)

That looks similar to what we have on R-Mobile: some devices are in controllable power areas, other are in an "always on" power area. All (most) devices have controllable clocks, which we also control through the PM domain. "git grep sysc-rmobile" will point you to the related code and DTS.

Quoted 7 lines

Quoted 5 lines

I guess I can also control the rest of the devices the same way, just that the genpd on/off for them would do nothing.
That way I don't have to use pm_clk_add_notifier() and can also associate the power domain (with no on/off control) to devices through DT (and there isn;t any duplication of code in the drivers)

Quoted 6 lines

That looks similar to what we have on R-Mobile: some devices are in controllable power areas, other are in an "always on" power area. All (most)
devices have controllable clocks, which we also control through the PM domain. "git grep sysc-rmobile" will point you to the related code and DTS.

Geert, thanks, I was wondering how you handle the !CONFIG_PM case for rmobile. I mean who turns the clocks on for the devices when you build with CONFIG_PM disabled?

Hi Rajendra,

Quoted 14 lines · Rajendra Nayak

Quoted 7 lines

Quoted 5 lines

I guess I can also control the rest of the devices the same way, just that the genpd on/off for them would do nothing.
That way I don't have to use pm_clk_add_notifier() and can also associate the power domain (with no on/off control) to devices through DT (and there isn;t any duplication of code in the drivers)

Quoted 6 lines

That looks similar to what we have on R-Mobile: some devices are in controllable power areas, other are in an "always on" power area. All (most)
devices have controllable clocks, which we also control through the PM domain. "git grep sysc-rmobile" will point you to the related code and DTS.

Quoted 3 lines

Geert, thanks, I was wondering how you handle the !CONFIG_PM case for rmobile. I mean who turns the clocks on for the devices when you build with CONFIG_PM disabled?

We still use pm_clk_add_notifier() in drivers/sh/pm_runtime.c if CONFIG_PM_GENERIC_DOMAINS_OF=n. Hence the second instance of pm_clk_notify() will enable the clocks at driver binding time. Hardware power domains are assumed enabled by reset state/the boot loader.

Given the amount of PM infrastructure involved when hardware power and clock domains are involved, I think at one point we'll have to start restricting our builds to CONFIG_PM=y.

Quoted 9 lines

Quoted 3 lines

Geert, thanks, I was wondering how you handle the !CONFIG_PM case for rmobile. I mean who turns the clocks on for the devices when you build with CONFIG_PM disabled?

We still use pm_clk_add_notifier() in drivers/sh/pm_runtime.c if CONFIG_PM_GENERIC_DOMAINS_OF=n. Hence the second instance of pm_clk_notify() will enable the clocks at driver binding time. Hardware power domains are assumed enabled by reset state/the boot loader.

Yes, it a bit tricky - but I guess that's the current only viable solution if we have when using the pm_clk API.

Quoted 3 lines

Given the amount of PM infrastructure involved when hardware power and clock domains are involved, I think at one point we'll have to start restricting our builds to CONFIG_PM=y.

I don't think that would solve the problem, since you may still have cross SoC drivers which may be used without CONFIG_PM.

So all SoC that uses a driver which expects clocks to be managed using PM clocks from a PM domain, will need the above "trick". Right?

Hi Ulf,

Quoted 17 lines · Ulf Hansson

Quoted 9 lines

Quoted 3 lines

Geert, thanks, I was wondering how you handle the !CONFIG_PM case for rmobile. I mean who turns the clocks on for the devices when you build with CONFIG_PM disabled?

We still use pm_clk_add_notifier() in drivers/sh/pm_runtime.c if CONFIG_PM_GENERIC_DOMAINS_OF=n. Hence the second instance of pm_clk_notify() will enable the clocks at driver binding time. Hardware power domains are assumed enabled by reset state/the boot loader.

Yes, it a bit tricky - but I guess that's the current only viable solution if we have when using the pm_clk API.

Quoted 3 lines

Given the amount of PM infrastructure involved when hardware power and clock domains are involved, I think at one point we'll have to start restricting our builds to CONFIG_PM=y.

Quoted 2 lines

I don't think that would solve the problem, since you may still have cross SoC drivers which may be used without CONFIG_PM.

That's not as much of a problem as it sounds:
- If the driver cares (needs to know) about the clock, it should already manage it itself.

- If it doesn't care about the clock, it just needs Runtime PM support. That will do the right thing on platforms with (needs PM=y) and without (doesn't care about PM=x) clock domains.

So the bigger "problem" is making sure all drivers have at least minimal Runtime PM support, else they can't be reused as-is on systems with PM domains.

Quoted 2 lines

So all SoC that uses a driver which expects clocks to be managed using PM clocks from a PM domain, will need the above "trick". Right?

One remaining issue with pm_clk_add_notifier() is that it applies to all platform devices in the system, not just the on-SoC devices. Hence it may inadvertently manage clocks for off-SoC devices it's not supposed to touch.

Quoted 20 lines · Geert Uytterhoeven

Hi Ulf,

Quoted 17 lines · Ulf Hansson

Quoted 9 lines

Quoted 3 lines

Geert, thanks, I was wondering how you handle the !CONFIG_PM case for rmobile. I mean who turns the clocks on for the devices when you build with CONFIG_PM disabled?

We still use pm_clk_add_notifier() in drivers/sh/pm_runtime.c if CONFIG_PM_GENERIC_DOMAINS_OF=n. Hence the second instance of pm_clk_notify() will enable the clocks at driver binding time. Hardware power domains are assumed enabled by reset state/the boot loader.

Yes, it a bit tricky - but I guess that's the current only viable solution if we have when using the pm_clk API.

Quoted 3 lines

Given the amount of PM infrastructure involved when hardware power and clock domains are involved, I think at one point we'll have to start restricting our builds to CONFIG_PM=y.

Quoted 6 lines

Quoted 2 lines

I don't think that would solve the problem, since you may still have cross SoC drivers which may be used without CONFIG_PM.

That's not as much of a problem as it sounds:
- If the driver cares (needs to know) about the clock, it should already manage it itself.

Agree!

Quoted 3 lines

- If it doesn't care about the clock, it just needs Runtime PM support. That will do the right thing on platforms with (needs PM=y) and without (doesn't care about PM=x) clock domains.

How about those drivers that cares about clocks and thus manages them, but also cares about runtime PM?

I believe we will get a clock reference count issue in these cases, since both the PM domain and the driver will manage the clocks.

I assume that's why we have had a few attempts to have "runtime PM clocks" specially marked, one was via DT, to have clear distinguish between who will be responsible to manage them.

Those attempts did get nacked, so the problem is still there I assume.

Quoted 7 lines

So the bigger "problem" is making sure all drivers have at least minimal Runtime PM support, else they can't be reused as-is on systems with PM domains.

Quoted 2 lines

So all SoC that uses a driver which expects clocks to be managed using PM clocks from a PM domain, will need the above "trick". Right?

Quoted 3 lines

One remaining issue with pm_clk_add_notifier() is that it applies to all platform devices in the system, not just the on-SoC devices. Hence it may inadvertently manage clocks for off-SoC devices it's not supposed to touch.

Yes. That's really bad. :-)

Additionally, it means devices that isn't part of the platform bus isn't able to use PM clk domains at all.

Within this context, I find it hard to advise people to use PM clk domains (via pm_clk_add_notifier()), since there just so many open issues. What works a _little_ better is to use PM clks via genpd.

Kind regards
Uffe

Quoted 43 lines · Ulf Hansson

Quoted 41 lines · Rajendra Nayak

Add runtime PM support to handle (core and iface) clocks for devices without a controllable power domain. Once the drivers for these devices are converted to use runtime PM apis, all clock handling (for core and iface) from these drivers can then be removed.

Signed-off-by Rajendra Nayak <[email protected]>

---
  drivers/clk/qcom/gdsc.c | 20 ++++++++++++++++++++
  1 file changed, 20 insertions(+)

drivers/clk/qcom/gdsc.c +15 -0

index 480ebf6..92b0f6d 100644
@@ -15,6 +15,7 @@
  #include <linux/err.h>
  #include <linux/jiffies.h>
  #include <linux/pm_clock.h>
+#include <linux/platform_device.h>
  #include <linux/slab.h>
  #include "gdsc.h"
 

@@ -226,3 +227,22 @@ void gdsc_unregister(struct device *dev)
  {
         of_genpd_del_provider(dev->of_node);
  }
+
+static struct dev_pm_domain default_qcom_pm_domain = {
+       .ops = {
+               USE_PM_CLK_RUNTIME_OPS
+               USE_PLATFORM_PM_SLEEP_OPS
+       },
+};
+
+static struct pm_clk_notifier_block qcom_pm_notifier = {
+       .pm_domain      = &default_qcom_pm_domain,
+       .con_ids        = { "core", "iface" },
+};
+
+static int __init qcom_pm_runtime_init(void)

Quoted 8 lines

drivers/clk/qcom/gdsc.c void gdsc_unregister(struct device *dev) · line 248

Quoted 5 lines

drivers/clk/qcom/gdsc.c +5 -0

+{
+       pm_clk_add_notifier(&platform_bus_type, &qcom_pm_notifier);
+       return 0;
+}
+core_initcall(qcom_pm_runtime_init);

First, I don't follow how this code is related to GDSC.

Actually its not. I should probably move it to a pm_runtime.c someplace in drivers/soc/qcom maybe.

Quoted 6 lines

Second, I want to see less users of pm_clk_add_notifier() since it's not the proper/long-term way of how to assign PM domain pointers to a device. Instead that shall be done at device registration point. In your case while parsing the DT nodes and adding devices onto to their buses.

but these are devices which do not really have a controllable power domain, they just have controllable clocks.

Quoted 10 lines

Yes, I understand that it will requires quite some work to adopt to this behaviour - but that how it shall be done.

Finally, an important note, you don't need to use PM domains for these devices at all and thus you don't need to fix what I propose. Instead you only have to implement the runtime PM callbacks for each driver and manage the clocks from there. That is probably also a easier solution.

But that would mean I repeat the same code in all drivers to do a clk_get/prepare/enable/disable/unprepare of the same "core" and "iface" clocks. I thought we have clock_ops.c just to avoid that (atleast up until we have a better way of doing it)
And there are atleast a few architecture which have used it to avoid the duplication across all drivers (omap1/davinci/sh/keystone)

With platform support now in place to manage clocks from within runtime PM callbacks, get rid of all clock handling from the driver and convert the driver to use runtime PM apis.

Signed-off-by Rajendra Nayak <[email protected]>

Cc <[email protected]>

---
 drivers/tty/serial/msm_serial.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

drivers/tty/serial/msm_serial.c +16 -17

index b73889c..eb66502 100644
@@ -33,6 +33,7 @@
 #include <linux/serial.h>
 #include <linux/clk.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/delay.h>
 #include <linux/of.h>
 #include <linux/of_device.h>

@@ -50,11 +51,11 @@ struct msm_port {
 	struct uart_port	uart;
 	char			name[16];
 	struct clk		*clk;
-	struct clk		*pclk;
 	unsigned int		imr;
 	int			is_uartdm;
 	unsigned int		old_snap_state;
 	bool			break_detected;
+	struct device		*dev;
 };
 
 static inline void wait_for_xmitr(struct uart_port *port)

@@ -464,12 +465,11 @@ static int msm_set_baud_rate(struct uart_port *port, unsigned int baud)
 	return baud;
 }
 
-static void msm_init_clock(struct uart_port *port)
+static void msm_init(struct uart_port *port)
 {
 	struct msm_port *msm_port = UART_TO_MSM(port);
 
-	clk_prepare_enable(msm_port->clk);
-	clk_prepare_enable(msm_port->pclk);
+	pm_runtime_get_sync(msm_port->dev);
 	msm_serial_set_mnd_regs(port);
 }
 

@@ -487,7 +487,7 @@ static int msm_startup(struct uart_port *port)
 	if (unlikely(ret))
 		return ret;
 
-	msm_init_clock(port);
+	msm_init(port);
 
 	if (likely(port->fifosize > 12))
 		rfr_level = port->fifosize - 12;

@@ -511,7 +511,7 @@ static void msm_shutdown(struct uart_port *port)
 	msm_port->imr = 0;
 	msm_write(port, 0, UART_IMR); /* disable interrupts */
 
-	clk_disable_unprepare(msm_port->clk);
+	pm_runtime_put_sync(msm_port->dev);
 
 	free_irq(port->irq, port);
 }

@@ -669,12 +669,10 @@ static void msm_power(struct uart_port *port, unsigned int state,
 
 	switch (state) {
 	case 0:
-		clk_prepare_enable(msm_port->clk);
-		clk_prepare_enable(msm_port->pclk);
+		pm_runtime_get_sync(msm_port->dev);
 		break;
 	case 3:
-		clk_disable_unprepare(msm_port->clk);
-		clk_disable_unprepare(msm_port->pclk);
+		pm_runtime_put_sync(msm_port->dev);
 		break;
 	default:
 		pr_err("msm_serial: Unknown PM state %d\n", state);

@@ -933,7 +931,7 @@ static int __init msm_console_setup(struct console *co, char *options)
 	if (unlikely(!port->membase))
 		return -ENXIO;
 
-	msm_init_clock(port);
+	msm_init(port);
 
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);

@@ -1057,13 +1055,10 @@ static int msm_serial_probe(struct platform_device *pdev)
 	if (IS_ERR(msm_port->clk))
 		return PTR_ERR(msm_port->clk);
 
-	if (msm_port->is_uartdm) {
-		msm_port->pclk = devm_clk_get(&pdev->dev, "iface");
-		if (IS_ERR(msm_port->pclk))
-			return PTR_ERR(msm_port->pclk);
-
+	if (msm_port->is_uartdm)
 		clk_set_rate(msm_port->clk, 1843200);
-	}
+
+	msm_port->dev = &pdev->dev;
 
 	port->uartclk = clk_get_rate(msm_port->clk);
 	dev_info(&pdev->dev, "uartclk = %d\n", port->uartclk);

@@ -1080,13 +1075,17 @@ static int msm_serial_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, port);
 
+	pm_runtime_enable(&pdev->dev);
+
 	return uart_add_one_port(&msm_uart_driver, port);
 }
 
 static int msm_serial_remove(struct platform_device *pdev)
 {
 	struct uart_port *port = platform_get_drvdata(pdev);
+	struct msm_port *msm_port = UART_TO_MSM(port);
 
+	pm_runtime_disable(msm_port->dev);
 	uart_remove_one_port(&msm_uart_driver, port);
 
 	return 0;

Quoted 6 lines · Rajendra Nayak

With platform support now in place to manage clocks from within runtime PM callbacks, get rid of all clock handling from the driver and convert the driver to use runtime PM apis.

Signed-off-by Rajendra Nayak <[email protected]>

Cc <[email protected]>

Pramod is changing the same file in the same area. Please work together if necessary.

-Stephen

Quoted 121 lines

---
  drivers/tty/serial/msm_serial.c | 33 ++++++++++++++++-----------------
  1 file changed, 16 insertions(+), 17 deletions(-)

drivers/tty/serial/msm_serial.c +16 -17

index b73889c..eb66502 100644
@@ -33,6 +33,7 @@
  #include <linux/serial.h>
  #include <linux/clk.h>
  #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
  #include <linux/delay.h>
  #include <linux/of.h>
  #include <linux/of_device.h>

@@ -50,11 +51,11 @@ struct msm_port {
  	struct uart_port	uart;
  	char			name[16];
  	struct clk		*clk;
-	struct clk		*pclk;
  	unsigned int		imr;
  	int			is_uartdm;
  	unsigned int		old_snap_state;
  	bool			break_detected;
+	struct device		*dev;
  };
 
  static inline void wait_for_xmitr(struct uart_port *port)

@@ -464,12 +465,11 @@ static int msm_set_baud_rate(struct uart_port *port, unsigned int baud)
  	return baud;
  }
 
-static void msm_init_clock(struct uart_port *port)
+static void msm_init(struct uart_port *port)
  {
  	struct msm_port *msm_port = UART_TO_MSM(port);
 
-	clk_prepare_enable(msm_port->clk);
-	clk_prepare_enable(msm_port->pclk);
+	pm_runtime_get_sync(msm_port->dev);
  	msm_serial_set_mnd_regs(port);
  }
 

@@ -487,7 +487,7 @@ static int msm_startup(struct uart_port *port)
  	if (unlikely(ret))
  		return ret;
 
-	msm_init_clock(port);
+	msm_init(port);
 
  	if (likely(port->fifosize > 12))
  		rfr_level = port->fifosize - 12;

@@ -511,7 +511,7 @@ static void msm_shutdown(struct uart_port *port)
  	msm_port->imr = 0;
  	msm_write(port, 0, UART_IMR); /* disable interrupts */
 
-	clk_disable_unprepare(msm_port->clk);
+	pm_runtime_put_sync(msm_port->dev);
 
  	free_irq(port->irq, port);
  }

@@ -669,12 +669,10 @@ static void msm_power(struct uart_port *port, unsigned int state,
 
  	switch (state) {
  	case 0:
-		clk_prepare_enable(msm_port->clk);
-		clk_prepare_enable(msm_port->pclk);
+		pm_runtime_get_sync(msm_port->dev);
  		break;
  	case 3:
-		clk_disable_unprepare(msm_port->clk);
-		clk_disable_unprepare(msm_port->pclk);
+		pm_runtime_put_sync(msm_port->dev);
  		break;
  	default:
  		pr_err("msm_serial: Unknown PM state %d\n", state);

@@ -933,7 +931,7 @@ static int __init msm_console_setup(struct console *co, char *options)
  	if (unlikely(!port->membase))
  		return -ENXIO;
 
-	msm_init_clock(port);
+	msm_init(port);
 
  	if (options)
  		uart_parse_options(options, &baud, &parity, &bits, &flow);

@@ -1057,13 +1055,10 @@ static int msm_serial_probe(struct platform_device *pdev)
  	if (IS_ERR(msm_port->clk))
  		return PTR_ERR(msm_port->clk);
 
-	if (msm_port->is_uartdm) {
-		msm_port->pclk = devm_clk_get(&pdev->dev, "iface");
-		if (IS_ERR(msm_port->pclk))
-			return PTR_ERR(msm_port->pclk);
-
+	if (msm_port->is_uartdm)
  		clk_set_rate(msm_port->clk, 1843200);
-	}
+
+	msm_port->dev = &pdev->dev;
 
  	port->uartclk = clk_get_rate(msm_port->clk);
  	dev_info(&pdev->dev, "uartclk = %d\n", port->uartclk);

@@ -1080,13 +1075,17 @@ static int msm_serial_probe(struct platform_device *pdev)
 
  	platform_set_drvdata(pdev, port);
 
+	pm_runtime_enable(&pdev->dev);
+
  	return uart_add_one_port(&msm_uart_driver, port);
  }
 
  static int msm_serial_remove(struct platform_device *pdev)
  {
  	struct uart_port *port = platform_get_drvdata(pdev);
+	struct msm_port *msm_port = UART_TO_MSM(port);
 
+	pm_runtime_disable(msm_port->dev);
  	uart_remove_one_port(&msm_uart_driver, port);
 
  	return 0;

With platform support now in place to manage clocks from within runtime PM callbacks, get rid of all clock handling from the driver and convert the driver to use runtime PM apis.

Signed-off-by Rajendra Nayak <[email protected]>

Cc <[email protected]>

---
 drivers/mmc/host/sdhci-msm.c | 46 +++++++++++---------------------------------
 1 file changed, 11 insertions(+), 35 deletions(-)

drivers/mmc/host/sdhci-msm.c +11 -36

index 4a09f76..3c62a77 100644
@@ -19,6 +19,7 @@
 #include <linux/delay.h>
 #include <linux/mmc/mmc.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
 
 #include "sdhci-pltfm.h"
 

@@ -56,8 +57,6 @@
 struct sdhci_msm_host {
        struct platform_device *pdev;
        void __iomem *core_mem; /* MSM SDCC mapped address */
-       struct clk *clk;        /* main SD/MMC bus clock */
-       struct clk *pclk;       /* SDHC peripheral bus clock */
        struct clk *bus_clk;    /* SDHC bus voter clock */
        struct mmc_host *mmc;
        struct sdhci_pltfm_data sdhci_msm_pdata;

@@ -469,29 +468,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
                        goto pltfm_free;
        }
 
-       /* Setup main peripheral bus clock */
-       msm_host->pclk = devm_clk_get(&pdev->dev, "iface");
-       if (IS_ERR(msm_host->pclk)) {
-               ret = PTR_ERR(msm_host->pclk);
-               dev_err(&pdev->dev, "Perpheral clk setup failed (%d)\n", ret);
-               goto bus_clk_disable;
-       }
-
-       ret = clk_prepare_enable(msm_host->pclk);
-       if (ret)
-               goto bus_clk_disable;
-
-       /* Setup SDC MMC clock */
-       msm_host->clk = devm_clk_get(&pdev->dev, "core");
-       if (IS_ERR(msm_host->clk)) {
-               ret = PTR_ERR(msm_host->clk);
-               dev_err(&pdev->dev, "SDC MMC clk setup failed (%d)\n", ret);
-               goto pclk_disable;
-       }
-
-       ret = clk_prepare_enable(msm_host->clk);
-       if (ret)
-               goto pclk_disable;
+       pm_runtime_enable(&pdev->dev);
+       pm_runtime_get_sync(&pdev->dev);
 
        core_memres = platform_get_resource(pdev, IORESOURCE_MEM, 1);
        msm_host->core_mem = devm_ioremap_resource(&pdev->dev, core_memres);

@@ -499,7 +477,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
        if (IS_ERR(msm_host->core_mem)) {
                dev_err(&pdev->dev, "Failed to remap registers\n");
                ret = PTR_ERR(msm_host->core_mem);
-               goto clk_disable;
+               goto err;
        }
 
        /* Reset the core and Enable SDHC mode */

@@ -511,7 +489,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
        if (readl(msm_host->core_mem + CORE_POWER) & CORE_SW_RST) {
                dev_err(&pdev->dev, "Stuck in reset\n");
                ret = -ETIMEDOUT;
-               goto clk_disable;
+               goto err;
        }
 
        /* Set HC_MODE_EN bit in HC_MODE register */

@@ -545,15 +523,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 
        ret = sdhci_add_host(host);
        if (ret)
-               goto clk_disable;
+               goto err;
 
        return 0;
 
-clk_disable:
-       clk_disable_unprepare(msm_host->clk);
-pclk_disable:
-       clk_disable_unprepare(msm_host->pclk);
-bus_clk_disable:
+err:
+       pm_runtime_put_sync(&pdev->dev);
+       pm_runtime_disable(&pdev->dev);
        if (!IS_ERR(msm_host->bus_clk))
                clk_disable_unprepare(msm_host->bus_clk);
 pltfm_free:

@@ -571,8 +547,8 @@ static int sdhci_msm_remove(struct platform_device *pdev)
 
        sdhci_remove_host(host, dead);
        sdhci_pltfm_free(pdev);
-       clk_disable_unprepare(msm_host->clk);
-       clk_disable_unprepare(msm_host->pclk);
+       pm_runtime_put_sync(&pdev->dev);
+       pm_runtime_disable(&pdev->dev);
        if (!IS_ERR(msm_host->bus_clk))
                clk_disable_unprepare(msm_host->bus_clk);
        return 0;
--

QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

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

Quoted 127 lines

With platform support now in place to manage clocks from within runtime PM
callbacks, get rid of all clock handling from the driver and convert the driver to use runtime PM apis.

Signed-off-by Rajendra Nayak <[email protected]>

Cc <[email protected]>

---
 drivers/mmc/host/sdhci-msm.c | 46
+++++++++++---------------------------------
 1 file changed, 11 insertions(+), 35 deletions(-)

drivers/mmc/host/sdhci-msm.c +3 -25

index 4a09f76..3c62a77 100644
@@ -19,6 +19,7 @@
 #include <linux/delay.h>
 #include <linux/mmc/mmc.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
 
 #include "sdhci-pltfm.h"
 

@@ -56,8 +57,6 @@
 struct sdhci_msm_host {
        struct platform_device *pdev;
        void __iomem *core_mem; /* MSM SDCC mapped address */
-       struct clk *clk;        /* main SD/MMC bus clock */
-       struct clk *pclk;       /* SDHC peripheral bus clock */
        struct clk *bus_clk;    /* SDHC bus voter clock */
        struct mmc_host *mmc;
        struct sdhci_pltfm_data sdhci_msm_pdata;

@@ -469,29 +468,8 @@ static int sdhci_msm_probe(struct platform_device
 *pdev)
                        goto pltfm_free;
        }
 
-       /* Setup main peripheral bus clock */
-       msm_host->pclk = devm_clk_get(&pdev->dev, "iface");
-       if (IS_ERR(msm_host->pclk)) {
-               ret = PTR_ERR(msm_host->pclk);
-               dev_err(&pdev->dev, "Perpheral clk setup failed (%d)\n",
 ret);
-               goto bus_clk_disable;
-       }
-
-       ret = clk_prepare_enable(msm_host->pclk);
-       if (ret)
-               goto bus_clk_disable;
-
-       /* Setup SDC MMC clock */
-       msm_host->clk = devm_clk_get(&pdev->dev, "core");
-       if (IS_ERR(msm_host->clk)) {
-               ret = PTR_ERR(msm_host->clk);
-               dev_err(&pdev->dev, "SDC MMC clk setup failed (%d)\n",
 ret);
-               goto pclk_disable;
-       }
-
-       ret = clk_prepare_enable(msm_host->clk);
-       if (ret)
-               goto pclk_disable;
+       pm_runtime_enable(&pdev->dev);
+       pm_runtime_get_sync(&pdev->dev);

        core_memres = platform_get_resource(pdev, IORESOURCE_MEM, 1);
        msm_host->core_mem = devm_ioremap_resource(&pdev->dev,

core_memres);

drivers/mmc/host/sdhci-msm.c +1 -1

@@ -499,7 +477,7 @@ static int sdhci_msm_probe(struct platform_device
 *pdev)
        if (IS_ERR(msm_host->core_mem)) {
                dev_err(&pdev->dev, "Failed to remap registers\n");
                ret = PTR_ERR(msm_host->core_mem);
-               goto clk_disable;
+               goto err;
        }
 

        /* Reset the core and Enable SDHC mode */

drivers/mmc/host/sdhci-msm.c +1 -1

@@ -511,7 +489,7 @@ static int sdhci_msm_probe(struct platform_device
 *pdev)
        if (readl(msm_host->core_mem + CORE_POWER) & CORE_SW_RST) {
                dev_err(&pdev->dev, "Stuck in reset\n");
                ret = -ETIMEDOUT;
-               goto clk_disable;
+               goto err;
        }
 

        /* Set HC_MODE_EN bit in HC_MODE register */

drivers/mmc/host/sdhci-msm.c +4 -6

@@ -545,15 +523,13 @@ static int sdhci_msm_probe(struct platform_device
 *pdev)
 
        ret = sdhci_add_host(host);
        if (ret)
-               goto clk_disable;
+               goto err;
 
        return 0;
 
-clk_disable:
-       clk_disable_unprepare(msm_host->clk);
-pclk_disable:
-       clk_disable_unprepare(msm_host->pclk);
-bus_clk_disable:
+err:
+       pm_runtime_put_sync(&pdev->dev);
+       pm_runtime_disable(&pdev->dev);
        if (!IS_ERR(msm_host->bus_clk))
                clk_disable_unprepare(msm_host->bus_clk);

pltfm_free:

drivers/mmc/host/sdhci-msm.c +2 -2

@@ -571,8 +547,8 @@ static int sdhci_msm_remove(struct platform_device
 *pdev)
 
        sdhci_remove_host(host, dead);
        sdhci_pltfm_free(pdev);
-       clk_disable_unprepare(msm_host->clk);
-       clk_disable_unprepare(msm_host->pclk);
+       pm_runtime_put_sync(&pdev->dev);
+       pm_runtime_disable(&pdev->dev);
        if (!IS_ERR(msm_host->bus_clk))
                clk_disable_unprepare(msm_host->bus_clk);

        return 0;

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Quoted 4 lines

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

This all looks wrong. The driver will no longer work unless CONFIG_PM is set.

Kind regards
Uffe

Quoted 128 lines · Rajendra Nayak

Quoted 127 lines

With platform support now in place to manage clocks from within runtime PM
callbacks, get rid of all clock handling from the driver and convert the driver to use runtime PM apis.

Signed-off-by Rajendra Nayak <[email protected]>

Cc <[email protected]>

---
 drivers/mmc/host/sdhci-msm.c | 46
+++++++++++---------------------------------
 1 file changed, 11 insertions(+), 35 deletions(-)

drivers/mmc/host/sdhci-msm.c +3 -25

index 4a09f76..3c62a77 100644
@@ -19,6 +19,7 @@
 #include <linux/delay.h>
 #include <linux/mmc/mmc.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
 
 #include "sdhci-pltfm.h"
 

@@ -56,8 +57,6 @@
 struct sdhci_msm_host {
        struct platform_device *pdev;
        void __iomem *core_mem; /* MSM SDCC mapped address */
-       struct clk *clk;        /* main SD/MMC bus clock */
-       struct clk *pclk;       /* SDHC peripheral bus clock */
        struct clk *bus_clk;    /* SDHC bus voter clock */
        struct mmc_host *mmc;
        struct sdhci_pltfm_data sdhci_msm_pdata;

@@ -469,29 +468,8 @@ static int sdhci_msm_probe(struct platform_device
 *pdev)
                        goto pltfm_free;
        }
 
-       /* Setup main peripheral bus clock */
-       msm_host->pclk = devm_clk_get(&pdev->dev, "iface");
-       if (IS_ERR(msm_host->pclk)) {
-               ret = PTR_ERR(msm_host->pclk);
-               dev_err(&pdev->dev, "Perpheral clk setup failed (%d)\n",
 ret);
-               goto bus_clk_disable;
-       }
-
-       ret = clk_prepare_enable(msm_host->pclk);
-       if (ret)
-               goto bus_clk_disable;
-
-       /* Setup SDC MMC clock */
-       msm_host->clk = devm_clk_get(&pdev->dev, "core");
-       if (IS_ERR(msm_host->clk)) {
-               ret = PTR_ERR(msm_host->clk);
-               dev_err(&pdev->dev, "SDC MMC clk setup failed (%d)\n",
 ret);
-               goto pclk_disable;
-       }
-
-       ret = clk_prepare_enable(msm_host->clk);
-       if (ret)
-               goto pclk_disable;
+       pm_runtime_enable(&pdev->dev);
+       pm_runtime_get_sync(&pdev->dev);

        core_memres = platform_get_resource(pdev, IORESOURCE_MEM, 1);
        msm_host->core_mem = devm_ioremap_resource(&pdev->dev,

core_memres);

drivers/mmc/host/sdhci-msm.c +1 -1

@@ -499,7 +477,7 @@ static int sdhci_msm_probe(struct platform_device
 *pdev)
        if (IS_ERR(msm_host->core_mem)) {
                dev_err(&pdev->dev, "Failed to remap registers\n");
                ret = PTR_ERR(msm_host->core_mem);
-               goto clk_disable;
+               goto err;
        }
 

        /* Reset the core and Enable SDHC mode */

drivers/mmc/host/sdhci-msm.c +1 -1

@@ -511,7 +489,7 @@ static int sdhci_msm_probe(struct platform_device
 *pdev)
        if (readl(msm_host->core_mem + CORE_POWER) & CORE_SW_RST) {
                dev_err(&pdev->dev, "Stuck in reset\n");
                ret = -ETIMEDOUT;
-               goto clk_disable;
+               goto err;
        }
 

        /* Set HC_MODE_EN bit in HC_MODE register */

drivers/mmc/host/sdhci-msm.c +4 -6

@@ -545,15 +523,13 @@ static int sdhci_msm_probe(struct platform_device
 *pdev)
 
        ret = sdhci_add_host(host);
        if (ret)
-               goto clk_disable;
+               goto err;
 
        return 0;
 
-clk_disable:
-       clk_disable_unprepare(msm_host->clk);
-pclk_disable:
-       clk_disable_unprepare(msm_host->pclk);
-bus_clk_disable:
+err:
+       pm_runtime_put_sync(&pdev->dev);
+       pm_runtime_disable(&pdev->dev);
        if (!IS_ERR(msm_host->bus_clk))
                clk_disable_unprepare(msm_host->bus_clk);

pltfm_free:

drivers/mmc/host/sdhci-msm.c +2 -2

@@ -571,8 +547,8 @@ static int sdhci_msm_remove(struct platform_device
 *pdev)
 
        sdhci_remove_host(host, dead);
        sdhci_pltfm_free(pdev);
-       clk_disable_unprepare(msm_host->clk);
-       clk_disable_unprepare(msm_host->pclk);
+       pm_runtime_put_sync(&pdev->dev);
+       pm_runtime_disable(&pdev->dev);
        if (!IS_ERR(msm_host->bus_clk))
                clk_disable_unprepare(msm_host->bus_clk);

        return 0;

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Quoted 7 lines

Quoted 4 lines

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

This all looks wrong. The driver will no longer work unless CONFIG_PM is set.

Right, I seem to have completely ignored the !CONFIG_PM case. Will look at how to handle that.

Quoted 120 lines · Rajendra Nayak

Quoted 120 lines

Quoted 118 lines · Rajendra Nayak

With platform support now in place to manage clocks from within runtime
PM
callbacks, get rid of all clock handling from the driver and convert the
driver to use runtime PM apis.

Signed-off-by Rajendra Nayak <[email protected]>

Cc <[email protected]>

---
 drivers/mmc/host/sdhci-msm.c | 46
+++++++++++---------------------------------
 1 file changed, 11 insertions(+), 35 deletions(-)

drivers/mmc/host/sdhci-msm.c +3 -25

index 4a09f76..3c62a77 100644
@@ -19,6 +19,7 @@
 #include <linux/delay.h>
 #include <linux/mmc/mmc.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
 
 #include "sdhci-pltfm.h"
 

@@ -56,8 +57,6 @@
 struct sdhci_msm_host {
        struct platform_device *pdev;
        void __iomem *core_mem; /* MSM SDCC mapped address */
-       struct clk *clk;        /* main SD/MMC bus clock */
-       struct clk *pclk;       /* SDHC peripheral bus clock */
        struct clk *bus_clk;    /* SDHC bus voter clock */
        struct mmc_host *mmc;
        struct sdhci_pltfm_data sdhci_msm_pdata;

@@ -469,29 +468,8 @@ static int sdhci_msm_probe(struct platform_device
 *pdev)
                        goto pltfm_free;
        }
 
-       /* Setup main peripheral bus clock */
-       msm_host->pclk = devm_clk_get(&pdev->dev, "iface");
-       if (IS_ERR(msm_host->pclk)) {
-               ret = PTR_ERR(msm_host->pclk);
-               dev_err(&pdev->dev, "Perpheral clk setup failed
 (%d)\n",
 ret);
-               goto bus_clk_disable;
-       }
-
-       ret = clk_prepare_enable(msm_host->pclk);
-       if (ret)
-               goto bus_clk_disable;
-
-       /* Setup SDC MMC clock */
-       msm_host->clk = devm_clk_get(&pdev->dev, "core");
-       if (IS_ERR(msm_host->clk)) {
-               ret = PTR_ERR(msm_host->clk);
-               dev_err(&pdev->dev, "SDC MMC clk setup failed (%d)\n",
 ret);
-               goto pclk_disable;
-       }
-
-       ret = clk_prepare_enable(msm_host->clk);
-       if (ret)
-               goto pclk_disable;
+       pm_runtime_enable(&pdev->dev);
+       pm_runtime_get_sync(&pdev->dev);
 
        core_memres = platform_get_resource(pdev, IORESOURCE_MEM, 1);
        msm_host->core_mem = devm_ioremap_resource(&pdev->dev,

core_memres);

drivers/mmc/host/sdhci-msm.c +1 -1

@@ -499,7 +477,7 @@ static int sdhci_msm_probe(struct platform_device
 *pdev)
        if (IS_ERR(msm_host->core_mem)) {
                dev_err(&pdev->dev, "Failed to remap registers\n");
                ret = PTR_ERR(msm_host->core_mem);
-               goto clk_disable;
+               goto err;
        }
 

        /* Reset the core and Enable SDHC mode */

drivers/mmc/host/sdhci-msm.c +1 -1

@@ -511,7 +489,7 @@ static int sdhci_msm_probe(struct platform_device
 *pdev)
        if (readl(msm_host->core_mem + CORE_POWER) & CORE_SW_RST) {
                dev_err(&pdev->dev, "Stuck in reset\n");
                ret = -ETIMEDOUT;
-               goto clk_disable;
+               goto err;
        }
 

        /* Set HC_MODE_EN bit in HC_MODE register */

drivers/mmc/host/sdhci-msm.c +0 -0

@@ -545,15 +523,13 @@ static int sdhci_msm_probe(struct

platform_device
*pdev)

        ret = sdhci_add_host(host);
        if (ret)

drivers/mmc/host/sdhci-msm.c +4 -6

-               goto clk_disable;
+               goto err;
 
        return 0;
 
-clk_disable:
-       clk_disable_unprepare(msm_host->clk);
-pclk_disable:
-       clk_disable_unprepare(msm_host->pclk);
-bus_clk_disable:
+err:
+       pm_runtime_put_sync(&pdev->dev);
+       pm_runtime_disable(&pdev->dev);
        if (!IS_ERR(msm_host->bus_clk))
                clk_disable_unprepare(msm_host->bus_clk);
 pltfm_free:

@@ -571,8 +547,8 @@ static int sdhci_msm_remove(struct platform_device
 *pdev)
 

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

drivers/mmc/host/sdhci-msm.c +2 -2

        sdhci_remove_host(host, dead);
        sdhci_pltfm_free(pdev);
-       clk_disable_unprepare(msm_host->clk);
-       clk_disable_unprepare(msm_host->pclk);
+       pm_runtime_put_sync(&pdev->dev);
+       pm_runtime_disable(&pdev->dev);
        if (!IS_ERR(msm_host->bus_clk))
                clk_disable_unprepare(msm_host->bus_clk);

        return 0;

Quoted 7 lines

Quoted 4 lines

This all looks wrong. The driver will no longer work unless CONFIG_PM is
set.

Right, I seem to have completely ignored the !CONFIG_PM case. Will look at how to handle that.

IMO, don't complicate the driver with that. Just enable (or leave enabled) all the clocks in the clock driver in the !CONFIG_PM case.

Quoted 127 lines

With platform support now in place to manage clocks from within runtime PM
callbacks, get rid of all clock handling from the driver and convert the driver to use runtime PM apis.

Signed-off-by Rajendra Nayak <[email protected]>

Cc <[email protected]>

---
 drivers/mmc/host/sdhci-msm.c | 46
+++++++++++---------------------------------
 1 file changed, 11 insertions(+), 35 deletions(-)

drivers/mmc/host/sdhci-msm.c +3 -25

index 4a09f76..3c62a77 100644
@@ -19,6 +19,7 @@
 #include <linux/delay.h>
 #include <linux/mmc/mmc.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
 
 #include "sdhci-pltfm.h"
 

@@ -56,8 +57,6 @@
 struct sdhci_msm_host {
        struct platform_device *pdev;
        void __iomem *core_mem; /* MSM SDCC mapped address */
-       struct clk *clk;        /* main SD/MMC bus clock */
-       struct clk *pclk;       /* SDHC peripheral bus clock */
        struct clk *bus_clk;    /* SDHC bus voter clock */
        struct mmc_host *mmc;
        struct sdhci_pltfm_data sdhci_msm_pdata;

@@ -469,29 +468,8 @@ static int sdhci_msm_probe(struct platform_device
 *pdev)
                        goto pltfm_free;
        }
 
-       /* Setup main peripheral bus clock */
-       msm_host->pclk = devm_clk_get(&pdev->dev, "iface");
-       if (IS_ERR(msm_host->pclk)) {
-               ret = PTR_ERR(msm_host->pclk);
-               dev_err(&pdev->dev, "Perpheral clk setup failed (%d)\n",
 ret);
-               goto bus_clk_disable;
-       }
-
-       ret = clk_prepare_enable(msm_host->pclk);
-       if (ret)
-               goto bus_clk_disable;
-
-       /* Setup SDC MMC clock */
-       msm_host->clk = devm_clk_get(&pdev->dev, "core");
-       if (IS_ERR(msm_host->clk)) {
-               ret = PTR_ERR(msm_host->clk);
-               dev_err(&pdev->dev, "SDC MMC clk setup failed (%d)\n",
 ret);
-               goto pclk_disable;
-       }
-
-       ret = clk_prepare_enable(msm_host->clk);
-       if (ret)
-               goto pclk_disable;
+       pm_runtime_enable(&pdev->dev);
+       pm_runtime_get_sync(&pdev->dev);

        core_memres = platform_get_resource(pdev, IORESOURCE_MEM, 1);
        msm_host->core_mem = devm_ioremap_resource(&pdev->dev,

core_memres);

drivers/mmc/host/sdhci-msm.c +1 -1

@@ -499,7 +477,7 @@ static int sdhci_msm_probe(struct platform_device
 *pdev)
        if (IS_ERR(msm_host->core_mem)) {
                dev_err(&pdev->dev, "Failed to remap registers\n");
                ret = PTR_ERR(msm_host->core_mem);
-               goto clk_disable;
+               goto err;
        }
 

        /* Reset the core and Enable SDHC mode */

drivers/mmc/host/sdhci-msm.c +1 -1

@@ -511,7 +489,7 @@ static int sdhci_msm_probe(struct platform_device
 *pdev)
        if (readl(msm_host->core_mem + CORE_POWER) & CORE_SW_RST) {
                dev_err(&pdev->dev, "Stuck in reset\n");
                ret = -ETIMEDOUT;
-               goto clk_disable;
+               goto err;
        }
 

        /* Set HC_MODE_EN bit in HC_MODE register */

drivers/mmc/host/sdhci-msm.c +4 -6

@@ -545,15 +523,13 @@ static int sdhci_msm_probe(struct platform_device
 *pdev)
 
        ret = sdhci_add_host(host);
        if (ret)
-               goto clk_disable;
+               goto err;
 
        return 0;
 
-clk_disable:
-       clk_disable_unprepare(msm_host->clk);
-pclk_disable:
-       clk_disable_unprepare(msm_host->pclk);
-bus_clk_disable:
+err:
+       pm_runtime_put_sync(&pdev->dev);
+       pm_runtime_disable(&pdev->dev);
        if (!IS_ERR(msm_host->bus_clk))
                clk_disable_unprepare(msm_host->bus_clk);

pltfm_free:

drivers/mmc/host/sdhci-msm.c +2 -2

@@ -571,8 +547,8 @@ static int sdhci_msm_remove(struct platform_device
 *pdev)
 
        sdhci_remove_host(host, dead);
        sdhci_pltfm_free(pdev);
-       clk_disable_unprepare(msm_host->clk);
-       clk_disable_unprepare(msm_host->pclk);
+       pm_runtime_put_sync(&pdev->dev);
+       pm_runtime_disable(&pdev->dev);
        if (!IS_ERR(msm_host->bus_clk))
                clk_disable_unprepare(msm_host->bus_clk);

        return 0;

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Quoted 4 lines

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

This all looks wrong. The driver will no longer work unless CONFIG_PM is set.

Kind regards
Uffe

Remove all clock handling from the driver as this is not handled from within platform runtime callbacks.

Quoted 2 lines · Rajendra Nayak

Remove all clock handling from the driver as this is not handled from within platform runtime callbacks.

s/not/now/ ??

Remove all clock handling from the driver as this is not handled from within platform runtime callbacks.

Signed-off-by Rajendra Nayak <[email protected]>

---
drivers/i2c/busses/i2c-qup.c | 74 ++++++++++----------------------------------

1 file changed, 16 insertions(+), 58 deletions(-)

drivers/i2c/busses/i2c-qup.c +5 -32

index fdcbdab..465a2b2 100644
@@ -104,7 +104,6 @@ struct qup_i2c_dev {
 	void __iomem		*base;
 	int			irq;
 	struct clk		*clk;
-	struct clk		*pclk;
 	struct i2c_adapter	adap;
 
 	int			clk_ctl;

@@ -532,24 +531,6 @@ static struct i2c_adapter_quirks qup_i2c_quirks = {
 	.max_read_len = QUP_READ_LIMIT,
 };
 
-static void qup_i2c_enable_clocks(struct qup_i2c_dev *qup)
-{
-	clk_prepare_enable(qup->clk);
-	clk_prepare_enable(qup->pclk);
-}
-
-static void qup_i2c_disable_clocks(struct qup_i2c_dev *qup)
-{
-	u32 config;
-
-	qup_i2c_change_state(qup, QUP_RESET_STATE);
-	clk_disable_unprepare(qup->clk);
-	config = readl(qup->base + QUP_CONFIG);
-	config |= QUP_CLOCK_AUTO_GATE;
-	writel(config, qup->base + QUP_CONFIG);
-	clk_disable_unprepare(qup->pclk);
-}
-
 static int qup_i2c_probe(struct platform_device *pdev)
 {
 	static const int blk_sizes[] = {4, 16, 32};

@@ -596,13 +577,10 @@ static int qup_i2c_probe(struct platform_device *pdev)
 		return PTR_ERR(qup->clk);
 	}
 
-	qup->pclk = devm_clk_get(qup->dev, "iface");
-	if (IS_ERR(qup->pclk)) {
-		dev_err(qup->dev, "Could not get iface clock\n");
-		return PTR_ERR(qup->pclk);
-	}
-
-	qup_i2c_enable_clocks(qup);
+	pm_runtime_set_autosuspend_delay(qup->dev, MSEC_PER_SEC);
+	pm_runtime_use_autosuspend(qup->dev);
+	pm_runtime_enable(qup->dev);
+	pm_runtime_get_sync(qup->dev);
 
 	/*
 	 * Bootloaders might leave a pending interrupt on certain QUP's,

@@ -673,22 +651,15 @@ static int qup_i2c_probe(struct platform_device *pdev)
 	qup->adap.dev.of_node = pdev->dev.of_node;
 	strlcpy(qup->adap.name, "QUP I2C adapter", sizeof(qup->adap.name));
 
-	pm_runtime_set_autosuspend_delay(qup->dev, MSEC_PER_SEC);
-	pm_runtime_use_autosuspend(qup->dev);
-	pm_runtime_set_active(qup->dev);
-	pm_runtime_enable(qup->dev);
-
 	ret = i2c_add_adapter(&qup->adap);
 	if (ret)
-		goto fail_runtime;
+		goto fail;
 

Hi Rajendra,

Quoted 81 lines · Rajendra Nayak

Remove all clock handling from the driver as this is not handled from within platform runtime callbacks.

Signed-off-by Rajendra Nayak <[email protected]>

---
drivers/i2c/busses/i2c-qup.c | 74 ++++++++++----------------------------------

1 file changed, 16 insertions(+), 58 deletions(-)

drivers/i2c/busses/i2c-qup.c +5 -32

index fdcbdab..465a2b2 100644
@@ -104,7 +104,6 @@ struct qup_i2c_dev {
 	void __iomem		*base;
 	int			irq;
 	struct clk		*clk;
-	struct clk		*pclk;
 	struct i2c_adapter	adap;
 
 	int			clk_ctl;

@@ -532,24 +531,6 @@ static struct i2c_adapter_quirks qup_i2c_quirks = {
 	.max_read_len = QUP_READ_LIMIT,
 };
 
-static void qup_i2c_enable_clocks(struct qup_i2c_dev *qup)
-{
-	clk_prepare_enable(qup->clk);
-	clk_prepare_enable(qup->pclk);
-}
-
-static void qup_i2c_disable_clocks(struct qup_i2c_dev *qup)
-{
-	u32 config;
-
-	qup_i2c_change_state(qup, QUP_RESET_STATE);
-	clk_disable_unprepare(qup->clk);
-	config = readl(qup->base + QUP_CONFIG);
-	config |= QUP_CLOCK_AUTO_GATE;
-	writel(config, qup->base + QUP_CONFIG);
-	clk_disable_unprepare(qup->pclk);
-}
-
 static int qup_i2c_probe(struct platform_device *pdev)
 {
 	static const int blk_sizes[] = {4, 16, 32};

@@ -596,13 +577,10 @@ static int qup_i2c_probe(struct platform_device *pdev)
 		return PTR_ERR(qup->clk);
 	}
 
-	qup->pclk = devm_clk_get(qup->dev, "iface");
-	if (IS_ERR(qup->pclk)) {
-		dev_err(qup->dev, "Could not get iface clock\n");
-		return PTR_ERR(qup->pclk);
-	}
-
-	qup_i2c_enable_clocks(qup);
+	pm_runtime_set_autosuspend_delay(qup->dev, MSEC_PER_SEC);
+	pm_runtime_use_autosuspend(qup->dev);
+	pm_runtime_enable(qup->dev);
+	pm_runtime_get_sync(qup->dev);
 
 	/*
 	 * Bootloaders might leave a pending interrupt on certain QUP's,

@@ -673,22 +651,15 @@ static int qup_i2c_probe(struct platform_device *pdev)
 	qup->adap.dev.of_node = pdev->dev.of_node;
 	strlcpy(qup->adap.name, "QUP I2C adapter", sizeof(qup->adap.name));
 
-	pm_runtime_set_autosuspend_delay(qup->dev, MSEC_PER_SEC);
-	pm_runtime_use_autosuspend(qup->dev);
-	pm_runtime_set_active(qup->dev);
-	pm_runtime_enable(qup->dev);
-
 	ret = i2c_add_adapter(&qup->adap);
 	if (ret)
-		goto fail_runtime;
+		goto fail;
 
 	return 0;
 

drivers/i2c/busses/i2c-qup.c static int qup_i2c_probe(struct platform_device *pdev) · line 664

Quoted 8 lines

drivers/i2c/busses/i2c-qup.c +1 -3

-fail_runtime:
+fail:
 	pm_runtime_disable(qup->dev);
 	pm_runtime_set_suspended(qup->dev);
-fail:
-	qup_i2c_disable_clocks(qup);
 	return ret;
 }

Same thing like in MMC driver. Clocks should be explicitly enabled because CONFIG_PM could be off.