| Message ID | 1368807872-2601-5-git-send-email-t.figa@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Tomasz, On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa <t.figa@samsung.com> wrote: > + if (ctrl->resume) > + ctrl->resume(drvdata); > + Having this resume at the beginning of the function seems right for restoring eint settings like you do in patch #6. ...but if you need to try to handle the old s3c64xx and s5p64x0 concept of "restored" to actually take GPIOs out of powerdown mode then you'll also need a callback at the end. Does it make sense to add a second callback at the end of this function? ...since it's unclear how we'll handle s3c64xx/s5p64x0 (or even if I'm misunderstanding and they're already handled somehow), I don't see any problems with this patch, so... On exynos5250-snow (pinmux backported to 3.8): Tested-by: Doug Anderson <dianders@chromium.org> Reviewed-by: Doug Anderson <dianders@chromium.org>
On Friday 17 of May 2013 12:24:29 Doug Anderson wrote: > Tomasz, > > On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa <t.figa@samsung.com> wrote: > > + if (ctrl->resume) > > + ctrl->resume(drvdata); > > + > > Having this resume at the beginning of the function seems right for > restoring eint settings like you do in patch #6. > > ...but if you need to try to handle the old s3c64xx and s5p64x0 > concept of "restored" to actually take GPIOs out of powerdown mode > then you'll also need a callback at the end. > > Does it make sense to add a second callback at the end of this function? Right. I haven't thought of this. It might make sense to add .resumed() callback as well to handle this. I guess it could be added as a part of patches for S3C64xx-specific pinctrl suspend/resume, that I will post some day. > ...since it's unclear how we'll handle s3c64xx/s5p64x0 (or even if I'm > misunderstanding and they're already handled somehow), I don't see any > problems with this patch, so... > > > On exynos5250-snow (pinmux backported to 3.8): > > Tested-by: Doug Anderson <dianders@chromium.org> > > Reviewed-by: Doug Anderson <dianders@chromium.org> Thanks. Best regards, Tomasz
On Fri, May 17, 2013 at 6:24 PM, Tomasz Figa <t.figa@samsung.com> wrote: > SoC-specific driver might require additional save and restore of > registers. This patch adds pair of SoC-specific callbacks per pinctrl > device to account for this. > > Signed-off-by: Tomasz Figa <t.figa@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> Patch applied for fixes. Hm this is quite a lot of code for "fixes", can you confirm that the system is really unusable without all these patches? Yours, Linus Walleij
Hi Linus, On Friday 24 of May 2013 11:07:41 Linus Walleij wrote: > On Fri, May 17, 2013 at 6:24 PM, Tomasz Figa <t.figa@samsung.com> wrote: > > SoC-specific driver might require additional save and restore of > > registers. This patch adds pair of SoC-specific callbacks per pinctrl > > device to account for this. > > > > Signed-off-by: Tomasz Figa <t.figa@samsung.com> > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > > Patch applied for fixes. Thanks. > Hm this is quite a lot of code for "fixes", can you confirm that > the system is really unusable without all these patches? Well, it is something that should have been already sent as a fix for 3.8, when this pin control driver was added, because since then suspend/resume has been broken on DT-enabled Exynos boards. Without this, suspending the board and then trying to wake it up is rather unpredictable, leading usually to board reset, interrupt storm or at least some devices failing to resume. Best regards,
diff --git a/drivers/pinctrl/pinctrl-samsung.c b/drivers/pinctrl/pinctrl-samsung.c index 15db258..63ac22e 100644 --- a/drivers/pinctrl/pinctrl-samsung.c +++ b/drivers/pinctrl/pinctrl-samsung.c @@ -1010,6 +1010,9 @@ static void samsung_pinctrl_suspend_dev( reg, bank->pm_save[PINCFG_TYPE_FUNC]); } } + + if (ctrl->suspend) + ctrl->suspend(drvdata); } /** @@ -1026,6 +1029,9 @@ static void samsung_pinctrl_resume_dev(struct samsung_pinctrl_drv_data *drvdata) void __iomem *virt_base = drvdata->virt_base; int i; + if (ctrl->resume) + ctrl->resume(drvdata); + for (i = 0; i < ctrl->nr_banks; i++) { struct samsung_pin_bank *bank = &ctrl->pin_banks[i]; void __iomem *reg = virt_base + bank->pctl_offset; diff --git a/drivers/pinctrl/pinctrl-samsung.h b/drivers/pinctrl/pinctrl-samsung.h index 9f5cc81..b316d9f 100644 --- a/drivers/pinctrl/pinctrl-samsung.h +++ b/drivers/pinctrl/pinctrl-samsung.h @@ -187,6 +187,9 @@ struct samsung_pin_ctrl { int (*eint_gpio_init)(struct samsung_pinctrl_drv_data *); int (*eint_wkup_init)(struct samsung_pinctrl_drv_data *); + void (*suspend)(struct samsung_pinctrl_drv_data *); + void (*resume)(struct samsung_pinctrl_drv_data *); + char *label; };