# RE: \[PATCH v5 4/5\] fsl\_pmc: Add API to enable device as wakeup event source

- From: Li Yang-R58472 \<hidden\>
- Date: 2012-06-05 16:49:43 
- List: linuxppc-dev
- Page: https://ratatoskr.run/linuxppc-dev/2012/06/3874648
- Thread: https://ratatoskr.run/linuxppc-dev/2012/05/3862718/t.md

```text


> -----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; <redacted>
;
> 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:
> >
> >
> >> -----Original Message-----
> >> From: Wood Scott-B07421
> >> Sent: Tuesday, June 05, 2012 7:03 AM
> >> To: Zhao Chenhui-B35336
> >> Cc: <redacted>; <redacted>;
> >> 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:
> >>> On Fri, Jun 01, 2012 at 05:08:52PM -0500, Scott Wood wrote:
> >>>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> >>>>> +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
> >>>> 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
> >>>> 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
```