diff options
Diffstat (limited to 'drivers/staging/vc04_services/interface/TODO')
-rw-r--r-- | drivers/staging/vc04_services/interface/TODO | 93 |
1 files changed, 93 insertions, 0 deletions
diff --git a/drivers/staging/vc04_services/interface/TODO b/drivers/staging/vc04_services/interface/TODO new file mode 100644 index 000000000000..fc2752bc95b2 --- /dev/null +++ b/drivers/staging/vc04_services/interface/TODO @@ -0,0 +1,93 @@ +1) Import drivers using VCHI. + +VCHI is just a tool to let drivers talk to the firmware. Here are +some of the ones we want: + + - vc_mem (https://github.com/raspberrypi/linux/blob/rpi-4.4.y/drivers/char/broadcom/vc_mem.c) + + This driver is what the vcdbg userspace program uses to set up its + requests to the firmware, which are transmitted across VCHIQ. vcdbg + is really useful for debugging firmware interactions. + + - VCSM (https://github.com/raspberrypi/linux/tree/rpi-4.4.y/drivers/char/broadcom/vc_sm) + + This driver is used for talking about regions of VC memory across + firmware protocols including VCHI. We'll want to extend this driver + to manage these buffers as dmabufs so that we can zero-copy import + camera images into vc4 for rendering/display. + +2) Garbage-collect unused code + +One of the reasons this driver wasn't upstreamed previously was that +there's a lot code that got built that's probably unnecessary these +days. Once we have the set of VCHI-using drivers we want in tree, we +should be able to do a sweep of the code to see what's left that's +unused. + +3) Make driver more portable + +Building this driver with arm/multi_v7_defconfig or arm64/defconfig +leads to data corruption during the following command: + + vchiq_test -f 1 + +This should be fixed. + +4) Fix kernel module support + +Even the VPU firmware doesn't support a VCHI re-connect, the driver +should properly handle a module unload. This also includes that all +resouces must be freed (kthreads, debugfs entries, ...) and global +variables avoided. + +5) Cleanup logging mechanism + +The driver should probably be using the standard kernel logging mechanisms +such as dev_info, dev_dbg, and friends. + +6) Documentation + +A short top-down description of this driver's architecture (function of +kthreads, userspace, limitations) could be very helpful for reviewers. + +7) Review and comment memory barriers + +There is a heavy use of memory barriers in this driver, it would be very +beneficial to go over all of them and, if correct, comment on their merits. +Extra points to whomever confidently reviews the remote_event_*() family of +functions. + +8) Get rid of custom function return values + +Most functions use a custom set of return values, we should force proper Linux +error numbers. Special care is needed for VCHIQ_RETRY. + +9) Reformat core code with more sane indentations + +The code follows the 80 characters limitation yet tends to go 3 or 4 levels of +indentation deep making it very unpleasant to read. This is specially relevant +in the character driver ioctl code and in the core thread functions. + +10) Reorganize file structure: Move char driver to it's own file and join both +platform files + +The cdev is defined alongside with the platform code in vchiq_arm.c. It would +be nice to completely decouple it from the actual core code. For instance to be +able to use bcm2835-audio without having /dev/vchiq created. One could argue +it's better for security reasons or general cleanliness. It could even be +interesting to create two different kernel modules, something the likes of +vchiq-core.ko and vchiq-dev.ko. This would also ease the upstreaming process. + +The code in vchiq_bcm2835_arm.c should fit in the generic platform file. + +12) Get rid of all the struct typedefs + +Most structs are typedefd, it's not encouraged in the kernel. + +13) Get rid of all non essential global structures and create a proper per +device structure + +The first thing one generally sees in a probe function is a memory allocation +for all the device specific data. This structure is then passed all over the +driver. This is good practice since it makes the driver work regardless of the +number of devices probed. |