From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko =?ISO-8859-1?Q?St=FCbner?= Subject: Re: [PATCH 1/2] drm/rockchip: dw_hdmi: Call drm_encoder_cleanup() in error path Date: Mon, 07 Mar 2016 19:56:18 +0100 Message-ID: <2630588.ZOL2q0FzuV@diego> References: <1457133723-24869-1-git-send-email-dianders@chromium.org> <1693114.6CV9KAkHOO@diego> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Doug Anderson Cc: Russell King - ARM Linux , John Keeping , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , "open list:ARM/Rockchip SoC..." , "linux-arm-kernel@lists.infradead.org" List-Id: linux-rockchip.vger.kernel.org SGkgRG91ZywKCkFtIE1vbnRhZywgNy4gTcOkcnogMjAxNiwgMTA6NDk6NTMgc2NocmllYiBEb3Vn IEFuZGVyc29uOgo+IE9uIE1vbiwgTWFyIDcsIDIwMTYgYXQgOTo1NyBBTSwgSGVpa28gU3TDvGJu ZXIgPGhlaWtvQHNudGVjaC5kZT4gd3JvdGU6Cj4gPiBBbSBNb250YWcsIDcuIE3DpHJ6IDIwMTYs IDA5OjM2OjA3IHNjaHJpZWIgRG91ZyBBbmRlcnNvbjoKPiA+PiBIaSwKPiA+PiAKPiA+PiBPbiBN b24sIE1hciA3LCAyMDE2IGF0IDEyOjM3IEFNLCBNYXJrIHlhbyA8bWFyay55YW9Acm9jay1jaGlw cy5jb20+IAp3cm90ZToKPiA+PiA+IE9uIDIwMTblubQwM+aciDA15pelIDIwOjM5LCBSdXNzZWxs IEtpbmcgLSBBUk0gTGludXggd3JvdGU6Cj4gPj4gPj4gT24gU2F0LCBNYXIgMDUsIDIwMTYgYXQg MTI6MTE6MTZQTSArMDAwMCwgSm9obiBLZWVwaW5nIHdyb3RlOgo+ID4+ID4+PiBPbiBGcmksIE1h ciAwNCwgMjAxNiBhdCAwMzoyMjowMVBNIC0wODAwLCBEb3VnbGFzIEFuZGVyc29uIHdyb3RlOgo+ ID4+ID4+Pj4gVGhlIGRybV9lbmNvZGVyX2NsZWFudXAoKSB3YXMgbWlzc2luZyBib3RoIGZyb20g dGhlIGVycm9yIHBhdGggb2YKPiA+PiA+Pj4+IGR3X2hkbWlfcm9ja2NoaXBfYmluZCgpLiAgVGhp cyBjYXVzZWQgYSBjcmFzaCB3aGVuIHNsdWJfZGVidWcgd2FzCj4gPj4gPj4+PiBlbmFibGVkIGFu ZCB3ZSBlbmRlZCB1cCBkZWZlcnJpbmcgcHJvYmUgb2YgSERNSSBhdCBib290Lgo+ID4+ID4+Pj4g Cj4gPj4gPj4+PiBUaGlzIGNhbGwgaXNuJ3QgbmVlZGVkIGZyb20gdW5iaW5kKCkgYmVjYXVzZSBp ZiBkd19oZG1pX2JpbmQoKQo+ID4+ID4+Pj4gcmV0dXJucwo+ID4+ID4+Pj4gbm8gZXJyb3IgdGhl biBpdCB0YWtlcyBvdmVyIHRoZSBqb2Igb2YgZnJlZWluZyB0aGUgZW5jb2RlciAoaW4KPiA+PiA+ Pj4+IGR3X2hkbWlfdW5iaW5kKS4KPiA+PiA+Pj4+IAo+ID4+ID4+Pj4gU2lnbmVkLW9mZi1ieTog RG91Z2xhcyBBbmRlcnNvbiA8ZGlhbmRlcnNAY2hyb21pdW0ub3JnPgo+ID4+ID4+Pj4gLS0tCj4g Pj4gPj4+IAo+ID4+ID4+PiBEb2VzIGR3X2hkbWktaW14IG5lZWQgYSBzaW1pbGFyIGNoYW5nZT8g IEkgd29uZGVyIGlmIGl0IHdvdWxkIGJlCj4gPj4gPj4+IGNsZWFuZXIKPiA+PiA+Pj4gdG8gcHVz aCB0aGlzIGludG8gZHdfaGRtaV9iaW5kKCkgaWYgaXQgYWZmZWN0cyBhbGwgb2YgdGhlIHBsYXRm b3Jtcy4uCj4gPj4gPj4gCj4gPj4gPj4gSSBkb24ndCB0aGluayBtb3ZpbmcgaXQgdGhlcmUgd291 bGQgbWFrZSBzZW5zZSAtIGtlZXAgdGhlCj4gPj4gPj4gaW5pdGlhbGlzYXRpb24KPiA+PiA+PiBh bmQgY2xlYW51cCB0b2dldGhlciBpbiB0aGUgc2FtZSBmaWxlIHNvIHRoYXQgaXQncyBjb250YWlu ZWQgdG9nZXRoZXIuCj4gPj4gPiAKPiA+PiA+IEkgZG9uJ3QgbGlrZSB0aGlzIHBhdGNoIHRvbywg aW5pdGlhbGlzYXRpb24gYW5kIGNsZWFudXAgbm90IGluIHRoZSBzYW1lCj4gPj4gPiBmaWxlIGxv b2tzIGJhZCwKPiA+PiA+IAo+ID4+ID4gSG93IGFib3V0Ogo+ID4+ID4gCj4gPj4gPiBkcml2ZXJz L2dwdS9kcm0vYnJpZGdlL2R3LWhkbWkuYwo+ID4+ID4gdm9pZCBkd19oZG1pX3VuYmluZChzdHJ1 Y3QgZGV2aWNlICpkZXYsIHN0cnVjdCBkZXZpY2UgKm1hc3Rlciwgdm9pZAo+ID4+ID4gKmRhdGEp Cj4gPj4gPiAKPiA+PiA+ICAgICAgICAgaGRtaV93cml0ZWIoaGRtaSwgfjAsIEhETUlfSUhfTVVU RV9QSFlfU1RBVDApOwo+ID4+ID4gCj4gPj4gPiBoZG1pLT5jb25uZWN0b3IuZnVuY3MtPmRlc3Ry b3koJmhkbWktPmNvbm5lY3Rvcik7Cj4gPj4gPiAtICAgICAgIGhkbWktPmVuY29kZXItPmZ1bmNz LT5kZXN0cm95KGhkbWktPmVuY29kZXIpOwo+ID4+ID4gCj4gPj4gPiBkcml2ZXJzL2dwdS9kcm0v cm9ja2NoaXAvZHdfaGRtaS1yb2NrY2hpcC5jCj4gPj4gPiBzdGF0aWMgaW50IGR3X2hkbWlfcm9j a2NoaXBfYmluZChzdHJ1Y3QgZGV2aWNlICpkZXYsIHN0cnVjdCBkZXZpY2UKPiA+PiA+ICptYXN0 ZXIsCj4gPj4gPiAKPiA+PiA+IC0gICAgICAgcmV0dXJuIGR3X2hkbWlfYmluZChkZXYsIG1hc3Rl ciwgZGF0YSwgZW5jb2RlciwgaW9yZXMsIGlycSwKPiA+PiA+IHBsYXRfZGF0YSk7Cj4gPj4gPiAr ICAgICAgIHJldCA9IGR3X2hkbWlfYmluZChkZXYsIG1hc3RlciwgZGF0YSwgZW5jb2RlciwgaW9y ZXMsIGlycSwKPiA+PiA+IHBsYXRfZGF0YSk7Cj4gPj4gPiArICAgICAgIGlmIChyZXQpCj4gPj4g PiArICAgICAgICAgICAgICAgZHJtX2VuY29kZXJfY2xlYW51cChlbmNvZGVyKTsKPiA+PiA+ICsK PiA+PiA+ICsgICAgICAgcmV0dXJuIHJldDsKPiA+PiA+IAo+ID4+ID4gIH0KPiA+PiA+ICAKPiA+ PiA+ICBzdGF0aWMgdm9pZCBkd19oZG1pX3JvY2tjaGlwX3VuYmluZChzdHJ1Y3QgZGV2aWNlICpk ZXYsIHN0cnVjdCBkZXZpY2UKPiA+PiA+IAo+ID4+ID4gKm1hc3RlciwKPiA+PiA+IAo+ID4+ID4g ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgdm9pZCAqZGF0YSkKPiA+PiA+ICAK PiA+PiA+ICB7Cj4gPj4gPiAKPiA+PiA+ICsgICAgICAgZHJtX2VuY29kZXJfY2xlYW51cCguLi4p Owo+ID4+ID4gCj4gPj4gPiAgICAgICAgIHJldHVybiBkd19oZG1pX3VuYmluZChkZXYsIG1hc3Rl ciwgZGF0YSk7Cj4gPj4gPiAgCj4gPj4gPiAgfQo+ID4+IAo+ID4+IFRoYXQnYSBhIHJlYXNvbmFi bGUgc3VnZ2VzdGlvbiBpbiB0aGVvcnkuICAuLi5idXQgd2UgcnVuIGludG8gdGhlIHNhbWUKPiA+ PiBwcm9ibGVtIEkndmUgcnVuIGludG8gYmVmb3JlIHdpdGggdGhlIHN0cmFuZ2UgcmVsYXRpb25z aGlwIGJldHdlZW4KPiA+PiBkd19oZG1pIGFuZCBpdHMgZGVzY2VuZGFudHMuCj4gPiAKPiA+IEkg ZG9uJ3QgdGhpbmsgaGFuZGluZyBvZmYgdGhlIGNsZWFudXAgcmVzcG9uc2liaWxpdHkgaXMgcmVh bGx5IGluIHF1ZXN0aW9uCj4gPiBoZXJlLiBJLmUuIEkgZG8gYmVsaWV2ZSBpdCBzaG91bGQgYWxz byBiZSBmaW5lIHRvIGV4cGVjdCAoYXMgZGVmaW5pdGlvbikKPiA+IHRoZSBjb3JlIGRyaXZlciB0 byBjbGVhbnVwIHRoZSBlbmNvZGVyIF9hZnRlcl8gaXQgc3VjZXNzZnVsbHkgY2xhaW1lZCBpdAo+ ID4gaW4gZHdfaGRtaV9iaW5kKCkuCj4gPiAKPiA+IFdlIGRvIHRoZSBzYW1lIGluIHRoZSByb2Nr Y2hpcCBwb3dlci1kb21haW5zLCBoYW5kaW5nIG9mZiB0aGUgc3RydWN0IGNsay0KPiA+IHBvaW50 ZXIgdG8gdGhlIHBtX2NsayBzdHVmZiAoZHVlIHRvIHRoZSBjbGstcG9pbnRlciBiZWluZyB1bmlx dWUKPiA+IHBlci1kZXZpY2UKPiA+IG5vd2FkYXlzKS4KPiA+IAo+ID4gU28ganVzdCBtYWtpbmcg c3VyZSBpdCBpcyBzdWNlc3NmdWxseSBoYW5kZWQgb2ZmIHNob3VsZCBhbHNvIGJlIG9rLgo+IAo+ IElmIEkgdW5kZXJzdGFuZCBjb3JyZWN0bHksIHRoYXQgbWVhbnMgeW91J2QgYmUgT0sgd2l0aCB0 aGUgb3JpZ2luYWwKPiBwYXRjaCBJIHBvc3RlZD8gIEluIHRoYXQgY2FzZSBjbGVhbnVwIGNvbnRp bnVlcyB0byBoYXBwZW4gaW4gdGhlIG1haW4KPiBkdy1oZG1pLmMgaWYgZHdfaGRtaV9iaW5kKCkg c3VjY2VlZHMgYW5kIG15IHBhdGNoIGZpeGVzIHRoZSBjbGVhbnVwCj4gd2hlbiBkd19oZG1pX2Jp bmQoKSBmYWlscyAoYW5kIHRodXMgY2xlYW51cCByZXNwb25zaWJpbGl0eSB3YXMgbm90Cj4gaGFu ZGVkIG9mZikuCgpjb3JyZWN0LiBJIGRvbid0IHNlZSB0aGUgbmVlZCB0byBkdXBsaWNhdGUgdGhl IGNsZWFudXAgKCthZGRlZCBpbmZyYXN0cnVjdHVyZSAKdG8gYWN0dWFsbHkgZ2V0IHRoZSBlbmNv ZGVyIGluIHVuYmluZCkgaW4gYWxsIGluc3RhbmNlcywgaWYgd2UganVzdCBkZWZpbmUgdGhhdCAK dGhlIGR3X2hkbWkgY29yZSB0YWtlcyBjb250cm9sIG9mIHRoZSBlbmNvZGVyIF9hZnRlcl8gaXQg c3VjZXNzZnVsbHkgYm91bmQuCgpTbyBvbmx5IGlmIGR3X2hkbWlfYmluZCgpIGZhaWxzIGRvZXMg dGhlIGh3LXNwZWNpZmljIGluc3RhbmNlIG5lZWQgdG8gY2xlYW4gdXAgCnRoZSBlbmNvZGVyIGl0 IGNyZWF0ZWQuCgoKPiBBbHNvOiBJIG5vdGljZWQgdGhhdCBSdXNzZWxsIGFsc28gZGlkbid0IHNl ZW0gdG8gc2F5IHRoYXQgbXkgb3JpZ2luYWwKPiBwYXRjaCB3YXMgYmFkLiAgSSB0aGluayBoZSBq dXN0IHNhaWQgdGhhdCBoZSBkaWRuJ3QgbGlrZSBKb2huCj4gS2VlcGluZydzIHN1Z2dlc3Rpb24u Cgp0aGF0IHdhcyBteSByZWFkaW5nIGFzIHdlbGwuCgoKSGVpa28KX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmkt ZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3Jn L21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: heiko@sntech.de (Heiko =?ISO-8859-1?Q?St=FCbner?=) Date: Mon, 07 Mar 2016 19:56:18 +0100 Subject: [PATCH 1/2] drm/rockchip: dw_hdmi: Call drm_encoder_cleanup() in error path In-Reply-To: References: <1457133723-24869-1-git-send-email-dianders@chromium.org> <1693114.6CV9KAkHOO@diego> Message-ID: <2630588.ZOL2q0FzuV@diego> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Doug, Am Montag, 7. M?rz 2016, 10:49:53 schrieb Doug Anderson: > On Mon, Mar 7, 2016 at 9:57 AM, Heiko St?bner wrote: > > Am Montag, 7. M?rz 2016, 09:36:07 schrieb Doug Anderson: > >> Hi, > >> > >> On Mon, Mar 7, 2016 at 12:37 AM, Mark yao wrote: > >> > On 2016?03?05? 20:39, Russell King - ARM Linux wrote: > >> >> On Sat, Mar 05, 2016 at 12:11:16PM +0000, John Keeping wrote: > >> >>> On Fri, Mar 04, 2016 at 03:22:01PM -0800, Douglas Anderson wrote: > >> >>>> The drm_encoder_cleanup() was missing both from the error path of > >> >>>> dw_hdmi_rockchip_bind(). This caused a crash when slub_debug was > >> >>>> enabled and we ended up deferring probe of HDMI at boot. > >> >>>> > >> >>>> This call isn't needed from unbind() because if dw_hdmi_bind() > >> >>>> returns > >> >>>> no error then it takes over the job of freeing the encoder (in > >> >>>> dw_hdmi_unbind). > >> >>>> > >> >>>> Signed-off-by: Douglas Anderson > >> >>>> --- > >> >>> > >> >>> Does dw_hdmi-imx need a similar change? I wonder if it would be > >> >>> cleaner > >> >>> to push this into dw_hdmi_bind() if it affects all of the platforms.. > >> >> > >> >> I don't think moving it there would make sense - keep the > >> >> initialisation > >> >> and cleanup together in the same file so that it's contained together. > >> > > >> > I don't like this patch too, initialisation and cleanup not in the same > >> > file looks bad, > >> > > >> > How about: > >> > > >> > drivers/gpu/drm/bridge/dw-hdmi.c > >> > void dw_hdmi_unbind(struct device *dev, struct device *master, void > >> > *data) > >> > > >> > hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0); > >> > > >> > hdmi->connector.funcs->destroy(&hdmi->connector); > >> > - hdmi->encoder->funcs->destroy(hdmi->encoder); > >> > > >> > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > >> > static int dw_hdmi_rockchip_bind(struct device *dev, struct device > >> > *master, > >> > > >> > - return dw_hdmi_bind(dev, master, data, encoder, iores, irq, > >> > plat_data); > >> > + ret = dw_hdmi_bind(dev, master, data, encoder, iores, irq, > >> > plat_data); > >> > + if (ret) > >> > + drm_encoder_cleanup(encoder); > >> > + > >> > + return ret; > >> > > >> > } > >> > > >> > static void dw_hdmi_rockchip_unbind(struct device *dev, struct device > >> > > >> > *master, > >> > > >> > void *data) > >> > > >> > { > >> > > >> > + drm_encoder_cleanup(...); > >> > > >> > return dw_hdmi_unbind(dev, master, data); > >> > > >> > } > >> > >> That'a a reasonable suggestion in theory. ...but we run into the same > >> problem I've run into before with the strange relationship between > >> dw_hdmi and its descendants. > > > > I don't think handing off the cleanup responsibility is really in question > > here. I.e. I do believe it should also be fine to expect (as definition) > > the core driver to cleanup the encoder _after_ it sucessfully claimed it > > in dw_hdmi_bind(). > > > > We do the same in the rockchip power-domains, handing off the struct clk- > > pointer to the pm_clk stuff (due to the clk-pointer being unique > > per-device > > nowadays). > > > > So just making sure it is sucessfully handed off should also be ok. > > If I understand correctly, that means you'd be OK with the original > patch I posted? In that case cleanup continues to happen in the main > dw-hdmi.c if dw_hdmi_bind() succeeds and my patch fixes the cleanup > when dw_hdmi_bind() fails (and thus cleanup responsibility was not > handed off). correct. I don't see the need to duplicate the cleanup (+added infrastructure to actually get the encoder in unbind) in all instances, if we just define that the dw_hdmi core takes control of the encoder _after_ it sucessfully bound. So only if dw_hdmi_bind() fails does the hw-specific instance need to clean up the encoder it created. > Also: I noticed that Russell also didn't seem to say that my original > patch was bad. I think he just said that he didn't like John > Keeping's suggestion. that was my reading as well. Heiko From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753279AbcCGS4a (ORCPT ); Mon, 7 Mar 2016 13:56:30 -0500 Received: from gloria.sntech.de ([95.129.55.99]:57280 "EHLO gloria.sntech.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752850AbcCGS4W convert rfc822-to-8bit (ORCPT ); Mon, 7 Mar 2016 13:56:22 -0500 From: Heiko =?ISO-8859-1?Q?St=FCbner?= To: Doug Anderson Cc: Mark yao , Russell King - ARM Linux , John Keeping , David Airlie , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , "open list:ARM/Rockchip SoC..." , Daniel Kurtz , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCH 1/2] drm/rockchip: dw_hdmi: Call drm_encoder_cleanup() in error path Date: Mon, 07 Mar 2016 19:56:18 +0100 Message-ID: <2630588.ZOL2q0FzuV@diego> User-Agent: KMail/4.14.10 (Linux/4.2.0-1-amd64; KDE/4.14.14; x86_64; ; ) In-Reply-To: References: <1457133723-24869-1-git-send-email-dianders@chromium.org> <1693114.6CV9KAkHOO@diego> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="utf-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Doug, Am Montag, 7. März 2016, 10:49:53 schrieb Doug Anderson: > On Mon, Mar 7, 2016 at 9:57 AM, Heiko Stübner wrote: > > Am Montag, 7. März 2016, 09:36:07 schrieb Doug Anderson: > >> Hi, > >> > >> On Mon, Mar 7, 2016 at 12:37 AM, Mark yao wrote: > >> > On 2016年03月05日 20:39, Russell King - ARM Linux wrote: > >> >> On Sat, Mar 05, 2016 at 12:11:16PM +0000, John Keeping wrote: > >> >>> On Fri, Mar 04, 2016 at 03:22:01PM -0800, Douglas Anderson wrote: > >> >>>> The drm_encoder_cleanup() was missing both from the error path of > >> >>>> dw_hdmi_rockchip_bind(). This caused a crash when slub_debug was > >> >>>> enabled and we ended up deferring probe of HDMI at boot. > >> >>>> > >> >>>> This call isn't needed from unbind() because if dw_hdmi_bind() > >> >>>> returns > >> >>>> no error then it takes over the job of freeing the encoder (in > >> >>>> dw_hdmi_unbind). > >> >>>> > >> >>>> Signed-off-by: Douglas Anderson > >> >>>> --- > >> >>> > >> >>> Does dw_hdmi-imx need a similar change? I wonder if it would be > >> >>> cleaner > >> >>> to push this into dw_hdmi_bind() if it affects all of the platforms.. > >> >> > >> >> I don't think moving it there would make sense - keep the > >> >> initialisation > >> >> and cleanup together in the same file so that it's contained together. > >> > > >> > I don't like this patch too, initialisation and cleanup not in the same > >> > file looks bad, > >> > > >> > How about: > >> > > >> > drivers/gpu/drm/bridge/dw-hdmi.c > >> > void dw_hdmi_unbind(struct device *dev, struct device *master, void > >> > *data) > >> > > >> > hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0); > >> > > >> > hdmi->connector.funcs->destroy(&hdmi->connector); > >> > - hdmi->encoder->funcs->destroy(hdmi->encoder); > >> > > >> > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > >> > static int dw_hdmi_rockchip_bind(struct device *dev, struct device > >> > *master, > >> > > >> > - return dw_hdmi_bind(dev, master, data, encoder, iores, irq, > >> > plat_data); > >> > + ret = dw_hdmi_bind(dev, master, data, encoder, iores, irq, > >> > plat_data); > >> > + if (ret) > >> > + drm_encoder_cleanup(encoder); > >> > + > >> > + return ret; > >> > > >> > } > >> > > >> > static void dw_hdmi_rockchip_unbind(struct device *dev, struct device > >> > > >> > *master, > >> > > >> > void *data) > >> > > >> > { > >> > > >> > + drm_encoder_cleanup(...); > >> > > >> > return dw_hdmi_unbind(dev, master, data); > >> > > >> > } > >> > >> That'a a reasonable suggestion in theory. ...but we run into the same > >> problem I've run into before with the strange relationship between > >> dw_hdmi and its descendants. > > > > I don't think handing off the cleanup responsibility is really in question > > here. I.e. I do believe it should also be fine to expect (as definition) > > the core driver to cleanup the encoder _after_ it sucessfully claimed it > > in dw_hdmi_bind(). > > > > We do the same in the rockchip power-domains, handing off the struct clk- > > pointer to the pm_clk stuff (due to the clk-pointer being unique > > per-device > > nowadays). > > > > So just making sure it is sucessfully handed off should also be ok. > > If I understand correctly, that means you'd be OK with the original > patch I posted? In that case cleanup continues to happen in the main > dw-hdmi.c if dw_hdmi_bind() succeeds and my patch fixes the cleanup > when dw_hdmi_bind() fails (and thus cleanup responsibility was not > handed off). correct. I don't see the need to duplicate the cleanup (+added infrastructure to actually get the encoder in unbind) in all instances, if we just define that the dw_hdmi core takes control of the encoder _after_ it sucessfully bound. So only if dw_hdmi_bind() fails does the hw-specific instance need to clean up the encoder it created. > Also: I noticed that Russell also didn't seem to say that my original > patch was bad. I think he just said that he didn't like John > Keeping's suggestion. that was my reading as well. Heiko