https://bugzilla.kernel.org/show_bug.cgi?id=201609 --- Comment #10 from Emil Velikov (emil.l.velikov@xxxxxxxxx) --- (In reply to James.Bottomley from comment #9) > On Wed, 2019-01-30 at 11:43 +0000, bugzilla-daemon@xxxxxxxxxxxxxxxxxxx > wrote: > > https://bugzilla.kernel.org/show_bug.cgi?id=201609 > > > > Emil Velikov (emil.l.velikov@xxxxxxxxx) changed: > > > > What |Removed |Added > > ------------------------------------------------------------------- > > --------- > > CC| |emil.l.velikov@gmail > > .com > > > > --- Comment #5 from Emil Velikov (emil.l.velikov@xxxxxxxxx) --- > > The other five files ({subsystem_,}{vendor,device} and class) do not > > have a prefix, so the more reasonable thing was it not add one for > > the revision file. > > No, it wasn't. This is the problem. In the early days we all grabbed > un-namespaced names in this directory. If you add another un- > namespaced file, a clash is likely to happen which has serious > consequences. If you want to add a generically named file, you have to > check all the drivers before doing it. If you clash with a file that's > in use by userspace, your new addition gets reverted because we can't > rename an in-use file because it's an exported ABI. > I see thank you. Can we get this and the (check every driver) documented please? Otherwise we'll end up in the same situation more often than needed. The usual regex in MAINTAINERS is unlikely to catch such patches, so it will be hard for experienced developers/maintainers to catch these easily :-\ Fwiw many platforms depend on the revision entry that I've introduced. From desktops (using the open-source graphics stack) to servers depending on libvirt. > > That said, I did spend a lot of time going through the Documentation > > looking for guidelines, rules or restrictions that should be applied > > and could not find any. The PCI subsystem maintainer did not suggest > > adding a prefix either, so I would not use "blatently silly" here. > > Either you prefix the addition or you check every driver ... neither > happened in this case. > Ack. Going through the kernel: $ git grep "\<DEVICE_ATTR\>.*\<revision\>" drivers/base/soc.c:static DEVICE_ATTR(revision, S_IRUGO, soc_info_get, NULL); drivers/scsi/aic94xx/aic94xx_init.c:static DEVICE_ATTR(revision, S_IRUGO, asd_show_dev_rev, NULL); drivers/video/fbdev/gbefb.c:static DEVICE_ATTR(revision, S_IRUGO, gbefb_show_rev, NULL); First is safe as is the last one (a platform_device). So only aic needs updating. > > On a more practical side - the aic94xx driver custom revision file > > solves the exact same problem my patch does. Admittedly, aic94xx > > lacks the leading "0x" for the hexadecimal number provided. > > > > My personal inclination is that we'd want to check if existing > > userspace > > requires the leading 0x and if so to what extend: > > - cosmetic - remove the aic94xx code, update userspace > > - unused - remove the aic94xx code > > - major impact - the aic94xx driver removes the generic revision > > file, before setting it's own > > > > Does that sound reasonable, am I missing something? > > I think we're safe and the aic file isn't used from userspace, so just > renaming the aic revision file should work. > Ack. Are you going to write a patch or shall I? Any name recommendations - aic94xx_revision, aic_revision, other? Thanks Emil -- You are receiving this mail because: You are watching the assignee of the bug.
