Re: simpleaudio: set sysclk before using it

[Klemens Nanni]

On Sun, Apr 04, 2021 at 10:01:50PM +0200, Mark Kettenis wrote:
> > Date: Sun, 4 Apr 2021 21:08:09 +0200
> > From: Klemens Nanni <k...@openbsd.org>
> > 
> > Feedback? Objections? OK?
> 
> Explanation?
> 
> Not sure what happened here, but the reply-to was completely garbled...
Sorry, I must've crippled the body before sending.

simpleaudio_set_params() calls set_params() which reads sysclk off the
"i2s_clk" property before it sets that very clock's dd_set_sysclk()
(in case there's multiplier specified).

Hence reverse the order so set_params() picks up the newly set rate.

The rate is still off on the Pinebook Pro, but I came across this when
reading the code and it seemed wrong, so I also checked NetBSD which did
the same with

sys/dev/fdt/ausoc.c r1.6
"Set sysclk rate at set_format time, so the link set_format callback can read 
the new sysclk"
https://github.com/NetBSD/src/commit/ac8f47d1e5f46949b081c8e9d95211cdfda1e327


Index: dev/fdt/simpleaudio.c
===================================================================
RCS file: /cvs/src/sys/dev/fdt/simpleaudio.c,v
retrieving revision 1.1
diff -u -p -r1.1 simpleaudio.c
--- dev/fdt/simpleaudio.c       10 Jun 2020 23:55:19 -0000      1.1
+++ dev/fdt/simpleaudio.c       4 Apr 2021 20:23:39 -0000
@@ -300,24 +300,6 @@ simpleaudio_set_params(void *cookie, int
        uint32_t rate;
        int error;
 
-       dai = sc->sc_dai_cpu;
-       hwif = dai->dd_hw_if;
-       if (hwif->set_params) {
-               error = hwif->set_params(dai->dd_cookie,
-                   setmode, usemode, play, rec);
-               if (error)
-                       return error;
-       }
-
-       dai = sc->sc_dai_codec;
-       hwif = dai->dd_hw_if;
-       if (hwif->set_params) {
-               error = hwif->set_params(dai->dd_cookie,
-                   setmode, usemode, play, rec);
-               if (error)
-                       return error;
-       }
-
        if (sc->sc_mclk_fs) {
                if (setmode & AUMODE_PLAY)
                        rate = play->sample_rate * sc->sc_mclk_fs;
@@ -337,6 +319,24 @@ simpleaudio_set_params(void *cookie, int
                        if (error)
                                return error;
                }
+       }
+
+       dai = sc->sc_dai_cpu;
+       hwif = dai->dd_hw_if;
+       if (hwif->set_params) {
+               error = hwif->set_params(dai->dd_cookie,
+                   setmode, usemode, play, rec);
+               if (error)
+                       return error;
+       }
+
+       dai = sc->sc_dai_codec;
+       hwif = dai->dd_hw_if;
+       if (hwif->set_params) {
+               error = hwif->set_params(dai->dd_cookie,
+                   setmode, usemode, play, rec);
+               if (error)
+                       return error;
        }
 
        return 0;