[alsa-devel] [PATCH 0/3] ALSA: firewire: fix minor bugs
Hi,
This patchset fixes some minor regressions and mis-programming. These bugs has quite less influences, thus they're not worth for stable maintenance.
I'd like to apply them into kernel 4.10 or later.
Takashi Sakamoto (3): ALSA: fireworks: fix asymmetric API call at unit removal ALSA: firewire-tascam: Fix to handle error from initialization of stream data ALSA: firewire-lib: change structure member with proper type
sound/firewire/amdtp-stream.c | 2 +- sound/firewire/amdtp-stream.h | 4 ++-- sound/firewire/fireworks/fireworks_stream.c | 2 +- sound/firewire/tascam/tascam-stream.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-)
ALSA fireworks driver has a bug not to call an API to destroy 'cmp_connection' structure for input direction. Currently this causes no issues because it just destroys 'mutex' structure, while it's better to fix it for future work.
Fix: d23c2cc4485d ("ALSA: fireworks/bebob/dice/oxfw: allow stream destructor after releasing runtime") Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/fireworks/fireworks_stream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/firewire/fireworks/fireworks_stream.c b/sound/firewire/fireworks/fireworks_stream.c index ee47924..827161b 100644 --- a/sound/firewire/fireworks/fireworks_stream.c +++ b/sound/firewire/fireworks/fireworks_stream.c @@ -117,7 +117,7 @@ destroy_stream(struct snd_efw *efw, struct amdtp_stream *stream) conn = &efw->in_conn;
amdtp_stream_destroy(stream); - cmp_connection_destroy(&efw->out_conn); + cmp_connection_destroy(conn); }
static int
This module has a bug not to return error code in a case that data structure for transmitted packets fails to be initialized.
This commit fixes the bug.
Fixes: 35efa5c489de ("ALSA: firewire-tascam: add streaming functionality") Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/tascam/tascam-stream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/firewire/tascam/tascam-stream.c b/sound/firewire/tascam/tascam-stream.c index 4ad3bd7..f1657a4 100644 --- a/sound/firewire/tascam/tascam-stream.c +++ b/sound/firewire/tascam/tascam-stream.c @@ -343,7 +343,7 @@ int snd_tscm_stream_init_duplex(struct snd_tscm *tscm) if (err < 0) amdtp_stream_destroy(&tscm->rx_stream);
- return 0; + return err; }
/* At bus reset, streaming is stopped and some registers are clear. */
The 'amdtp_stream' structure is initialized by a call of 'amdtp_stream_init()'. Although a parameter of this function is for bit flags of packet attributes, its type is enumerator.
This commit changes the type so that it's proper for a bit flags.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 2 +- sound/firewire/amdtp-stream.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 00060c4..8ce93cd 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -69,7 +69,7 @@ static void pcm_period_tasklet(unsigned long data); * @protocol_size: the size to allocate newly for protocol */ int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit, - enum amdtp_stream_direction dir, enum cip_flags flags, + enum amdtp_stream_direction dir, int flags, unsigned int fmt, amdtp_stream_process_data_blocks_t process_data_blocks, unsigned int protocol_size) diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index c1bc7fa..7be2142 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -93,7 +93,7 @@ typedef unsigned int (*amdtp_stream_process_data_blocks_t)( unsigned int *syt); struct amdtp_stream { struct fw_unit *unit; - enum cip_flags flags; + int flags; enum amdtp_stream_direction direction; struct mutex mutex;
@@ -137,7 +137,7 @@ struct amdtp_stream { };
int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit, - enum amdtp_stream_direction dir, enum cip_flags flags, + enum amdtp_stream_direction dir, int flags, unsigned int fmt, amdtp_stream_process_data_blocks_t process_data_blocks, unsigned int protocol_size);
Takashi Sakamoto wrote:
The 'amdtp_stream' structure is initialized by a call of 'amdtp_stream_init()'. Although a parameter of this function is for bit flags of packet attributes, its type is enumerator.
This is the correct type to use for an enumeration consisting of bit flags. What problem do you see with this?
int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit,
enum amdtp_stream_direction dir, enum cip_flags flags,
enum amdtp_stream_direction dir, int flags, unsigned int fmt,
Regards, Clemens
Hi Clemens,
On Jan 4 2017 20:13, Clemens Ladisch wrote:
Takashi Sakamoto wrote:
The 'amdtp_stream' structure is initialized by a call of 'amdtp_stream_init()'. Although a parameter of this function is for bit flags of packet attributes, its type is enumerator.
This is the correct type to use for an enumeration consisting of bit flags. What problem do you see with this?
We have cases that an actual value to the parameter is not one of enumeration-constants. I had this concern and posted the patch.
But this is permitted, as you said. C language specification has loose constrains to the actual value of the type, just within a range of enumeration-constants. I understand that a list of enumeration-constants is just used to decide the actual integer type of the type.
I'll post revert patch. Thanks for your indication!
int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit,
enum amdtp_stream_direction dir, enum cip_flags flags,
enum amdtp_stream_direction dir, int flags, unsigned int fmt,
Regards
Takashi Sakamoto
On Tue, 03 Jan 2017 03:58:31 +0100, Takashi Sakamoto wrote:
Hi,
This patchset fixes some minor regressions and mis-programming. These bugs has quite less influences, thus they're not worth for stable maintenance.
I'd like to apply them into kernel 4.10 or later.
Takashi Sakamoto (3): ALSA: fireworks: fix asymmetric API call at unit removal ALSA: firewire-tascam: Fix to handle error from initialization of stream data ALSA: firewire-lib: change structure member with proper type
Applied all three patches to for-linus branch. Thanks.
Takashi
participants (3)
-
Clemens Ladisch -
Takashi Iwai -
Takashi Sakamoto