[PATCH] mfd: tc6393xb fail ohci suspend if full state restore is required

[PATCH] mfd: tc6393xb fail ohci suspend if full state restore is required

[Dmitry Eremin-Solenikov]

Some boards with TC6393XB chip require full state restore during system
resume thanks to chip's VCC being cut off during suspend (Sharp SL-6000
tosa is one of them). Failing to do so would result in ohci Oops on
resume due to internal memory contentes being changed. Fail ohci suspend
on tc6393xb is full state restore is required.

Recommended workaround is to unbind tmio-ohci driver before suspend and
rebind it after resume.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Cc: stable@vger.kernel.org
---
 drivers/mfd/tc6393xb.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/tc6393xb.c b/drivers/mfd/tc6393xb.c
index 11c19e5..48579e5 100644
--- a/drivers/mfd/tc6393xb.c
+++ b/drivers/mfd/tc6393xb.c
@@ -263,6 +263,17 @@ static int tc6393xb_ohci_disable(struct platform_device *dev)
 	return 0;
 }
 
+static int tc6393xb_ohci_suspend(struct platform_device *dev)
+{
+	struct tc6393xb_platform_data *tcpd = dev_get_platdata(dev->dev.parent);
+
+	/* We can't properly store/restore OHCI state, so fail here */
+	if (tcpd->resume_restore)
+		return -EBUSY;
+
+	return tc6393xb_ohci_disable(dev);
+}
+
 static int tc6393xb_fb_enable(struct platform_device *dev)
 {
 	struct tc6393xb *tc6393xb = dev_get_drvdata(dev->dev.parent);
@@ -403,7 +414,7 @@ static struct mfd_cell tc6393xb_cells[] = {
 		.num_resources = ARRAY_SIZE(tc6393xb_ohci_resources),
 		.resources = tc6393xb_ohci_resources,
 		.enable = tc6393xb_ohci_enable,
-		.suspend = tc6393xb_ohci_disable,
+		.suspend = tc6393xb_ohci_suspend,
 		.resume = tc6393xb_ohci_enable,
 		.disable = tc6393xb_ohci_disable,
 	},
-- 
2.1.1

Re: [PATCH] mfd: tc6393xb fail ohci suspend if full state restore is required

[Dmitry Eremin-Solenikov]

2014-10-24 20:19 GMT+03:00 Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>:
> Some boards with TC6393XB chip require full state restore during system
> resume thanks to chip's VCC being cut off during suspend (Sharp SL-6000
> tosa is one of them). Failing to do so would result in ohci Oops on
> resume due to internal memory contentes being changed. Fail ohci suspend
> on tc6393xb is full state restore is required.
>
> Recommended workaround is to unbind tmio-ohci driver before suspend and
> rebind it after resume.

ping

>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/mfd/tc6393xb.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)



-- 
With best wishes
Dmitry

Re: [PATCH] mfd: tc6393xb fail ohci suspend if full state restore is required

[Lee Jones]

On Wed, 29 Oct 2014, Dmitry Eremin-Solenikov wrote:

> 2014-10-24 20:19 GMT+03:00 Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>:
> > Some boards with TC6393XB chip require full state restore during system
> > resume thanks to chip's VCC being cut off during suspend (Sharp SL-6000
> > tosa is one of them). Failing to do so would result in ohci Oops on
> > resume due to internal memory contentes being changed. Fail ohci suspend
> > on tc6393xb is full state restore is required.
> >
> > Recommended workaround is to unbind tmio-ohci driver before suspend and
> > rebind it after resume.
> 
> ping

Don't do that.  If you think the patch has slipped through the net
please sent it again with a [RESEND] tag in the email's subject line.

As it happens it hasn't been forgotten about, I was on vacation last
week and will get round to reviewing this shortly.

> > Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> > Cc: stable@vger.kernel.org
> > ---
> >  drivers/mfd/tc6393xb.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] mfd: tc6393xb fail ohci suspend if full state restore is required

[Lee Jones]

On Fri, 24 Oct 2014, Dmitry Eremin-Solenikov wrote:

> Some boards with TC6393XB chip require full state restore during system
> resume thanks to chip's VCC being cut off during suspend (Sharp SL-6000
> tosa is one of them). Failing to do so would result in ohci Oops on
> resume due to internal memory contentes being changed. Fail ohci suspend
> on tc6393xb is full state restore is required.
> 
> Recommended workaround is to unbind tmio-ohci driver before suspend and
> rebind it after resume.
> 
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/mfd/tc6393xb.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/tc6393xb.c b/drivers/mfd/tc6393xb.c
> index 11c19e5..48579e5 100644
> --- a/drivers/mfd/tc6393xb.c
> +++ b/drivers/mfd/tc6393xb.c
> @@ -263,6 +263,17 @@ static int tc6393xb_ohci_disable(struct platform_device *dev)
>  	return 0;
>  }
>  
> +static int tc6393xb_ohci_suspend(struct platform_device *dev)
> +{
> +	struct tc6393xb_platform_data *tcpd = dev_get_platdata(dev->dev.parent);
> +
> +	/* We can't properly store/restore OHCI state, so fail here */
> +	if (tcpd->resume_restore)
> +		return -EBUSY;

Wouldn't it be easier to just put these three lines into
tc6393xb_ohci_disable() instead off adding 7 superfluous lines and
making the Ops names inconsistent?


> +	return tc6393xb_ohci_disable(dev);
> +}
> +
>  static int tc6393xb_fb_enable(struct platform_device *dev)
>  {
>  	struct tc6393xb *tc6393xb = dev_get_drvdata(dev->dev.parent);
> @@ -403,7 +414,7 @@ static struct mfd_cell tc6393xb_cells[] = {
>  		.num_resources = ARRAY_SIZE(tc6393xb_ohci_resources),
>  		.resources = tc6393xb_ohci_resources,
>  		.enable = tc6393xb_ohci_enable,
> -		.suspend = tc6393xb_ohci_disable,
> +		.suspend = tc6393xb_ohci_suspend,
>  		.resume = tc6393xb_ohci_enable,
>  		.disable = tc6393xb_ohci_disable,
>  	},

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] mfd: tc6393xb fail ohci suspend if full state restore is required

[Dmitry Eremin-Solenikov]

Hello.

2014-11-03 17:56 GMT+03:00 Lee Jones <lee.jones@linaro.org>:
> On Fri, 24 Oct 2014, Dmitry Eremin-Solenikov wrote:
>
>> Some boards with TC6393XB chip require full state restore during system
>> resume thanks to chip's VCC being cut off during suspend (Sharp SL-6000
>> tosa is one of them). Failing to do so would result in ohci Oops on
>> resume due to internal memory contentes being changed. Fail ohci suspend
>> on tc6393xb is full state restore is required.
>>
>> Recommended workaround is to unbind tmio-ohci driver before suspend and
>> rebind it after resume.
>>
>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>> Cc: stable@vger.kernel.org
>> ---
>>  drivers/mfd/tc6393xb.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mfd/tc6393xb.c b/drivers/mfd/tc6393xb.c
>> index 11c19e5..48579e5 100644
>> --- a/drivers/mfd/tc6393xb.c
>> +++ b/drivers/mfd/tc6393xb.c
>> @@ -263,6 +263,17 @@ static int tc6393xb_ohci_disable(struct platform_device *dev)
>>       return 0;
>>  }
>>
>> +static int tc6393xb_ohci_suspend(struct platform_device *dev)
>> +{
>> +     struct tc6393xb_platform_data *tcpd = dev_get_platdata(dev->dev.parent);
>> +
>> +     /* We can't properly store/restore OHCI state, so fail here */
>> +     if (tcpd->resume_restore)
>> +             return -EBUSY;
>
> Wouldn't it be easier to just put these three lines into
> tc6393xb_ohci_disable() instead off adding 7 superfluous lines and
> making the Ops names inconsistent?

No. The problem only exists on suspend/resume path. tc6393xb_ohci_disable
is also used on driver removal and can (should?) be used on device shutdown.
Using the tc6393xb_ohci_disable() function in that paths is not a problem.

Also with this patch the Oops name will not be inconsistent - returning -EBUSY
from this disable callback will cancel the device suspension with
clearly stating
that OHCI is busy in dmesg.

>
>
>> +     return tc6393xb_ohci_disable(dev);
>> +}
>> +
>>  static int tc6393xb_fb_enable(struct platform_device *dev)
>>  {
>>       struct tc6393xb *tc6393xb = dev_get_drvdata(dev->dev.parent);
>> @@ -403,7 +414,7 @@ static struct mfd_cell tc6393xb_cells[] = {
>>               .num_resources = ARRAY_SIZE(tc6393xb_ohci_resources),
>>               .resources = tc6393xb_ohci_resources,
>>               .enable = tc6393xb_ohci_enable,
>> -             .suspend = tc6393xb_ohci_disable,
>> +             .suspend = tc6393xb_ohci_suspend,
>>               .resume = tc6393xb_ohci_enable,
>>               .disable = tc6393xb_ohci_disable,
>>       },


-- 
With best wishes
Dmitry

Re: [PATCH] mfd: tc6393xb fail ohci suspend if full state restore is required

[Lee Jones]

On Tue, 04 Nov 2014, Dmitry Eremin-Solenikov wrote:

> Hello.
> 
> 2014-11-03 17:56 GMT+03:00 Lee Jones <lee.jones@linaro.org>:
> > On Fri, 24 Oct 2014, Dmitry Eremin-Solenikov wrote:
> >
> >> Some boards with TC6393XB chip require full state restore during system
> >> resume thanks to chip's VCC being cut off during suspend (Sharp SL-6000
> >> tosa is one of them). Failing to do so would result in ohci Oops on
> >> resume due to internal memory contentes being changed. Fail ohci suspend
> >> on tc6393xb is full state restore is required.
> >>
> >> Recommended workaround is to unbind tmio-ohci driver before suspend and
> >> rebind it after resume.
> >>
> >> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> >> Cc: stable@vger.kernel.org
> >> ---
> >>  drivers/mfd/tc6393xb.c | 13 ++++++++++++-
> >>  1 file changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mfd/tc6393xb.c b/drivers/mfd/tc6393xb.c
> >> index 11c19e5..48579e5 100644
> >> --- a/drivers/mfd/tc6393xb.c
> >> +++ b/drivers/mfd/tc6393xb.c
> >> @@ -263,6 +263,17 @@ static int tc6393xb_ohci_disable(struct platform_device *dev)
> >>       return 0;
> >>  }
> >>
> >> +static int tc6393xb_ohci_suspend(struct platform_device *dev)
> >> +{
> >> +     struct tc6393xb_platform_data *tcpd = dev_get_platdata(dev->dev.parent);
> >> +
> >> +     /* We can't properly store/restore OHCI state, so fail here */
> >> +     if (tcpd->resume_restore)
> >> +             return -EBUSY;
> >
> > Wouldn't it be easier to just put these three lines into
> > tc6393xb_ohci_disable() instead off adding 7 superfluous lines and
> > making the Ops names inconsistent?
> 
> No. The problem only exists on suspend/resume path. tc6393xb_ohci_disable
> is also used on driver removal and can (should?) be used on device shutdown.
> Using the tc6393xb_ohci_disable() function in that paths is not a problem.
> 
> Also with this patch the Oops name will not be inconsistent - returning -EBUSY
> from this disable callback will cancel the device suspension with
> clearly stating
> that OHCI is busy in dmesg.

Understood.  Thanks for the explanation.

> >> +     return tc6393xb_ohci_disable(dev);
> >> +}
> >> +
> >>  static int tc6393xb_fb_enable(struct platform_device *dev)
> >>  {
> >>       struct tc6393xb *tc6393xb = dev_get_drvdata(dev->dev.parent);
> >> @@ -403,7 +414,7 @@ static struct mfd_cell tc6393xb_cells[] = {
> >>               .num_resources = ARRAY_SIZE(tc6393xb_ohci_resources),
> >>               .resources = tc6393xb_ohci_resources,
> >>               .enable = tc6393xb_ohci_enable,
> >> -             .suspend = tc6393xb_ohci_disable,
> >> +             .suspend = tc6393xb_ohci_suspend,
> >>               .resume = tc6393xb_ohci_enable,
> >>               .disable = tc6393xb_ohci_disable,
> >>       },
> 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] mfd: tc6393xb fail ohci suspend if full state restore is required

[Lee Jones]

On Fri, 24 Oct 2014, Dmitry Eremin-Solenikov wrote:

> Some boards with TC6393XB chip require full state restore during system
> resume thanks to chip's VCC being cut off during suspend (Sharp SL-6000
> tosa is one of them). Failing to do so would result in ohci Oops on
> resume due to internal memory contentes being changed. Fail ohci suspend
> on tc6393xb is full state restore is required.
> 
> Recommended workaround is to unbind tmio-ohci driver before suspend and
> rebind it after resume.
> 
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/mfd/tc6393xb.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

Applied, thanks.

> diff --git a/drivers/mfd/tc6393xb.c b/drivers/mfd/tc6393xb.c
> index 11c19e5..48579e5 100644
> --- a/drivers/mfd/tc6393xb.c
> +++ b/drivers/mfd/tc6393xb.c
> @@ -263,6 +263,17 @@ static int tc6393xb_ohci_disable(struct platform_device *dev)
>  	return 0;
>  }
>  
> +static int tc6393xb_ohci_suspend(struct platform_device *dev)
> +{
> +	struct tc6393xb_platform_data *tcpd = dev_get_platdata(dev->dev.parent);
> +
> +	/* We can't properly store/restore OHCI state, so fail here */
> +	if (tcpd->resume_restore)
> +		return -EBUSY;
> +
> +	return tc6393xb_ohci_disable(dev);
> +}
> +
>  static int tc6393xb_fb_enable(struct platform_device *dev)
>  {
>  	struct tc6393xb *tc6393xb = dev_get_drvdata(dev->dev.parent);
> @@ -403,7 +414,7 @@ static struct mfd_cell tc6393xb_cells[] = {
>  		.num_resources = ARRAY_SIZE(tc6393xb_ohci_resources),
>  		.resources = tc6393xb_ohci_resources,
>  		.enable = tc6393xb_ohci_enable,
> -		.suspend = tc6393xb_ohci_disable,
> +		.suspend = tc6393xb_ohci_suspend,
>  		.resume = tc6393xb_ohci_enable,
>  		.disable = tc6393xb_ohci_disable,
>  	},

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog