> Date: Sun, 4 Apr 2021 22:24:57 +0200
> From: Klemens Nanni <[email protected]>
> Cc: Mark Kettenis <[email protected]>
>
> 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 <[email protected]>
> > >
> > > 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
OK. So NetBSD's _set_format() is the equivalent of our _set_params().
So changing the order makes us match how NetBSD does things. That
makes some sense.
ok kettenis@, but give Patrick a chance to comment as well.
> 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;
>