Re: [PATCH v3 07/25] ASoC: qcom: qdsp6: Add support to Q6ADM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx 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
--
To unsubscribe from this list: send the line "unsubscribe linux-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux