-----Original Message-----
From: Wood Scott-B07421
Sent: Wednesday, June 06, 2012 12:12 AM
To: Li Yang-R58472
Cc: Wood Scott-B07421; Zhao Chenhui-B35336; linuxppc-dev@lists.ozlabs.org=

;

linux-kernel@vger.kernel.org; galak@kernel.crashing.org
Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup
event source
=20
On 06/04/2012 11:08 PM, Li Yang-R58472 wrote:

quoted

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Tuesday, June 05, 2012 7:03 AM
To: Zhao Chenhui-B35336
Cc: linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org;
galak@kernel.crashing.org; Li Yang-R58472
Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as
wakeup event source

On 06/04/2012 06:36 AM, Zhao Chenhui wrote:

quoted

On Fri, Jun 01, 2012 at 05:08:52PM -0500, Scott Wood wrote:

quoted

On 05/11/2012 06:53 AM, Zhao Chenhui wrote:

quoted

+int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool
+enable)

Why does it have to be a platform_device?  Would a bare device_node

work

quoted

quoted

here?  If it's for stuff like device_may_wakeup() that could be in
a platform_device wrapper function.

It does not have to be a platform_device. I think it can be a struct

device.

Why does it even need that?  The low level mechanism for influencing
PMCDR should only need a device node, not a Linux device struct.

It does no harm to pass the device structure and makes more sense for

object oriented interface design.
=20
It does do harm if you don't have a device structure to pass, if for some
reason you found the device by directly looking for it rather than going
through the device model.

Whether or not a device is a wakeup source not only depends on the SoC spec=
ification but also the configuration and current state for the device.  I o=
nly expect the device driver to have this knowledge and call this function =
rather than some standalone platform code.  Therefore I don't think your co=
ncern matters.
=20

=20

quoted

quoted

quoted

quoted

Who is setting can_wakeup for these devices?

The device driver is responsible to set can_wakeup.

How would the device driver know how to set it?  Wouldn't this depend
on the particular SoC and low power mode?

It is based on the "fsl,magic-packet" and "fsl,wake-on-filer" device

tree properties.
=20
fsl,magic-packet was a mistake.  It is equivalent to checking the
compatible for etsec.  It does not convey any information about whether

It can be described either by explicit feature property or by the compatibl=
e.  I don't think it is a problem that we choose one against another.

the eTSEC is still active in a given low power mode.

Whether or not the eTSEC is still active in both sleep and deep sleep is on=
ly depending on if we set it to be a wakeup source.  If it behaves differen=
tly for low power modes in the future, we could address that by adding addi=
tional property.

=20
How is fsl,wake-os-filer relevant to this decision?  When will it be set
but not fsl,magic-packet?

I mean either fsl,magic-packet or fsl,wake-on-filer shows it can be a wakeu=
p source.  Currently we don't have an SoC to have wake-on-filer while not w=
ake-on-magic.  But I think it's better to consider both as they are indepen=
dent features.

=20
What about devices other than ethernet?  What about differences between
ordinary sleep and deep sleep?

There is no difference for sleep and deep sleep for all wakeup sources curr=
ently.  We can address the problem if it is not the case in the future.

Leo