> >
> >> Given we know some OEM platforms have problems in current generations
> >> with constraints it would probably need to be restricted to this behavior only on a future
> >> SOC that we are confident of all drivers and firmware are doing the right thing.
> >>
> >> By passing the information to amd_pmc we can keep that logic restricting the behavior to
> >> only those platforms that we're confident on that behavior.
> >
> > Honestly, I'm not quite sure why it is a good idea to prevent the
> > platform from attempting to get into S0ix via suspend-to-idle in any
> > case.
> >
> > You know you have to suspend. You don't know how much time you will
> > be suspended. The constraints can only tell you what's the
> > lowest-power state you can achieve at this point, but why is it
> > relevant?
>
> Having thought through and said all I did above, I do concede you're
> right with the Linux approach to sleep the constraints really don't add
> a lot of value. If a device fails to enter it's intended sleep states
> the suspend will "fail". If the suspend succeeds but the constraints
> table doesn't match, it's just a hint where to focus on problems.
Exactly.
> I appreciate your thoughts and I will drop the constraints passing patch
> in this series.
Thanks!
> With that intent of dropping that would you still like this reworked as
> a notifier chain or keep it as this design?
I would introduce something like
struct acpi_s2idle_dev_ops {
struct list_head list_node;
void (*prepare)(struct device *dev);
void (*restore)(struct device *dev);
};
and let whoever wants to use one of these pass the pointer to it to a
"register" function (that will only do a "list add"). The reason why
I wouldn't allow ->prepare() to fail is that failing suspend at this
point isn't particularly useful (and arguably it isn't really useful
at all, but that's a whole different topic).
This can be a const struct in a driver, so it cannot be modified even
by mistake which reduces the attack surface a bit.
Then, make changes to acpi_s2idle_prepare_late() and
acpi_s2idle_restore_early() like in the $subject patch, except that
the extra locking may not be needed if the "register" function uses
lock_system_sleep() for mutual exclusion.
