lkml.org 
[lkml]   [2018]   [Mar]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v3 07/25] ASoC: qcom: qdsp6: Add support to Q6ADM
    From
    Date
    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

    \
     
     \ /
      Last update: 2018-03-06 10:28    [W:2.718 / U:0.408 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site