Fix fw boot retry - V3

[ranj063]

This is a follow-up to #507.

[keyonjie]

good fixes. minor change comment inline may be better.

[keyonjie]

How about changing it like this:

	if (!ret) {
dev_dbg(sdev->dev, "Firmware download successful, booting...\n");
goto done;
}
dev_err(sdev->dev, "error: load fw failed ret: %d\n", ret);
err:

[plbossart]

+1

[plbossart]

first patch is good, second one less clear, needs more work. Thanks!

[plbossart]

+1

[plbossart]

going from here to done: looks odd, why would you get the stream tag at this point?

[ranj063]

@plbossart@keyonjie I've fixed this to disable the DSP after stream cleanup.

[plbossart]

nicer code but you still search for the same stream twice when you could run the search loop only once.

[plbossart]

One doubt still on the error on the returned value, see below. one more version and we can merge I guess.

[plbossart]

is there a reason why you want to return here without doing the DSP disable? Or is it a leftover from the previous code before you added the DSP disable?

[ranj063]

@plbossart we should disable the DSP only in case of errors. if ret is 0, it means fw boot was successful.

[plbossart]

@ranj063 the diff format is misleading, see the entire code

ret1=cl_cleanup(sdev, &sdev->dmab, stream);
if (ret1<0) {
dev_err(sdev->dev, "error: Code loader DSP cleanup failed\n"); ret=ret1;
}
if (!ret)
returnret;

Why would you return if ret1 was not zero? the cleanup failed at this point

[ranj063]

@plbossart if cleanup fails, ret1 will be negative and I set ret to ret1 and the condition (!ret) will not be true. So we will not return but proceed with disabling the DSP.

[ranj063]

@plbossart maybe I should add some comments to make it clearer

[ranj063]

@plbossart I've added some comments to make the error flow a bit clearer. Please let me know if thats helpful.

[plbossart]

LGTM, waiting for CI results before merge.