Re: Replace audio(9) get_props() with duplex check in open() for play-only drivers
> Date: Thu, 27 Oct 2022 13:27:43 +
> From: Klemens Nanni
>
> Make drivers which do *not* adverise AUDIO_PROP_FULLDPLEX return ENXIO
> in their open() if full-duplex mode was requested.
>
> This way, sys/dev/audio.c:audio_open() will fail immediately rather than
> later through the to-be-removed get_props() check.
>
> These are all drivers which simply don't support recording.
>
> In device-tree based drivers like simpleaudio(4)/rkiis(4) and newer
> Apple ones like aplaudio(4)/aplmca(4), this adds a new open() stub to
> the low-level drivers which merely does the duplex check.
>
> The rkiis(4) and aplmca(4) stubs don't open anything, but are now
> required for simpleaudio(4) and aplaudio(4) to actually do the low-level
> driver specific duplex check.
>
> My Pinebook Pro keeps playing audio and recording silence with this diff
> just like before (rkiis(4) is currently play-only):
> simpleaudio0 at mainbus0
> simpleaudio1 at mainbus0
> audio0 at simpleaudio1
>
> $ aucat -i song69.wav -o rec.wav
>
> Builds fine on amd64 and arm64.
> Feedback? Objection? OK?
Sorry, but I'm still not sure I understand how this is supposed to
work. Because...
> ---
> sys/arch/arm64/dev/aplaudio.c | 15 ---
> sys/arch/arm64/dev/aplmca.c| 20
> sys/arch/luna88k/cbus/nec86.c | 1 -
> sys/arch/luna88k/cbus/nec86hw.c| 10 --
> sys/arch/luna88k/cbus/nec86hwvar.h | 2 --
> sys/dev/fdt/graphaudio.c | 15 ---
> sys/dev/fdt/rkiis.c| 20 +++-
> sys/dev/fdt/simpleaudio.c | 15 ---
> sys/dev/ic/arcofi.c| 11 +++
> sys/dev/pci/maestro.c | 13 +++--
> 10 files changed, 33 insertions(+), 89 deletions(-)
>
> diff --git a/sys/arch/arm64/dev/aplaudio.c b/sys/arch/arm64/dev/aplaudio.c
> index 2e06448a882..49b0aac2f38 100644
> --- a/sys/arch/arm64/dev/aplaudio.c
> +++ b/sys/arch/arm64/dev/aplaudio.c
> @@ -55,7 +55,6 @@ voidaplaudio_freem(void *, void *, int);
> int aplaudio_set_port(void *, mixer_ctrl_t *);
> int aplaudio_get_port(void *, mixer_ctrl_t *);
> int aplaudio_query_devinfo(void *, mixer_devinfo_t *);
> -int aplaudio_get_props(void *);
> int aplaudio_round_blocksize(void *, int);
> size_t aplaudio_round_buffersize(void *, int, size_t);
> int aplaudio_trigger_output(void *, void *, void *, int,
> @@ -74,7 +73,6 @@ const struct audio_hw_if aplaudio_hw_if = {
> .set_port = aplaudio_set_port,
> .get_port = aplaudio_get_port,
> .query_devinfo = aplaudio_query_devinfo,
> - .get_props = aplaudio_get_props,
> .round_blocksize = aplaudio_round_blocksize,
> .round_buffersize = aplaudio_round_buffersize,
> .trigger_output = aplaudio_trigger_output,
> @@ -401,19 +399,6 @@ aplaudio_query_devinfo(void *cookie, mixer_devinfo_t
> *dip)
> return ENXIO;
> }
>
> -int
> -aplaudio_get_props(void *cookie)
> -{
> - struct aplaudio_softc *sc = cookie;
> - struct dai_device *dai = sc->sc_dai_cpu;
> - const struct audio_hw_if *hwif = dai->dd_hw_if;
> -
> - if (hwif->get_props)
> - return hwif->get_props(dai->dd_cookie);
> -
> - return 0;
> -}
> -
> int
> aplaudio_round_blocksize(void *cookie, int block)
> {
> diff --git a/sys/arch/arm64/dev/aplmca.c b/sys/arch/arm64/dev/aplmca.c
> index 559dd765933..0d323ffc0c6 100644
> --- a/sys/arch/arm64/dev/aplmca.c
> +++ b/sys/arch/arm64/dev/aplmca.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -123,11 +124,11 @@ struct aplmca_softc {
> int aplmca_set_format(void *, uint32_t, uint32_t, uint32_t);
> int aplmca_set_sysclk(void *, uint32_t);
>
> +int aplmca_open(void *, int);
> int aplmca_set_params(void *, int, int,
> struct audio_params *, struct audio_params *);
> void *aplmca_allocm(void *, int, size_t, int, int);
> void aplmca_freem(void *, void *, int);
> -int aplmca_get_props(void *);
> int aplmca_trigger_output(void *, void *, void *, int,
> void (*)(void *), void *, struct audio_params *);
> int aplmca_trigger_input(void *, void *, void *, int,
> @@ -136,8 +137,8 @@ int aplmca_halt_output(void *);
> int aplmca_halt_input(void *);
>
> const struct audio_hw_if aplmca_hw_if = {
> + .open = aplmca_open,
> .set_params = aplmca_set_params,
> - .get_props = aplmca_get_props,
> .allocm = aplmca_allocm,
> .freem = aplmca_freem,
> .trigger_output = aplmca_trigger_output,
> @@ -380,6 +381,15 @@ aplmca_set_sysclk(void *cookie, uint32_t rate)
> return clock_set_frequency_idx(sc->sc_node, ad->ad_cluster, rate);
> }
>
> +int
> +aplmca_open(void *cookie, int flags)
> +{
> + if (flags & (FWRITE | FREAD))
> + return ENXIO;
Doesn't this mean that you return ENXIO even if only playback is
requested? Because then flags would still contain
Re: Replace audio(9) get_props() with duplex check in open() for play-only drivers
On Thu, Oct 27, 2022 at 01:27:43PM +, Klemens Nanni wrote: > Make drivers which do *not* adverise AUDIO_PROP_FULLDPLEX return ENXIO > in their open() if full-duplex mode was requested. > > This way, sys/dev/audio.c:audio_open() will fail immediately rather than > later through the to-be-removed get_props() check. > > These are all drivers which simply don't support recording. > > In device-tree based drivers like simpleaudio(4)/rkiis(4) and newer > Apple ones like aplaudio(4)/aplmca(4), this adds a new open() stub to > the low-level drivers which merely does the duplex check. > > The rkiis(4) and aplmca(4) stubs don't open anything, but are now > required for simpleaudio(4) and aplaudio(4) to actually do the low-level > driver specific duplex check. > > My Pinebook Pro keeps playing audio and recording silence with this diff > just like before (rkiis(4) is currently play-only): > simpleaudio0 at mainbus0 > simpleaudio1 at mainbus0 > audio0 at simpleaudio1 > > $ aucat -i song69.wav -o rec.wav > > Builds fine on amd64 and arm64. > Feedback? Objection? OK? IMO, for play-only drivers, the check should be: if (flags & FREAD) return ENXIO; which will reject record-only and full-duplex.
Replace audio(9) get_props() with duplex check in open() for play-only drivers
Make drivers which do *not* adverise AUDIO_PROP_FULLDPLEX return ENXIO
in their open() if full-duplex mode was requested.
This way, sys/dev/audio.c:audio_open() will fail immediately rather than
later through the to-be-removed get_props() check.
These are all drivers which simply don't support recording.
In device-tree based drivers like simpleaudio(4)/rkiis(4) and newer
Apple ones like aplaudio(4)/aplmca(4), this adds a new open() stub to
the low-level drivers which merely does the duplex check.
The rkiis(4) and aplmca(4) stubs don't open anything, but are now
required for simpleaudio(4) and aplaudio(4) to actually do the low-level
driver specific duplex check.
My Pinebook Pro keeps playing audio and recording silence with this diff
just like before (rkiis(4) is currently play-only):
simpleaudio0 at mainbus0
simpleaudio1 at mainbus0
audio0 at simpleaudio1
$ aucat -i song69.wav -o rec.wav
Builds fine on amd64 and arm64.
Feedback? Objection? OK?
---
sys/arch/arm64/dev/aplaudio.c | 15 ---
sys/arch/arm64/dev/aplmca.c| 20
sys/arch/luna88k/cbus/nec86.c | 1 -
sys/arch/luna88k/cbus/nec86hw.c| 10 --
sys/arch/luna88k/cbus/nec86hwvar.h | 2 --
sys/dev/fdt/graphaudio.c | 15 ---
sys/dev/fdt/rkiis.c| 20 +++-
sys/dev/fdt/simpleaudio.c | 15 ---
sys/dev/ic/arcofi.c| 11 +++
sys/dev/pci/maestro.c | 13 +++--
10 files changed, 33 insertions(+), 89 deletions(-)
diff --git a/sys/arch/arm64/dev/aplaudio.c b/sys/arch/arm64/dev/aplaudio.c
index 2e06448a882..49b0aac2f38 100644
--- a/sys/arch/arm64/dev/aplaudio.c
+++ b/sys/arch/arm64/dev/aplaudio.c
@@ -55,7 +55,6 @@ void aplaudio_freem(void *, void *, int);
intaplaudio_set_port(void *, mixer_ctrl_t *);
intaplaudio_get_port(void *, mixer_ctrl_t *);
intaplaudio_query_devinfo(void *, mixer_devinfo_t *);
-intaplaudio_get_props(void *);
intaplaudio_round_blocksize(void *, int);
size_t aplaudio_round_buffersize(void *, int, size_t);
intaplaudio_trigger_output(void *, void *, void *, int,
@@ -74,7 +73,6 @@ const struct audio_hw_if aplaudio_hw_if = {
.set_port = aplaudio_set_port,
.get_port = aplaudio_get_port,
.query_devinfo = aplaudio_query_devinfo,
- .get_props = aplaudio_get_props,
.round_blocksize = aplaudio_round_blocksize,
.round_buffersize = aplaudio_round_buffersize,
.trigger_output = aplaudio_trigger_output,
@@ -401,19 +399,6 @@ aplaudio_query_devinfo(void *cookie, mixer_devinfo_t *dip)
return ENXIO;
}
-int
-aplaudio_get_props(void *cookie)
-{
- struct aplaudio_softc *sc = cookie;
- struct dai_device *dai = sc->sc_dai_cpu;
- const struct audio_hw_if *hwif = dai->dd_hw_if;
-
- if (hwif->get_props)
- return hwif->get_props(dai->dd_cookie);
-
- return 0;
-}
-
int
aplaudio_round_blocksize(void *cookie, int block)
{
diff --git a/sys/arch/arm64/dev/aplmca.c b/sys/arch/arm64/dev/aplmca.c
index 559dd765933..0d323ffc0c6 100644
--- a/sys/arch/arm64/dev/aplmca.c
+++ b/sys/arch/arm64/dev/aplmca.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -123,11 +124,11 @@ struct aplmca_softc {
intaplmca_set_format(void *, uint32_t, uint32_t, uint32_t);
intaplmca_set_sysclk(void *, uint32_t);
+intaplmca_open(void *, int);
intaplmca_set_params(void *, int, int,
struct audio_params *, struct audio_params *);
void *aplmca_allocm(void *, int, size_t, int, int);
void aplmca_freem(void *, void *, int);
-intaplmca_get_props(void *);
intaplmca_trigger_output(void *, void *, void *, int,
void (*)(void *), void *, struct audio_params *);
intaplmca_trigger_input(void *, void *, void *, int,
@@ -136,8 +137,8 @@ int aplmca_halt_output(void *);
intaplmca_halt_input(void *);
const struct audio_hw_if aplmca_hw_if = {
+ .open = aplmca_open,
.set_params = aplmca_set_params,
- .get_props = aplmca_get_props,
.allocm = aplmca_allocm,
.freem = aplmca_freem,
.trigger_output = aplmca_trigger_output,
@@ -380,6 +381,15 @@ aplmca_set_sysclk(void *cookie, uint32_t rate)
return clock_set_frequency_idx(sc->sc_node, ad->ad_cluster, rate);
}
+int
+aplmca_open(void *cookie, int flags)
+{
+ if (flags & (FWRITE | FREAD))
+ return ENXIO;
+
+ return 0;
+}
+
int
aplmca_set_params(void *cookie, int setmode, int usemode,
struct audio_params *play, struct audio_params *rec)
@@ -396,12 +406,6 @@ aplmca_set_params(void *cookie, int setmode, int usemode,
return 0;
}
-int
-aplmca_get_props(void *cookie)
-{
- return 0;
-}
-
void *
aplmca_allocm(void *cookie, int direction, size_t size, int type,
int flags)
diff --git a/sys/arch/luna88k/cbus/nec86.c
