1.0 cleanup (#31)

* Only show configuration items under RMIBus
* Use VoodooSMBus repo for trackpoint
* Update README
* Fix inconsistent finger count for F12

.gitignore

@@ -3,3 +3,4 @@ VoodooInput/
 **/xcuserdata/
 **/*.kext
 build
+DerivedData

.gitmodules

@@ -1,6 +1,6 @@
 [submodule "Dependencies/VoodooTrackpoint"]
 	path = Dependencies/VoodooTrackpoint
-	url = https://github.com/1Revenger1/VoodooTrackpoint.git
+	url = https://github.com/VoodooSMBus/VoodooTrackpoint.git
 [submodule "Dependencies/VoodooSMBus"]
 	path = Dependencies/VoodooSMBus
 	url = https://github.com/VoodooSMBus/VoodooSMBus.git

README.md

@@ -15,16 +15,20 @@ This driver communicates over SMBus or I2C.
 
 ## How do I know if my device is compatible?
 **SMBus**  
+
 Windows:
 * Check under Device Manager for a Synaptics SMBus device
 
+![](/docs/images/Windows-SMBus-Device.png)
+
 Linux:
 * If you are using intertouch (i.e. psmouse.intertouch=1) for your synaptics trackpad, then it's compatible
 * Get `i2c-tools` from your package manager. Run `i2cdetect -l`, and note the number for SMBus (It's usually zero). Run `i2cdetect #` where # is the number you got from running the prior command. Synaptic devices are always at address 0x2c, so check at that address for anything other than `--`. It will usually appear as `UU` in my experiance if it's a Synaptics device.
-  * I've seen one or two examples where there was a trackpad that supported SMBus but nothing appeared at the address (Wack!)
+  * If the trackpad does not show up, there is a chance that it will still work. There have been one or two examples of the trackpad not showing up but still being compatible.
 * Likely compatible if you run `dmesg` and find a message along the lines of `"Your touchpad x says it can support a different bus."` and it's a synaptics trackpad.
 
 **I2C**
+
 Windows:
 * Check for `HID-compliant touch pad` in device manager
   * In properties, verify `location` is `on I2C HID Device` in `General` and `Hardware Ids` contains `SYNA` in `Details`
@@ -38,7 +42,9 @@ Linux:
 **SMBus**
 * [VoodooSMBus](https://github.com/VoodooSMBus/VoodooSMBus)
   * Apple's SMBus **PCI** controller cannot load, as it interfers with VoodooSMBus.
-* You likely want VoodooPS2 for keyboard as well. Make sure VoodooPS2Mouse/VoodooPS2Trackpad does not load.
+* VoodooPS2
+  * Needed for PS2 reset of the trackpad
+  * Generally users should only add VoodooPS2Controller and VoodooPS2Keyboard. Trackpad/Mouse will cause VoodooRMI to not attach.
   * OpenCore users can just disable Mouse/Trackpad in their config.plist.
   * Clover users - go inside the VoodooPS2 kext and remove Mouse/Trackpad from the PlugIns folder.
 
@@ -63,14 +69,15 @@ Linux:
 
 ## Installation
 1) Add the required kexts to your bootloader
-2) Disable VoodooPS2Mouse, VoodooPS2Trackpad, and if applicable, the VoodooInput from within the PS2 kext.
-3) For OpenCore users, make sure to add VoodooInput, VoodooTrackpoint and RMISMBus/RMII2C (add after VoodooRMI), as well to your Config.plist, it's under `VoodooRMI.kext/Contents/PlugIns/`
-Note: If you change any configuration values in the Info.plist, they must be integers/whole numbers
+2) Disable VoodooPS2Mouse, VoodooPS2Trackpad, and if applicable, VoodooInput from within the PS2 kext.
+3) For OpenCore users, make sure to add VoodooInput, VoodooTrackpoint and RMISMBus/RMII2C to your Config.plist.
+    * RMISMBus/RMII2C should be after VoodooRMI
+    * All dependencies are found under `VoodooRMI.kext/Contents/PlugIns/`
 
 ## Configuration
 
 The values below can be edited under Info.plist within the kext itself - these can be changed without recompiling  
-Note that using non-integer values causes undefined behaviour which may prevent the kext from loading
+**Note** that using non-integer values causes undefined behaviour which may prevent the kext from loading
 
 | Value | Default | Description |
 | ----- | ------- | ----------- |
@@ -84,6 +91,5 @@ Note that using non-integer values causes undefined behaviour which may prevent
 | `MinYDiffThumbDetection` | 200 | Minimum distance between the second lowest and lowest finger in which Minimum Y logic is used to detect the thumb rather than using the z value from the trackpad. Setting this higher means that the thumb must be farther from the other fingers before the y coordinate is used to detect the thumb, rather than using finger area. Keeping this smaller is preferable as finger area logic seems to only be useful when all 4 fingers are grouped together closely, where the thumb is more likely to be pressing down more |
 
 ## Building
-1) `git submodule init`
-2) `git submodule update`
-3) Build within XCode
+1) `git submodule update --init --recursive`
+2) Build within XCode

VoodooRMI/Functions/F12.cpp

@@ -314,7 +314,7 @@ void F12::getReport()
 {
     AbsoluteTime timestamp;
 
-    if (!sensor)
+    if (!sensor || !data1)
         return;
 
     int retval = rmiBus->readBlock(fn_descriptor->data_base_addr, sensor->data_pkt,
@@ -325,28 +325,19 @@ void F12::getReport()
         return;
     }
 
-    if (!data1) return;
-
-    int objects = data1->num_subpackets;
-    u8 *data = &sensor->data_pkt[data1_offset];
-
-    if ((data1->num_subpackets * F12_DATA1_BYTES_PER_OBJ) > sensor->pkt_size)
-        objects = sensor->pkt_size / F12_DATA1_BYTES_PER_OBJ;
-
     clock_get_uptime(&timestamp);
-
     if (sensor->shouldDiscardReport(timestamp))
         return;
 
     IOLogDebug("F12 Packet");
-
 #if DEBUG
     if (sensor->nbr_fingers > 5) {
         IOLogDebug("More than 5 fingers!");
     }
 #endif // debug
 
     int fingers = min (sensor->nbr_fingers, 5);
+    u8 *data = &sensor->data_pkt[data1_offset];
 
     for (int i = 0; i < fingers; i++) {
         rmi_2d_sensor_abs_object *obj = &report.objs[i];
@@ -433,10 +424,12 @@ int F12::rmi_read_register_desc(u16 addr,
     rdesc->num_registers = bitmap_weight(rdesc->presense_map,
                                          RMI_REG_DESC_PRESENSE_BITS);
 
-    rdesc->registers = reinterpret_cast<rmi_register_desc_item *>(IOMalloc(rdesc->num_registers * sizeof(struct rmi_register_desc_item)));
+    rdesc->registers = reinterpret_cast<rmi_register_desc_item *>(IOMalloc(rdesc->num_registers * sizeof(rmi_register_desc_item)));
     if (!rdesc->registers)
         return -ENOMEM;
 
+    memset (rdesc->registers, 0, rdesc->num_registers * sizeof(rmi_register_desc_item));
+
     /*
      * Allocate a temporary buffer to hold the register structure.
      * I'm not using devm_kzalloc here since it will not be retained

VoodooRMI/RMIBus.cpp

@@ -59,7 +59,7 @@ bool RMIBus::start(IOService *provider) {
         return false;
     int retval;
 
-    retval = rmi_init_functions(data);
+    retval = rmi_init_functions(this, data);
     if (retval)
         goto err;
 

VoodooRMI/RMI_2D_Sensor.cpp

@@ -23,7 +23,7 @@ bool RMI2DSensor::init(OSDictionary *dictionary)
     forceTouchEmulation = Configuration::loadBoolConfiguration(dictionary, "ForceTouchEmulation", true);
     minYDiffGesture = Configuration::loadUInt32Configuration(dictionary, "MinYDiffThumbDetection", 200);
 
-    return super::init(dictionary);
+    return super::init();
 }
 
 bool RMI2DSensor::start(IOService *provider)

VoodooRMI/Transports/I2C/Info.plist

@@ -13,7 +13,7 @@
 	<key>CFBundleName</key>
 	<string>$(PRODUCT_NAME)</string>
 	<key>CFBundlePackageType</key>
-	<string>$(PRODUCT_BUNDLE_PACKAGE_TYPE)</string>
+	<string>KEXT</string>
 	<key>CFBundleShortVersionString</key>
 	<string>1.0</string>
 	<key>CFBundleVersion</key>

VoodooRMI/Transports/I2C/RMII2C.cpp

@@ -22,7 +22,7 @@ RMII2C *RMII2C::probe(IOService *provider, SInt32 *score) {
     name = (const char *)(data->getBytesNoCopy());
 
     IOLog("%s::%s probing\n", getName(), name);
-
+    
     OSBoolean *isLegacy= OSDynamicCast(OSBoolean, getProperty("Legacy"));
     if (isLegacy == nullptr) {
         IOLog("%s::%s Legacy mode not set, default to false", getName(), name);
@@ -44,7 +44,9 @@ RMII2C *RMII2C::probe(IOService *provider, SInt32 *score) {
     }
 
     do {
+#if DEBUG
         IOLog("%s::%s Trying to set mode, attempt %d\n", getName(), name, attempts);
+#endif //DEBUG
         error = rmi_set_mode(reportMode);
         IOSleep(500);
     } while (error < 0 && attempts++ < 5);
@@ -257,7 +259,6 @@ int RMII2C::readBlock(u16 rmiaddr, u8 *databuff, size_t len) {
 
     memcpy(databuff, i2cInput+4, len);
 exit:
-//    IOLog("read %zd bytes at %#06x: %d (%*ph)\n", len, rmiaddr, ret, (int)len, databuff);
     delete[] i2cInput;
     IOLockUnlock(page_mutex);
     return retval;
@@ -292,7 +293,6 @@ int RMII2C::blockWrite(u16 rmiaddr, u8 *buf, size_t len) {
     retval = 0;
 
 exit:
-//    IOLog("write %zd bytes at %#06x: %d (%*ph)\n", len, rmiaddr, ret, (int)len, buf);
     delete [] writeReport;
     IOLockUnlock(page_mutex);
     return retval;

VoodooRMI/Transports/RMITransport.hpp

@@ -13,7 +13,6 @@
 
 #include <IOKit/IOLib.h>
 #include <IOKit/IOService.h>
-//#include <libkern/OSMalloc.h>
 #include "../Utility/LinuxCompat.h"
 
 #define kIOMessageVoodooSMBusHostNotify iokit_vendor_specific_msg(420)

VoodooRMI/rmi.h

@@ -12,8 +12,6 @@
 
 #include <IOKit/IOService.h>
 
-class RMIBus;
-
 // macOS kernel/math has absolute value in it. It's only for doubles though
 #define abs(x) ((x < 0) ? (-x) : (x))
 
@@ -93,7 +91,6 @@ struct rmi_function_descriptor {
 struct rmi_function {
     int size;
     struct rmi_function_descriptor fd;
-    RMIBus *dev;
 
     unsigned int num_of_irqs;
     int irq[RMI_FN_MAX_IRQS];
@@ -252,8 +249,6 @@ struct rmi_2d_sensor_platform_data {
 };
 
 struct rmi_driver_data {
-    RMIBus *rmi_dev;
-
     rmi_function *f01_container;
     rmi_function *f34_container;
     bool bootloader_mode;
@@ -275,20 +270,6 @@ struct rmi_driver_data {
 
     bool enabled;
     IOLock *enabled_mutex;
-
-    rmi4_attn_data attn_data;
-
-    struct {
-        union {
-            struct __kfifo          kfifo;
-            struct rmi4_attn_data   *type;
-            const  rmi4_attn_data   *const_type;
-            char                    (*rectype)[0];
-            struct rmi4_attn_data   *ptr;
-            struct rmi4_attn_data const *ptr_const;
-        };
-        rmi4_attn_data buf[16];
-    } attn_fifo;
 };
 
 #endif /* rmi_h */

VoodooRMI/rmi_driver.cpp

@@ -27,11 +27,7 @@
 int rmi_driver_probe(RMIBus *dev)
 {
     int retval;
-
-//    pdata = rmi_get_platform_data(rmi_dev);
-
-    dev->data->rmi_dev = dev;
-
+
     /*
      * Right before a warm boot, the sensor might be in some unusual state,
      * such as F54 diagnostics, or F34 bootloader mode after a firmware
@@ -68,26 +64,16 @@ int rmi_driver_probe(RMIBus *dev)
                  PDT_PROPERTIES_LOCATION, retval);
     }
 
-    retval = rmi_probe_interrupts(dev->data);
+    retval = rmi_probe_interrupts(dev, dev->data);
     if (retval)
         goto err;
 
-    // allocate device
-
-
     return 0;
-//
-//err_disable_irq:
-//    rmi_disable_irq(dev, false);
 err:
     IOLogError("Could not probe");
     return retval;
 }
 
-#define RMI_SCAN_CONTINUE    0
-#define RMI_SCAN_DONE        1
-
-
 static int rmi_read_pdt_entry(RMIBus *rmi_dev,
                               struct pdt_entry *entry, u16 pdt_address)
 {
@@ -152,7 +138,7 @@ static int rmi_scan_pdt_page(RMIBus *dev,
         *empty_pages = 0;
 
     return (data->bootloader_mode || *empty_pages >= 2) ?
-    RMI_SCAN_DONE : RMI_SCAN_CONTINUE;
+        RMI_SCAN_DONE : RMI_SCAN_CONTINUE;
 }
 
 int rmi_scan_pdt(RMIBus *dev, void *ctx,
@@ -178,28 +164,12 @@ int rmi_initial_reset(RMIBus *dev, void *ctx, const struct pdt_entry *pdt)
     int error;
 
     if (pdt->function_number == 0x01) {
-        u16 cmd_addr = pdt->page_start + pdt->command_base_addr;
-        u8 cmd_buf = RMI_DEVICE_RESET_CMD;
-
         error = dev->reset();
         if (error < 0) {
             IOLogError("Unable to reset");
             return error;
         }
 
-        if (error > 0)
-            return RMI_SCAN_DONE;
-
-        // Only send reset if there is no reset in transport (SMBus has one which just gets version)
-        IOLogDebug("Sending Reset\n");
-        error = dev->blockWrite(cmd_addr, &cmd_buf, 1);
-        if (error) {
-            IOLogError("Initial reset failed. Code = %d\n", error);
-            return error;
-        }
-
-        IOSleep(DEFAULT_RESET_DELAY_MS);
-
         return RMI_SCAN_DONE;
     }
 
@@ -252,9 +222,8 @@ static int rmi_count_irqs(RMIBus *rmi_dev,
     return RMI_SCAN_CONTINUE;
 }
 
-int rmi_probe_interrupts(rmi_driver_data *data)
+int rmi_probe_interrupts(RMIBus *rmi_dev, rmi_driver_data *data)
 {
-    RMIBus *rmi_dev = data->rmi_dev;
     int irq_count = 0;
     int retval;
 
@@ -326,8 +295,6 @@ static int rmi_create_function(RMIBus *rmi_dev,
 
     rmi_driver_copy_pdt_to_fd(pdt, &fn->fd);
 
-    fn->dev = rmi_dev;
-
     fn->num_of_irqs = pdt->interrupt_source_count;
     fn->irq_pos = *current_irq_count;
     *current_irq_count += fn->num_of_irqs;
@@ -339,13 +306,9 @@ static int rmi_create_function(RMIBus *rmi_dev,
     if (error)
         return error;
 
-    // TODO: Why are these stored?
+    // Keep F01 around for reading/writing IRQ
     if (pdt->function_number == 0x01)
         data->f01_container = fn;
-    else if (pdt->function_number == 0x34)
-        data->f34_container = fn;
-    // We don't need the function data anymore,
-    // just the descriptors stored in the Functions
     else IOFree(fn, size);
 
     return RMI_SCAN_CONTINUE;
@@ -453,18 +416,15 @@ int rmi_enable_sensor(RMIBus *rmi_dev)
     if (retval < 0)
         return retval;
 
-//    return rmi_process_interrupt_requests(rmi_dev);
-
     retval = rmi_driver_set_irq_bits(rmi_dev);
     if (retval < 0)
         return retval;
 
     return 0;
 }
 
-int rmi_init_functions(rmi_driver_data *data)
+int rmi_init_functions(RMIBus *rmi_dev, rmi_driver_data *data)
 {
-    RMIBus *rmi_dev = data->rmi_dev;
     int irq_count = 0;
     int retval;
 
@@ -500,9 +460,6 @@ void rmi_free_function_list(RMIBus *rmi_dev)
 
     if (data->f01_container)
         IOFree(data->f01_container, sizeof(rmi_function));
-    if (data->f34_container)
-        IOFree(data->f34_container, sizeof(rmi_function));
     data->f01_container = NULL;
-    data->f34_container = NULL;
 }