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 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 997BCC001B0 for ; Thu, 13 Jul 2023 12:48:34 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 668E810E687; Thu, 13 Jul 2023 12:48:31 +0000 (UTC) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id F2AF410E143; Wed, 12 Jul 2023 11:14:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1689160486; x=1720696486; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=pa2Bj6VqT5cElyyhfEvI8AxkZz4IX9juc4rRJw0HbBE=; b=gaflAFx5VZTk90cRxsoZ6xtpXo3QqZFz1W0I1AvvxO8tSbO2id+zhepa kjdRdAzyY9uzbzT+Cqckdt/FQkhaumynR/EU82CiAHgs6g+EoJMXknaP6 OWuvibY031yz+kU/r4IyW7sb2380+RJf267SDhzy9VhR+5vlO+3MN4YCf DvBwECBuTwF0YT9+eVQtKFg4ARg3Wem1NW+hdJLONqc5/pmi5NWfUjp7i YYcqeVKYdSVsKBZYmmY2OVOF6tZzkhad1KE0ZCSVN1QSdtkrg2Foz5Dgg 3x1vUVfEl9e087oOxybF1QhrCRR23mtZ4LyPBJZHeqXSIXjz0cebxDR7u A==; X-IronPort-AV: E=McAfee;i="6600,9927,10768"; a="368396151" X-IronPort-AV: E=Sophos;i="6.01,199,1684825200"; d="scan'208";a="368396151" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Jul 2023 04:14:42 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10768"; a="1052148842" X-IronPort-AV: E=Sophos;i="6.01,199,1684825200"; d="scan'208";a="1052148842" Received: from ahajda-mobl.ger.corp.intel.com (HELO [10.213.31.249]) ([10.213.31.249]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Jul 2023 04:13:48 -0700 Message-ID: <60a183df-9776-1f10-bbd7-248531921888@intel.com> Date: Wed, 12 Jul 2023 13:13:45 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Firefox/102.0 Thunderbird/102.13.0 Content-Language: en-US To: Julia Lawall , =?UTF-8?Q?Uwe_Kleine-K=c3=b6nig?= References: <20230712094702.1770121-1-u.kleine-koenig@pengutronix.de> <94eb6e4d-9384-152f-351b-ebb217411da9@amd.com> <20230712110253.paoyrmcbvlhpfxbf@pengutronix.de> From: Andrzej Hajda Organization: Intel Technology Poland sp. z o.o. - ul. Slowackiego 173, 80-298 Gdansk - KRS 101882 - NIP 957-07-52-316 In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Mailman-Approved-At: Thu, 13 Jul 2023 12:48:29 +0000 Subject: Re: [Intel-gfx] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: =?UTF-8?Q?Heiko_St=c3=bcbner?= , Geert Uytterhoeven , Xinliang Liu , Linus Walleij , Tomi Valkeinen , Alexey Kodanev , dri-devel@lists.freedesktop.org, Alim Akhtar , Anitha Chrisanthus , Marijn Suijten , Jonathan Hunter , Jerome Brunet , Liu Shixin , linux-samsung-soc@vger.kernel.org, Samuel Holland , Bhawanpreet Lakha , Wenjing Liu , Javier Martinez Canillas , Danilo Krummrich , NXP Linux Team , spice-devel@lists.freedesktop.org, linux-sunxi@lists.linux.dev, Tim Huang , =?UTF-8?Q?Andr=c3=a9_Almeida?= , Yifan Zhang , Jani Nikula , Sascha Hauer , Lucas De Marchi , Inki Dae , Hersen Wu , Jessica Zhang , Kamlesh Gurudasani , Matt Roper , =?UTF-8?Q?=c5=81ukasz_Bartosik?= , Andrew Jeffery , Seung-Woo Kim , =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= , kernel@pengutronix.de, Alex Deucher , freedreno@lists.freedesktop.org, Claudiu Beznea , Zack Rusin , Gerd Hoffmann , Alexandre Belloni , linux-aspeed@lists.ozlabs.org, nouveau@lists.freedesktop.org, virtualization@lists.linux-foundation.org, Yongqin Liu , Mario Limonciello , David Lechner , "Jiri Slaby \(SUSE\)" , David Francis , Aaron Liu , Vinod Polimera , linux-rockchip@lists.infradead.org, Fangzhi Zuo , Aurabindo Pillai , VMware Graphics Reviewers , Ben Skeggs , Dave Airlie , linux-mips@vger.kernel.org, Maxime Coquelin , Gurchetan Singh , Martin Blumenstingl , linux-arm-msm@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Maxime Ripard , Biju Das , linux-amlogic@lists.infradead.org, Evan Quan , Michal Simek , linux-arm-kernel@lists.infradead.org, Neil Armstrong , Boris Brezillon , Chunyan Zhang , Qingqing Zhuo , Sandy Huang , John Stultz , Paul Kocialkowski , Kyungmin Park , Drew Davenport , Kevin Hilman , Hawking Zhang , Haneen Mohammed , Dan Carpenter , Karol Herbst , linux-hyperv@vger.kernel.org, Stefan Agner , Melissa Wen , =?UTF-8?Q?Ma=c3=adra_Canal?= , Luca Coelho , Laurent Pinchart , Likun Gao , Sam Ravnborg , Alain Volmat , Xinwei Kong , Jernej Skrabec , Deepak Rawat , Chen-Yu Tsai , Joel Stanley , Philipp Zabel , Harry Wentland , Sumit Semwal , Alan Liu , Philip Yang , intel-gfx@lists.freedesktop.org, Alison Wang , Wolfram Sang , Abhinav Kumar , Baolin Wang , Rodrigo Vivi , Mikko Perttunen , Rodrigo Siqueira , Tomi Valkeinen , Deepak R Varma , "Pan, Xinhui" , Chia-I Wu , Konrad Dybcio , Kieran Bingham , Tian Tao , Shawn Guo , =?UTF-8?Q?Christian_K=c3=b6nig?= , linux-stm32@st-md-mailman.stormreply.com, Emma Anholt , Chun-Kuang Hu , Alexandre Torgue , Roman Li , Paul Cercueil , Hamza Mahfooz , David Airlie , Marek Vasut , Jiapeng Chong , xen-devel@lists.xenproject.org, Guchun Chen , Oleksandr Andrushchenko , Raphael Gallais-Pou , Rodrigo Siqueira , Russell King , Leo Li , Jiasheng Jiang , Srinivasan Shanmugam , Thomas Zimmermann , linux-tegra@vger.kernel.org, =?UTF-8?B?TWFyZWsgT2zFocOhaw==?= , =?UTF-8?Q?Joaqu=c3=adn_Ignacio_Aramend=c3=ada?= , Melissa Wen , Hans de Goede , linux-mediatek@lists.infradead.org, Fabio Estevam , Laurentiu Palcu , Matthias Brugger , David Tadokoro , AngeloGioacchino Del Regno , Orson Zhai , amd-gfx@lists.freedesktop.org, Jyri Sarha , Yannick Fertre , Nicolas Ferre , Krzysztof Kozlowski , Philippe Cornu , Daniel Vetter , Wayne Lin , Dmitry Baryshkov , Nirmoy Das , Lang Yu , Lucas Stach Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On 12.07.2023 13:07, Julia Lawall wrote: > > On Wed, 12 Jul 2023, Uwe Kleine-König wrote: > >> On Wed, Jul 12, 2023 at 12:46:33PM +0200, Christian König wrote: >>> Am 12.07.23 um 11:46 schrieb Uwe Kleine-König: >>>> Hello, >>>> >>>> while I debugged an issue in the imx-lcdc driver I was constantly >>>> irritated about struct drm_device pointer variables being named "dev" >>>> because with that name I usually expect a struct device pointer. >>>> >>>> I think there is a big benefit when these are all renamed to "drm_dev". >>>> I have no strong preference here though, so "drmdev" or "drm" are fine >>>> for me, too. Let the bikesheding begin! >>>> >>>> Some statistics: >>>> >>>> $ git grep -ohE 'struct drm_device *\* *[^ (),;]*' v6.5-rc1 | sort | uniq -c | sort -n >>>> 1 struct drm_device *adev_to_drm >>>> 1 struct drm_device *drm_ >>>> 1 struct drm_device *drm_dev >>>> 1 struct drm_device *drm_dev >>>> 1 struct drm_device *pdev >>>> 1 struct drm_device *rdev >>>> 1 struct drm_device *vdev >>>> 2 struct drm_device *dcss_drv_dev_to_drm >>>> 2 struct drm_device **ddev >>>> 2 struct drm_device *drm_dev_alloc >>>> 2 struct drm_device *mock >>>> 2 struct drm_device *p_ddev >>>> 5 struct drm_device *device >>>> 9 struct drm_device * dev >>>> 25 struct drm_device *d >>>> 95 struct drm_device * >>>> 216 struct drm_device *ddev >>>> 234 struct drm_device *drm_dev >>>> 611 struct drm_device *drm >>>> 4190 struct drm_device *dev >>>> >>>> This series starts with renaming struct drm_crtc::dev to drm_dev. If >>>> it's not only me and others like the result of this effort it should be >>>> followed up by adapting the other structs and the individual usages in >>>> the different drivers. >>>> >>>> To make this series a bit easier handleable, I first added an alias for >>>> drm_crtc::dev, then converted the drivers one after another and the last >>>> patch drops the "dev" name. This has the advantage of being easier to >>>> review, and if I should have missed an instance only the last patch must >>>> be dropped/reverted. Also this series might conflict with other patches, >>>> in this case the remaining patches can still go in (apart from the last >>>> one of course). Maybe it also makes sense to delay applying the last >>>> patch by one development cycle? >>> When you automatically generate the patch (with cocci for example) I usually >>> prefer a single patch instead. >> Maybe I'm too stupid, but only parts of this patch were created by >> coccinelle. I failed to convert code like >> >> - spin_lock_irq(&crtc->dev->event_lock); >> + spin_lock_irq(&crtc->drm_dev->event_lock); >> >> Added Julia to Cc, maybe she has a hint?! > A priori, I see no reason why the rule below should not apply to the above > code. Is there a parsing problem in the containing function? You can run > > spatch --parse-c file.c > > If there is a paring problem, please let me know and i will try to fix it > so the while thing can be done automatically. I guess some clever macros can fool spatch, at least I observe such things in i915 which often uses custom iterators. Regards Andrzej > > julia > >> (Up to now it's only >> >> @@ >> struct drm_crtc *crtc; >> @@ >> -crtc->dev >> +crtc->drm_dev >> >> ) >> >>> Background is that this makes merge conflicts easier to handle and detect. >> Really? Each file (apart from include/drm/drm_crtc.h) is only touched >> once. So unless I'm missing something you don't get less or easier >> conflicts by doing it all in a single patch. But you gain the freedom to >> drop a patch for one driver without having to drop the rest with it. So >> I still like the split version better, but I'm open to a more verbose >> reasoning from your side. >> >>> When you have multiple patches and a merge conflict because of some added >>> lines using the old field the build breaks only on the last patch which >>> removes the old field. >> Then you can revert/drop the last patch without having to respin the >> whole single patch and thus caring for still more conflicts that arise >> until the new version is sent. >> >> Best regards >> Uwe >> >> -- >> Pengutronix e.K. | Uwe Kleine-König | >> Industrial Linux Solutions | https://www.pengutronix.de/ | > >