11) Import drivers using VCHI.
2
3VCHI is just a tool to let drivers talk to the firmware.  Here are
4some of the ones we want:
5
6  - vc_mem (https://github.com/raspberrypi/linux/blob/rpi-4.4.y/drivers/char/broadcom/vc_mem.c)
7
8  This driver is what the vcdbg userspace program uses to set up its
9  requests to the firmware, which are transmitted across VCHIQ.  vcdbg
10  is really useful for debugging firmware interactions.
11
12  - VCSM (https://github.com/raspberrypi/linux/tree/rpi-4.4.y/drivers/char/broadcom/vc_sm)
13
14  This driver is used for talking about regions of VC memory across
15  firmware protocols including VCHI.  We'll want to extend this driver
16  to manage these buffers as dmabufs so that we can zero-copy import
17  camera images into vc4 for rendering/display.
18
192) Garbage-collect unused code
20
21One of the reasons this driver wasn't upstreamed previously was that
22there's a lot code that got built that's probably unnecessary these
23days.  Once we have the set of VCHI-using drivers we want in tree, we
24should be able to do a sweep of the code to see what's left that's
25unused.
26
273) Make driver more portable
28
29Building this driver with arm/multi_v7_defconfig or arm64/defconfig
30leads to data corruption during the following command:
31
32  vchiq_test -f 1
33
34This should be fixed.
35
364) Fix kernel module support
37
38Even the VPU firmware doesn't support a VCHI re-connect, the driver
39should properly handle a module unload. This also includes that all
40resources must be freed (kthreads, debugfs entries, ...) and global
41variables avoided.
42
435) Cleanup logging mechanism
44
45The driver should probably be using the standard kernel logging mechanisms
46such as dev_info, dev_dbg, and friends.
47
486) Documentation
49
50A short top-down description of this driver's architecture (function of
51kthreads, userspace, limitations) could be very helpful for reviewers.
52
537) Review and comment memory barriers
54
55There is a heavy use of memory barriers in this driver, it would be very
56beneficial to go over all of them and, if correct, comment on their merits.
57Extra points to whomever confidently reviews the remote_event_*() family of
58functions.
59
608) Get rid of custom function return values
61
62Most functions use a custom set of return values, we should force proper Linux
63error numbers. Special care is needed for VCHIQ_RETRY.
64
659) Reformat core code with more sane indentations
66
67The code follows the 80 characters limitation yet tends to go 3 or 4 levels of
68indentation deep making it very unpleasant to read. This is specially relevant
69in the character driver ioctl code and in the core thread functions.
70
7110) Reorganize file structure: Move char driver to it's own file and join both
72platform files
73
74The cdev is defined alongside with the platform code in vchiq_arm.c. It would
75be nice to completely decouple it from the actual core code. For instance to be
76able to use bcm2835-audio without having /dev/vchiq created. One could argue
77it's better for security reasons or general cleanliness. It could even be
78interesting to create two different kernel modules, something the likes of
79vchiq-core.ko and vchiq-dev.ko. This would also ease the upstreaming process.
80
81The code in vchiq_bcm2835_arm.c should fit in the generic platform file.
82
8312) Get rid of all the struct typedefs
84
85Most structs are typedefd, it's not encouraged in the kernel.
86
8713) Get rid of all non essential global structures and create a proper per
88device structure
89
90The first thing one generally sees in a probe function is a memory allocation
91for all the device specific data. This structure is then passed all over the
92driver. This is good practice since it makes the driver work regardless of the
93number of devices probed.
94
9514) Clean up Sparse warnings from __user annotations. See
96vchiq_irq_queue_bulk_tx_rx(). Ensure that the address of "&waiter->bulk_waiter"
97is never disclosed to userspace.
98