Skip to content

Conversation

@Bin-QA
Copy link
Contributor

@Bin-QA Bin-QA commented Dec 25, 2019

After patch: thesofproject/linux#1676 merged
add "error: status" and "error: cl_dsp_init" into ignore string
to avoid catch DPS reset which can be ignored

Signed-off-by: Wu, BinX binx.wu@intel.com

@Bin-QA Bin-QA requested review from libinyang and xiulipan December 25, 2019 07:20
@bardliao
Copy link
Contributor

How about changing the message on kernel?
https://github.com/thesofproject/linux/blob/topic/sof-dev/sound/soc/sof/intel/hda-loader.c#L331
Maybe use dev_warn and "warning:" instead of dev_err and "error:".

@Bin-QA
Copy link
Contributor Author

Bin-QA commented Dec 26, 2019

How about changing the message on kernel?
https://github.com/thesofproject/linux/blob/topic/sof-dev/sound/soc/sof/intel/hda-loader.c#L331
Maybe use dev_warn and "warning:" instead of dev_err and "error:".

        # 0 error: cl_dsp_init
        # 1 error: status
        # 2 error: Error
        # 3 error: iteration

If you want modify the source code those information also need to modify:
Line 0 error: cl_dsp_init
https://github.com/thesofproject/linux/blob/topic/sof-dev/sound/soc/sof/intel/hda-loader.c#L81
Line 1 error: status
https://github.com/thesofproject/linux/blob/topic/sof-dev/sound/soc/sof/intel/hda.c#L204
Line 2 error: Error
https://github.com/thesofproject/linux/blob/topic/sof-dev/sound/soc/sof/intel/hda-loader.c#L331
Line 3 error: iteration
https://github.com/thesofproject/linux/blob/topic/sof-dev/sound/soc/sof/intel/hda-loader.c#L336

Because from the kernel log, catch 4 error message when DSP reset
On the other hand, I format the error message to avoid miss some really error

dsp_count=$(echo "$err"|grep 'error: cl_dsp_init' -c)
allow_count=$(echo "$err"|grep 'error: cl_dsp_init' -A3|grep 'iteration [01]' -c)

@bardliao

@xiulipan
Copy link
Contributor

@bardliao @Bin-QA Let wait for the final discussion about the dsp init message. I think this error is not user friendly.

@wenqingfu
Copy link

wenqingfu commented Jan 8, 2020

@bardliao @Bin-QA Let wait for the final discussion about the dsp init message. I think this error is not user friendly.

Which discussion do you refer to @xiulipan ?

I don't see this discussion pattern is finalizing. I agree with @bardliao. Similarly, the unexpected reply shouldn't be called error, either. If they are not real errors, then don't call it error. It freak people out.

small fix, big good, please just act with your patch. @bardliao , can you please fix it?

@bardliao
Copy link
Contributor

bardliao commented Jan 9, 2020

@bardliao @Bin-QA Let wait for the final discussion about the dsp init message. I think this error is not user friendly.

Which discussion do you refer to @xiulipan ?

I don't see this discussion pattern is finalizing. I agree with @bardliao. Similarly, the unexpected reply shouldn't be called error, either. If they are not real errors, then don't call it error. It freak people out.

small fix, big good, please just act with your patch. @bardliao , can you please fix it?

Yes, I will send a kernel PR to fix it.

@xiulipan
Copy link
Contributor

xiulipan commented Jan 9, 2020

@wenqingfu It seems the vacation break the discussion. I also agree with @bardliao that we should not output error when we have retry method.
@ranj063 what is your opinion here, even the S0ix refine thesofproject/linux#1631 may have benefit on this. But the error with retry will still make people get panic.

@wenqingfu
Copy link

Thanks, @xiulipan @bardliao . connect the dot with thesofproject/linux#1676 , Seems we have overwhelmingly consensus of "let's don't freak people out" there. :)

After patch: thesofproject/linux#1676 merged
add "error: status" and "error: cl_dsp_init" into ignore string
to avoid catch DPS reset which can be ignored

Signed-off-by: Wu, BinX <binx.wu@intel.com>
@Bin-QA
Copy link
Contributor Author

Bin-QA commented Jan 14, 2020

Update for kernel patch merged
@xiulipan @libinyang please help to review this patch

Copy link
Contributor

@xiulipan xiulipan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let keep this as temp solution here. I think the error will be very misleading in none-debug build.

Copy link
Contributor

@stripes416 stripes416 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM.

@xiulipan xiulipan merged commit 50f6ccf into thesofproject:master Jan 15, 2020
@Bin-QA
Copy link
Contributor Author

Bin-QA commented Jan 15, 2020

@xiulipan
ignore_str="no reply expected,|error: debugfs write failed to idle -16|error: status |error: cl_dsp_init|iteration [01]"

How about remove 'failed' keyword in err_str
if here just use 'failed' to catch DSP reset

@xiulipan
Copy link
Contributor

@Bin-QA I think there will also be IPC failed, so let keep it to see if we got any other issues.
Please send the patch to the change
ignore_str="no reply expected,|error: debugfs write failed to idle -16|error: status |error: cl_dsp_init|iteration [01]"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants