aboutsummaryrefslogtreecommitdiffstatshomepage
path: root/drivers/staging/vc04_services/interface/TODO
diff options
context:
space:
mode:
Diffstat (limited to 'drivers/staging/vc04_services/interface/TODO')
-rw-r--r--drivers/staging/vc04_services/interface/TODO93
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.