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! -- Péter
