At Wed, 4 Jun 2014 15:25:32 +0900, Takashi Sakamoto wrote:

All assignment for local variables in these functions are not related to critical section.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp

Applied, thanks.

Takashi

---

sound/firewire/bebob/bebob_stream.c | 4 ++-- sound/firewire/fireworks/fireworks_stream.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c index bc4f827..ef4d0c9 100644 --- a/sound/firewire/bebob/bebob_stream.c +++ b/sound/firewire/bebob/bebob_stream.c @@ -655,8 +655,6 @@ void snd_bebob_stream_stop_duplex(struct snd_bebob *bebob) struct amdtp_stream *master, *slave; atomic_t *master_substreams, *slave_substreams;

• mutex_lock(&bebob->mutex);
• if (bebob->master == &bebob->rx_stream) { slave = &bebob->tx_stream; master = &bebob->rx_stream;

@@ -669,6 +667,8 @@ void snd_bebob_stream_stop_duplex(struct snd_bebob *bebob) master_substreams = &bebob->capture_substreams; }

• mutex_lock(&bebob->mutex);
• if (atomic_read(slave_substreams) == 0) { amdtp_stream_pcm_abort(slave); amdtp_stream_stop(slave);

diff --git a/sound/firewire/fireworks/fireworks_stream.c b/sound/firewire/fireworks/fireworks_stream.c index 5415690..b985fc5 100644 --- a/sound/firewire/fireworks/fireworks_stream.c +++ b/sound/firewire/fireworks/fireworks_stream.c @@ -284,8 +284,6 @@ void snd_efw_stream_stop_duplex(struct snd_efw *efw) struct amdtp_stream *master, *slave; atomic_t *master_substreams, *slave_substreams;

• mutex_lock(&efw->mutex);
• if (efw->master == &efw->rx_stream) { slave = &efw->tx_stream; master = &efw->rx_stream;

@@ -298,6 +296,8 @@ void snd_efw_stream_stop_duplex(struct snd_efw *efw) master_substreams = &efw->capture_substreams; }

• mutex_lock(&efw->mutex);
• if (atomic_read(slave_substreams) == 0) { stop_stream(efw, slave);

-- 1.8.3.2

At Wed, 4 Jun 2014 15:25:33 +0900, Takashi Sakamoto wrote:

To reverse a pointer for the ring buffer, subtraction by buffer size is better than assignment to the beginning of the buffer.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp

Applied, thanks.

Takashi

---

sound/firewire/fireworks/fireworks_hwdep.c | 2 +- sound/firewire/fireworks/fireworks_transaction.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/firewire/fireworks/fireworks_hwdep.c b/sound/firewire/fireworks/fireworks_hwdep.c index 4f8216f..33df865 100644 --- a/sound/firewire/fireworks/fireworks_hwdep.c +++ b/sound/firewire/fireworks/fireworks_hwdep.c @@ -58,7 +58,7 @@ hwdep_read_resp_buf(struct snd_efw *efw, char __user *buf, long remained, efw->pull_ptr += till_end; if (efw->pull_ptr >= efw->resp_buf + snd_efw_resp_buf_size)

• 		efw->pull_ptr = efw->resp_buf;
  

• 		efw->pull_ptr -= snd_efw_resp_buf_size;
  
  length -= till_end;
  buf += till_end;
  

diff --git a/sound/firewire/fireworks/fireworks_transaction.c b/sound/firewire/fireworks/fireworks_transaction.c index aa56b8a..a6a9e9f 100644 --- a/sound/firewire/fireworks/fireworks_transaction.c +++ b/sound/firewire/fireworks/fireworks_transaction.c @@ -148,7 +148,7 @@ copy_resp_to_buf(struct snd_efw *efw, void *data, size_t length, int *rcode)

efw->push_ptr += till_end;
if (efw->push_ptr >= efw->resp_buf + snd_efw_resp_buf_size)

• 	efw->push_ptr = efw->resp_buf;
  

• 	efw->push_ptr -= snd_efw_resp_buf_size;
  
  

  length -= till_end; data += till_end;

-- 1.8.3.2

At Wed, 4 Jun 2014 15:25:34 +0900, Takashi Sakamoto wrote:

It includes descriptions to cause misreading.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp

Applied, thanks.

Takashi

---

sound/firewire/fireworks/fireworks_transaction.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/sound/firewire/fireworks/fireworks_transaction.c b/sound/firewire/fireworks/fireworks_transaction.c index a6a9e9f..255dabc 100644 --- a/sound/firewire/fireworks/fireworks_transaction.c +++ b/sound/firewire/fireworks/fireworks_transaction.c @@ -8,19 +8,19 @@

/*

• Fireworks have its own transaction. The transaction can be delivered by AV/C

• • Vendor Specific command. But at least Windows driver and firmware version 5.5
• • or later don't use it.

• • Vendor Specific command frame or usual asynchronous transaction. At least,
• • Windows driver and firmware version 5.5 or later don't use AV/C command.
  • Transaction substance:

• • At first, 6 data exist. Following to the 6 data, parameters for each
• • commands exists. All of parameters are 32 bit alighed to big endian.

• • At first, 6 data exist. Following to the data, parameters for each command
• • exist. All of the parameters are 32 bit alighed to big endian.
  • data[0]: Length of transaction substance
  • data[1]: Transaction version
  • data[2]: Sequence number. This is incremented by the device

• • data[3]: transaction category
• • data[4]: transaction command
• • data[5]: return value in response.
• • data[6-]: parameters

• • data[3]: Transaction category
• • data[4]: Transaction command
• • data[5]: Return value in response.
• • data[6-]: Parameters
  • Transaction address:
  • command: 0xecc000000000

-- 1.8.3.2

At Wed, 4 Jun 2014 15:25:35 +0900, Takashi Sakamoto wrote:

The constants of enum snd_efw_grp_type is for struct snd_efw_phys_grp.type. But this member is 1 byte. Although the value is between 0x00-0xff, a constant has 0x10000. This constant is meaningless.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp

Applied, thanks.

Takashi

---

sound/firewire/fireworks/fireworks.h | 1 - 1 file changed, 1 deletion(-)

diff --git a/sound/firewire/fireworks/fireworks.h b/sound/firewire/fireworks/fireworks.h index d2b36be..4f0201a 100644 --- a/sound/firewire/fireworks/fireworks.h +++ b/sound/firewire/fireworks/fireworks.h @@ -162,7 +162,6 @@ enum snd_efw_grp_type { SND_EFW_CH_TYPE_GUITAR = 7, SND_EFW_CH_TYPE_PIEZO_GUITAR = 8, SND_EFW_CH_TYPE_GUITAR_STRING = 9,

• SND_EFW_CH_TYPE_VIRTUAL = 0x10000, SND_EFW_CH_TYPE_DUMMY

}; struct snd_efw_phys_meters { -- 1.8.3.2

At Wed, 4 Jun 2014 15:25:36 +0900, Takashi Sakamoto wrote:

Currently mutex_unlock() is called in module's cleanup function. But after cleaned up, this mutex is automatically released. So this function call is meaningless.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp

It must be meant as mutex_destroy()? I applied it with the typo fixes. Thanks.

Takashi

---

sound/firewire/fireworks/fireworks.c | 1 - 1 file changed, 1 deletion(-)

diff --git a/sound/firewire/fireworks/fireworks.c b/sound/firewire/fireworks/fireworks.c index 996fdc4..3e2ed8e 100644 --- a/sound/firewire/fireworks/fireworks.c +++ b/sound/firewire/fireworks/fireworks.c @@ -346,7 +346,6 @@ static void __exit snd_efw_exit(void) { snd_efw_transaction_unregister(); driver_unregister(&efw_driver.driver);

• mutex_destroy(&devices_mutex);

}

module_init(snd_efw_init);

1.8.3.2