Hi Dan,
On 21/03/2023 16:46, Dan Carpenter wrote:
On Tue, Mar 21, 2023 at 04:40:05PM +0200, Péter Ujfalusi wrote:
On 21/03/2023 16:16, Dan Carpenter wrote:
On Tue, Mar 21, 2023 at 03:49:19PM +0200, Peter Ujfalusi wrote:
The patch adding the bytes control support moved the error check outside of the list_for_each_entry() which will cause issues when we will have support for multiple controls per widgets.
Even now it causes an issue. We're exiting the list_for_each_entry() without hitting a break statement so the scontrol points to somewhere in the middle of the sdev instead of to a valid scontrol entry.
The scontrol->comp_id will be some garbage value.
I'm not sure what you see
No, the patch is correct. My issue is with the commit message because it says "will cause issues when we will have support for multiple controls per widgets." The bug already causes issues now.
It bugged me a great deal how could I have missed this initially as I was testing the firmware side and user space, I had not one failure with this until all the pieces found there places...
The reason is simple: we have one control per swidget and in the error print: dev_err(sdev->dev, "kcontrol %d set up failed for widget %s\n", scontrol->comp_id, swidget->widget->name);
only the widget's name gave usable information for a human. If we would have taken the comp_id from swidget->comp_id then this would not have been discovered.
scontrol was not invalid (but ignored), swidget name was correct, one control per swidget, all looked about right for the eye ;)
Again, thanks for the report!