Thanks for the review,
On 01/03/18 21:24, Mark Brown wrote:
On Tue, Feb 13, 2018 at 04:58:19PM +0000, srinivas.kandagatla@linaro.org wrote:
+static struct copp *adm_find_copp(struct q6adm *adm, int port_idx,
int copp_idx)+{
- struct copp *c;
- spin_lock(&adm->copps_list_lock);
- list_for_each_entry(c, &adm->copps_list, node) {
if ((port_idx == c->afe_port) && (copp_idx == c->copp_idx)) {spin_unlock(&adm->copps_list_lock);return c;}- }
- spin_unlock(&adm->copps_list_lock);
We've again got this use of spinlocks here but no IRQ safety - what exactly is going on with the locking? In general all of the locking in this stuff is raising very serious alarm bells with me, I don't understand what is being protected against what and there's some very obvious bugs. We could probably use some documentation about what the locking is supposed to be doing.
I agree, there are locking issues here, Am revisiting them all before I send a next version.
- case ADM_CMDRSP_DEVICE_OPEN_V5: {
copp->id = open->copp_id;wake_up(&copp->wait);- }
- break;
- default:
This indentation is confusing.
I agree, will fix such instances in next version.
thanks, srini