diff mbox

[V1,2/2] spi: fsl-lpspi: add VLLS mode support

Message ID 1483522697-6691-2-git-send-email-pandy.gao@nxp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gao Pan Jan. 4, 2017, 9:38 a.m. UTC
When system enters VLLS mode, module power is turned off. As a result,
all registers are reset to HW default value. After exiting VLLS mode,
registers are still in default mode. As a result, the pinctrl settings
are incorrect, which will affect the module function.

The patch recovers the pinctrl setting when exit VLLS mode.

Signed-off-by: Gao Pan <pandy.gao@nxp.com>
---
 drivers/spi/spi-fsl-lpspi.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Comments

Mark Brown Jan. 9, 2017, 7:33 p.m. UTC | #1
On Wed, Jan 04, 2017 at 05:38:17PM +0800, Gao Pan wrote:
> When system enters VLLS mode, module power is turned off. As a result,
> all registers are reset to HW default value. After exiting VLLS mode,
> registers are still in default mode. As a result, the pinctrl settings
> are incorrect, which will affect the module function.
> 
> The patch recovers the pinctrl setting when exit VLLS mode.

Why is this a change here and not in the pinctrl driver?  It sounds like
it's the pinctrl driver that's forgetting the settings and should be
restoring them on resume but perhaps I'm missing something here.
Gao Pan Jan. 17, 2017, 7:03 a.m. UTC | #2
From: Mark Brown <mailto:broonie@kernel.org>   Sent: Tuesday, January 10, 2017 3:34 AM
> To: Pandy Gao <pandy.gao@nxp.com>
> Cc: linux-spi@vger.kernel.org
> Subject: Re: [Patch V1 2/2] spi: fsl-lpspi: add VLLS mode support
> 
> On Wed, Jan 04, 2017 at 05:38:17PM +0800, Gao Pan wrote:
> > When system enters VLLS mode, module power is turned off. As a
> result,
> > all registers are reset to HW default value. After exiting VLLS mode,
> > registers are still in default mode. As a result, the pinctrl settings
> > are incorrect, which will affect the module function.
> >
> > The patch recovers the pinctrl setting when exit VLLS mode.
> 
> Why is this a change here and not in the pinctrl driver?  It sounds like
> it's the pinctrl driver that's forgetting the settings and should be
> restoring them on resume but perhaps I'm missing something here.
 
If the pinctrl driver restores the setting on resume, then all device node
should be scanned,  which is irregular. 

Even though the pinctrl driver can handle all the child node of iomuxc,
it's still difficult for the pinctrl driver to know which device node is disabled
and which device node is okay. Moreover, there may be some pin conflicts
among the child node of iomuxc.

So it's better to do the restore in spi driver other than the pinctrl driver.

Best  Regards
Gao  Pan
--
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 Jan. 17, 2017, 7:04 p.m. UTC | #3
On Tue, Jan 17, 2017 at 07:03:28AM +0000, Pandy Gao wrote:

> > > The patch recovers the pinctrl setting when exit VLLS mode.

> > Why is this a change here and not in the pinctrl driver?  It sounds like
> > it's the pinctrl driver that's forgetting the settings and should be
> > restoring them on resume but perhaps I'm missing something here.

> If the pinctrl driver restores the setting on resume, then all device node
> should be scanned,  which is irregular. 

What would restoring the configuration have to do with scanning the
device tree?  The changelog says this is just reapplying the same
settings we started out with so it's just a case of restoring the state
of the device after a runtime resume which is a perfectly normal thing
for a driver to do.
Gao Pan Jan. 18, 2017, 3:50 a.m. UTC | #4
From: Mark Brown <mailto:broonie@kernel.org> Sent: Wednesday, January 18, 2017 3:04 AM
> To: Pandy Gao <pandy.gao@nxp.com>
> Cc: linux-spi@vger.kernel.org
> Subject: Re: [Patch V1 2/2] spi: fsl-lpspi: add VLLS mode support
> 
> On Tue, Jan 17, 2017 at 07:03:28AM +0000, Pandy Gao wrote:
> 
> > > > The patch recovers the pinctrl setting when exit VLLS mode.
> 
> > > Why is this a change here and not in the pinctrl driver?  It sounds
> > > like it's the pinctrl driver that's forgetting the settings and
> > > should be restoring them on resume but perhaps I'm missing
> something here.
> 
> > If the pinctrl driver restores the setting on resume, then all device
> > node should be scanned,  which is irregular.
> 
> What would restoring the configuration have to do with scanning the
> device tree?  The changelog says this is just reapplying the same
> settings we started out with so it's just a case of restoring the state of
> the device after a runtime resume which is a perfectly normal thing for
> a driver to do.

We can reapply  the initial settings as the configurations defined in
device node iomuxc in pinctrl driver.  However, I think there are some
limitations.  For example, conflicts may exist between enet1grp and
ecspi3grp, which is not easy for the pinctrl  driver to find out.

&iomuxc {
	pinctrl_ecspi3: ecspi3grp 
	{
		...
	};

            	pinctrl_enet1: enet1grp
	 {
		...
	};
}

Best Regards
Gao Pan
--
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
diff mbox

Patch

diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
index 8ec8f55..8557728 100644
--- a/drivers/spi/spi-fsl-lpspi.c
+++ b/drivers/spi/spi-fsl-lpspi.c
@@ -510,10 +510,32 @@  static int fsl_lpspi_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int fsl_lpspi_suspend(struct device *dev)
+{
+	pinctrl_pm_select_sleep_state(dev);
+
+	return 0;
+}
+
+static int fsl_lpspi_resume(struct device *dev)
+{
+	pinctrl_pm_select_default_state(dev);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(imx_lpspi_pm, fsl_lpspi_suspend, fsl_lpspi_resume);
+#define IMX_LPSPI_PM    (&imx_lpspi_pm)
+#else
+#define IMX_LPSPI_PM    NULL
+#endif
+
 static struct platform_driver fsl_lpspi_driver = {
 	.driver = {
 		.name = DRIVER_NAME,
 		.of_match_table = fsl_lpspi_dt_ids,
+		.pm = IMX_LPSPI_PM,
 	},
 	.probe = fsl_lpspi_probe,
 	.remove = fsl_lpspi_remove,