Hi,
On 2017-05-16 13:36:21 +0200, Geert Uytterhoeven wrote:
> Hi Simon,
>
> On Tue, May 16, 2017 at 1:01 PM, Simon Horman <ho...@verge.net.au> wrote:
> > On Tue, May 16, 2017 at 11:07:34AM +0200, Geert Uytterhoeven wrote:
> >> On Tue, May 16, 2017 at 11:02 AM, Niklas Söderlund
> >> <niklas.soderl...@ragnatech.se> wrote:
> >> >> > Whit all this being said I still like to withdraw this patch as I
> >> >> > found
> >> >> > another fault with it, ravb_wol_restore() will unconditionally be
> >> >> > called
> >> >> > while ravb_wol_setup() will only be called if netif_running(ndev).
> >> >> > This
> >> >> > is en easy fix and I will send out a v2 once we figure out what to do
> >> >> > about the clock.
> >> >>
> >> >> The clock issue is external to the ravb driver. If it works with
> >> >> s2idle, it should
> >> >> be OK.
> >> >
> >> > Do you think it's a good idea to move ahead with a v2 of the ravb WoL
> >> > patch to fix the unrelated issue and aim for it to be picked up prior to
> >> > suspend/resume support is added to the CPG/MSSR?
> >>
> >> Sure.
> >>
> >> It can still be used on R-Car Gen2, where we're not s*d by mandatory PSCI.
> >
> > Is there some way for - e.g. the driver - to not enable WoL on Gen3 SoCs
> > until the clock issues is sorted out? I'm quite happy to enable features
>
> "priv->chip_id != RCAR_GEN3". However, as we don't have RAVB enabled
> on any R-Car Gen2 board, its use is limited.
I agree that it's not so useful to enable this on Gen2 only.
>
> > where they work; not so much where they don't.
>
> Agreed.
>
> One workaround could be to disable/enable the module clock in the WoL
> resume path, to make sure it is enabled. Once the enable count reaches
> 0, CCF will know it's disabled, and will really enable next time.
> You may need a double disable/double enable though, without testing I
> don't know remember the enable count is 1 or 2 at that point (due to PM
> runtime).
I thought about this but it feels like such a hack I did not dare
suggest it :-) But at the same time it would be nice to enable WoL for
the s2idle use-case where it works. Only resume from PSCI with WoL
enabled that is broken, and WoL in PSCI suspend will never work :-)
How about I add another patch in v2 on-top of this that adds the clock
disable/enable hack? That way it's clear that this is a workaround and
once we have support for suspend/resume in CPG/MSSR just that patch can
be reverted? Or is it cleaner to fold it in to this patch with a big
comment that this is a workaround? Or is it maybe better to hold of on
this until CPG/MSSR supports suspend/resume?
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
--
Regards,
Niklas Söderlund
