From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5DA5EC48BD5 for ; Tue, 25 Jun 2019 13:48:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 2FF40213F2 for ; Tue, 25 Jun 2019 13:48:59 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="r1rdcjeX" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726599AbfFYNs6 (ORCPT ); Tue, 25 Jun 2019 09:48:58 -0400 Received: from mail-io1-f68.google.com ([209.85.166.68]:38867 "EHLO mail-io1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726423AbfFYNs6 (ORCPT ); Tue, 25 Jun 2019 09:48:58 -0400 Received: by mail-io1-f68.google.com with SMTP id j6so711907ioa.5 for ; Tue, 25 Jun 2019 06:48:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=0GZB7vPzRz7lX9qbxkSEpP1KLhPlWAT4/FgiLzGhH+s=; b=r1rdcjeXi4MAwAvMD46wul0KccgFY0EgllO1e9xUMldteC7SW/pHDO3uvCrEiKmgGm aeFrfHq7saV5CRGC8Mz5umOUNpPmMbZXsMLqBS7ucu1YSLhevwBnGhM2FGo1ozCAQWR6 1wnuRW3oW2HVeeVkRJXe3Y4aumn+LpEdPss5UyvBVSp3Q8TL/nislh28U3UKaH3uscsh 7j4FlrKstl88AlccywZ5TdgX2fLFN7WMgqbGU5+7VAKnIYwNW7hrROX+njb6Cq8hz31c 9++qX0i7JwQZB+cQp2yjIREQSQWyeGIyt/d9qm83GBBsgYTMzx/dtL0FED0FW4cjbL5U poXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=0GZB7vPzRz7lX9qbxkSEpP1KLhPlWAT4/FgiLzGhH+s=; b=H46uDs2U1DwnltLU9LVFFymr+h5SNgYL3dJcbSDhKCQ5Hj7Zm79mUMCI0T6Iit2KMe SMQeYbr0+/vVIL0gFwKPznhzVGCzEOeMDzKp9VCahxuUJzSutibP9GZOx9luAnBu3CAG U1oDdv0jREH/oNN09bwLeNBjpUBtBOMrz8nko25ciO+WZkLmnB3yYygJlloKiRpn1NeX QuJfu5JZi10Ur12NJqSVh8W6iBbmAVCicwYgxz220abn0bxS0yKka9qmDLaK0hZA+ZxM 8K/VOXQyhYrNsN+822Ux6voDqL+t/k5/DvfYoe0qr01f/cRfcr1CVeBEQjAA8aUV8xDm 0V7Q== X-Gm-Message-State: APjAAAWPwvSwAT+u5QrATYOSEXeeFcLuYG4uBruDvNS77biA907ftII+ 0H+dU8YpO/fIFM0KYbjW+AznU+FxOz/plocxPTmhSw== X-Google-Smtp-Source: APXvYqxZXtosOMgMyOjQCcCtq+V7ie85OCZKFxgrYamt76zeQqFJw1tOsZ7F8w3e+47pEtLQnoEgW2aOruUNwGm/lSM= X-Received: by 2002:a02:554a:: with SMTP id e71mr31810557jab.144.1561470537559; Tue, 25 Jun 2019 06:48:57 -0700 (PDT) MIME-Version: 1.0 References: <20190624160330.38048-1-hverkuil-cisco@xs4all.nl> <20190624160330.38048-4-hverkuil-cisco@xs4all.nl> In-Reply-To: <20190624160330.38048-4-hverkuil-cisco@xs4all.nl> From: Dariusz Marcinkiewicz Date: Tue, 25 Jun 2019 15:48:46 +0200 Message-ID: Subject: Re: [PATCHv8 03/13] cec: add new notifier functions To: Hans Verkuil Cc: linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, Cheng-yi Chiang Content-Type: text/plain; charset="UTF-8" Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Hello. Some small comments/questions. On Mon, Jun 24, 2019 at 6:03 PM Hans Verkuil wrote: > ... > @@ -22,9 +22,11 @@ struct cec_notifier { > struct list_head head; > struct kref kref; > struct device *hdmi_dev; > + struct cec_connector_info conn_info; > const char *conn_name; > struct cec_adapter *cec_adap; > void (*callback)(struct cec_adapter *adap, u16 pa); > + bool called_cec_notifier_register; If I read his correctly callback is set only by cec_notifier_register (and not by the cec_notifier_cec_adap_register) so maybe that boolean is not needed and just checking if the callback is set is enough to tell those 2 cases apart? ... > +struct cec_notifier * > +cec_notifier_cec_adap_register(struct device *hdmi_dev, const char *conn_name, > + struct cec_adapter *adap) > +{ > + struct cec_notifier *n; > + > + if (WARN_ON(!adap)) > + return NULL; > + > + n = cec_notifier_get_conn(hdmi_dev, conn_name); > + if (!n) > + return n; > + > + mutex_lock(&n->lock); > + n->cec_adap = adap; > + adap->conn_info = n->conn_info; Would it make sense to use cec_s_conn_info? There is a certain asymmetry here - cec_s_phys_addr is used to configure physical address, which also takes the adapter's lock while setting the physical address. That lock is not taken while the connector info is being set (not sure if that really matters for the code paths that would call into this function). > + adap->notifier = n; > + cec_s_phys_addr(adap, n->phys_addr, false); > + mutex_unlock(&n->lock); > + return n; > +} > +EXPORT_SYMBOL_GPL(cec_notifier_cec_adap_register); > + > +void cec_notifier_cec_adap_unregister(struct cec_notifier *n) > +{ > + if (!n) > + return; > + > + mutex_lock(&n->lock); > + memset(&n->cec_adap->conn_info, 0, sizeof(n->cec_adap->conn_info)); Could cec_s_conn_info be used to reset the connector info? Also, we explicitly clear connector info here. Since the notifier provides both connector info and physical address, maybe it would make sense to clear physical address as well? ... > void cec_notifier_unregister(struct cec_notifier *n) > { > + /* Do nothing unless cec_notifier_register was called first */ > + if (!n->called_cec_notifier_register) Could this check be made with n->lock held? cec_notifier_register sets this value while holding that lock. ... Thank you. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dariusz Marcinkiewicz Subject: Re: [PATCHv8 03/13] cec: add new notifier functions Date: Tue, 25 Jun 2019 15:48:46 +0200 Message-ID: References: <20190624160330.38048-1-hverkuil-cisco@xs4all.nl> <20190624160330.38048-4-hverkuil-cisco@xs4all.nl> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-io1-xd44.google.com (mail-io1-xd44.google.com [IPv6:2607:f8b0:4864:20::d44]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7762D6E0F6 for ; Tue, 25 Jun 2019 13:48:58 +0000 (UTC) Received: by mail-io1-xd44.google.com with SMTP id k20so784113ios.10 for ; Tue, 25 Jun 2019 06:48:58 -0700 (PDT) In-Reply-To: <20190624160330.38048-4-hverkuil-cisco@xs4all.nl> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Hans Verkuil Cc: Cheng-yi Chiang , dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org SGVsbG8uCgpTb21lIHNtYWxsIGNvbW1lbnRzL3F1ZXN0aW9ucy4KCk9uIE1vbiwgSnVuIDI0LCAy MDE5IGF0IDY6MDMgUE0gSGFucyBWZXJrdWlsIDxodmVya3VpbC1jaXNjb0B4czRhbGwubmw+IHdy b3RlOgo+Ci4uLgo+IEBAIC0yMiw5ICsyMiwxMSBAQCBzdHJ1Y3QgY2VjX25vdGlmaWVyIHsKPiAg ICAgICAgIHN0cnVjdCBsaXN0X2hlYWQgaGVhZDsKPiAgICAgICAgIHN0cnVjdCBrcmVmIGtyZWY7 Cj4gICAgICAgICBzdHJ1Y3QgZGV2aWNlICpoZG1pX2RldjsKPiArICAgICAgIHN0cnVjdCBjZWNf Y29ubmVjdG9yX2luZm8gY29ubl9pbmZvOwo+ICAgICAgICAgY29uc3QgY2hhciAqY29ubl9uYW1l Owo+ICAgICAgICAgc3RydWN0IGNlY19hZGFwdGVyICpjZWNfYWRhcDsKPiAgICAgICAgIHZvaWQg KCpjYWxsYmFjaykoc3RydWN0IGNlY19hZGFwdGVyICphZGFwLCB1MTYgcGEpOwo+ICsgICAgICAg Ym9vbCBjYWxsZWRfY2VjX25vdGlmaWVyX3JlZ2lzdGVyOwpJZiBJIHJlYWQgaGlzIGNvcnJlY3Rs eSBjYWxsYmFjayBpcyBzZXQgb25seSBieSBjZWNfbm90aWZpZXJfcmVnaXN0ZXIKKGFuZCBub3Qg YnkgdGhlIGNlY19ub3RpZmllcl9jZWNfYWRhcF9yZWdpc3Rlcikgc28gbWF5YmUgdGhhdCBib29s ZWFuCmlzIG5vdCBuZWVkZWQgYW5kIGp1c3QgY2hlY2tpbmcgaWYgdGhlIGNhbGxiYWNrIGlzIHNl dCBpcyBlbm91Z2ggdG8KdGVsbCB0aG9zZSAyIGNhc2VzIGFwYXJ0PwoKLi4uCj4gK3N0cnVjdCBj ZWNfbm90aWZpZXIgKgo+ICtjZWNfbm90aWZpZXJfY2VjX2FkYXBfcmVnaXN0ZXIoc3RydWN0IGRl dmljZSAqaGRtaV9kZXYsIGNvbnN0IGNoYXIgKmNvbm5fbmFtZSwKPiArICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgc3RydWN0IGNlY19hZGFwdGVyICphZGFwKQo+ICt7Cj4gKyAgICAgICBz dHJ1Y3QgY2VjX25vdGlmaWVyICpuOwo+ICsKPiArICAgICAgIGlmIChXQVJOX09OKCFhZGFwKSkK PiArICAgICAgICAgICAgICAgcmV0dXJuIE5VTEw7Cj4gKwo+ICsgICAgICAgbiA9IGNlY19ub3Rp Zmllcl9nZXRfY29ubihoZG1pX2RldiwgY29ubl9uYW1lKTsKPiArICAgICAgIGlmICghbikKPiAr ICAgICAgICAgICAgICAgcmV0dXJuIG47Cj4gKwo+ICsgICAgICAgbXV0ZXhfbG9jaygmbi0+bG9j ayk7Cj4gKyAgICAgICBuLT5jZWNfYWRhcCA9IGFkYXA7Cj4gKyAgICAgICBhZGFwLT5jb25uX2lu Zm8gPSBuLT5jb25uX2luZm87CldvdWxkIGl0IG1ha2Ugc2Vuc2UgdG8gdXNlIGNlY19zX2Nvbm5f aW5mbz8gVGhlcmUgaXMgYSBjZXJ0YWluCmFzeW1tZXRyeSBoZXJlICAtIGNlY19zX3BoeXNfYWRk ciBpcyB1c2VkIHRvIGNvbmZpZ3VyZSBwaHlzaWNhbAphZGRyZXNzLCB3aGljaCBhbHNvIHRha2Vz IHRoZSBhZGFwdGVyJ3MgbG9jayB3aGlsZSBzZXR0aW5nIHRoZQpwaHlzaWNhbCBhZGRyZXNzLiBU aGF0IGxvY2sgaXMgbm90IHRha2VuIHdoaWxlIHRoZSBjb25uZWN0b3IgaW5mbyBpcwpiZWluZyBz ZXQgKG5vdCBzdXJlIGlmIHRoYXQgcmVhbGx5IG1hdHRlcnMgZm9yIHRoZSBjb2RlIHBhdGhzIHRo YXQKd291bGQgY2FsbCBpbnRvIHRoaXMgZnVuY3Rpb24pLgoKPiArICAgICAgIGFkYXAtPm5vdGlm aWVyID0gbjsKPiArICAgICAgIGNlY19zX3BoeXNfYWRkcihhZGFwLCBuLT5waHlzX2FkZHIsIGZh bHNlKTsKPiArICAgICAgIG11dGV4X3VubG9jaygmbi0+bG9jayk7Cj4gKyAgICAgICByZXR1cm4g bjsKPiArfQo+ICtFWFBPUlRfU1lNQk9MX0dQTChjZWNfbm90aWZpZXJfY2VjX2FkYXBfcmVnaXN0 ZXIpOwo+ICsKPiArdm9pZCBjZWNfbm90aWZpZXJfY2VjX2FkYXBfdW5yZWdpc3RlcihzdHJ1Y3Qg Y2VjX25vdGlmaWVyICpuKQo+ICt7Cj4gKyAgICAgICBpZiAoIW4pCj4gKyAgICAgICAgICAgICAg IHJldHVybjsKPiArCj4gKyAgICAgICBtdXRleF9sb2NrKCZuLT5sb2NrKTsKPiArICAgICAgIG1l bXNldCgmbi0+Y2VjX2FkYXAtPmNvbm5faW5mbywgMCwgc2l6ZW9mKG4tPmNlY19hZGFwLT5jb25u X2luZm8pKTsKQ291bGQgY2VjX3NfY29ubl9pbmZvIGJlIHVzZWQgdG8gcmVzZXQgdGhlIGNvbm5l Y3RvciBpbmZvPwpBbHNvLCB3ZSBleHBsaWNpdGx5IGNsZWFyIGNvbm5lY3RvciBpbmZvIGhlcmUu IFNpbmNlIHRoZSBub3RpZmllcgpwcm92aWRlcyBib3RoIGNvbm5lY3RvciBpbmZvIGFuZCBwaHlz aWNhbCBhZGRyZXNzLCBtYXliZSBpdCB3b3VsZCBtYWtlCnNlbnNlIHRvIGNsZWFyIHBoeXNpY2Fs IGFkZHJlc3MgYXMgd2VsbD8KCgouLi4KPiAgdm9pZCBjZWNfbm90aWZpZXJfdW5yZWdpc3Rlcihz dHJ1Y3QgY2VjX25vdGlmaWVyICpuKQo+ICB7Cj4gKyAgICAgICAvKiBEbyBub3RoaW5nIHVubGVz cyBjZWNfbm90aWZpZXJfcmVnaXN0ZXIgd2FzIGNhbGxlZCBmaXJzdCAqLwo+ICsgICAgICAgaWYg KCFuLT5jYWxsZWRfY2VjX25vdGlmaWVyX3JlZ2lzdGVyKQpDb3VsZCB0aGlzIGNoZWNrIGJlIG1h ZGUgd2l0aCBuLT5sb2NrIGhlbGQ/IGNlY19ub3RpZmllcl9yZWdpc3RlciBzZXRzCnRoaXMgdmFs dWUgd2hpbGUgaG9sZGluZyB0aGF0IGxvY2suCi4uLgoKClRoYW5rIHlvdS4KX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlz dApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0 b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVs