From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brian Norris Subject: Re: [PATCH v4 1/3] drm/bridge/synopsys: dsi: stop clobbering drvdata Date: Tue, 5 Dec 2017 10:56:52 -0800 Message-ID: <20171205185651.GA136446@google.com> References: <1512100685-4015-1-git-send-email-nickey.yang@rock-chips.com> <1512100685-4015-2-git-send-email-nickey.yang@rock-chips.com> <5605c3c2-56ad-2289-c3c0-78cde1152123@st.com> <1700743e-04b0-90ea-d393-dbfebf93742f@st.com> <206b071f-2b57-c4a2-31b3-2f469091e5e2@rock-chips.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <206b071f-2b57-c4a2-31b3-2f469091e5e2@rock-chips.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Nickey Yang Cc: "mark.rutland@arm.com" , "airlied@linux.ie" , "hoegsberg@gmail.com" , Philippe CORNU , "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , Yannick FERTRE , "linux-rockchip@lists.infradead.org" , "robh+dt@kernel.org" , "laurent.pinchart@ideasonboard.com" , "zyw@rock-chips.com" , "xbl@rock-chips.com" , "mka@chromium.org" , "hl@rock-chips.com" List-Id: linux-rockchip.vger.kernel.org SGkgTmlja2V5LAoKT24gVHVlLCBEZWMgMDUsIDIwMTcgYXQgMDU6MTQ6MTFQTSArMDgwMCwgTmlj a2V5IFlhbmcgd3JvdGU6Cj4gT24gMjAxN+W5tDEy5pyIMDHml6UgMTg6MDcsIFBoaWxpcHBlIENP Uk5VIHdyb3RlOgo+ID5PbiAxMi8wMS8yMDE3IDEwOjExIEFNLCBOaWNrZXkgWWFuZyB3cm90ZToK PiA+Pk9uIDIwMTflubQxMuaciDAx5pelIDE2OjMyLCBQaGlsaXBwZSBDT1JOVSB3cm90ZToKPiA+ Pj5JIGFtIHNvcnJ5IHRvIHNheSB0aGF0IGJ1dCB5b3UgY2FuIG5vdCBhZGQgbXkgIkFja2VkLWJ5 IiB0byB0aGlzIHBhdGNoCj4gPj4+YmVjYXVzZSB0aGlzIGNvZGUgaXMgZGlmZmVyZW50IGZyb20g dGhlICJvcmlnaW5hbCIgb25lIGZyb20gQnJpYW4gKHdoaWNoCj4gPj4+Z290IG15ICJBY2tlZC1i eSIpLgo+ID4+SSdtIHNvcnJ5IEkgZGlkbid0IHRoaW5rIG11Y2ggYWJvdXQgaXQsIFRoYW5rIHlv dSBmb3IgY29ycmVjdGluZyBtZS4KPiA+Pj5Tb21ldGltZXMgaXQgaXMgbm90IGFuIGlzc3VlIGJl Y2F1c2UgZGlmZmVyZW5jZXMgYXJlIG5vdCBpbXBvcnRhbnQgYnV0Cj4gPj4+aW4gdGhpcyBwYXJ0 aWN1bGFyIGNhc2UsIHRoZSBjb2RlIGlzIHJlYWxseSBkaWZmZXJlbnQ6IHlvdSBoYXZlIHJlbW92 ZQo+ID4+PnBsYXRmb3JtX3NldF9kcnZkYXRhKCkgJiBwbGF0Zm9ybV9nZXRfZHJ2ZGF0YSgpIGlu IHRoZSBzdG0gcGFydC4KPiA+Pj4KPiA+Pj5Db3VsZCB5b3UgcGxlYXNlIGdvIGJhY2sgdG8gdGhl IG9yaWdpbmFsIGNvZGUgb3IgcHJvcG9zZSBtZSBhbiB1cGRhdGVkCj4gPj4+dmVyc2lvbiBvZiB0 aGlzIGNvZGUuCj4gPj5Db3VsZCB5b3UgaGVscCB1cGRhdGUgbmV3IHZlcnNpb24gb2YgdGhpcyBj b2RlKHN0bSBwYXJ0KSBhbmQgdGhlbiB0ZXN0IG9uCj4gPj5zdG0gcGxhdGZvcm0/Cj4gPkkgdGhp bmsgeW91IGNhbiBzaW1wbHkgZ29lcyBiYWNrIHRvIHRoZSBvcmlnaW5hbCB2ZXJzaW9uIGZyb20g QnJpYW4gKHNlZQo+ID50aGUgZGlzY3Vzc2lvbiB0aHJlYWQgaW4gaHR0cHM6Ly9wYXRjaHdvcmsu a2VybmVsLm9yZy9wYXRjaC8xMDA3ODQ5My8pCj4gPnVubGVzcyB5b3UgaGF2ZSBzcGVjaWZpYy9n b29kIHJlYXNvbnMgZm9yIG1vZGlmeWluZyB0aGUgY29kZSBhcyB5b3UgZGlkLgo+IG1tbSxJJ20g c29ycnksIEkgZmVlbCBhIGxpdHRsZSBwdXp6bGVkLiBEbyB5b3UgbWVhbnMgd2Ugc2hvdWxkIGFi YW5kb24KPiBCcmlhbidzIHBhdGNoIChodHRwczovL3BhdGNod29yay5rZXJuZWwub3JnL3BhdGNo LzEwMDc4NDkzLyk/Cj4gSSB0aGluayB3ZSBuZWVkIHRvIGFkanVzdCBzdG0gcGFydCBiZWNhdXNl wqAgZHdfbWlwaV9kc2lfc3RtLmMgY2FsbHMKPiBicmlkZ2UncyBkcml2ZXJzIGlmIHdlIHdhbnQg bWVyZ2UgQnJpYW4ncyBwYXRjaC4KCkl0J3MgcmVhbGx5IHNpbXBsZS4gWW91ciBjb2RlIGlzIGRp ZmZlcmVudCBmcm9tIHRoZSBwYXRjaCBJIHNlbnQsIGFuZCBpbgphIHdheSB0aGF0IFBoaWxpcHBl IGRpZCBub3QgbGlrZS4gSSdsbCBoaWdobGlnaHQgaXQgYWdhaW4gYmVsb3c6Cgo+ID4+Pj5kaWZm IC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL3N0bS9kd19taXBpX2RzaS1zdG0uYyBiL2RyaXZlcnMv Z3B1L2RybS9zdG0vZHdfbWlwaV9kc2ktc3RtLmMKPiA+Pj4+aW5kZXggZTViNjMxMC4uODBmOTk1 MCAxMDA2NDQKPiA+Pj4+LS0tIGEvZHJpdmVycy9ncHUvZHJtL3N0bS9kd19taXBpX2RzaS1zdG0u Ywo+ID4+Pj4rKysgYi9kcml2ZXJzL2dwdS9kcm0vc3RtL2R3X21pcGlfZHNpLXN0bS5jCj4gPj4+ PkBAIC02Niw2ICs2Niw3IEBAIGVudW0gZHNpX2NvbG9yIHsKPiA+Pj4+ICAgIHN0cnVjdCBkd19t aXBpX2RzaV9zdG0gewo+ID4+Pj4gICAgCXZvaWQgX19pb21lbSAqYmFzZTsKPiA+Pj4+ICAgIAlz dHJ1Y3QgY2xrICpwbGxyZWZfY2xrOwo+ID4+Pj4rCXN0cnVjdCBkd19taXBpX2RzaSAqZG1kOwo+ ID4+Pj4gICAgfTsKPiA+Pj4+ICAgIHN0YXRpYyBpbmxpbmUgdm9pZCBkc2lfd3JpdGUoc3RydWN0 IGR3X21pcGlfZHNpX3N0bSAqZHNpLCB1MzIgcmVnLCB1MzIgdmFsKQo+ID4+Pj5AQCAtMzE4LDEw ICszMTksMTEgQEAgc3RhdGljIGludCBkd19taXBpX2RzaV9zdG1fcHJvYmUoc3RydWN0IHBsYXRm b3JtX2RldmljZSAqcGRldikKPiA+Pj4+ICAgIAlkd19taXBpX2RzaV9zdG1fcGxhdF9kYXRhLmJh c2UgPSBkc2ktPmJhc2U7Cj4gPj4+PiAgICAJZHdfbWlwaV9kc2lfc3RtX3BsYXRfZGF0YS5wcml2 X2RhdGEgPSBkc2k7Cj4gPj4+Pi0JcmV0ID0gZHdfbWlwaV9kc2lfcHJvYmUocGRldiwgJmR3X21p cGlfZHNpX3N0bV9wbGF0X2RhdGEpOwo+ID4+Pj4tCWlmIChyZXQpIHsKPiA+Pj4+Kwlkc2ktPmRt ZCA9IGR3X21pcGlfZHNpX3Byb2JlKHBkZXYsICZkd19taXBpX2RzaV9zdG1fcGxhdF9kYXRhKTsK PiA+Pj4+KwlpZiAoSVNfRVJSKGRzaS0+ZG1kKSkgewo+ID4+Pj4gICAgCQlEUk1fRVJST1IoIkZh aWxlZCB0byBpbml0aWFsaXplIG1pcGkgZHNpIGhvc3RcbiIpOwo+ID4+Pj4gICAgCQljbGtfZGlz YWJsZV91bnByZXBhcmUoZHNpLT5wbGxyZWZfY2xrKTsKPiA+Pj4+KwkJcmV0dXJuIFBUUl9FUlIo ZHNpLT5kbWQpOwo+ID4+Pj4gICAgCX0KPiA+Pj4+ICAgIAlyZXR1cm4gcmV0Owo+ID4+Pj5AQCAt MzMyLDcgKzMzNCw3IEBAIHN0YXRpYyBpbnQgZHdfbWlwaV9kc2lfc3RtX3JlbW92ZShzdHJ1Y3Qg cGxhdGZvcm1fZGV2aWNlICpwZGV2KQo+ID4+Pj4gICAgCXN0cnVjdCBkd19taXBpX2RzaV9zdG0g KmRzaSA9IGR3X21pcGlfZHNpX3N0bV9wbGF0X2RhdGEucHJpdl9kYXRhOwo+ID4+Pj4gICAgCWNs a19kaXNhYmxlX3VucHJlcGFyZShkc2ktPnBsbHJlZl9jbGspOwo+ID4+Pj4tCWR3X21pcGlfZHNp X3JlbW92ZShwZGV2KTsKPiA+Pj4+Kwlkd19taXBpX2RzaV9yZW1vdmUoZHNpLT5kbWQpOwo+ID4+ Pj4gICAgCXJldHVybiAwOwo+ID4+Pj4gICAgfQogCkFib3ZlIGlzIHlvdXIgZGlmZiBmb3IgZHdf bWlwaV9kc2ktc3RtLmMuIFBhcnRpY3VsYXJseSwgbm90aWNlIHRoYXQKcmVtb3ZlKCkgaXMgZGly ZWN0bHkgcmVmZXJlbmNpbmcgdGhlIHN0YXRpYyBkd19taXBpX2RzaV9zdG1fcGxhdF9kYXRhCnN0 cnVjdC4KCklmIHlvdSBsb29rIGJhY2sgYXQgbXkgcGF0Y2ggWzFdIHlvdSdsbCBzZWUgdGhhdCB5 b3UncmUgbWlzc2luZyBodW5rcwpsaWtlIHRoaXM6Cgogc3RhdGljIGludCBkd19taXBpX2RzaV9z dG1fcmVtb3ZlKHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYpCiB7Ci0Jc3RydWN0IGR3X21p cGlfZHNpX3N0bSAqZHNpID0gZHdfbWlwaV9kc2lfc3RtX3BsYXRfZGF0YS5wcml2X2RhdGE7CisJ c3RydWN0IGR3X21pcGlfZHNpX3N0bSAqZHNpID0gcGxhdGZvcm1fZ2V0X2RydmRhdGEocGRldik7 CiAKIAljbGtfZGlzYWJsZV91bnByZXBhcmUoZHNpLT5wbGxyZWZfY2xrKTsKWy4uLl0KCkJyaWFu CgpbMV0gaHR0cHM6Ly9wYXRjaHdvcmsua2VybmVsLm9yZy9wYXRjaC8xMDA3ODQ5My8KX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxp bmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJl ZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752762AbdLES46 (ORCPT ); Tue, 5 Dec 2017 13:56:58 -0500 Received: from mail-it0-f67.google.com ([209.85.214.67]:45825 "EHLO mail-it0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752415AbdLES45 (ORCPT ); Tue, 5 Dec 2017 13:56:57 -0500 X-Google-Smtp-Source: AGs4zMail7c5/eAvOdhEGQIIVQHJbPCON6R/Tz85TD4rI9jD22e+a7jYS0gMwkNurHtEu4XQJ2LPfw== Date: Tue, 5 Dec 2017 10:56:52 -0800 From: Brian Norris To: Nickey Yang Cc: Philippe CORNU , "robh+dt@kernel.org" , "heiko@sntech.de" , "mark.rutland@arm.com" , "airlied@linux.ie" , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , "linux-rockchip@lists.infradead.org" , "laurent.pinchart@ideasonboard.com" , "seanpaul@chromium.org" , "mka@chromium.org" , "hoegsberg@gmail.com" , "architt@codeaurora.org" , Yannick FERTRE , "hl@rock-chips.com" , "zyw@rock-chips.com" , "xbl@rock-chips.com" Subject: Re: [PATCH v4 1/3] drm/bridge/synopsys: dsi: stop clobbering drvdata Message-ID: <20171205185651.GA136446@google.com> References: <1512100685-4015-1-git-send-email-nickey.yang@rock-chips.com> <1512100685-4015-2-git-send-email-nickey.yang@rock-chips.com> <5605c3c2-56ad-2289-c3c0-78cde1152123@st.com> <1700743e-04b0-90ea-d393-dbfebf93742f@st.com> <206b071f-2b57-c4a2-31b3-2f469091e5e2@rock-chips.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <206b071f-2b57-c4a2-31b3-2f469091e5e2@rock-chips.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Nickey, On Tue, Dec 05, 2017 at 05:14:11PM +0800, Nickey Yang wrote: > On 2017年12月01日 18:07, Philippe CORNU wrote: > >On 12/01/2017 10:11 AM, Nickey Yang wrote: > >>On 2017年12月01日 16:32, Philippe CORNU wrote: > >>>I am sorry to say that but you can not add my "Acked-by" to this patch > >>>because this code is different from the "original" one from Brian (which > >>>got my "Acked-by"). > >>I'm sorry I didn't think much about it, Thank you for correcting me. > >>>Sometimes it is not an issue because differences are not important but > >>>in this particular case, the code is really different: you have remove > >>>platform_set_drvdata() & platform_get_drvdata() in the stm part. > >>> > >>>Could you please go back to the original code or propose me an updated > >>>version of this code. > >>Could you help update new version of this code(stm part) and then test on > >>stm platform? > >I think you can simply goes back to the original version from Brian (see > >the discussion thread in https://patchwork.kernel.org/patch/10078493/) > >unless you have specific/good reasons for modifying the code as you did. > mmm,I'm sorry, I feel a little puzzled. Do you means we should abandon > Brian's patch (https://patchwork.kernel.org/patch/10078493/)? > I think we need to adjust stm part because  dw_mipi_dsi_stm.c calls > bridge's drivers if we want merge Brian's patch. It's really simple. Your code is different from the patch I sent, and in a way that Philippe did not like. I'll highlight it again below: > >>>>diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > >>>>index e5b6310..80f9950 100644 > >>>>--- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > >>>>+++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > >>>>@@ -66,6 +66,7 @@ enum dsi_color { > >>>> struct dw_mipi_dsi_stm { > >>>> void __iomem *base; > >>>> struct clk *pllref_clk; > >>>>+ struct dw_mipi_dsi *dmd; > >>>> }; > >>>> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val) > >>>>@@ -318,10 +319,11 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev) > >>>> dw_mipi_dsi_stm_plat_data.base = dsi->base; > >>>> dw_mipi_dsi_stm_plat_data.priv_data = dsi; > >>>>- ret = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); > >>>>- if (ret) { > >>>>+ dsi->dmd = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); > >>>>+ if (IS_ERR(dsi->dmd)) { > >>>> DRM_ERROR("Failed to initialize mipi dsi host\n"); > >>>> clk_disable_unprepare(dsi->pllref_clk); > >>>>+ return PTR_ERR(dsi->dmd); > >>>> } > >>>> return ret; > >>>>@@ -332,7 +334,7 @@ static int dw_mipi_dsi_stm_remove(struct platform_device *pdev) > >>>> struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data; > >>>> clk_disable_unprepare(dsi->pllref_clk); > >>>>- dw_mipi_dsi_remove(pdev); > >>>>+ dw_mipi_dsi_remove(dsi->dmd); > >>>> return 0; > >>>> } Above is your diff for dw_mipi_dsi-stm.c. Particularly, notice that remove() is directly referencing the static dw_mipi_dsi_stm_plat_data struct. If you look back at my patch [1] you'll see that you're missing hunks like this: static int dw_mipi_dsi_stm_remove(struct platform_device *pdev) { - struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data; + struct dw_mipi_dsi_stm *dsi = platform_get_drvdata(pdev); clk_disable_unprepare(dsi->pllref_clk); [...] Brian [1] https://patchwork.kernel.org/patch/10078493/