| Message ID | 1393558072-25926-19-git-send-email-o-takashi@sakamocchi.jp (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Takashi Iwai |
| Headers | show |
On Feb 28 Takashi Sakamoto wrote: > Linux Firewire Subsystem currently gives > 200msec for default timeout (See DEFAULT_SPLIT_TIMEOUT in core-card.c). It's 2000 ms actually. And this is the IEEE 1394 split transaction timeout of course. FCP transactions on the other hand are transported by means of two IEEE 1394 transactions: One in which the FCP requester is IEEE 1394 requester, and another one in which the FCP responder is IEEE 1394 requester. firewire-core's split transaction timeout is only relevant to the request subaction of an FCP transaction (i.e. the first one of the two IEEE 1394 transaction), and only if this one is being performed as a split transaction rather than a unified transaction, which depends on the FCP responder hardware. (I am not aware of any hardware which handles FCP request subactions as unified transactions.) IEC 61883-1 does not specify any FCP transaction timeout. The 1394 TA document "AV/C Digital Interface Command Set, Rev. 1.0, 1996" defines FCP transactions (for use with AV/C command set) much more precisely than IEC 61883-1 does: - To each FCP request, there may be 0..n FCP responses. - An AV/C target may emit a final response (one with response code < 0xf) up to 100 ms after reception of an FCP request. - Or the AV/C target may emit an intermediate response (one with response code 0xf for interim response) up to 100 ms after reception of an FCP request. The target shall emit no additional interim response in this transaction, but one final response. There is no timeout specified for this final-after-interim response, i.e. it may take an undefined, possibly very long time. (The AV/C specification v4.0, 2001 adds that subunit specifications may define such timeouts for certain commands.) - Or the AV/C target may still be busy processing a previous command. In this case, the specification accepts that the target does not send any response to subsequent requests at all until it is done with the previous work. The requester may the retry such a failed request after the mentioned 100 ms timeout. (Plus IEEE 1394 transport overhead, but the AV/C specification v1.0 does not mention this. AV/C v4.0 explains that the requester should take certain transport dependent delays into account in addition to the 100 ms FCP timeout, e.g. physical layer repeater delays, or busy retry delays. Furthermore, AV/C v4.0 recommends that a busy target returns certain acknowledge codes or response codes to pipelined requests that exceed its capacity to handle concurrent FCP transactions.) - An "IEEE 1394.0 reset" (sic) terminates any outstanding FCP transaction. (I suppose a 1394 bus reset is meant by this. AV/C v4.0 clarifies this to be an IEEE 1394 bus reset indeed.) From a quick glance, the FCP transaction specification looks unchanged from AV/C 4.0, 2001 to v4.1, 2001 and to v4.2, 2004.
On Feb 28 Stefan Richter wrote: > - Or the AV/C target may still be busy processing a previous command. > In this case, the specification accepts that the target does not send > any response to subsequent requests at all until it is done with the > previous work. To be more precise: The specification accepts that the target *ignores* incoming AV/C command frames while processing an AV/C command. This is indicated to the requester by lack of a response to any ignored request. To me, this implies that the target must send a response to any request that it did not ignore, i.e. to any command that it executed. On the other hand, I don't think there is any rollback behavior specified or practical in cases when the target is unable to deliver a response to the initiator.
Stefan, At first, I introduce this is for a patch below. You can see actual logs: [alsa-devel] [PATCH 39/39] bebob: Add support for M-Audio special Firewire series http://mailman.alsa-project.org/pipermail/alsa-devel/2014-February/073484.html > It's 2000 ms actually. Oops. Here I made a mistake to refer to unrelated value here. I might consider about the other thing when writing this commit, perhaps because of noon in Friday and a bit tired... Well, yes. It's 2000 msec. (Feb 28 2014 12:27), Takashi Sakamoto wrote: > As a result, in worst case that all of three retries are timeout > and devices don't send responses, fcp_avc_transaction() returns 1sec > later ((200 + 125) msec * 3 times). So in this case, this function returns 6,375msec later. (but it's rare, again.) > And this is the IEEE 1394 split transaction timeout of course. FCP > transactions on the other hand are transported by means of two IEEE 1394 > transactions: One in which the FCP requester is IEEE 1394 requester, and > another one in which the FCP responder is IEEE 1394 requester. > > firewire-core's split transaction timeout is only relevant to the request > subaction of an FCP transaction (i.e. the first one of the two IEEE 1394 > transaction), OK. > and only if this one is being performed as a split > transaction rather than a unified transaction, which depends on the FCP > responder hardware. (I am not aware of any hardware which handles > FCP request subactions as unified transactions.) As You explained in this message: [alsa-devel] Echo Fireworks control protocol (was Re: Sample program for hwdep interface) http://mailman.alsa-project.org/pipermail/alsa-devel/2014-February/071978.html > IEC 61883-1 does not specify any FCP transaction timeout. I've confirmed it. > The 1394 TA document "AV/C Digital Interface Command Set, Rev. 1.0, > 1996" defines FCP transactions (for use with AV/C command set) much more > precisely than IEC 61883-1 does: > > - To each FCP request, there may be 0..n FCP responses. > > - An AV/C target may emit a final response (one with response code < > 0xf) up to 100 ms after reception of an FCP request. > > - Or the AV/C target may emit an intermediate response (one with > response code 0xf for interim response) up to 100 ms after reception > of an FCP request. The target shall emit no additional interim > response in this transaction, but one final response. There is no > timeout specified for this final-after-interim response, i.e. it may > take an undefined, possibly very long time. (The AV/C > specification v4.0, 2001 adds that subunit specifications may define > such timeouts for certain commands.) > > - Or the AV/C target may still be busy processing a previous command. > In this case, the specification accepts that the target does not send > any response to subsequent requests at all until it is done with the > previous work. The requester may the retry such a failed request after > the mentioned 100 ms timeout. (Plus IEEE 1394 transport overhead, but > the AV/C specification v1.0 does not mention this. AV/C v4.0 explains > that the requester should take certain transport dependent delays > into account in addition to the 100 ms FCP timeout, e.g. physical layer > repeater delays, or busy retry delays. Furthermore, AV/C v4.0 > recommends that a busy target returns certain acknowledge codes or > response codes to pipelined requests that exceed its capacity > to handle concurrent FCP transactions.) > > - An "IEEE 1394.0 reset" (sic) terminates any outstanding FCP transaction. > (I suppose a 1394 bus reset is meant by this. AV/C v4.0 clarifies this > to be an IEEE 1394 bus reset indeed.) Additionally, there are two types of AV/C transactions, 'immediate transaction' and 'deffered transaction'. Current 'firewire-lib' don't implement 'deffered transaction' so fcp_avc_transaction() always returns first AV/C response frame. This will be response=INTERIM against ctype=STATUS/NOTIFY. The gap between a first response and a second response of 'deffered transaction' is defined as 'unspecified interval'. But this is not related to an issue about which this patch mentions. (Mar 01 2014 05:39), Stefan Richter wrote: > To be more precise: The specification accepts that the target*ignores* > incoming AV/C command frames while processing an AV/C command. This is > indicated to the requester by lack of a response to any ignored request. > > To me, this implies that the target must send a response to any request > that it did not ignore, i.e. to any command that it executed. On the > other hand, I don't think there is any rollback behavior specified or > practical in cases when the target is unable to deliver a response to the > initiator. > >>From a quick glance, the FCP transaction specification looks unchanged > from AV/C 4.0, 2001 to v4.1, 2001 and to v4.2, 2004. I cannot find some sections about such ignorance in 'AV/C Digital Interface Command Set General Specification Version 4.2 (Sep 01, 2004, 1394TA)' and 'IEC 61883-1 ed2'. Can I request you any pointers about this ignorance? Thanks Takashi Sakamoto o-takashi@sakamocchi.jp
On Mar 01 Takashi Sakamoto wrote: > Stefan, > > At first, I introduce this is for a patch below. You can see actual logs: > [alsa-devel] [PATCH 39/39] bebob: Add support for M-Audio special > Firewire series > http://mailman.alsa-project.org/pipermail/alsa-devel/2014-February/073484.html Right, I saw it (and your note that the iso resource management related transactions still need a similar workaround). > > It's 2000 ms actually. > > Oops. Here I made a mistake to refer to unrelated value here. I might > consider about the other thing when writing this commit, perhaps because > of noon in Friday and a bit tired... > > Well, yes. It's 2000 msec. > > (Feb 28 2014 12:27), Takashi Sakamoto wrote: > > As a result, in worst case that all of three retries are timeout > > and devices don't send responses, fcp_avc_transaction() returns 1sec > > later ((200 + 125) msec * 3 times). > > So in this case, this function returns 6,375msec later. (but it's rare, > again.) This 125 ms part of the calculation made me wonder what the split transaction timeout of FCP transactions is (I didn't remember), so I looked it up and wrote down what I found. So, all of the rest of my reply was not really a direct comment to your patch but merely a loosely related side note. > > And this is the IEEE 1394 split transaction timeout of course. FCP > > transactions on the other hand are transported by means of two IEEE 1394 > > transactions: One in which the FCP requester is IEEE 1394 requester, and > > another one in which the FCP responder is IEEE 1394 requester. > > > > firewire-core's split transaction timeout is only relevant to the request > > subaction of an FCP transaction (i.e. the first one of the two IEEE 1394 > > transaction), > > OK. (Per IEEE 1394 specification, it should in fact also be the upper bound of our own 1394 response timing, e.g. when we respond to FCP response frame reception. But we haven't implemented such a time limit for outbound 1394 response packets yet.) > > and only if this one is being performed as a split > > transaction rather than a unified transaction, which depends on the FCP > > responder hardware. (I am not aware of any hardware which handles > > FCP request subactions as unified transactions.) > > As You explained in this message: > [alsa-devel] Echo Fireworks control protocol (was Re: Sample program for > hwdep interface) > http://mailman.alsa-project.org/pipermail/alsa-devel/2014-February/071978.html > > > IEC 61883-1 does not specify any FCP transaction timeout. > > I've confirmed it. > > > The 1394 TA document "AV/C Digital Interface Command Set, Rev. 1.0, > > 1996" defines FCP transactions (for use with AV/C command set) much more > > precisely than IEC 61883-1 does: > > > > - To each FCP request, there may be 0..n FCP responses. Together with the rest of the spec, it's 0..2 FCP responses, precisely. > > - An AV/C target may emit a final response (one with response code < > > 0xf) up to 100 ms after reception of an FCP request. > > > > - Or the AV/C target may emit an intermediate response (one with > > response code 0xf for interim response) up to 100 ms after reception > > of an FCP request. The target shall emit no additional interim > > response in this transaction, but one final response. There is no > > timeout specified for this final-after-interim response, i.e. it may > > take an undefined, possibly very long time. (The AV/C > > specification v4.0, 2001 adds that subunit specifications may define > > such timeouts for certain commands.) > > > > - Or the AV/C target may still be busy processing a previous command. > > In this case, the specification accepts that the target does not send > > any response to subsequent requests at all until it is done with the > > previous work. The requester may the retry such a failed request after > > the mentioned 100 ms timeout. (Plus IEEE 1394 transport overhead, but > > the AV/C specification v1.0 does not mention this. AV/C v4.0 explains > > that the requester should take certain transport dependent delays > > into account in addition to the 100 ms FCP timeout, e.g. physical layer > > repeater delays, or busy retry delays. Furthermore, AV/C v4.0 > > recommends that a busy target returns certain acknowledge codes or > > response codes to pipelined requests that exceed its capacity > > to handle concurrent FCP transactions.) > > > > - An "IEEE 1394.0 reset" (sic) terminates any outstanding FCP transaction. > > (I suppose a 1394 bus reset is meant by this. AV/C v4.0 clarifies this > > to be an IEEE 1394 bus reset indeed.) > > Additionally, there are two types of AV/C transactions, 'immediate > transaction' and 'deffered transaction'. These are the first two cases in my list above. Put into other words, here are all possible cases of 0, 1, or 2 responses to one and the same request: Cases with 0 responses: a) The target ignored a command with reserved ctype value. b) The target ignored a command that it received before it finished processing a prior command. c) A bus reset happened before the target had a chance to send the first response. The FCP transaction specification part of the AV/C spec does not say what kind of effects such an aborted transaction may have. Perhaps some of the various command related definitions specify this, I haven't looked. d) The specification of a particular command defines that handling of this command shall (or may) not be followed up by a response. I haven't looked whether such commands actually exist. Cases with 1 response: e) The target sends a final response as first response. f) The target sent an interim response as first response, but a bus reset happened before it could send the final response. Cases with 2 responses: g) The target sends an interim response, and then a final response. This is only possible with CONTROL and NOTIFY commands. > Current 'firewire-lib' don't > implement 'deffered transaction' so fcp_avc_transaction() always returns > first AV/C response frame. This will be response=INTERIM against > ctype=STATUS/NOTIFY. Correction: CONTROL or NOTIFY. (In contrast, STATUS or SPECIFIC INQUIRY or GENERAL INQUIRY commands can only be performed in immediate transactions. See AV/C v4.2 table 19. Likewise, already AV/C v1.0 allows INTERIM response only to CONTROL or NOTIFY commands.) > The gap between a first response and a second > response of 'deffered transaction' is defined as 'unspecified interval'. I wonder: Now that firewire-lib's use cases are expanded far beyond LaCie Speakers and Griffin FireWave, do we need to implement deferred transactions in firewire-lib too? As far as I can tell, FFADO simply ignores INTERIM responses, and treats deferred transactions which are completed within 200...400 ms the same way as if it was an immediate transaction. (This is 200 ms from IEEE1394SERVICE_FCP_RESPONSE_TIMEOUT_USEC and 400 ms from IEEE1394SERVICE_FCP_POLL_TIMEOUT_MSEC in the FFADO code.) (By the way, there is another FCP transaction layer implementation existing in the kernel: drivers/media/firewire/firedtv-avc.c. This one does in fact implement deferred transactions, but only for one particular command, SFE_VENDOR_OPCODE_REGISTER_REMOTE_CONTROL.) A related question: Since FFADO applies 200 ms or more as FCP transaction timeout, shouldn't firewire-lib's fcp.c increase FCP_TIMEOUT_MS from 125 to 200 or more as well? > But this is not related to an issue about which this patch mentions. Right. > (Mar 01 2014 05:39), Stefan Richter wrote: > > To be more precise: The specification accepts that the target*ignores* > > incoming AV/C command frames while processing an AV/C command. This is > > indicated to the requester by lack of a response to any ignored request. > > > > To me, this implies that the target must send a response to any request > > that it did not ignore, i.e. to any command that it executed. On the > > other hand, I don't think there is any rollback behavior specified or > > practical in cases when the target is unable to deliver a response to the > > initiator. > > > >>From a quick glance, the FCP transaction specification looks unchanged > > from AV/C 4.0, 2001 to v4.1, 2001 and to v4.2, 2004. > > I cannot find some sections about such ignorance in 'AV/C Digital > Interface Command Set General Specification Version 4.2 (Sep 01, 2004, > 1394TA)' and 'IEC 61883-1 ed2'. > > Can I request you any pointers about this ignorance? This is about case b) and, now that I think about it, also about case a) in my list further above. AV/C v1.0 says about case a)... If the AV/C command frame contains a reserved value in the ctype field, the target shall ignore the command and shall not generate a response frame. ...and about case b): If the target is already occupied with a previous command, it may ignore any AV/C command frames received. Note that the receipt of an AV/C command frame shall always be acknowledged by a target. The target ignores a command frame by a failure to return a response frame. [...] If the target is not occupied with a previous command, it shall create an AV/C response frame [...] AV/C v4.2 says about case a)... If the AV/C command frame contains a reserved value in the ctype field, the target shall ignore the command and shall not generate a response frame. ...and about case b): Before sending a first response to a command, the number of AV/C commands that a target can process is implementation dependent. If the target cannot process an additional command, it should return a 1394 ack_busy, ack_conflict_error, or resp_conflict_error to the additional command. NOTE – Some targets that are compliant to the previous version of this specification may ignore the additional command in the above case.
On Feb 28 Takashi Sakamoto wrote: > --- a/sound/firewire/lib.c > +++ b/sound/firewire/lib.c > @@ -22,7 +22,7 @@ > * @length: length of @buffer > * @flags: use %FW_FIXED_GENERATION and add the generation value to attempt the > * request only in that generation; use %FW_QUIET to suppress error > - * messages > + * messages: use %FW_RETURN_TIMEOUT to avoid retry at RCODE_CANCELLED For clarity, this should be a semicolon instead of a colon, i.e. messages; use %FW_RETURN_TIMEOUT to avoid retry at RCODE_CANCELLED
Hi Stefan, > This 125 ms part of the calculation made me wonder what the split > transaction timeout of FCP transactions is (I didn't remember), so I > looked it up and wrote down what I found. So, all of the rest of my > reply was not really a direct comment to your patch but merely a loosely > related side note. I think it's not a proper action in this ML... My intent of this comment is for a notice about time till returining from fcp_avc_transaction(). It might be much time against developer's expectation. > Correction: CONTROL or NOTIFY. (In contrast, STATUS or SPECIFIC INQUIRY > or GENERAL INQUIRY commands can only be performed in immediate > transactions. See AV/C v4.2 table 19. Likewise, already AV/C v1.0 allows > INTERIM response only to CONTROL or NOTIFY commands.) I confirm my mistake, thanks for your correction ;) > I wonder: Now that firewire-lib's use cases are expanded far beyond LaCie > Speakers and Griffin FireWave, do we need to implement deferred > transactions in firewire-lib too? Yes. I confirmed some devices use deffered transaction for AV/C CONTROL request. But I still pending this issue, because of the way to handle the 'unspecified interval'. I plan to solve this issue for my future work. This series of patch is the most I can do, now. > A related question: Since FFADO applies 200 ms or more as FCP transaction > timeout, shouldn't firewire-lib's fcp.c increase FCP_TIMEOUT_MS from 125 > to 200 or more as well? For this developing, I've spent much time with my test devices. But I've never experienced disadvantages under FCP_TIMEOUT_MS=125msec. So feel no importance. If you feel this importance, please post your patch with proper reasons. > This is about case b) and, now that I think about it, also about case a) > in my list further above. The case a) is for ctype=reserved. Current ALSA Firewire drivers don't utilize it. So I have never considered about this case. For the case b), I'm optimistic because current fcp_avc_transaction() don't return till the first AV/C response comes (or AV/C request is error). This means that the driver basically don't send any AV/C transaction at the same time. Furthermore, my drivers basically generate some AV/C transaction in driver's probe/remove state or starting/stopping playback/capture of PCM samples/MIDI messages. So I believe that FFADO/ALSA rarely transfer some AV/C transactions at the same time. In these reasons, I'm optimistic for device's ignorance of AV/C request. Anyway, I appreciate your advices. Thank you to spend time for my developing. Takashi Sakamoto o-takashi@sakamocchi.jp
(Mar 02 2014 19:34), Stefan Richter wrote: >> - * messages >> + * messages: use %FW_RETURN_TIMEOUT to avoid retry at RCODE_CANCELLED > > For clarity, this should be a semicolon instead of a colon, i.e. > > messages; use %FW_RETURN_TIMEOUT to avoid retry at RCODE_CANCELLED Exactly. Thank you. Takashi Sakamoto o-takashi@sakamocchi.jp
On Mar 01 Takashi Sakamoto wrote: [...] >> I wonder: Now that firewire-lib's use cases are expanded far >> beyond LaCie Speakers and Griffin FireWave, do we need to implement >> deferred transactions in firewire-lib too? > > Yes. I confirmed some devices use deffered transaction for AV/C > CONTROL request. > But I still pending this issue, because of the way to handle the > 'unspecified interval'. Are these devices already supported or reported to work with FFADO? If yes, then the interval is probably typically less than FFADO's built-in FCP transaction timeout, i.e. 200 (or 400?) ms. > I plan to solve this issue for my future work. OK. >> A related question: Since FFADO applies 200 ms or more as FCP >> transaction timeout, shouldn't firewire-lib's fcp.c increase >> FCP_TIMEOUT_MS from 125 to 200 or more as well? > > For this developing, I've spent much time with my test devices. > But I've never experienced disadvantages under FCP_TIMEOUT_MS=125msec. > So feel no importance. > > If you feel this importance, please post your patch with proper reasons. This is mostly a question to the ffado-devel subscribers. 125 ms is of course enough for devices which comply with the specification in this regard. The question is whether FFADO developers know of devices (or suspect devices) which exceed the standard 100 ms and need more like 200 ms. In any case, this question is unrelated to your patch (apart from the unlikely worst-case timeout which you noted in the changelog). >> This is about case b) and, now that I think about it, also about case >> a) in my list further above. > > The case a) is for ctype=reserved. Current ALSA Firewire drivers don't > utilize it. > So I have never considered about this case. Of course; the kernel is never going to emit a reserved ctype. And if malicious or faulty userspace does, the specified target behavior is exactly the desired behavior. > For the case b), I'm optimistic because current fcp_avc_transaction() > don't return till the first AV/C response comes (or AV/C request is > error). This means that the driver basically don't send any AV/C > transaction at the same time. > > Furthermore, my drivers basically generate some AV/C transaction in > driver's probe/remove state or starting/stopping playback/capture of > PCM samples/MIDI messages. So I believe that FFADO/ALSA rarely transfer > some AV/C transactions at the same time. > > In these reasons, I'm optimistic for device's ignorance of AV/C request. From what I understood so far, I agree. (And besides, the specified behavior of ignoring the command and not sending a response is of course better than, for example, execution of a command but failure to send the response.)
On Sat, Mar 01, 2014 at 03:20:50PM +0100, Stefan Richter wrote: > On Mar 01 Takashi Sakamoto wrote: > >> A related question: Since FFADO applies 200 ms or more as FCP > >> transaction timeout, shouldn't firewire-lib's fcp.c increase > >> FCP_TIMEOUT_MS from 125 to 200 or more as well? > > > > For this developing, I've spent much time with my test devices. > > But I've never experienced disadvantages under FCP_TIMEOUT_MS=125msec. > > So feel no importance. > > > > If you feel this importance, please post your patch with proper reasons. > > This is mostly a question to the ffado-devel subscribers. 125 ms is of > course enough for devices which comply with the specification in this > regard. The question is whether FFADO developers know of devices (or > suspect devices) which exceed the standard 100 ms and need more like 200 > ms. I've personally had nothing to do with devices utilising FCP transactions so unfortunately I don't really know. My feeling is that the authors of the respective FFADO drivers would not have applied an FCP timeout of 200 ms if there was no demonstrated need for it. Therefore in the absence of other evidence I would be assuming that there are devices which require the higher timeout allowed for in FFADO's streaming code. However, I don't know which specific devices these might be. Regards jonathan
On Mar 04 Jonathan Woithe wrote: > On Sat, Mar 01, 2014 at 03:20:50PM +0100, Stefan Richter wrote: > > >> Since FFADO applies 200 ms or more as FCP > > >> transaction timeout, shouldn't firewire-lib's fcp.c increase > > >> FCP_TIMEOUT_MS from 125 to 200 or more as well? [...] > I've personally had nothing to do with devices utilising FCP transactions so > unfortunately I don't really know. My feeling is that the authors of the > respective FFADO drivers would not have applied an FCP timeout of 200 ms if > there was no demonstrated need for it. Therefore in the absence of other > evidence I would be assuming that there are devices which require the higher > timeout allowed for in FFADO's streaming code. However, I don't know which > specific devices these might be. ./libffado/config.h.in: #define IEEE1394SERVICE_FCP_RESPONSE_TIMEOUT_USEC 200000 was added in this FFADO revision: http://subversion.ffado.org/changeset/1371 Timestamp: 10/23/08 09:00:47 (5 years ago) Author: ppalmers Message: * implement our own code to do FCP transactions. the code from libavc had too much side-effects. * remove libavc1394 as a dependency * set the SPLIT_TIMEOUT value for the host controller such that late responses by the DM1x00 based devices are not discarded. Should fix the issues with FA-101 discovery. (re: #155, #162) I will look through the libavc1394 source history.
diff --git a/sound/firewire/cmp.c b/sound/firewire/cmp.c index a8199fd..73f5f26 100644 --- a/sound/firewire/cmp.c +++ b/sound/firewire/cmp.c @@ -89,13 +89,32 @@ static int pcr_modify(struct cmp_connection *c, err = snd_fw_transaction( c->resources.unit, TCODE_LOCK_COMPARE_SWAP, get_offset(c, false), buffer, 8, - FW_FIXED_GENERATION | c->resources.generation); + FW_FIXED_GENERATION | c->resources.generation | + FW_RETURN_TIMEOUT); if (err < 0) { if (err == -EAGAIN && bus_reset_handling == SUCCEED_ON_BUS_RESET) err = 0; - return err; + + if (err != -ETIMEDOUT) + return err; + + /* Check current PCR. */ + err = snd_fw_transaction( + c->resources.unit, TCODE_READ_QUADLET_REQUEST, + get_offset(c, false), buffer, 4, + FW_FIXED_GENERATION | c->resources.generation); + if (err < 0) + return err; + + /* The lock transaction may be failed, retry */ + if (buffer[0] != buffer[1]) { + buffer[0] = c->last_pcr_value; + continue; + } + + break; } if (buffer[0] == old_arg) /* success? */ diff --git a/sound/firewire/fcp.c b/sound/firewire/fcp.c index e903ddb..6fc813f 100644 --- a/sound/firewire/fcp.c +++ b/sound/firewire/fcp.c @@ -245,8 +245,9 @@ int fcp_avc_transaction(struct fw_unit *unit, : TCODE_WRITE_BLOCK_REQUEST; ret = snd_fw_transaction(t.unit, tcode, CSR_REGISTER_BASE + CSR_FCP_COMMAND, - (void *)command, command_size, 0); - if (ret < 0) + (void *)command, command_size, + FW_RETURN_TIMEOUT); + if ((ret < 0) && (ret != -ETIMEDOUT)) break; wait_event_timeout(t.wait, t.state != STATE_PENDING, diff --git a/sound/firewire/lib.c b/sound/firewire/lib.c index 7409edb..6c3278b 100644 --- a/sound/firewire/lib.c +++ b/sound/firewire/lib.c @@ -22,7 +22,7 @@ * @length: length of @buffer * @flags: use %FW_FIXED_GENERATION and add the generation value to attempt the * request only in that generation; use %FW_QUIET to suppress error - * messages + * messages: use %FW_RETURN_TIMEOUT to avoid retry at RCODE_CANCELLED * * Submits an asynchronous request to the target device, and waits for the * response. The node ID and the current generation are derived from @unit. @@ -53,6 +53,10 @@ int snd_fw_transaction(struct fw_unit *unit, int tcode, if (rcode == RCODE_GENERATION && (flags & FW_FIXED_GENERATION)) return -EAGAIN; + /* timeout */ + if ((rcode == RCODE_CANCELLED) && (flags & FW_RETURN_TIMEOUT)) + return -ETIMEDOUT; + if (rcode_is_permanent_error(rcode) || ++tries >= 3) { if (!(flags & FW_QUIET)) dev_err(&unit->device, diff --git a/sound/firewire/lib.h b/sound/firewire/lib.h index 02cfabc..b8ea90b 100644 --- a/sound/firewire/lib.h +++ b/sound/firewire/lib.h @@ -9,6 +9,7 @@ struct fw_unit; #define FW_GENERATION_MASK 0x00ff #define FW_FIXED_GENERATION 0x0100 #define FW_QUIET 0x0200 +#define FW_RETURN_TIMEOUT 0x0400 int snd_fw_transaction(struct fw_unit *unit, int tcode, u64 offset, void *buffer, size_t length,
I confirm that some devices often send no response against driver's request even if the devices handle the request. In this case, drivers should not retry the request because it's not clear whether the devices already handled an operation of the request or not. The second request may bring errors or make no sence if the request was handled. This patch adds a flag, FW_RETURN_TIMEOUT, for snd_fw_transaction(). With this flag, this function returns -ETIMEOUT when fw_run_transaction() returns RCODE_CALCELLED. For FCP, this patch modifies fcp_avc_transaction() with this flag. The -ETIMEDOUT is handled as success. Linux Firewire Subsystem currently gives 200msec for default timeout (See DEFAULT_SPLIT_TIMEOUT in core-card.c). firewire-lib gives 125msec for wait_event_timeout(). As a result, in worst case that all of three retries are timeout and devices don't send responses, fcp_avc_transaction() returns 1sec later ((200 + 125) msec * 3 times). But this is rare. For CMP, this patch modifies pcr_modify() with this flag. The -ETIMEOUT generates another read transaction to check current value of PCR. If this read transaction returns errors, then pcr_modify() returns the error code. Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> --- sound/firewire/cmp.c | 23 +++++++++++++++++++++-- sound/firewire/fcp.c | 5 +++-- sound/firewire/lib.c | 6 +++++- sound/firewire/lib.h | 1 + 4 files changed, 30 insertions(+), 5 deletions(-)