> 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;
> 

Reply via email to