Re: [PATCH] staging: vchiq: convert to use a miscdevice

[Ojaswin Mujoo <ojaswin98@gmail.com>]

On Fri, Sep 10, 2021 at 02:05:19PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Sep 10, 2021 at 05:10:04PM +0530, Ojaswin Mujoo wrote:
> > On Tue, Sep 07, 2021 at 01:50:45PM +0200, Greg Kroah-Hartman wrote:
> > > Using a struct class, a cdev, and another device just for a single minor
> > > device is total overkill.  Just use a dynamic misc device instead,
> > > saving lots of logic and memory.
> > 
> > Hello Greg,
> > 
> > I got some time to test this out at my end. This seems to work correctly
> > however there's a small change in permissions applied to /dev/vchiq that
> > is causing tests to break.
> > 
> > * Permissions before the patch *
> > $ ls -l /dev/vchiq 
> > crw-rw---- 1 root video 235, 0 May  7 17:33 vchiq
> > 
> > * Permissions after the patch *
> > $ ls -l /dev/vchiq 
> > crw------- 1 root root 10, 125 May  7 17:30 vchiq
> > 
> > As seen above, after the patch, the cdev is only accessible by root user,
> > which is causing the tests ($ vchiq_test -f 10) to fail when run as
> > non-root.
> 
> Ah, that's not under the kernel's control, but as you point out, it's a
> udev issue.
> 
> > I believe assigning the permission and "video" group to /dev/vchiq is
> > handled by udev, in the downstream pi OS, as seen in this line in
> > /lib/udev/rules.d/10-local-rpi.rules file:
> > 
> > SUBSYSTEM=="vchiq", GROUP="video", MODE="0660" 
> > 
> > I'm not completely sure how the SUBSYSTEM part is passed to udev from
> > the kernel modules, however seems like the miscdevice is not notifying
> > udev correctly (?).
> 
> No, the SUBSYSTEM for this device has changed from "vchiq" to "misc".
> 
> Having a whole subsystem for just one character device is crazy, which
> is why I did the kernel change.
> 
> Try changing the line in the udev file to:
> 
> 	NAME=="vchiq", GROUP="video", MODE="0660"
> 
> (SUBSYSTEM changes to NAME) and see if that works both on the newer
> kernel, and on the older ones as
> well.
Hello, thanks for the explanation and pointers.

The "NAME==vchiq" change doesn't seem to work but I was able to get it
correctly working by using "KERNEL=vchiq" instead:

   KERNEL=="vchiq", GROUP="video", MODE="0660"

I tested this with and without this patch and it works as expected. 

Regards,
Ojaswin
> 
> thanks,
> 
> greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel