On Tue, 04 Nov 2014, Dmitry Eremin-Solenikov wrote: > Hello. > > 2014-11-03 17:56 GMT+03:00 Lee Jones <lee.jones@xxxxxxxxxx>: > > 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@xxxxxxxxx> > >> Cc: stable@xxxxxxxxxxxxxxx > >> --- > >> 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 -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html