[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
- To: Jani Nikula <jani.nikula@xxxxxxxxx>
- From: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
- Date: Thu, 13 Jul 2023 11:54:27 +0200
- Cc: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>, Xinliang Liu <xinliang.liu@xxxxxxxxxx>, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>, Alexey Kodanev <aleksei.kodanev@xxxxxxxxxxx>, dri-devel@xxxxxxxxxxxxxxxxxxxxx, Vandita Kulkarni <vandita.kulkarni@xxxxxxxxx>, Alim Akhtar <alim.akhtar@xxxxxxxxxxx>, Anitha Chrisanthus <anitha.chrisanthus@xxxxxxxxx>, Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>, Jonathan Hunter <jonathanh@xxxxxxxxxx>, Arun R Murthy <arun.r.murthy@xxxxxxxxx>, Jerome Brunet <jbrunet@xxxxxxxxxxxx>, linux-samsung-soc@xxxxxxxxxxxxxxx, Samuel Holland <samuel@xxxxxxxxxxxx>, Matt Roper <matthew.d.roper@xxxxxxxxx>, Wenjing Liu <wenjing.liu@xxxxxxx>, Javier Martinez Canillas <javierm@xxxxxxxxxx>, Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>, Danilo Krummrich <dakr@xxxxxxxxxx>, NXP Linux Team <linux-imx@xxxxxxx>, spice-devel@xxxxxxxxxxxxxxxxxxxxx, Niranjana Vishwanathapura <niranjana.vishwanathapura@xxxxxxxxx>, Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>, linux-sunxi@xxxxxxxxxxxxxxx, Matthias Brugger <matthias.bgg@xxxxxxxxx>, Stylon Wang <stylon.wang@xxxxxxx>, Tim Huang <Tim.Huang@xxxxxxx>, Suraj Kandpal <suraj.kandpal@xxxxxxxxx>, André Almeida <andrealmeid@xxxxxxxxxx>, Mika Kahola <mika.kahola@xxxxxxxxx>, Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>, Leo Li <sunpeng.li@xxxxxxx>, Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>, Lucas De Marchi <lucas.demarchi@xxxxxxxxx>, Hersen Wu <hersenxs.wu@xxxxxxx>, Dave Airlie <airlied@xxxxxxxxxx>, Kamlesh Gurudasani <kamlesh.gurudasani@xxxxxxxxx>, Bhawanpreet Lakha <Bhawanpreet.Lakha@xxxxxxx>, Łukasz Bartosik <lb@xxxxxxxxxxxx>, Radhakrishna Sripada <radhakrishna.sripada@xxxxxxxxx>, Andrew Jeffery <andrew@xxxxxxxx>, Seung-Woo Kim <sw0312.kim@xxxxxxxxxxx>, Noralf Trønnes <noralf@xxxxxxxxxxx>, kernel@xxxxxxxxxxxxxx, Alex Deucher <alexander.deucher@xxxxxxx>, freedreno@xxxxxxxxxxxxxxxxxxxxx, Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx>, Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>, Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx>, linux-aspeed@xxxxxxxxxxxxxxxx, nouveau@xxxxxxxxxxxxxxxxxxxxx, Mitul Golani <mitulkumar.ajitkumar.golani@xxxxxxxxx>, José Roberto de Souza <jose.souza@xxxxxxxxx>, virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx, Thierry Reding <thierry.reding@xxxxxxxxx>, Yongqin Liu <yongqin.liu@xxxxxxxxxx>, Mario Limonciello <mario.limonciello@xxxxxxx>, Fei Yang <fei.yang@xxxxxxxxx>, Juha-Pekka Heikkila <juhapekka.heikkila@xxxxxxxxx>, Chunyan Zhang <zhang.lyra@xxxxxxxxx>, David Francis <David.Francis@xxxxxxx>, Vinod Govindapillai <vinod.govindapillai@xxxxxxxxx>, Aaron Liu <aaron.liu@xxxxxxx>, Vinod Polimera <quic_vpolimer@xxxxxxxxxxx>, linux-rockchip@xxxxxxxxxxxxxxxxxxx, Fangzhi Zuo <jerry.zuo@xxxxxxx>, Aurabindo Pillai <aurabindo.pillai@xxxxxxx>, VMware Graphics Reviewers <linux-graphics-maintainer@xxxxxxxxxx>, Ben Skeggs <bskeggs@xxxxxxxxxx>, Jouni Högander <jouni.hogander@xxxxxxxxx>, Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>, Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>, linux-arm-msm@xxxxxxxxxxxxxxx, Animesh Manna <animesh.manna@xxxxxxxxx>, Nicolas Ferre <nicolas.ferre@xxxxxxxxxxxxx>, Maxime Ripard <mripard@xxxxxxxxxx>, Chaitanya Kumar Borah <chaitanya.kumar.borah@xxxxxxxxx>, Tian Tao <tiantao6@xxxxxxxxxxxxx>, Biju Das <biju.das.jz@xxxxxxxxxxxxxx>, linux-amlogic@xxxxxxxxxxxxxxxxxxx, Evan Quan <evan.quan@xxxxxxx>, Michal Simek <michal.simek@xxxxxxx>, linux-arm-kernel@xxxxxxxxxxxxxxxxxxx, Sean Paul <sean@xxxxxxxxxx>, Neil Armstrong <neil.armstrong@xxxxxxxxxx>, Kai Vehmanen <kai.vehmanen@xxxxxxxxxxxxxxx>, Boris Brezillon <bbrezillon@xxxxxxxxxx>, Shawn Guo <shawnguo@xxxxxxxxxx>, Qingqing Zhuo <qingqing.zhuo@xxxxxxx>, Sandy Huang <hjc@xxxxxxxxxxxxxx>, Swati Sharma <swati2.sharma@xxxxxxxxx>, linux-renesas-soc@xxxxxxxxxxxxxxx, Kyungmin Park <kyungmin.park@xxxxxxxxxxx>, Maxime Coquelin <mcoquelin.stm32@xxxxxxxxx>, Kevin Hilman <khilman@xxxxxxxxxxxx>, Hawking Zhang <Hawking.Zhang@xxxxxxx>, Haneen Mohammed <hamohammed.sa@xxxxxxxxx>, Paul Cercueil <paul@xxxxxxxxxxxxxxx>, Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>, Dan Carpenter <error27@xxxxxxxxx>, Karol Herbst <kherbst@xxxxxxxxxx>, linux-hyperv@xxxxxxxxxxxxxxx, Melissa Wen <melissa.srw@xxxxxxxxx>, Maíra Canal <mairacanal@xxxxxxxxxx>, Luca Coelho <luciano.coelho@xxxxxxxxx>, Gerd Hoffmann <kraxel@xxxxxxxxxx>, Andrzej Hajda <andrzej.hajda@xxxxxxxxx>, Likun Gao <Likun.Gao@xxxxxxx>, "Jiri Slaby (SUSE)" <jirislaby@xxxxxxxxxx>, Emma Anholt <emma@xxxxxxxxxx>, Alain Volmat <alain.volmat@xxxxxxxxxxx>, Chen-Yu Tsai <wens@xxxxxxxx>, Jernej Skrabec <jernej.skrabec@xxxxxxxxx>, Deepak Rawat <drawat.floss@xxxxxxxxx>, Xinwei Kong <kong.kongxinwei@xxxxxxxxxxxxx>, Joel Stanley <joel@xxxxxxxxx>, Orson Zhai <orsonzhai@xxxxxxxxx>, Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>, Alan Liu <haoping.liu@xxxxxxx>, Philip Yang <Philip.Yang@xxxxxxx>, intel-gfx@xxxxxxxxxxxxxxxxxxxxx, Alison Wang <alison.wang@xxxxxxx>, Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>, Gustavo Sousa <gustavo.sousa@xxxxxxxxx>, Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>, Tomi Valkeinen <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>, Mikko Perttunen <mperttunen@xxxxxxxxxx>, Yifan Zhang <yifan1.zhang@xxxxxxx>, Rodrigo Siqueira <rodrigosiqueiramelo@xxxxxxxxx>, Tomi Valkeinen <tomba@xxxxxxxxxx>, Deepak R Varma <drv@xxxxxxxxx>, "Pan, Xinhui" <Xinhui.Pan@xxxxxxx>, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>, Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>, John Stultz <jstultz@xxxxxxxxxx>, Roman Li <roman.li@xxxxxxx>, Sumit Semwal <sumit.semwal@xxxxxxxxxx>, Christian König <christian.koenig@xxxxxxx>, Khaled Almahallawy <khaled.almahallawy@xxxxxxxxx>, linux-stm32@xxxxxxxxxxxxxxxxxxxxxxxxxxxx, Sam Ravnborg <sam@xxxxxxxxxxxx>, Chun-Kuang Hu <chunkuang.hu@xxxxxxxxxx>, Liviu Dudau <liviu.dudau@xxxxxxx>, Alexandre Torgue <alexandre.torgue@xxxxxxxxxxx>, Gurchetan Singh <gurchetansingh@xxxxxxxxxxxx>, Liu Shixin <liushixin2@xxxxxxxxxx>, Hamza Mahfooz <hamza.mahfooz@xxxxxxx>, Marek Vasut <marex@xxxxxxx>, Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, Guchun Chen <guchun.chen@xxxxxxx>, Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>, Raphael Gallais-Pou <raphael.gallais-pou@xxxxxxxxxxx>, Rodrigo Siqueira <Rodrigo.Siqueira@xxxxxxx>, Russell King <linux@xxxxxxxxxxxxxxx>, Uma Shankar <uma.shankar@xxxxxxxxx>, Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>, Jiasheng Jiang <jiasheng@xxxxxxxxxxx>, Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx>, David Lechner <david@xxxxxxxxxxxxxx>, Jiapeng Chong <jiapeng.chong@xxxxxxxxxxxxxxxxx>, Marek Olšák <marek.olsak@xxxxxxx>, Joaquín Ignacio Aramendía <samsagax@xxxxxxxxx>, Melissa Wen <mwen@xxxxxxxxxx>, Hans de Goede <hdegoede@xxxxxxxxxx>, linux-mediatek@xxxxxxxxxxxxxxxxxxx, Laurentiu Palcu <laurentiu.palcu@xxxxxxxxxxx>, linux-tegra@xxxxxxxxxxxxxxx, David Tadokoro <davidbtadokoro@xxxxxx>, AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>, amd-gfx@xxxxxxxxxxxxxxxxxxxxx, Lang Yu <Lang.Yu@xxxxxxx>, Yannick Fertre <yannick.fertre@xxxxxxxxxxx>, linux-mips@xxxxxxxxxxxxxxx, Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>, Philippe Cornu <philippe.cornu@xxxxxxxxxxx>, Thomas Zimmermann <tzimmermann@xxxxxxx>, Wayne Lin <Wayne.Lin@xxxxxxx>, Drew Davenport <ddavenport@xxxxxxxxxxxx>, Nirmoy Das <nirmoy.das@xxxxxxxxx>, Jyri Sarha <jyri.sarha@xxxxxx>
- Delivery-date: Thu, 13 Jul 2023 09:57:01 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Thu, Jul 13, 2023 at 12:03:05PM +0300, Jani Nikula wrote:
> On Wed, 12 Jul 2023, Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote:
> > Hello Jani,
> >
> > On Wed, Jul 12, 2023 at 05:34:28PM +0300, Jani Nikula wrote:
> >> On Wed, 12 Jul 2023, Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> >> wrote:
> >> > 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.
> >>
> >> I think this is an unnecessary change. In drm, a dev is usually a drm
> >> device, i.e. struct drm_device *.
> >
> > Well, unless it's not. Prominently there is
> >
> > struct drm_device {
> > ...
> > struct device *dev;
> > ...
> > };
> >
> > which yields quite a few code locations using dev->dev which is
> > IMHO unnecessary irritating:
> >
> > $ git grep '\<dev->dev' v6.5-rc1 drivers/gpu/drm | wc -l
> > 1633
> >
> > Also the functions that deal with both a struct device and a struct
> > drm_device often use "dev" for the struct device and then "ddev" for
> > the drm_device (see for example amdgpu_device_get_pcie_replay_count()).
>
> Why is specifically struct drm_device *dev so irritating to you?
>
> You lead us to believe it's an outlier in kernel, something that goes
> against common kernel style, but it's really not:
>
> $ git grep -how "struct [A-Za-z0-9_]\+ \*dev" | sort | uniq -c | sort -rn |
> head -20
> 38494 struct device *dev
> 16388 struct net_device *dev
> 4184 struct drm_device *dev
> 2780 struct pci_dev *dev
> 1916 struct comedi_device *dev
> 1510 struct mlx5_core_dev *dev
> 1057 struct mlx4_dev *dev
> 894 struct b43_wldev *dev
> 762 struct input_dev *dev
> 623 struct usbnet *dev
> 561 struct mlx5_ib_dev *dev
> 525 struct mt76_dev *dev
> 465 struct mt76x02_dev *dev
> 435 struct platform_device *dev
> 431 struct usb_device *dev
> 411 struct mt7915_dev *dev
> 398 struct cx231xx *dev
> 378 struct mei_device *dev
> 363 struct ksz_device *dev
> 359 struct mthca_dev *dev
>
> A good portion of the above also have a dev member.
Yeah, other subsystems and drivers have the same problem. You're lucky
that I noticed drm first and invested some effort to improve it. IMHO
other subsystems being bad shouldn't stop drm to improve here.
And note that for example for pci_dev there are 5794 instances that are
named "pdev" and there are 9971 struct platform_device that are called
"pdev", too. So the majority for these does it better. And agreed,
net_device and others are also inconsistent. If you want an area that is
better, look at the naming of i2c_client or spi_device. (And take into
account that these are spread all over the tree and so are not in
control of a single maintainer team.)
> Are you planning on changing all of the above too, or are you only
> annoyed by drm?
Would you be more welcoming if I promised to tackle some of the above,
too? If so: I might. I hesitate a bit because I didn't suffer from the
others. (Apart from asking ctags for "dev" is a nightmare.)
And regarding the second part of your question: I was annoyed by drm
because that was the one that offended me while researching a problem in
a drm driver. And the variable/struct member name irritated me enough to
believe that with consistent naming I would have found it quicker.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
|