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. 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/ | >
