diff mbox

spi: omap2-mcspi: Add calls for pinctrl state select

Message ID 1431519316-30989-1-git-send-email-pascal.huerst@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Pascal Huerst May 13, 2015, 12:15 p.m. UTC
From: Pascal Huerst <pascal.huerst@gmail.com>

This adds calls to pinctrl subsystem in order to switch pin states
on suspend/resume if you provide a "sleep" state in DT.

If no "sleep" state is provided in DT, these calls turn
to NOPs, so we don't need error checking here.

Signed-off-by: Pascal Huerst <pascal.huerst@gmail.com>
---
 drivers/spi/spi-omap2-mcspi.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

--
2.1.0

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

Comments

Mark Brown May 13, 2015, 6:15 p.m. UTC | #1
On Wed, May 13, 2015 at 02:15:16PM +0200, pascal.huerst@gmail.com wrote:

> This adds calls to pinctrl subsystem in order to switch pin states
> on suspend/resume if you provide a "sleep" state in DT.

> If no "sleep" state is provided in DT, these calls turn
> to NOPs, so we don't need error checking here.

We should still be checking if we succesfully changed the state when
there is a state provided shouldn't we?
Pascal Huerst May 18, 2015, 12:02 p.m. UTC | #2
On 13.05.2015 20:15, Mark Brown wrote:
> On Wed, May 13, 2015 at 02:15:16PM +0200, pascal.huerst@gmail.com wrote:
> 
>> This adds calls to pinctrl subsystem in order to switch pin states
>> on suspend/resume if you provide a "sleep" state in DT.
> 
>> If no "sleep" state is provided in DT, these calls turn
>> to NOPs, so we don't need error checking here.
> 
> We should still be checking if we succesfully changed the state when
> there is a state provided shouldn't we?

I agree, but if I git grep for such calls, nobody seems to check the
return values of these functions. Shall I add a check and print a
message if the call fails?
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown May 19, 2015, 12:12 p.m. UTC | #3
On Mon, May 18, 2015 at 02:02:04PM +0200, Pascal Huerst wrote:
> On 13.05.2015 20:15, Mark Brown wrote:

> > We should still be checking if we succesfully changed the state when
> > there is a state provided shouldn't we?

> I agree, but if I git grep for such calls, nobody seems to check the
> return values of these functions. Shall I add a check and print a
> message if the call fails?

Yes, please.
Pascal Huerst June 9, 2015, 11:16 a.m. UTC | #4
Hey Mark,

On 19.05.2015 14:12, Mark Brown wrote:
> On Mon, May 18, 2015 at 02:02:04PM +0200, Pascal Huerst wrote:
>> On 13.05.2015 20:15, Mark Brown wrote:
> 
>>> We should still be checking if we succesfully changed the state when
>>> there is a state provided shouldn't we?
> 
>> I agree, but if I git grep for such calls, nobody seems to check the
>> return values of these functions. Shall I add a check and print a
>> message if the call fails?
> 
> Yes, please.

Sorry for the late reply.

There is already a message printed on failure in the subsystem. See:
http://lxr.free-electrons.com/source/drivers/pinctrl/core.c#L1250

So I don't see the need for another message, right?
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Sept. 14, 2015, 4:10 p.m. UTC | #5
On Tue, Jun 09, 2015 at 01:16:33PM +0200, Pascal Huerst wrote:
> On 19.05.2015 14:12, Mark Brown wrote:
> > On Mon, May 18, 2015 at 02:02:04PM +0200, Pascal Huerst wrote:
> >> On 13.05.2015 20:15, Mark Brown wrote:

> >>> We should still be checking if we succesfully changed the state when
> >>> there is a state provided shouldn't we?

> >> I agree, but if I git grep for such calls, nobody seems to check the
> >> return values of these functions. Shall I add a check and print a
> >> message if the call fails?

> > Yes, please.

> Sorry for the late reply.

> There is already a message printed on failure in the subsystem. See:
> http://lxr.free-electrons.com/source/drivers/pinctrl/core.c#L1250

> So I don't see the need for another message, right?

I was expecting to see us pass back an error code more than just
printing an error message.
diff mbox

Patch

diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
index 4df8942..424f8cd 100644
--- a/drivers/spi/spi-omap2-mcspi.c
+++ b/drivers/spi/spi-omap2-mcspi.c
@@ -24,6 +24,7 @@ 
 #include <linux/dma-mapping.h>
 #include <linux/dmaengine.h>
 #include <linux/omap-dma.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/err.h>
 #include <linux/clk.h>
@@ -1502,14 +1503,27 @@  static int omap2_mcspi_resume(struct device *dev)
 	}
 	pm_runtime_mark_last_busy(mcspi->dev);
 	pm_runtime_put_autosuspend(mcspi->dev);
+
+	pinctrl_pm_select_default_state(dev);
+
+	return 0;
+}
+
+static int omap2_mcspi_suspend(struct device *dev)
+{
+	pinctrl_pm_select_sleep_state(dev);
+
 	return 0;
 }
+
 #else
+#define omap2_mcspi_suspend	NULL
 #define	omap2_mcspi_resume	NULL
 #endif

 static const struct dev_pm_ops omap2_mcspi_pm_ops = {
 	.resume = omap2_mcspi_resume,
+	.suspend = omap2_mcspi_suspend,
 	.runtime_resume	= omap_mcspi_runtime_resume,
 };