From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Stuebner Subject: Re: [PATCH v3] drm/rockchip: Refactor the component match logic. Date: Wed, 15 Mar 2017 18:23:27 +0100 Message-ID: <2110531.OeyX13mlk5@phil> References: <1489573247-9628-1-git-send-email-jeffy.chen@rock-chips.com> <19101003.9xjQ92OS5C@phil> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <19101003.9xjQ92OS5C@phil> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jeffy Chen Cc: linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org, Brian Norris , Douglas Anderson , dri-devel@lists.freedesktop.org, Tomasz Figa , Guenter Roeck , linux-arm-kernel@lists.infradead.org List-Id: linux-rockchip.vger.kernel.org QW0gTWl0dHdvY2gsIDE1LiBNw6RyeiAyMDE3LCAxODowMDowNCBDRVQgc2NocmllYiBIZWlrbyBT dHVlYm5lcjoKPiBBbSBNaXR0d29jaCwgMTUuIE3DpHJ6IDIwMTcsIDE4OjIwOjQ3IENFVCBzY2hy aWViIEplZmZ5IENoZW46Cj4gPiBDdXJyZW50bHkgd2UgYXJlIGFkZGluZyBhbGwgY29tcG9uZW50 cyBmcm9tIHRoZSBkdHMsIGlmIG9uZSBvZiB0aGVpcgo+ID4gZHJpdmVycyBiZWVuIGRpc2FibGVk LCB3ZSB3b3VsZCBub3QgYmUgYWJsZSB0byBicmluZyB1cCBvdGhlcnMuCj4gPiAKPiA+IFJlZmFj dG9yIGNvbXBvbmVudCBtYXRjaCBsb2dpYywgZm9sbG93IGV4eW5vcyBkcm0uCj4gPiAKPiA+IFNp Z25lZC1vZmYtYnk6IEplZmZ5IENoZW4gPGplZmZ5LmNoZW5Acm9jay1jaGlwcy5jb20+Cj4gPiBS ZXZpZXdlZC1ieTogQW5kcnplaiBIYWpkYSA8YS5oYWpkYUBzYW1zdW5nLmNvbT4KPiAKPiBUaGlz IHJlbGlhYmx5IHByb2R1Y2VzIG51bGwgcG9pbnRlciBkZXJlZmVyZW5jZSBlcnJvcnMgaW4KPiAJ X19wbGF0Zm9ybV9kcml2ZXJfcmVnaXN0ZXIgY2FsbGVkIGZyb20gcm9ja2NoaXBfZHJtX2luaXQK PiBvbiBhdCBsZWFzdCByazMwMzYgYW5kIHJrMzI4OCAocHJvYmFibHkgbW9yZSkgd2hlbiBhcHBs aWVkIG9uIHRvcCBvZgo+IExpbnVzJyB0cmVlIGZyb20gdG9kYXkuIExvZyBhdHRhY2hlZCBhbmQg Um9ja2NoaXAgZHJtIGNvbXBpbGVkIGFzIG1vZHVsZS4KPiAKPiBJJ20gY3VycmVudGx5IGR1ZyBp bnRvIG90aGVyIGFyZWFzLCBzbyBoYWRuJ3QgaGF2ZSB0aW1lIHRvIGludmVzdGlnYXRlCj4gZnVy dGhlciB5ZXQuCgo+ICsjZGVmaW5lIERSVl9QVFIoZHJ2LCBjb25kKSAoSVNfRU5BQkxFRChjb25k KSA/ICZkcnYgOiBOVUxMKQo+ICsKPiArc3RhdGljIHN0cnVjdCBwbGF0Zm9ybV9kcml2ZXIgKnJv Y2tjaGlwX2RybV9jb21wX2RydnNbXSA9IHsKPiArCSZ2b3BfcGxhdGZvcm1fZHJpdmVyLAo+ICsJ RFJWX1BUUihyb2NrY2hpcF9kcF9kcml2ZXIsIENPTkZJR19ST0NLQ0hJUF9BTkFMT0dJWF9EUCks Cj4gKwlEUlZfUFRSKGNkbl9kcF9kcml2ZXIsIENPTkZJR19ST0NLQ0hJUF9DRE5fRFApLAo+ICsJ RFJWX1BUUihkd19oZG1pX3JvY2tjaGlwX3BsdGZtX2RyaXZlciwgQ09ORklHX1JPQ0tDSElQX0RX X0hETUkpLAo+ICsJRFJWX1BUUihkd19taXBpX2RzaV9kcml2ZXIsIENPTkZJR19ST0NLQ0hJUF9E V19NSVBJX0RTSSksCj4gKwlEUlZfUFRSKGlubm9faGRtaV9kcml2ZXIsIENPTkZJR19ST0NLQ0hJ UF9JTk5PX0hETUkpLAo+ICt9OwoKWy4uLl0KCj4gK3N0YXRpYyBpbnQgcm9ja2NoaXBfZHJtX3Jl Z2lzdGVyX2RyaXZlcnModm9pZCkKPiArewo+ICsJaW50IGksIHJldDsKPiArCj4gKwlmb3IgKGkg PSAwOyBpIDwgQVJSQVlfU0laRShyb2NrY2hpcF9kcm1fY29tcF9kcnZzKTsgaSsrKSB7Cj4gKwkJ cmV0ID0gcGxhdGZvcm1fZHJpdmVyX3JlZ2lzdGVyKHJvY2tjaGlwX2RybV9jb21wX2RydnNbaV0p Owo+ICsJCWlmIChyZXQpCj4gKwkJCWdvdG8gZXJyX3VucmVnOwo+ICsJfQoKVGhpcyBvZiBjb3Vy c2Ugd29uJ3Qgd29yayBpbiB0aGUgTlVMTCBjYXNlLCBhcyBwbGF0Zm9ybV9kcml2ZXJfcmVnaXN0 ZXIgYWx3YXlzIApkZXJlZmVyZW5jZXMgaXRzIHBhcmFtZXRlciBbMF0sIHNvIHlvdSBzaG91bGQg b25seSBjYWxsIGl0IGZvciB0aGUgYWN0dWFsIG5vbi0KbnVsbCBhcnJheSBlbGVtZW50cyAtIG9m IGNvdXJzZSBzYW1lIGZvciB1bnJlZ2lzdGVyLgoKCkhlaWtvCgpbMF0gaHR0cDovL2x4ci5mcmVl LWVsZWN0cm9ucy5jb20vc291cmNlL2RyaXZlcnMvYmFzZS9wbGF0Zm9ybS5jI0w2MTcKCl9fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWls aW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZy ZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: heiko@sntech.de (Heiko Stuebner) Date: Wed, 15 Mar 2017 18:23:27 +0100 Subject: [PATCH v3] drm/rockchip: Refactor the component match logic. In-Reply-To: <19101003.9xjQ92OS5C@phil> References: <1489573247-9628-1-git-send-email-jeffy.chen@rock-chips.com> <19101003.9xjQ92OS5C@phil> Message-ID: <2110531.OeyX13mlk5@phil> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Am Mittwoch, 15. M?rz 2017, 18:00:04 CET schrieb Heiko Stuebner: > Am Mittwoch, 15. M?rz 2017, 18:20:47 CET schrieb Jeffy Chen: > > Currently we are adding all components from the dts, if one of their > > drivers been disabled, we would not be able to bring up others. > > > > Refactor component match logic, follow exynos drm. > > > > Signed-off-by: Jeffy Chen > > Reviewed-by: Andrzej Hajda > > This reliably produces null pointer dereference errors in > __platform_driver_register called from rockchip_drm_init > on at least rk3036 and rk3288 (probably more) when applied on top of > Linus' tree from today. Log attached and Rockchip drm compiled as module. > > I'm currently dug into other areas, so hadn't have time to investigate > further yet. > +#define DRV_PTR(drv, cond) (IS_ENABLED(cond) ? &drv : NULL) > + > +static struct platform_driver *rockchip_drm_comp_drvs[] = { > + &vop_platform_driver, > + DRV_PTR(rockchip_dp_driver, CONFIG_ROCKCHIP_ANALOGIX_DP), > + DRV_PTR(cdn_dp_driver, CONFIG_ROCKCHIP_CDN_DP), > + DRV_PTR(dw_hdmi_rockchip_pltfm_driver, CONFIG_ROCKCHIP_DW_HDMI), > + DRV_PTR(dw_mipi_dsi_driver, CONFIG_ROCKCHIP_DW_MIPI_DSI), > + DRV_PTR(inno_hdmi_driver, CONFIG_ROCKCHIP_INNO_HDMI), > +}; [...] > +static int rockchip_drm_register_drivers(void) > +{ > + int i, ret; > + > + for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) { > + ret = platform_driver_register(rockchip_drm_comp_drvs[i]); > + if (ret) > + goto err_unreg; > + } This of course won't work in the NULL case, as platform_driver_register always dereferences its parameter [0], so you should only call it for the actual non- null array elements - of course same for unregister. Heiko [0] http://lxr.free-electrons.com/source/drivers/base/platform.c#L617 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752672AbdCORXv convert rfc822-to-8bit (ORCPT ); Wed, 15 Mar 2017 13:23:51 -0400 Received: from gloria.sntech.de ([95.129.55.99]:50632 "EHLO gloria.sntech.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751310AbdCORXn (ORCPT ); Wed, 15 Mar 2017 13:23:43 -0400 From: Heiko Stuebner To: Jeffy Chen Cc: Mark Yao , Andrzej Hajda , Guenter Roeck , Brian Norris , Tomasz Figa , Douglas Anderson , Sean Paul , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-rockchip@lists.infradead.org, David Airlie , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3] drm/rockchip: Refactor the component match logic. Date: Wed, 15 Mar 2017 18:23:27 +0100 Message-ID: <2110531.OeyX13mlk5@phil> User-Agent: KMail/5.2.3 (Linux/4.9.0-1-amd64; KDE/5.27.0; x86_64; ; ) In-Reply-To: <19101003.9xjQ92OS5C@phil> References: <1489573247-9628-1-git-send-email-jeffy.chen@rock-chips.com> <19101003.9xjQ92OS5C@phil> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="iso-8859-1" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am Mittwoch, 15. März 2017, 18:00:04 CET schrieb Heiko Stuebner: > Am Mittwoch, 15. März 2017, 18:20:47 CET schrieb Jeffy Chen: > > Currently we are adding all components from the dts, if one of their > > drivers been disabled, we would not be able to bring up others. > > > > Refactor component match logic, follow exynos drm. > > > > Signed-off-by: Jeffy Chen > > Reviewed-by: Andrzej Hajda > > This reliably produces null pointer dereference errors in > __platform_driver_register called from rockchip_drm_init > on at least rk3036 and rk3288 (probably more) when applied on top of > Linus' tree from today. Log attached and Rockchip drm compiled as module. > > I'm currently dug into other areas, so hadn't have time to investigate > further yet. > +#define DRV_PTR(drv, cond) (IS_ENABLED(cond) ? &drv : NULL) > + > +static struct platform_driver *rockchip_drm_comp_drvs[] = { > + &vop_platform_driver, > + DRV_PTR(rockchip_dp_driver, CONFIG_ROCKCHIP_ANALOGIX_DP), > + DRV_PTR(cdn_dp_driver, CONFIG_ROCKCHIP_CDN_DP), > + DRV_PTR(dw_hdmi_rockchip_pltfm_driver, CONFIG_ROCKCHIP_DW_HDMI), > + DRV_PTR(dw_mipi_dsi_driver, CONFIG_ROCKCHIP_DW_MIPI_DSI), > + DRV_PTR(inno_hdmi_driver, CONFIG_ROCKCHIP_INNO_HDMI), > +}; [...] > +static int rockchip_drm_register_drivers(void) > +{ > + int i, ret; > + > + for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) { > + ret = platform_driver_register(rockchip_drm_comp_drvs[i]); > + if (ret) > + goto err_unreg; > + } This of course won't work in the NULL case, as platform_driver_register always dereferences its parameter [0], so you should only call it for the actual non- null array elements - of course same for unregister. Heiko [0] http://lxr.free-electrons.com/source/drivers/base/platform.c#L617