[PATCH] [MMC] wbsd pnp suspend

[PATCH] [MMC] wbsd pnp suspend

[Pierre Ossman]

Allow the wbsd driver to use the new suspend/resume functions added to
the PnP layer.

Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>
---

 drivers/mmc/wbsd.c |  106 ++++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 90 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/wbsd.c b/drivers/mmc/wbsd.c
--- a/drivers/mmc/wbsd.c
+++ b/drivers/mmc/wbsd.c
@@ -1980,37 +1980,53 @@ static void __devexit wbsd_pnp_remove(st
 
 #ifdef CONFIG_PM
 
-static int wbsd_suspend(struct device *dev, pm_message_t state)
+static int wbsd_suspend(struct wbsd_host *host, pm_message_t state)
+{
+	BUG_ON(host == NULL);
+
+	return mmc_suspend_host(host->mmc, state);
+}
+
+static int wbsd_resume(struct wbsd_host *host)
+{
+	BUG_ON(host == NULL);
+
+	wbsd_init_device(host);
+
+	return mmc_resume_host(host->mmc);
+}
+
+static int wbsd_platform_suspend(struct device *dev, pm_message_t state)
 {
 	struct mmc_host *mmc = dev_get_drvdata(dev);
 	struct wbsd_host *host;
 	int ret;
 
-	if (!mmc)
+	if (mmc == NULL)
 		return 0;
 
-	DBG("Suspending...\n");
-
-	ret = mmc_suspend_host(mmc, state);
-	if (!ret)
-		return ret;
+	DBGF("Suspending...\n");
 
 	host = mmc_priv(mmc);
 
+	ret = wbsd_suspend(host, state);
+	if (ret)
+		return ret;
+
 	wbsd_chip_poweroff(host);
 
 	return 0;
 }
 
-static int wbsd_resume(struct device *dev)
+static int wbsd_platform_resume(struct device *dev)
 {
 	struct mmc_host *mmc = dev_get_drvdata(dev);
 	struct wbsd_host *host;
 
-	if (!mmc)
+	if (mmc == NULL)
 		return 0;
 
-	DBG("Resuming...\n");
+	DBGF("Resuming...\n");
 
 	host = mmc_priv(mmc);
 
@@ -2021,15 +2037,70 @@ static int wbsd_resume(struct device *de
 	 */
 	mdelay(5);
 
-	wbsd_init_device(host);
+	return wbsd_resume(host);
+}
+
+#ifdef CONFIG_PNP
+
+static int wbsd_pnp_suspend(struct pnp_dev *pnp_dev, pm_message_t state)
+{
+	struct mmc_host *mmc = dev_get_drvdata(&pnp_dev->dev);
+	struct wbsd_host *host;
+
+	if (mmc == NULL)
+		return 0;
+
+	DBGF("Suspending...\n");
+
+	host = mmc_priv(mmc);
 
-	return mmc_resume_host(mmc);
+	return wbsd_suspend(host, state);
 }
 
+static int wbsd_pnp_resume(struct pnp_dev *pnp_dev)
+{
+	struct mmc_host *mmc = dev_get_drvdata(&pnp_dev->dev);
+	struct wbsd_host *host;
+
+	if (mmc == NULL)
+		return 0;
+
+	DBGF("Resuming...\n");
+
+	host = mmc_priv(mmc);
+
+	/*
+	 * See if chip needs to be configured.
+	 */
+	if (host->config != 0)
+	{
+		if (!wbsd_chip_validate(host))
+		{
+			printk(KERN_WARNING DRIVER_NAME
+				": PnP active but chip not configured! "
+				"You probably have a buggy BIOS. "
+				"Configuring chip manually.\n");
+			wbsd_chip_config(host);
+		}
+	}
+
+	/*
+	 * Allow device to initialise itself properly.
+	 */
+	mdelay(5);
+
+	return wbsd_resume(host);
+}
+
+#endif /* CONFIG_PNP */
+
 #else /* CONFIG_PM */
 
-#define wbsd_suspend NULL
-#define wbsd_resume NULL
+#define wbsd_platform_suspend NULL
+#define wbsd_platform_resume NULL
+
+#define wbsd_pnp_suspend NULL
+#define wbsd_pnp_resume NULL
 
 #endif /* CONFIG_PM */
 
@@ -2041,8 +2112,8 @@ static struct device_driver wbsd_driver 
 	.probe		= wbsd_probe,
 	.remove		= wbsd_remove,
 
-	.suspend	= wbsd_suspend,
-	.resume		= wbsd_resume,
+	.suspend	= wbsd_platform_suspend,
+	.resume		= wbsd_platform_resume,
 };
 
 #ifdef CONFIG_PNP
@@ -2052,6 +2123,9 @@ static struct pnp_driver wbsd_pnp_driver
 	.id_table	= pnp_dev_table,
 	.probe		= wbsd_pnp_probe,
 	.remove		= wbsd_pnp_remove,
+
+	.suspend	= wbsd_pnp_suspend,
+	.resume		= wbsd_pnp_resume,
 };
 
 #endif /* CONFIG_PNP */

Re: [PATCH] [MMC] wbsd pnp suspend

[Andrew Morton]

Pierre Ossman <drzeus@drzeus.cx> wrote:
>
> Allow the wbsd driver to use the new suspend/resume functions added to
> the PnP layer.
> 

Doesn't Russell handle mmc stuff?

> -static int wbsd_suspend(struct device *dev, pm_message_t state)
> +static int wbsd_suspend(struct wbsd_host *host, pm_message_t state)
> +{
> +	BUG_ON(host == NULL);
> +
> +	return mmc_suspend_host(host->mmc, state);
> +}

There's not much point in this BUG_ON.  If host==0 then we'll get a
perfectly good oops in the next statement - it's just as informative.

> +static int wbsd_resume(struct wbsd_host *host)
> +{
> +	BUG_ON(host == NULL);

Ditto.

> +	if (host->config != 0)
> +	{
> +		if (!wbsd_chip_validate(host))
> +		{

Like:

	if (host->config != 0) {
		if (!wbsd_chip_validate(host)) {

please.

Re: [PATCH] [MMC] wbsd pnp suspend

[Pierre Ossman]

Andrew Morton wrote:
> Pierre Ossman <drzeus@drzeus.cx> wrote:
>> Allow the wbsd driver to use the new suspend/resume functions added to
>> the PnP layer.
>>
> 
> Doesn't Russell handle mmc stuff?
> 

Yup. But this needs the PnP suspend stuff in your patch set.

>> -static int wbsd_suspend(struct device *dev, pm_message_t state)
>> +static int wbsd_suspend(struct wbsd_host *host, pm_message_t state)
>> +{
>> +	BUG_ON(host == NULL);
>> +
>> +	return mmc_suspend_host(host->mmc, state);
>> +}
> 
> There's not much point in this BUG_ON.  If host==0 then we'll get a
> perfectly good oops in the next statement - it's just as informative.
> 

I suppose. I just have a tendency to scatter assertions all over the 
place. :)

>> +	if (host->config != 0)
>> +	{
>> +		if (!wbsd_chip_validate(host))
>> +		{
> 
> Like:
> 
> 	if (host->config != 0) {
> 		if (!wbsd_chip_validate(host)) {
> 
> please.
> 

We had this discussion the last patch for this driver. It's horribly 
wrong when it comes to coding style so keeping patches in the same style 
as the rest of the driver is the lesser evil (IMHO).

Rgds
Pierre

Re: [PATCH] [MMC] wbsd pnp suspend

[Dmitry Torokhov]

On Tuesday 08 November 2005 01:57, Pierre Ossman wrote:
> Andrew Morton wrote:
> > Pierre Ossman <drzeus@drzeus.cx> wrote: 
> >> +	if (host->config != 0)
> >> +	{
> >> +		if (!wbsd_chip_validate(host))
> >> +		{
> > 
> > Like:
> > 
> > 	if (host->config != 0) {
> > 		if (!wbsd_chip_validate(host)) {
> > 
> > please.
> > 
> 
> We had this discussion the last patch for this driver. It's horribly 
> wrong when it comes to coding style so keeping patches in the same style 
> as the rest of the driver is the lesser evil (IMHO).
>

Aside from unusual brace placement it does not look too bad. How about
running it through Lindent to fix it once and for all?

-- 
Dmitry

Re: [PATCH] [MMC] wbsd pnp suspend

[Pierre Ossman]

Dmitry Torokhov wrote:
>
> Aside from unusual brace placement it does not look too bad. How about
> running it through Lindent to fix it once and for all?
>
>   

I could, just haven't gotten around to it. :)

I'll try to remember to have a look at it when there aren't any pending 
patches that will get borked by a lindent.

Rgds
Pierre

[PATCH] [MMC] wbsd pnp suspend

[Pierre Ossman]

Allow the wbsd driver to use the new suspend/resume functions added to
the PnP layer.

Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>
---

 drivers/mmc/wbsd.c |  106 ++++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 90 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/wbsd.c b/drivers/mmc/wbsd.c
index 4f13bd2..60afc12 100644
--- a/drivers/mmc/wbsd.c
+++ b/drivers/mmc/wbsd.c
@@ -1980,37 +1980,53 @@ static void __devexit wbsd_pnp_remove(st
 
 #ifdef CONFIG_PM
 
-static int wbsd_suspend(struct platform_device *dev, pm_message_t state)
+static int wbsd_suspend(struct wbsd_host *host, pm_message_t state)
+{
+	BUG_ON(host == NULL);
+
+	return mmc_suspend_host(host->mmc, state);
+}
+
+static int wbsd_resume(struct wbsd_host *host)
+{
+	BUG_ON(host == NULL);
+
+	wbsd_init_device(host);
+
+	return mmc_resume_host(host->mmc);
+}
+
+static int wbsd_platform_suspend(struct platform_device *dev, pm_message_t state)
 {
 	struct mmc_host *mmc = platform_get_drvdata(dev);
 	struct wbsd_host *host;
 	int ret;
 
-	if (!mmc)
+	if (mmc == NULL)
 		return 0;
 
-	DBG("Suspending...\n");
-
-	ret = mmc_suspend_host(mmc, state);
-	if (!ret)
-		return ret;
+	DBGF("Suspending...\n");
 
 	host = mmc_priv(mmc);
 
+	ret = wbsd_suspend(host, state);
+	if (ret)
+		return ret;
+
 	wbsd_chip_poweroff(host);
 
 	return 0;
 }
 
-static int wbsd_resume(struct platform_device *dev)
+static int wbsd_platform_resume(struct platform_device *dev)
 {
 	struct mmc_host *mmc = platform_get_drvdata(dev);
 	struct wbsd_host *host;
 
-	if (!mmc)
+	if (mmc == NULL)
 		return 0;
 
-	DBG("Resuming...\n");
+	DBGF("Resuming...\n");
 
 	host = mmc_priv(mmc);
 
@@ -2021,15 +2037,70 @@ static int wbsd_resume(struct platform_d
 	 */
 	mdelay(5);
 
-	wbsd_init_device(host);
+	return wbsd_resume(host);
+}
+
+#ifdef CONFIG_PNP
+
+static int wbsd_pnp_suspend(struct pnp_dev *pnp_dev, pm_message_t state)
+{
+	struct mmc_host *mmc = dev_get_drvdata(&pnp_dev->dev);
+	struct wbsd_host *host;
+
+	if (mmc == NULL)
+		return 0;
+
+	DBGF("Suspending...\n");
+
+	host = mmc_priv(mmc);
 
-	return mmc_resume_host(mmc);
+	return wbsd_suspend(host, state);
 }
 
+static int wbsd_pnp_resume(struct pnp_dev *pnp_dev)
+{
+	struct mmc_host *mmc = dev_get_drvdata(&pnp_dev->dev);
+	struct wbsd_host *host;
+
+	if (mmc == NULL)
+		return 0;
+
+	DBGF("Resuming...\n");
+
+	host = mmc_priv(mmc);
+
+	/*
+	 * See if chip needs to be configured.
+	 */
+	if (host->config != 0)
+	{
+		if (!wbsd_chip_validate(host))
+		{
+			printk(KERN_WARNING DRIVER_NAME
+				": PnP active but chip not configured! "
+				"You probably have a buggy BIOS. "
+				"Configuring chip manually.\n");
+			wbsd_chip_config(host);
+		}
+	}
+
+	/*
+	 * Allow device to initialise itself properly.
+	 */
+	mdelay(5);
+
+	return wbsd_resume(host);
+}
+
+#endif /* CONFIG_PNP */
+
 #else /* CONFIG_PM */
 
-#define wbsd_suspend NULL
-#define wbsd_resume NULL
+#define wbsd_platform_suspend NULL
+#define wbsd_platform_resume NULL
+
+#define wbsd_pnp_suspend NULL
+#define wbsd_pnp_resume NULL
 
 #endif /* CONFIG_PM */
 
@@ -2039,8 +2110,8 @@ static struct platform_driver wbsd_drive
 	.probe		= wbsd_probe,
 	.remove		= __devexit_p(wbsd_remove),
 
-	.suspend	= wbsd_suspend,
-	.resume		= wbsd_resume,
+	.suspend	= wbsd_platform_suspend,
+	.resume		= wbsd_platform_resume,
 	.driver		= {
 		.name	= DRIVER_NAME,
 	},
@@ -2053,6 +2124,9 @@ static struct pnp_driver wbsd_pnp_driver
 	.id_table	= pnp_dev_table,
 	.probe		= wbsd_pnp_probe,
 	.remove		= __devexit_p(wbsd_pnp_remove),
+
+	.suspend	= wbsd_pnp_suspend,
+	.resume		= wbsd_pnp_resume,
 };
 
 #endif /* CONFIG_PNP */

Re: [PATCH] [MMC] wbsd pnp suspend

[Russell King]

On Sun, Jan 08, 2006 at 12:02:01AM +0100, Pierre Ossman wrote:
> Allow the wbsd driver to use the new suspend/resume functions added to
> the PnP layer.

Applied, thanks.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core