[PATCH 0/3] ALSA: firewire: fix minor bugs

[PATCH 0/3] ALSA: firewire: fix minor bugs

[Takashi Sakamoto]

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(-)

-- 
2.9.3

[PATCH 1/3] ALSA: fireworks: fix asymmetric API call at unit removal

[Takashi Sakamoto]

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

[PATCH 2/3] ALSA: firewire-tascam: Fix to handle error from initialization of stream data

[Takashi Sakamoto]

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. */
-- 
2.9.3

[PATCH 3/3] ALSA: firewire-lib: change structure member with proper type

[Takashi Sakamoto]

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

Re: [PATCH 0/3] ALSA: firewire: fix minor bugs

[Takashi Iwai]

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

Re: [PATCH 3/3] ALSA: firewire-lib: change structure member with proper type

[Clemens Ladisch]

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

Re: [PATCH 3/3] ALSA: firewire-lib: change structure member with proper type

[Takashi Sakamoto]

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