At least freeze, restore and thaw need to be set in order for the driver
to support system hibernation. The existing suspend/resume functions can
be reused since those functions don't touch the device's power state or
wakeup capability. Use the helper macros SET_SYSTEM_SLEEP_PM_OPS and
SET_NOIRQ_SYSTEM_SLEEP_PM_OPS for symmetry with similar drivers.
Signed-off-by: Anton Eliasson <[email protected]>
---
I have not investigated the impact of adding the additional noirq
handler functions. The hardware that I tested on (Axis ARTPEC-8) appears
to work both with and without them assigned. Other similar drivers that
use noirq handlers assign them to both resume, thaw and restore so I
follow that style also.
---
drivers/tty/serial/samsung_tty.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index b29e9dfd81a6..e2247c11067d 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -2273,9 +2273,8 @@ static int s3c24xx_serial_resume_noirq(struct device *dev)
}
static const struct dev_pm_ops s3c24xx_serial_pm_ops = {
- .suspend = s3c24xx_serial_suspend,
- .resume = s3c24xx_serial_resume,
- .resume_noirq = s3c24xx_serial_resume_noirq,
+ SET_SYSTEM_SLEEP_PM_OPS(s3c24xx_serial_suspend, s3c24xx_serial_resume)
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(NULL, s3c24xx_serial_resume_noirq)
};
#define SERIAL_SAMSUNG_PM_OPS (&s3c24xx_serial_pm_ops)
---
base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
change-id: 20230731-samsung_tty_pm_ops-7c98f309a4e9
Best regards,
--
Anton Eliasson <[email protected]>
Hi Anton,
On Thu, Aug 03, 2023 at 01:26:42PM +0200, Anton Eliasson wrote:
> At least freeze, restore and thaw need to be set in order for the driver
> to support system hibernation. The existing suspend/resume functions can
> be reused since those functions don't touch the device's power state or
> wakeup capability. Use the helper macros SET_SYSTEM_SLEEP_PM_OPS and
> SET_NOIRQ_SYSTEM_SLEEP_PM_OPS for symmetry with similar drivers.
and why do we need hibernation in this device?
Andi
On 05/08/2023 23.38, Andi Shyti wrote:
> Hi Anton,
>
> On Thu, Aug 03, 2023 at 01:26:42PM +0200, Anton Eliasson wrote:
>> At least freeze, restore and thaw need to be set in order for the driver
>> to support system hibernation. The existing suspend/resume functions can
>> be reused since those functions don't touch the device's power state or
>> wakeup capability. Use the helper macros SET_SYSTEM_SLEEP_PM_OPS and
>> SET_NOIRQ_SYSTEM_SLEEP_PM_OPS for symmetry with similar drivers.
> and why do we need hibernation in this device?
>
> Andi
Hi!
I wanted to test whether hibernation is possible on our SoC, even though
it is not a common feature on embedded ARM systems. This is the only
mainline driver that I found that needed modification, for my
proof-of-concept anyway, and I couldn't see any harm in the change.
Anton Eliasson
On 03/08/2023 13:26, Anton Eliasson wrote:
> At least freeze, restore and thaw need to be set in order for the driver
> to support system hibernation. The existing suspend/resume functions can
> be reused since those functions don't touch the device's power state or
> wakeup capability. Use the helper macros SET_SYSTEM_SLEEP_PM_OPS and
> SET_NOIRQ_SYSTEM_SLEEP_PM_OPS for symmetry with similar drivers.
>
Looks sensible, although you should also test the other sleep methods,
e.g. suspend to idle.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
Hi Anton,
On Mon, Aug 07, 2023 at 11:57:04AM +0200, Anton Eliasson wrote:
> On 05/08/2023 23.38, Andi Shyti wrote:
> > Hi Anton,
> >
> > On Thu, Aug 03, 2023 at 01:26:42PM +0200, Anton Eliasson wrote:
> > > At least freeze, restore and thaw need to be set in order for the driver
> > > to support system hibernation. The existing suspend/resume functions can
> > > be reused since those functions don't touch the device's power state or
> > > wakeup capability. Use the helper macros SET_SYSTEM_SLEEP_PM_OPS and
> > > SET_NOIRQ_SYSTEM_SLEEP_PM_OPS for symmetry with similar drivers.
> > and why do we need hibernation in this device?
> >
> > Andi
>
> Hi!
>
> I wanted to test whether hibernation is possible on our SoC, even though it
> is not a common feature on embedded ARM systems. This is the only mainline
> driver that I found that needed modification, for my proof-of-concept
> anyway, and I couldn't see any harm in the change.
Thanks, makes sense, mine was just curiosity, can I know which
SoC you are testing that is using the samsung serial device?
You can add my r-b, anyway:
Reviewed-by: Andi Shyti <[email protected]>
Thanks,
Andi
On 07/08/2023 23.34, Andi Shyti wrote:
> Hi Anton,
>
> On Mon, Aug 07, 2023 at 11:57:04AM +0200, Anton Eliasson wrote:
>> On 05/08/2023 23.38, Andi Shyti wrote:
>>> Hi Anton,
>>>
>>> On Thu, Aug 03, 2023 at 01:26:42PM +0200, Anton Eliasson wrote:
>>>> At least freeze, restore and thaw need to be set in order for the driver
>>>> to support system hibernation. The existing suspend/resume functions can
>>>> be reused since those functions don't touch the device's power state or
>>>> wakeup capability. Use the helper macros SET_SYSTEM_SLEEP_PM_OPS and
>>>> SET_NOIRQ_SYSTEM_SLEEP_PM_OPS for symmetry with similar drivers.
>>> and why do we need hibernation in this device?
>>>
>>> Andi
>> Hi!
>>
>> I wanted to test whether hibernation is possible on our SoC, even though it
>> is not a common feature on embedded ARM systems. This is the only mainline
>> driver that I found that needed modification, for my proof-of-concept
>> anyway, and I couldn't see any harm in the change.
> Thanks, makes sense, mine was just curiosity, can I know which
> SoC you are testing that is using the samsung serial device?
>
> You can add my r-b, anyway:
>
> Reviewed-by: Andi Shyti <[email protected]>
>
> Thanks,
> Andi
It's the Axis Communications ARTPEC-8, an SoC for surveillance cameras:
https://www.axis.com/solutions/system-on-chip
Thanks for the review!
Anton Eliasson
On 07/08/2023 15.50, Krzysztof Kozlowski wrote:
> On 03/08/2023 13:26, Anton Eliasson wrote:
>> At least freeze, restore and thaw need to be set in order for the driver
>> to support system hibernation. The existing suspend/resume functions can
>> be reused since those functions don't touch the device's power state or
>> wakeup capability. Use the helper macros SET_SYSTEM_SLEEP_PM_OPS and
>> SET_NOIRQ_SYSTEM_SLEEP_PM_OPS for symmetry with similar drivers.
>>
> Looks sensible, although you should also test the other sleep methods,
> e.g. suspend to idle.
>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>
> Best regards,
> Krzysztof
>
Yes, s2idle still works. Standby / Power-On Suspend and s2ram we don't
have support for so I can't test it. Thanks for the review!
Anton Eliasson