From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4213B200D7 for ; Wed, 12 Jul 2023 11:14:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1689160485; x=1720696485; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=pa2Bj6VqT5cElyyhfEvI8AxkZz4IX9juc4rRJw0HbBE=; b=hFYg/N22Sh9CfxaftH8T5xpVegf2lB1XDELWh8aWRe23i4sHP1idMeX/ ZNTeJ8EwlFlVb0jtLfJfoxFbqAQKLwJUl5Z7KAeBWro6MaWe0l7DOmtMu 9p56+g98xyXcP+yo7vREKzlNHJMO7Bhjrt2YVPh8JjWFrsfvVdtKoYN6j jnPHTGE+oCIawcNeF0atihkCmqCA3+cWV9JWs8ADXE9ZnFoOrYgNTjuyM rEBakjD4CkOf6TPuMrn2ozKAbgIOqti63kiZ0enkc3WQM8nF/09yrw3wi IRn9tDWTTI86UplLNhiwI5rCsankuzs/RZyfRETT2g6G4zkMRDJ3CLpv+ g==; X-IronPort-AV: E=McAfee;i="6600,9927,10768"; a="368396160" X-IronPort-AV: E=Sophos;i="6.01,199,1684825200"; d="scan'208";a="368396160" 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 Precedence: bulk X-Mailing-List: linux-sunxi@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: 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 Subject: Re: [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev Content-Language: en-US To: Julia Lawall , =?UTF-8?Q?Uwe_Kleine-K=c3=b6nig?= Cc: =?UTF-8?Q?Christian_K=c3=b6nig?= , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Daniel Vetter , Alex Deucher , "Pan, Xinhui" , Harry Wentland , Leo Li , Rodrigo Siqueira , Hamza Mahfooz , Javier Martinez Canillas , Guchun Chen , Srinivasan Shanmugam , Evan Quan , Likun Gao , =?UTF-8?B?TWFyZWsgT2zFocOhaw==?= , David Francis , Hawking Zhang , Lang Yu , Philip Yang , Yifan Zhang , Tim Huang , Zack Rusin , Sam Ravnborg , Jani Nikula , Laurent Pinchart , =?UTF-8?Q?Ma=c3=adra_Canal?= , =?UTF-8?Q?Andr=c3=a9_Almeida?= , Qingqing Zhuo , Aurabindo Pillai , Hersen Wu , Fangzhi Zuo , Stylon Wang , Alan Liu , Wayne Lin , Aaron Liu , Melissa Wen , Bhawanpreet Lakha , David Tadokoro , Wenjing Liu , Jiapeng Chong , Mario Limonciello , Alexey Kodanev , Roman Li , =?UTF-8?Q?Joaqu=c3=adn_Ignacio_Aramend=c3=ada?= , Dave Airlie , Russell King , Liviu Dudau , Joel Stanley , Boris Brezillon , Nicolas Ferre , Alexandre Belloni , Claudiu Beznea , Inki Dae , Seung-Woo Kim , Kyungmin Park , Krzysztof Kozlowski , Stefan Agner , Alison Wang , Patrik Jakobsson , =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= , Xinliang Liu , Tian Tao , Danilo Krummrich , Deepak Rawat , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Tvrtko Ursulin , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Lucas De Marchi , Ankit Nautiyal , Matt Roper , Stanislav Lisovskiy , Radhakrishna Sripada , Hans de Goede , Luca Coelho , Niranjana Vishwanathapura , Kai Vehmanen , Vinod Govindapillai , =?UTF-8?Q?=c5=81ukasz_Bartosik?= , Anusha Srivatsa , Chaitanya Kumar Borah , Uma Shankar , Imre Deak , Mitul Golani , Swati Sharma , =?UTF-8?Q?Jouni_H=c3=b6gander?= , Mika Kahola , =?UTF-8?Q?Jos=c3=a9_Roberto_de_Souza?= , Arun R Murthy , Gustavo Sousa , Khaled Almahallawy , Juha-Pekka Heikkila , Andi Shyti , Nirmoy Das , Fei Yang , Animesh Manna , Deepak R Varma , "Jiri Slaby (SUSE)" , Dmitry Baryshkov , Vandita Kulkarni , Suraj Kandpal , Drew Davenport , Laurentiu Palcu , Shawn Guo , Sascha Hauer , Philipp Zabel , Dan Carpenter , Paul Cercueil , Anitha Chrisanthus , Paul Kocialkowski , Linus Walleij , Chun-Kuang Hu , Matthias Brugger , Neil Armstrong , Kevin Hilman , Rob Clark , Abhinav Kumar , Vinod Polimera , Jiasheng Jiang , Konrad Dybcio , Jessica Zhang , Liu Shixin , Marek Vasut , Ben Skeggs , Karol Herbst , Lyude Paul , Tomi Valkeinen , Emma Anholt , Gerd Hoffmann , Kieran Bingham , Tomi Valkeinen , Wolfram Sang , Geert Uytterhoeven , Biju Das , Sandy Huang , =?UTF-8?Q?Heiko_St=c3=bcbner?= , Orson Zhai , Baolin Wang , Chunyan Zhang , Alain Volmat , Yannick Fertre , Raphael Gallais-Pou , Philippe Cornu , Maxime Coquelin , Alexandre Torgue , Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , Thierry Reding , Mikko Perttunen , Jonathan Hunter , Jyri Sarha , David Lechner , Kamlesh Gurudasani , Rodrigo Siqueira , Melissa Wen , Oleksandr Andrushchenko , Michal Simek , Haneen Mohammed , linux-hyperv@vger.kernel.org, linux-aspeed@lists.ozlabs.org, nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org, virtualization@lists.linux-foundation.org, Yongqin Liu , Alim Akhtar , Marijn Suijten , Fabio Estevam , Sumit Semwal , Jerome Brunet , linux-samsung-soc@vger.kernel.org, amd-gfx@lists.freedesktop.org, linux-stm32@st-md-mailman.stormreply.com, linux-rockchip@lists.infradead.org, Xinwei Kong , VMware Graphics Reviewers , NXP Linux Team , spice-devel@lists.freedesktop.org, linux-sunxi@lists.linux.dev, Martin Blumenstingl , linux-arm-msm@vger.kernel.org, intel-gfx@lists.freedesktop.org, linux-mediatek@lists.infradead.org, xen-devel@lists.xenproject.org, linux-tegra@vger.kernel.org, linux-amlogic@lists.infradead.org, Gurchetan Singh , Sean Paul , linux-arm-kernel@lists.infradead.org, AngeloGioacchino Del Regno , Andrew Jeffery , linux-mips@vger.kernel.org, Chia-I Wu , linux-renesas-soc@vger.kernel.org, kernel@pengutronix.de, John Stultz , freedreno@lists.freedesktop.org, Lucas Stach 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 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/ | > >