Hi Geert,
On 2017-05-18 10:52:25 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
>
> On Tue, May 16, 2017 at 2:16 PM, Niklas Söderlund
> <niklas.soderlund@xxxxxxxxxxxx> wrote:
> > On 2017-05-16 13:36:21 +0200, Geert Uytterhoeven wrote:
> >> On Tue, May 16, 2017 at 1:01 PM, Simon Horman <horms@xxxxxxxxxxxx> wrote:
> >> > 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
> >> > 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 :-)
>
> Indeed.
>
> > 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?
>
> Personally, I would have no problems of having the workaround integrated (and
> documented, of course) in the WoL patch, as it avoids having broken PSCI
> suspend in between WoL-without-workaround and a separate workaround.
You have now posted '[PATCH/RFC] clk: renesas: cpg-mssr: Restore module
clock registers during resume' which solves this issue. Do you think it
would be OK for me to resubmit this patch due to an unrelated fix and
state that it depends on your patch or do you feel it still would be
valuable to include a workaround in the RAVB driver as to not make it
dependent on your patch?
>
> It's not that dissimilar from the initial R-Car Gen3 support patch limiting
> ravb to 100 Mbps.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
>
> 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
