| Message ID | 1432304474-6533-4-git-send-email-o-takashi@sakamocchi.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
At Fri, 22 May 2015 23:21:13 +0900, Takashi Sakamoto wrote: > > When detecting invalid value in 'dbs' field of CIP header or packet > discontinuity, current implementation reports the status by err_info(). dev_info() > In most cases this state is caused by model-specific issue due to > vendor's customization and should be reported to developers. > > This commit use dev_err() instead of dev_info() for this purpose. > In the cases, packet streaming is aborted, thus no message floading > occurs. > > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> > --- > sound/firewire/amdtp.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c > index 29efbda..93cf93a 100644 > --- a/sound/firewire/amdtp.c > +++ b/sound/firewire/amdtp.c > @@ -723,7 +723,7 @@ static int handle_in_packet(struct amdtp_stream *s, > (cip_header[0] & CIP_DBS_MASK) >> CIP_DBS_SHIFT; > /* avoid division by zero */ > if (data_block_quadlets == 0) { > - dev_info_ratelimited(&s->unit->device, > + dev_err(&s->unit->device, Here you dropped _ratelimited(). Are you sure that it won't give any problem? Takashi > "Detect invalid value in dbs field: %08X\n", > cip_header[0]); > return -EIO; > @@ -756,9 +756,9 @@ static int handle_in_packet(struct amdtp_stream *s, > } > > if (lost) { > - dev_info(&s->unit->device, > - "Detect discontinuity of CIP: %02X %02X\n", > - s->data_block_counter, data_block_counter); > + dev_err(&s->unit->device, > + "Detect discontinuity of CIP: %02X %02X\n", > + s->data_block_counter, data_block_counter); > return -EIO; > } > > -- > 2.1.4 >
Takashi Sakamoto wrote: > When detecting invalid value in 'dbs' field of CIP header or packet > discontinuity, current implementation reports the status by err_info(). > [...] > This commit use dev_err() instead of dev_info() for this purpose. > In the cases, packet streaming is aborted, thus no message floading > occurs. An aborted stream ends up in the XRUN state; the application is likely to prepare and start the stream again. So it is likely that there will be message flooding. Regards, Clemens
Hi, On May 23 2015 00:08, Takashi Iwai wrote: > Here you dropped _ratelimited(). Are you sure that it won't give any > problem? Similar situation as we discussed before: http://mailman.alsa-project.org/pipermail/alsa-devel/2015-May/092192.html On May 23 2015 03:05, Clemens Ladisch wrote: > An aborted stream ends up in the XRUN state; the application is likely > to prepare and start the stream again. So it is likely that there > will be message flooding. Considering about how frequently this message can be generated at receiving such problematic packets, Not so flooding. In the situation, all of committed drivers (fireworks/bebob/oxfw/dice) stops isochronous contexts and actual streams once, then start new streams and isochronous contexts. This is because our old packet stream falls to error state. Therefore, when devices transfers such problematic packets, different isochronous contexts (old one and new one) generate the error messages. This means that there're a gap between the generated error messages, approximately several hundred seconds because it cost expensive for devices to restart isochronous streams. In my opinion, this is not such flooding. Regards Takashi Sakamoto
On May 23 2015 13:16, Takashi Sakamoto wrote: > This means that there're a gap between the generated > error messages, approximately several hundred seconds because it cost > expensive for devices to restart isochronous streams. In my opinion, > this is not such flooding. Oops, "several hundred mili-seconds"... Regards Takashi Sakamoto
diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c index 29efbda..93cf93a 100644 --- a/sound/firewire/amdtp.c +++ b/sound/firewire/amdtp.c @@ -723,7 +723,7 @@ static int handle_in_packet(struct amdtp_stream *s, (cip_header[0] & CIP_DBS_MASK) >> CIP_DBS_SHIFT; /* avoid division by zero */ if (data_block_quadlets == 0) { - dev_info_ratelimited(&s->unit->device, + dev_err(&s->unit->device, "Detect invalid value in dbs field: %08X\n", cip_header[0]); return -EIO; @@ -756,9 +756,9 @@ static int handle_in_packet(struct amdtp_stream *s, } if (lost) { - dev_info(&s->unit->device, - "Detect discontinuity of CIP: %02X %02X\n", - s->data_block_counter, data_block_counter); + dev_err(&s->unit->device, + "Detect discontinuity of CIP: %02X %02X\n", + s->data_block_counter, data_block_counter); return -EIO; }
When detecting invalid value in 'dbs' field of CIP header or packet discontinuity, current implementation reports the status by err_info(). In most cases this state is caused by model-specific issue due to vendor's customization and should be reported to developers. This commit use dev_err() instead of dev_info() for this purpose. In the cases, packet streaming is aborted, thus no message floading occurs. Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> --- sound/firewire/amdtp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)