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=-3.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 A6EA0C433DF for ; Thu, 20 Aug 2020 14:49:34 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7044B2075E for ; Thu, 20 Aug 2020 14:49:34 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="0vKxzUWv" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7044B2075E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ravnborg.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=mswOnx9dbQSmqnjk8bYJSmWjOzqPwK1r0hfbqM/yb1w=; b=0vKxzUWvwRHU4rBbs65qY7yRp FAV66ftXfhN/vEDB0pVYYOTWuR2Uhbc/EsJv/xfeWNxL2BHugK6Nxkmsq05dHUX8QXI/GXJQJ8UWs IWGOL0kOHxdAaSZQkMLpHgS5XnRcFPSRrNjRJ1lqgSsHLh4PeWxc2l6kSpV9Llj96KBEqw6A6kRWK 3nyj9hd2GM58RiOEKzgVlUvxuovlICaBwCc89AnsEGos2odjQplQtCV9pVbGdr1Kbx2dAomyTKvq0 uauZ9mx5Ki22BGwwum/17+sT71YycDcqFkPyYjI9zn8yzt17ymnYE1lb+GF7Pqg4GGqwu4iD5cZXM etep4Bh/w==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k8lrY-0007Cs-8P; Thu, 20 Aug 2020 14:48:20 +0000 Received: from asavdk3.altibox.net ([109.247.116.14]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1k8lrV-0007Bo-Ma for linux-arm-kernel@lists.infradead.org; Thu, 20 Aug 2020 14:48:18 +0000 Received: from ravnborg.org (unknown [188.228.123.71]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by asavdk3.altibox.net (Postfix) with ESMTPS id B6D2820026; Thu, 20 Aug 2020 16:48:09 +0200 (CEST) Date: Thu, 20 Aug 2020 16:48:08 +0200 From: Sam Ravnborg To: Mauro Carvalho Chehab Subject: Re: [PATCH 00/49] DRM driver for Hikey 970 Message-ID: <20200820144808.GA186324@ravnborg.org> References: <20200819152120.GA106437@ravnborg.org> <20200819174027.70b39ee9@coco.lan> <20200819173558.GA3733@ravnborg.org> <20200820160649.54741194@coco.lan> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200820160649.54741194@coco.lan> X-CMAE-Score: 0 X-CMAE-Analysis: v=2.3 cv=f+hm+t6M c=1 sm=1 tr=0 a=S6zTFyMACwkrwXSdXUNehg==:117 a=S6zTFyMACwkrwXSdXUNehg==:17 a=kj9zAlcOel0A:10 a=7gkXJVJtAAAA:8 a=FQPn5FuWFEp6BKnZ598A:9 a=CjuIK1q_8ugA:10 a=E9Po1WZjFZOl8hwRPBS3:22 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200820_104817_996323_65E65F80 X-CRM114-Status: GOOD ( 26.12 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Neil Armstrong , Xinliang Liu , Wanchun Zheng , linuxarm@huawei.com, dri-devel , Andrzej Hajda , Laurent Pinchart , devel@driverdev.osuosl.org, Daniel Borkmann , John Fastabend , Xiubin Zhang , Wei Xu , David Airlie , Xinwei Kong , Tomi Valkeinen , Bogdan Togorean , Jakub Kicinski , Laurentiu Palcu , linux-media@vger.kernel.org, devicetree@vger.kernel.org, Liwei Cai , Jesper Dangaard Brouer , Manivannan Sadhasivam , Chen Feng , Alexei Starovoitov , linaro-mm-sig@lists.linaro.org, Rob Herring , mauro.chehab@huawei.com, Rob Clark , linux-arm-kernel@lists.infradead.org, Greg Kroah-Hartman , linux-kernel@vger.kernel.org, Liuyao An , netdev@vger.kernel.org, Rongrong Zou , bpf@vger.kernel.org, "David S. Miller" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Mauro. On Thu, Aug 20, 2020 at 04:06:49PM +0200, Mauro Carvalho Chehab wrote: > Em Wed, 19 Aug 2020 19:35:58 +0200 > Sam Ravnborg escreveu: > > I'm already handling the other comments from your review (I'll send a > more complete comment about them after finishing), If you get back only on things you do not understand or do not agree on that would be fine. The rest should be visible in the changelog on the updated patch - no need to do extra work here. > but I have a doubt what you meant about this: > > > +static int kirin_drm_bind(struct device *dev) > > > +{ > > > + struct drm_driver *driver = &kirin_drm_driver; > > > + struct drm_device *drm_dev; > > > + struct kirin_drm_private *priv; > > > + int ret; > > > + > > > + drm_dev = drm_dev_alloc(driver, dev); > > > + if (!drm_dev) > > > + return -ENOMEM; > > > + > > > + ret = kirin_drm_kms_init(drm_dev); > > > + if (ret) > > > + goto err_drm_dev_unref; > > > + > > > + ret = drm_dev_register(drm_dev, 0); > > There is better ways to do this. See drm_drv.c for the code example. > > Not sure if I understood your comment here. The drm_drv.c example also calls > drm_dev_register(). This is indeed not obvious from my comments but what I wnated to say is that the driver should embed drm_device in some struct, maybe in "struct kirin_drm_private". This should also be part of the referenced example. I hope this clarifies it. Sam > > The only difference is that it calls it inside driver_probe(), while > on this driver, it uses: > > static const struct component_master_ops kirin_drm_ops = { > .bind = kirin_drm_bind, > .unbind = kirin_drm_unbind, > }; > > static int kirin_drm_platform_probe(struct platform_device *pdev) > { > ... > drm_of_component_match_add(dev, &match, compare_of, remote); > ... > return component_master_add_with_match(dev, &kirin_drm_ops, match); > } > > Are you meaning that I should get rid of this component API and call > kirin_drm_bind() from kirin_drm_platform_probe()? > > Thanks, > Mauro _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel