Am Sun, Apr 04, 2021 at 11:17:54PM +0200 schrieb Mark Kettenis:
> > 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.
>
ok patrick@
>
> > 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;
> >
>
