From 7fa31d2b05458e636d1412215558a46a534814f7 Mon Sep 17 00:00:00 2001 From: ben9923 Date: Tue, 13 Aug 2019 05:23:05 +0300 Subject: [PATCH 01/10] Clear interrupt status in mask/unmask callback Can resolve unexpected behaviors, notably on ELAN1200 I2C trackpad. Ported from torvalds/linux@670784f Also ports torvalds/linux@e58926e --- VoodooGPIO/VoodooGPIO.cpp | 48 ++++++++++++--------------------------- VoodooGPIO/VoodooGPIO.hpp | 3 --- 2 files changed, 14 insertions(+), 37 deletions(-) diff --git a/VoodooGPIO/VoodooGPIO.cpp b/VoodooGPIO/VoodooGPIO.cpp index d29fc75..9d02f4c 100644 --- a/VoodooGPIO/VoodooGPIO.cpp +++ b/VoodooGPIO/VoodooGPIO.cpp @@ -34,13 +34,13 @@ OSDefineMetaClassAndStructors(VoodooGPIO, IOService); #define PADOWN_BITS 4 #define PADOWN_SHIFT(p) ((p) % 8 * PADOWN_BITS) -#define PADOWN_MASK(p) (0xf << PADOWN_SHIFT(p)) +#define PADOWN_MASK(p) (GENMASK(3, 0) << PADOWN_SHIFT(p)) #define PADOWN_GPP(p) ((p) / 8) /* Offset from pad_regs */ #define PADCFG0 0x000 #define PADCFG0_RXEVCFG_SHIFT 25 -#define PADCFG0_RXEVCFG_MASK (3 << PADCFG0_RXEVCFG_SHIFT) +#define PADCFG0_RXEVCFG_MASK GENMASK(26, 25) #define PADCFG0_RXEVCFG_LEVEL 0 #define PADCFG0_RXEVCFG_EDGE 1 #define PADCFG0_RXEVCFG_DISABLED 2 @@ -52,7 +52,7 @@ OSDefineMetaClassAndStructors(VoodooGPIO, IOService); #define PADCFG0_GPIROUTSMI BIT(18) #define PADCFG0_GPIROUTNMI BIT(17) #define PADCFG0_PMODE_SHIFT 10 -#define PADCFG0_PMODE_MASK (0xf << PADCFG0_PMODE_SHIFT) +#define PADCFG0_PMODE_MASK GENMASK(13, 10) #define PADCFG0_GPIORXDIS BIT(9) #define PADCFG0_GPIOTXDIS BIT(8) #define PADCFG0_GPIORXSTATE BIT(1) @@ -61,7 +61,7 @@ OSDefineMetaClassAndStructors(VoodooGPIO, IOService); #define PADCFG1 0x004 #define PADCFG1_TERM_UP BIT(13) #define PADCFG1_TERM_SHIFT 10 -#define PADCFG1_TERM_MASK (7 << PADCFG1_TERM_SHIFT) +#define PADCFG1_TERM_MASK GENMASK(12, 10) #define PADCFG1_TERM_20K 4 #define PADCFG1_TERM_2K 3 #define PADCFG1_TERM_5K 2 @@ -269,30 +269,6 @@ SInt32 VoodooGPIO::intel_gpio_to_pin(UInt32 offset, return -1; } -/** - * @param pin Hardware GPIO pin number to enable. - */ -void VoodooGPIO::intel_gpio_irq_enable(UInt32 pin) { - const struct intel_community *community = intel_get_community(pin); - if (community) { - const struct intel_padgroup *padgrp = intel_community_get_padgroup(community, pin); - if (!padgrp) - return; - - UInt32 gpp, gpp_offset; - UInt32 value; - - gpp = padgrp->reg_num; - gpp_offset = padgroup_offset(padgrp, pin); - /* Clear interrupt status first to avoid unexpected interrupt */ - writel(BIT(gpp_offset), community->regs + GPI_IS + gpp * 4); - - value = readl(community->regs + community->ie_offset + gpp * 4); - value |= BIT(gpp_offset); - writel(value, community->regs + community->ie_offset + gpp * 4); - } -} - /** * @param pin Hardware GPIO pin number to mask. * @param mask Whether to mask or unmask. @@ -305,14 +281,18 @@ void VoodooGPIO::intel_gpio_irq_mask_unmask(unsigned pin, bool mask) { return; unsigned gpp, gpp_offset; - IOVirtualAddress reg; + IOVirtualAddress reg, is; UInt32 value; gpp = padgrp->reg_num; gpp_offset = padgroup_offset(padgrp, pin); - + reg = community->regs + community->ie_offset + gpp * 4; - + is = community->regs + GPI_IS + gpp * 4; + + /* Clear interrupt status first to avoid unexpected interrupt */ + writel(static_cast(BIT(gpp_offset)), is); + value = readl(reg); if (mask) value &= ~BIT(gpp_offset); @@ -752,7 +732,7 @@ IOReturn VoodooGPIO::setPowerState(unsigned long powerState, IOService *whatDevi if (pinInterruptOwner) { int pin = community->pin_base + j; intel_gpio_irq_set_type(pin, community->interruptTypes[j]); - intel_gpio_irq_enable(pin); + intel_gpio_irq_mask_unmask(pin, false); } } } @@ -795,7 +775,7 @@ void VoodooGPIO::intel_gpio_community_irq_handler(struct intel_community *commun } if (community->interruptTypes[pin] & IRQ_TYPE_LEVEL_MASK) - intel_gpio_irq_enable(community->pin_base + pin); // For Level interrupts, we need to clear the interrupt status or we get too many interrupts + intel_gpio_irq_mask_unmask(community->pin_base + pin, false); // For Level interrupts, we need to clear the interrupt status or we get too many interrupts } } } @@ -868,7 +848,7 @@ IOReturn VoodooGPIO::enableInterrupt(int pin) { unsigned communityidx = hw_pin - community->pin_base; if (community->pinInterruptActionOwners[communityidx]) { intel_gpio_irq_set_type(hw_pin, community->interruptTypes[communityidx]); - intel_gpio_irq_enable(hw_pin); + intel_gpio_irq_mask_unmask(hw_pin, false); return kIOReturnSuccess; } return kIOReturnNoInterrupt; diff --git a/VoodooGPIO/VoodooGPIO.hpp b/VoodooGPIO/VoodooGPIO.hpp index 265e0ce..fc6b8d1 100644 --- a/VoodooGPIO/VoodooGPIO.hpp +++ b/VoodooGPIO/VoodooGPIO.hpp @@ -216,7 +216,6 @@ class VoodooGPIO : public IOService { SInt32 intel_gpio_to_pin(UInt32 offset, const struct intel_community **community, const struct intel_padgroup **padgrp); - void intel_gpio_irq_enable(UInt32 pin); void intel_gpio_irq_mask_unmask(unsigned pin, bool mask); bool intel_gpio_irq_set_type(unsigned pin, unsigned type); @@ -234,8 +233,6 @@ class VoodooGPIO : public IOService { void InterruptOccurred(OSObject *owner, IOInterruptEventSource *src, int intCount); void interruptOccurredGated(); - void TouchpadInterruptOccurred(OSObject *owner, IOInterruptEventSource *src, int intCount); - public: IOReturn getInterruptType(int pin, int *interruptType) override; IOReturn registerInterrupt(int pin, OSObject *target, IOInterruptAction handler, void *refcon) override; From 3f42a0b99543f04673d7b8b38fe64a0aef2fb30a Mon Sep 17 00:00:00 2001 From: Erictoby <54815823+Erictoby@users.noreply.github.com> Date: Mon, 2 Sep 2019 15:49:39 -0400 Subject: [PATCH 02/10] Fixes cursor movement lagging or jumping since 2.1.4+ --- VoodooGPIO/VoodooGPIO.cpp | 20 -------------------- VoodooGPIO/VoodooGPIO.hpp | 2 -- 2 files changed, 22 deletions(-) diff --git a/VoodooGPIO/VoodooGPIO.cpp b/VoodooGPIO/VoodooGPIO.cpp index d29fc75..e919fb0 100644 --- a/VoodooGPIO/VoodooGPIO.cpp +++ b/VoodooGPIO/VoodooGPIO.cpp @@ -85,26 +85,6 @@ void VoodooGPIO::writel(UInt32 b, IOVirtualAddress addr) { *(volatile UInt32 *)(addr) = b; } -IOWorkLoop* VoodooGPIO::getWorkLoop() { - // Do we have a work loop already?, if so return it NOW. - if ((vm_address_t) workLoop >> 1) - return workLoop; - - if (OSCompareAndSwap(0, 1, reinterpret_cast(&workLoop))) { - // Construct the workloop and set the cntrlSync variable - // to whatever the result is and return - workLoop = IOWorkLoop::workLoop(); - } else { - while (reinterpret_cast(workLoop) == reinterpret_cast(1)) { - // Spin around the cntrlSync variable until the - // initialization finishes. - thread_block(0); - } - } - - return workLoop; -} - struct intel_community *VoodooGPIO::intel_get_community(unsigned pin) { struct intel_community *community; for (int i = 0; i < ncommunities; i++) { diff --git a/VoodooGPIO/VoodooGPIO.hpp b/VoodooGPIO/VoodooGPIO.hpp index 265e0ce..3f3518b 100644 --- a/VoodooGPIO/VoodooGPIO.hpp +++ b/VoodooGPIO/VoodooGPIO.hpp @@ -203,8 +203,6 @@ class VoodooGPIO : public IOService { UInt32 readl(IOVirtualAddress addr); void writel(UInt32 b, IOVirtualAddress addr); - IOWorkLoop* getWorkLoop(); - struct intel_community *intel_get_community(unsigned pin); const struct intel_padgroup *intel_community_get_padgroup(const struct intel_community *community, unsigned pin); IOVirtualAddress intel_get_padcfg(unsigned pin, unsigned reg); From cf8387fbf82982643348847f594a471747b6188d Mon Sep 17 00:00:00 2001 From: erictoby <54815823+Erictoby@users.noreply.github.com> Date: Sun, 8 Sep 2019 12:28:14 -0400 Subject: [PATCH 03/10] Fixes deadlock or delays in interrupt loop. runAction is not allowed in interrupt loop. --- VoodooGPIO/VoodooGPIO.cpp | 20 ++++++++++++++++---- VoodooGPIO/VoodooGPIO.hpp | 5 +++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/VoodooGPIO/VoodooGPIO.cpp b/VoodooGPIO/VoodooGPIO.cpp index e919fb0..11ed167 100644 --- a/VoodooGPIO/VoodooGPIO.cpp +++ b/VoodooGPIO/VoodooGPIO.cpp @@ -265,7 +265,7 @@ void VoodooGPIO::intel_gpio_irq_enable(UInt32 pin) { gpp = padgrp->reg_num; gpp_offset = padgroup_offset(padgrp, pin); /* Clear interrupt status first to avoid unexpected interrupt */ - writel(BIT(gpp_offset), community->regs + GPI_IS + gpp * 4); + writel((UInt32)BIT(gpp_offset), community->regs + GPI_IS + gpp * 4); value = readl(community->regs + community->ie_offset + gpp * 4); value |= BIT(gpp_offset); @@ -365,7 +365,7 @@ bool VoodooGPIO::intel_pinctrl_add_padgroups(intel_community *community) { unsigned gpp_size = community->gpp_size; gpps[i].reg_num = i; gpps[i].base = community->pin_base + i * gpp_size; - gpps[i].size = min(gpp_size, npins); + gpps[i].size = (UInt32)min(gpp_size, npins); gpps[i].gpio_base = 0; npins -= gpps[i].size; } @@ -544,6 +544,8 @@ bool VoodooGPIO::start(IOService *provider) { if (!IOService::start(provider)) return false; + isInterruptBusy = false; + PMinit(); workLoop = getWorkLoop(); @@ -696,7 +698,7 @@ void VoodooGPIO::stop(IOService *provider) { } if (workLoop) { - workLoop->release(); + // workLoop->release(); workLoop = NULL; } @@ -882,11 +884,21 @@ IOReturn VoodooGPIO::setInterruptTypeForPin(int pin, int type) { } void VoodooGPIO::InterruptOccurred(OSObject *owner, IOInterruptEventSource *src, int intCount) { - command_gate->runAction(OSMemberFunctionCast(IOCommandGate::Action, this, &VoodooGPIO::interruptOccurredGated)); + if (isInterruptBusy) + return; + isInterruptBusy = true; + + IOReturn ret = command_gate->attemptAction(OSMemberFunctionCast(IOCommandGate::Action, this, &VoodooGPIO::interruptOccurredGated)); + if (ret != kIOReturnSuccess) { + isInterruptBusy = false; + IOLog("%s:InterruptOccurred:Failed on attemptAction(). Error code = %X", getName(), ret); + } } + void VoodooGPIO::interruptOccurredGated() { for (int i = 0; i < ncommunities; i++) { struct intel_community *community = &communities[i]; intel_gpio_community_irq_handler(community); } + isInterruptBusy = false; } diff --git a/VoodooGPIO/VoodooGPIO.hpp b/VoodooGPIO/VoodooGPIO.hpp index 3f3518b..389bc40 100644 --- a/VoodooGPIO/VoodooGPIO.hpp +++ b/VoodooGPIO/VoodooGPIO.hpp @@ -197,8 +197,9 @@ class VoodooGPIO : public IOService { bool controllerIsAwake; IOWorkLoop *workLoop; - IOInterruptEventSource *interruptSource; - IOCommandGate* command_gate; + IOInterruptEventSource *interruptSource = NULL; + IOCommandGate* command_gate = NULL; + bool isInterruptBusy; UInt32 readl(IOVirtualAddress addr); void writel(UInt32 b, IOVirtualAddress addr); From eb42516854b9cb5fbe7eb2ad071cbdf1da316425 Mon Sep 17 00:00:00 2001 From: erictoby <54815823+Erictoby@users.noreply.github.com> Date: Sat, 14 Sep 2019 18:29:59 -0400 Subject: [PATCH 04/10] Enable GPIO interrupt after initing all parameters The bug can cause panic or unresponsive touchpad. --- VoodooGPIO/VoodooGPIO.cpp | 44 +++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/VoodooGPIO/VoodooGPIO.cpp b/VoodooGPIO/VoodooGPIO.cpp index 11ed167..8c935f4 100644 --- a/VoodooGPIO/VoodooGPIO.cpp +++ b/VoodooGPIO/VoodooGPIO.cpp @@ -544,7 +544,7 @@ bool VoodooGPIO::start(IOService *provider) { if (!IOService::start(provider)) return false; - isInterruptBusy = false; + isInterruptBusy = true; PMinit(); @@ -556,22 +556,21 @@ bool VoodooGPIO::start(IOService *provider) { } workLoop->retain(); + command_gate = IOCommandGate::commandGate(this); + if (!command_gate || (workLoop->addEventSource(command_gate) != kIOReturnSuccess)) { + IOLog("%s Could not open command gate\n", getName()); + stop(provider); + return false; + } + interruptSource = IOInterruptEventSource::interruptEventSource(this, OSMemberFunctionCast(IOInterruptEventAction, this, &VoodooGPIO::InterruptOccurred), provider); if (!interruptSource) { IOLog("%s::Failed to get GPIO Controller interrupt!\n", getName()); stop(provider); return false; } - workLoop->addEventSource(interruptSource); interruptSource->enable(); - - command_gate = IOCommandGate::commandGate(this); - if (!command_gate || (workLoop->addEventSource(command_gate) != kIOReturnSuccess)) { - IOLog("%s Could not open command gate\n", getName()); - stop(provider); - return false; - } IOLog("%s::VoodooGPIO Init!\n", getName()); @@ -634,6 +633,7 @@ bool VoodooGPIO::start(IOService *provider) { intel_pinctrl_pm_init(); + isInterruptBusy = false; controllerIsAwake = true; registerService(); @@ -666,6 +666,17 @@ bool VoodooGPIO::start(IOService *provider) { void VoodooGPIO::stop(IOService *provider) { IOLog("%s::VoodooGPIO stop!\n", getName()); + if (interruptSource) { + interruptSource->disable(); + workLoop->removeEventSource(interruptSource); + OSSafeReleaseNULL(interruptSource); + } + + if (command_gate) { + workLoop->removeEventSource(command_gate); + OSSafeReleaseNULL(command_gate); + } + intel_pinctrl_pm_release(); for (int i = 0; i < ncommunities; i++) { @@ -686,17 +697,6 @@ void VoodooGPIO::stop(IOService *provider) { IOFree(communities[i].pinInterruptRefcons, sizeof(void *) * communities[i].npins); } - if (interruptSource) { - interruptSource->disable(); - workLoop->removeEventSource(interruptSource); - OSSafeReleaseNULL(interruptSource); - } - - if (command_gate) { - workLoop->removeEventSource(command_gate); - OSSafeReleaseNULL(command_gate); - } - if (workLoop) { // workLoop->release(); workLoop = NULL; @@ -806,7 +806,7 @@ IOReturn VoodooGPIO::registerInterrupt(int pin, OSObject *target, IOInterruptAct if (hw_pin < 0) return kIOReturnNoInterrupt; - IOLog("%s::Registering hardware pin %d for GPIO IRQ pin %u", getName(), hw_pin, pin); + IOLog("%s::Registering hardware pin %d for GPIO IRQ pin %u\n", getName(), hw_pin, pin); unsigned communityidx = hw_pin - community->pin_base; @@ -891,7 +891,7 @@ void VoodooGPIO::InterruptOccurred(OSObject *owner, IOInterruptEventSource *src, IOReturn ret = command_gate->attemptAction(OSMemberFunctionCast(IOCommandGate::Action, this, &VoodooGPIO::interruptOccurredGated)); if (ret != kIOReturnSuccess) { isInterruptBusy = false; - IOLog("%s:InterruptOccurred:Failed on attemptAction(). Error code = %X", getName(), ret); + IOLog("%s:InterruptOccurred:Failed on attemptAction(). Error code = %X\n", getName(), ret); } } From bd3b177ae396fce8c4a9a4df5e0c2b8ac83792f9 Mon Sep 17 00:00:00 2001 From: erictoby <54815823+Erictoby@users.noreply.github.com> Date: Mon, 23 Sep 2019 12:07:44 -0400 Subject: [PATCH 05/10] Minor change - PMInit location. --- VoodooGPIO/VoodooGPIO.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/VoodooGPIO/VoodooGPIO.cpp b/VoodooGPIO/VoodooGPIO.cpp index 8c935f4..74b530f 100644 --- a/VoodooGPIO/VoodooGPIO.cpp +++ b/VoodooGPIO/VoodooGPIO.cpp @@ -546,8 +546,6 @@ bool VoodooGPIO::start(IOService *provider) { isInterruptBusy = true; - PMinit(); - workLoop = getWorkLoop(); if (!workLoop) { IOLog("%s::Failed to get workloop!\n", getName()); @@ -656,6 +654,7 @@ bool VoodooGPIO::start(IOService *provider) { myPowerStates[1].outputPowerCharacter = kIOPMPowerOn; myPowerStates[1].inputPowerRequirement = kIOPMPowerOn; + PMinit(); provider->joinPMtree(this); registerPowerDriver(this, myPowerStates, kMyNumberOfStates); @@ -698,7 +697,7 @@ void VoodooGPIO::stop(IOService *provider) { } if (workLoop) { - // workLoop->release(); + workLoop->release(); workLoop = NULL; } From ec0b2d97acf0bdcfd41c903040bf5a0ff20f3b52 Mon Sep 17 00:00:00 2001 From: erictoby <54815823+Erictoby@users.noreply.github.com> Date: Sat, 28 Sep 2019 17:36:21 -0400 Subject: [PATCH 06/10] Reduce CPU load caused by too many GPIO interrupts Removed unnecessary read activities and added passive delay instead. --- VoodooGPIO/VoodooGPIO.cpp | 44 +++++++++++++++++++++++++++++---------- VoodooGPIO/VoodooGPIO.hpp | 8 ++++--- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/VoodooGPIO/VoodooGPIO.cpp b/VoodooGPIO/VoodooGPIO.cpp index 74b530f..788202f 100644 --- a/VoodooGPIO/VoodooGPIO.cpp +++ b/VoodooGPIO/VoodooGPIO.cpp @@ -562,12 +562,11 @@ bool VoodooGPIO::start(IOService *provider) { } interruptSource = IOInterruptEventSource::interruptEventSource(this, OSMemberFunctionCast(IOInterruptEventAction, this, &VoodooGPIO::InterruptOccurred), provider); - if (!interruptSource) { + if (!interruptSource || (workLoop->addEventSource(interruptSource) != kIOReturnSuccess)) { IOLog("%s::Failed to get GPIO Controller interrupt!\n", getName()); stop(provider); return false; } - workLoop->addEventSource(interruptSource); interruptSource->enable(); IOLog("%s::VoodooGPIO Init!\n", getName()); @@ -627,7 +626,11 @@ bool VoodooGPIO::start(IOService *provider) { sz = sizeof(void *) * communities[i].npins; communities[i].pinInterruptRefcons = (void **)IOMalloc(sz); memset(communities[i].pinInterruptRefcons, 0, sz); + + communities[i].isActiveCommunity = (bool *)IOMalloc(1);; + *communities[i].isActiveCommunity = false; } + nInactiveCommunities = (UInt32)ncommunities - 1; intel_pinctrl_pm_init(); @@ -694,6 +697,7 @@ void VoodooGPIO::stop(IOService *provider) { IOFree(communities[i].pinInterruptAction, sizeof(IOInterruptAction) * communities[i].npins); IOFree(communities[i].interruptTypes, sizeof(unsigned) * communities[i].npins); IOFree(communities[i].pinInterruptRefcons, sizeof(void *) * communities[i].npins); + IOFree(communities[i].isActiveCommunity, 1); } if (workLoop) { @@ -746,9 +750,13 @@ IOReturn VoodooGPIO::setPowerState(unsigned long powerState, IOService *whatDevi return kIOPMAckImplied; } -void VoodooGPIO::intel_gpio_community_irq_handler(struct intel_community *community) { +void VoodooGPIO::intel_gpio_community_irq_handler(struct intel_community *community, bool *firstdelay) { for (int gpp = 0; gpp < community->ngpps; gpp++) { const struct intel_padgroup *padgrp = &community->gpps[gpp]; + + unsigned padno = padgrp->base - community->pin_base; + if (padno >= community->npins) + break; unsigned long pending, enabled; @@ -759,13 +767,12 @@ void VoodooGPIO::intel_gpio_community_irq_handler(struct intel_community *commun /* Only interrupts that are enabled */ pending &= enabled; - unsigned padno = padgrp->base - community->pin_base; - if (padno >= community->npins) - break; - - for (int i = 0; i < 32; i++) { - bool isPin = (pending >> i) & 0x1; - if (isPin) { + if (pending) { + for (int i = 0; i < 32; i++) { + bool isPin = (pending >> i) & 0x1; + if (!isPin) + continue; + unsigned pin = padno + i; OSObject *owner = community->pinInterruptActionOwners[pin]; @@ -773,6 +780,10 @@ void VoodooGPIO::intel_gpio_community_irq_handler(struct intel_community *commun IOInterruptAction handler = community->pinInterruptAction[pin]; void *refcon = community->pinInterruptRefcons[pin]; handler(owner, refcon, this, pin); + if (*firstdelay) { + *firstdelay = false; + IODelay(25 * nInactiveCommunities); // Reduce CPU load. 25~30us per inactive community was reasonable. + } } if (community->interruptTypes[pin] & IRQ_TYPE_LEVEL_MASK) @@ -815,6 +826,7 @@ IOReturn VoodooGPIO::registerInterrupt(int pin, OSObject *target, IOInterruptAct community->pinInterruptActionOwners[communityidx] = target; community->pinInterruptAction[communityidx] = handler; community->pinInterruptRefcons[communityidx] = refcon; + *community->isActiveCommunity = true; return kIOReturnSuccess; } @@ -879,6 +891,8 @@ IOReturn VoodooGPIO::setInterruptTypeForPin(int pin, int type) { unsigned communityidx = hw_pin - community->pin_base; community->interruptTypes[communityidx] = type; + if (type & IRQ_TYPE_LEVEL_MASK) + *community->isActiveCommunity = true; return kIOReturnSuccess; } @@ -895,9 +909,17 @@ void VoodooGPIO::InterruptOccurred(OSObject *owner, IOInterruptEventSource *src, } void VoodooGPIO::interruptOccurredGated() { + UInt32 inactive = 0; + bool firstdelay = true; + for (int i = 0; i < ncommunities; i++) { struct intel_community *community = &communities[i]; - intel_gpio_community_irq_handler(community); + if (*community->isActiveCommunity) + intel_gpio_community_irq_handler(community, &firstdelay); + else + inactive++; } + + nInactiveCommunities = (inactive < ncommunities)? inactive : ((UInt32)ncommunities - 1); isInterruptBusy = false; } diff --git a/VoodooGPIO/VoodooGPIO.hpp b/VoodooGPIO/VoodooGPIO.hpp index 389bc40..9f58685 100644 --- a/VoodooGPIO/VoodooGPIO.hpp +++ b/VoodooGPIO/VoodooGPIO.hpp @@ -120,6 +120,7 @@ struct intel_community { const struct intel_padgroup *gpps; size_t ngpps; bool gpps_alloc; + bool *isActiveCommunity; /* Reserved for the core driver */ IOMemoryMap *mmap; IOVirtualAddress regs; @@ -197,9 +198,10 @@ class VoodooGPIO : public IOService { bool controllerIsAwake; IOWorkLoop *workLoop; - IOInterruptEventSource *interruptSource = NULL; - IOCommandGate* command_gate = NULL; + IOInterruptEventSource *interruptSource; + IOCommandGate* command_gate; bool isInterruptBusy; + UInt32 nInactiveCommunities; UInt32 readl(IOVirtualAddress addr); void writel(UInt32 b, IOVirtualAddress addr); @@ -228,7 +230,7 @@ class VoodooGPIO : public IOService { void intel_gpio_irq_init(); void intel_pinctrl_resume(); - void intel_gpio_community_irq_handler(struct intel_community *community); + void intel_gpio_community_irq_handler(struct intel_community *community, bool *firstdelay); void InterruptOccurred(OSObject *owner, IOInterruptEventSource *src, int intCount); void interruptOccurredGated(); From d6d06a5a948d9ba727a79aa3ef34c1489e7e0dad Mon Sep 17 00:00:00 2001 From: ben9923 Date: Fri, 11 Oct 2019 01:00:51 +0300 Subject: [PATCH 07/10] Simplify offset validation in intel_get_padcfg() Ported from torvalds/linux@7eb7ecd --- VoodooGPIO/VoodooGPIO.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VoodooGPIO/VoodooGPIO.cpp b/VoodooGPIO/VoodooGPIO.cpp index 9d02f4c..e5e7dfd 100644 --- a/VoodooGPIO/VoodooGPIO.cpp +++ b/VoodooGPIO/VoodooGPIO.cpp @@ -140,7 +140,7 @@ IOVirtualAddress VoodooGPIO::intel_get_padcfg(unsigned pin, unsigned reg) { padno = pin_to_padno(community, pin); nregs = (community->features & PINCTRL_FEATURE_DEBOUNCE) ? 4 : 2; - if (reg == PADCFG2 && !(community->features & PINCTRL_FEATURE_DEBOUNCE)) + if (reg >= nregs * 4) return NULL; return community->pad_regs + reg + padno * nregs * 4; From a9d1dd914af3b6c6bbdaa9dbf1494d5e6f6091fb Mon Sep 17 00:00:00 2001 From: ben9923 Date: Fri, 11 Oct 2019 01:44:55 +0300 Subject: [PATCH 08/10] Allow to request locked pads Uncertain if functional difference exists for VoodooGPIO Ports torvalds/linux@1bd2315 --- VoodooGPIO/VoodooGPIO.cpp | 46 +++++++++++++++++++++++++++++---------- VoodooGPIO/VoodooGPIO.hpp | 3 ++- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/VoodooGPIO/VoodooGPIO.cpp b/VoodooGPIO/VoodooGPIO.cpp index e5e7dfd..ce842ac 100644 --- a/VoodooGPIO/VoodooGPIO.cpp +++ b/VoodooGPIO/VoodooGPIO.cpp @@ -194,40 +194,64 @@ bool VoodooGPIO::intel_pad_acpi_mode(unsigned pin) { return !(readl(hostown) & BIT(gpp_offset)); } -bool VoodooGPIO::intel_pad_locked(unsigned pin) { +/** + * enum - Locking variants of the pad configuration. + * + * @PAD_UNLOCKED: Pad is fully controlled by the configuration registers + * @PAD_LOCKED: Pad configuration registers, except TX state, are locked + * @PAD_LOCKED_TX: Pad configuration TX state is locked + * @PAD_LOCKED_FULL: Pad configuration registers are locked completely + * + * Locking is considered as read-only mode for corresponding registers and + * their respective fields. That said, TX state bit is locked separately from + * the main locking scheme. + */ +enum { + PAD_UNLOCKED = 0, + PAD_LOCKED = 1, + PAD_LOCKED_TX = 2, + PAD_LOCKED_FULL = PAD_LOCKED | PAD_LOCKED_TX, +}; + +int VoodooGPIO::intel_pad_locked(unsigned int pin) { const struct intel_community *community; const struct intel_padgroup *padgrp; unsigned offset, gpp_offset; UInt32 value; - + int ret = PAD_UNLOCKED; + community = intel_get_community(pin); if (!community) - return true; + return PAD_LOCKED_FULL; if (!community->padcfglock_offset) - return false; + return PAD_UNLOCKED; padgrp = intel_community_get_padgroup(community, pin); if (!padgrp) - return true; + return PAD_LOCKED_FULL; gpp_offset = padgroup_offset(padgrp, pin); /* * If PADCFGLOCK and PADCFGLOCKTX bits are both clear for this pad, * the pad is considered unlocked. Any other case means that it is - * either fully or partially locked and we don't touch it. + * either fully or partially locked. */ - offset = community->padcfglock_offset + padgrp->reg_num * 8; + offset = community->padcfglock_offset + 0 + padgrp->reg_num * 8; value = readl(community->regs + offset); if (value & BIT(gpp_offset)) - return true; + ret |= PAD_LOCKED; offset = community->padcfglock_offset + 4 + padgrp->reg_num * 8; value = readl(community->regs + offset); if (value & BIT(gpp_offset)) - return true; + ret |= PAD_LOCKED_TX; - return false; + return ret; +} + +bool VoodooGPIO::intel_pad_is_unlocked(unsigned int pin) { + return (intel_pad_locked(pin) & PAD_LOCKED) == PAD_UNLOCKED; } /** @@ -397,7 +421,7 @@ bool VoodooGPIO::intel_pinctrl_add_padgroups(intel_community *community) { } bool VoodooGPIO::intel_pinctrl_should_save(unsigned pin) { - if (!(intel_pad_owned_by_host(pin) && !intel_pad_locked(pin))) + if (!(intel_pad_owned_by_host(pin) && intel_pad_is_unlocked(pin))) return false; struct intel_community *community = intel_get_community(pin); diff --git a/VoodooGPIO/VoodooGPIO.hpp b/VoodooGPIO/VoodooGPIO.hpp index fc6b8d1..6dbd42c 100644 --- a/VoodooGPIO/VoodooGPIO.hpp +++ b/VoodooGPIO/VoodooGPIO.hpp @@ -211,7 +211,8 @@ class VoodooGPIO : public IOService { bool intel_pad_owned_by_host(unsigned pin); bool intel_pad_acpi_mode(unsigned pin); - bool intel_pad_locked(unsigned pin); + int intel_pad_locked(unsigned pin); + bool intel_pad_is_unlocked(unsigned int pin); SInt32 intel_gpio_to_pin(UInt32 offset, const struct intel_community **community, From 390bd8c8a5e9fcdbb30b156ce1d333331a15527f Mon Sep 17 00:00:00 2001 From: erictoby <54815823+Erictoby@users.noreply.github.com> Date: Fri, 11 Oct 2019 14:20:01 -0400 Subject: [PATCH 09/10] Fix a possibility to access array out of bounds in interrupt handler. It won't happen in normal condition though. --- VoodooGPIO/VoodooGPIO.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/VoodooGPIO/VoodooGPIO.cpp b/VoodooGPIO/VoodooGPIO.cpp index fdc391a..74c801b 100644 --- a/VoodooGPIO/VoodooGPIO.cpp +++ b/VoodooGPIO/VoodooGPIO.cpp @@ -766,14 +766,15 @@ void VoodooGPIO::intel_gpio_community_irq_handler(struct intel_community *commun /* Only interrupts that are enabled */ pending &= enabled; - - if (pending) { - for (int i = 0; i < 32; i++) { - bool isPin = (pending >> i) & 0x1; - if (!isPin) - continue; + if (!pending) + continue; + for (int i = 0; i < 32; i++) { + bool isPin = (pending >> i) & 0x1; + if (isPin) { unsigned pin = padno + i; + if (pin >= community->npins) + break; OSObject *owner = community->pinInterruptActionOwners[pin]; if (owner) { @@ -816,7 +817,7 @@ IOReturn VoodooGPIO::registerInterrupt(int pin, OSObject *target, IOInterruptAct if (hw_pin < 0) return kIOReturnNoInterrupt; - IOLog("%s::Registering hardware pin %d for GPIO IRQ pin %u\n", getName(), hw_pin, pin); + IOLog("%s::Registering hardware pin 0x%02X for GPIO IRQ pin 0x%02X\n", getName(), hw_pin, pin); unsigned communityidx = hw_pin - community->pin_base; From 8507d8ddb0c057d46c111ed20ff4126e5212c017 Mon Sep 17 00:00:00 2001 From: ben9923 Date: Fri, 1 Nov 2019 19:03:45 +0200 Subject: [PATCH 10/10] Fix kernel panic during stop after failed initialization Use IONew/IODelete macros for memory allocation/free Remove const definition of some variables --- VoodooGPIO.xcodeproj/project.pbxproj | 2 - .../CannonLake-H/VoodooGPIOCannonLakeH.hpp | 10 +- .../CannonLake-LP/VoodooGPIOCannonLakeLP.hpp | 6 +- VoodooGPIO/VoodooGPIO.cpp | 133 +++++++++--------- VoodooGPIO/VoodooGPIO.hpp | 29 ++-- 5 files changed, 93 insertions(+), 87 deletions(-) diff --git a/VoodooGPIO.xcodeproj/project.pbxproj b/VoodooGPIO.xcodeproj/project.pbxproj index 7f80879..889bb9b 100644 --- a/VoodooGPIO.xcodeproj/project.pbxproj +++ b/VoodooGPIO.xcodeproj/project.pbxproj @@ -321,7 +321,6 @@ INFOPLIST_FILE = VoodooGPIO/Info.plist; MODULE_NAME = org.coolstar.VoodooGPIO; MODULE_VERSION = 1.0.0d1; - "OTHER_CPLUSPLUSFLAGS[arch=*]" = "-Wno-inconsistent-missing-override"; PRODUCT_BUNDLE_IDENTIFIER = org.coolstar.VoodooGPIO; PRODUCT_NAME = "$(TARGET_NAME)"; WRAPPER_EXTENSION = kext; @@ -336,7 +335,6 @@ INFOPLIST_FILE = VoodooGPIO/Info.plist; MODULE_NAME = org.coolstar.VoodooGPIO; MODULE_VERSION = 1.0.0d1; - "OTHER_CPLUSPLUSFLAGS[arch=*]" = "-Wno-inconsistent-missing-override"; PRODUCT_BUNDLE_IDENTIFIER = org.coolstar.VoodooGPIO; PRODUCT_NAME = "$(TARGET_NAME)"; WRAPPER_EXTENSION = kext; diff --git a/VoodooGPIO/CannonLake-H/VoodooGPIOCannonLakeH.hpp b/VoodooGPIO/CannonLake-H/VoodooGPIOCannonLakeH.hpp index b3991e5..0a7f96e 100644 --- a/VoodooGPIO/CannonLake-H/VoodooGPIOCannonLakeH.hpp +++ b/VoodooGPIO/CannonLake-H/VoodooGPIOCannonLakeH.hpp @@ -374,7 +374,7 @@ static unsigned int cnlh_i2c1_pins[] = { 69, 70 }; static unsigned int cnlh_i2c2_pins[] = { 88, 89 }; static unsigned int cnlh_i2c3_pins[] = { 79, 98 }; -static const struct intel_pingroup cnlh_groups[] = { +static struct intel_pingroup cnlh_groups[] = { PIN_GROUP((char *)"spi0_grp", cnlh_spi0_pins, 1), PIN_GROUP((char *)"spi1_grp", cnlh_spi1_pins, 1), PIN_GROUP((char *)"spi2_grp", cnlh_spi2_pins, 3), @@ -411,12 +411,12 @@ static struct intel_function cnlh_functions[] = { FUNCTION((char *)"i2c3", cnlh_i2c3_groups), }; -static const struct intel_padgroup cnlh_community0_gpps[] = { +static struct intel_padgroup cnlh_community0_gpps[] = { CNL_GPP(0, 0, 24, 0), /* GPP_A */ CNL_GPP(1, 25, 50, 32), /* GPP_B */ }; -static const struct intel_padgroup cnlh_community1_gpps[] = { +static struct intel_padgroup cnlh_community1_gpps[] = { CNL_GPP(0, 51, 74, 64), /* GPP_C */ CNL_GPP(1, 75, 98, 96), /* GPP_D */ CNL_GPP(2, 99, 106, 128), /* GPP_G */ @@ -425,7 +425,7 @@ static const struct intel_padgroup cnlh_community1_gpps[] = { CNL_GPP(5, 147, 154, CNL_NO_GPIO), /* vGPIO_1 */ }; -static const struct intel_padgroup cnlh_community3_gpps[] = { +static struct intel_padgroup cnlh_community3_gpps[] = { CNL_GPP(0, 155, 178, 192), /* GPP_K */ CNL_GPP(1, 179, 202, 224), /* GPP_H */ CNL_GPP(2, 203, 215, 256), /* GPP_E */ @@ -433,7 +433,7 @@ static const struct intel_padgroup cnlh_community3_gpps[] = { CNL_GPP(4, 240, 248, CNL_NO_GPIO), /* SPI */ }; -static const struct intel_padgroup cnlh_community4_gpps[] = { +static struct intel_padgroup cnlh_community4_gpps[] = { CNL_GPP(0, 249, 259, CNL_NO_GPIO), /* CPU */ CNL_GPP(1, 260, 268, CNL_NO_GPIO), /* JTAG */ CNL_GPP(2, 269, 286, 320), /* GPP_I */ diff --git a/VoodooGPIO/CannonLake-LP/VoodooGPIOCannonLakeLP.hpp b/VoodooGPIO/CannonLake-LP/VoodooGPIOCannonLakeLP.hpp index d2f4e3d..e235dbd 100644 --- a/VoodooGPIO/CannonLake-LP/VoodooGPIOCannonLakeLP.hpp +++ b/VoodooGPIO/CannonLake-LP/VoodooGPIOCannonLakeLP.hpp @@ -363,14 +363,14 @@ static struct intel_function cnllp_functions[] = { FUNCTION((char *)"uart2", cnllp_uart2_groups), }; -static const struct intel_padgroup cnllp_community0_gpps[] = { +static struct intel_padgroup cnllp_community0_gpps[] = { CNL_GPP(0, 0, 24, 0), /* GPP_A */ CNL_GPP(1, 25, 50, 32), /* GPP_B */ CNL_GPP(2, 51, 58, 64), /* GPP_G */ CNL_GPP(3, 59, 67, CNL_NO_GPIO), /* SPI */ }; -static const struct intel_padgroup cnllp_community1_gpps[] = { +static struct intel_padgroup cnllp_community1_gpps[] = { CNL_GPP(0, 68, 92, 96), /* GPP_D */ CNL_GPP(1, 93, 116, 128), /* GPP_F */ CNL_GPP(2, 117, 140, 160), /* GPP_H */ @@ -378,7 +378,7 @@ static const struct intel_padgroup cnllp_community1_gpps[] = { CNL_GPP(4, 173, 180, 224), /* vGPIO */ }; -static const struct intel_padgroup cnllp_community4_gpps[] = { +static struct intel_padgroup cnllp_community4_gpps[] = { CNL_GPP(0, 181, 204, 256), /* GPP_C */ CNL_GPP(1, 205, 228, 288), /* GPP_E */ CNL_GPP(2, 229, 237, CNL_NO_GPIO), /* JTAG */ diff --git a/VoodooGPIO/VoodooGPIO.cpp b/VoodooGPIO/VoodooGPIO.cpp index 3c3fe40..66b5347 100644 --- a/VoodooGPIO/VoodooGPIO.cpp +++ b/VoodooGPIO/VoodooGPIO.cpp @@ -117,9 +117,9 @@ struct intel_community *VoodooGPIO::intel_get_community(unsigned pin) { return NULL; } -const struct intel_padgroup *VoodooGPIO::intel_community_get_padgroup(const struct intel_community *community, unsigned pin) { +struct intel_padgroup *VoodooGPIO::intel_community_get_padgroup(struct intel_community *community, unsigned pin) { for (int i = 0; i < community->ngpps; i++) { - const struct intel_padgroup *padgrp = &community->gpps[i]; + struct intel_padgroup *padgrp = &community->gpps[i]; if (pin >= padgrp->base && pin < padgrp->base + padgrp->size) return padgrp; } @@ -129,7 +129,7 @@ const struct intel_padgroup *VoodooGPIO::intel_community_get_padgroup(const stru } IOVirtualAddress VoodooGPIO::intel_get_padcfg(unsigned pin, unsigned reg) { - const struct intel_community *community; + struct intel_community *community; unsigned padno; size_t nregs; @@ -147,8 +147,8 @@ IOVirtualAddress VoodooGPIO::intel_get_padcfg(unsigned pin, unsigned reg) { } bool VoodooGPIO::intel_pad_owned_by_host(unsigned pin) { - const struct intel_community *community; - const struct intel_padgroup *padgrp; + struct intel_community *community; + struct intel_padgroup *padgrp; unsigned gpp, offset, gpp_offset; IOVirtualAddress padown; @@ -172,8 +172,8 @@ bool VoodooGPIO::intel_pad_owned_by_host(unsigned pin) { } bool VoodooGPIO::intel_pad_acpi_mode(unsigned pin) { - const struct intel_community *community; - const struct intel_padgroup *padgrp; + struct intel_community *community; + struct intel_padgroup *padgrp; unsigned offset, gpp_offset; IOVirtualAddress hostown; @@ -214,8 +214,8 @@ enum { }; int VoodooGPIO::intel_pad_locked(unsigned int pin) { - const struct intel_community *community; - const struct intel_padgroup *padgrp; + struct intel_community *community; + struct intel_padgroup *padgrp; unsigned offset, gpp_offset; UInt32 value; int ret = PAD_UNLOCKED; @@ -264,14 +264,14 @@ bool VoodooGPIO::intel_pad_is_unlocked(unsigned int pin) { * @return Hardware GPIO pin number. -1 if not found. */ SInt32 VoodooGPIO::intel_gpio_to_pin(UInt32 offset, - const struct intel_community **community, - const struct intel_padgroup **padgrp) { + struct intel_community **community, + struct intel_padgroup **padgrp) { int i; for (i = 0; i < ncommunities; i++) { - const struct intel_community *comm = &communities[i]; + struct intel_community *comm = &communities[i]; int j; for (j = 0; j < comm->ngpps; j++) { - const struct intel_padgroup *pgrp = &comm->gpps[j]; + struct intel_padgroup *pgrp = &comm->gpps[j]; if (pgrp->gpio_base < 0) continue; @@ -298,9 +298,9 @@ SInt32 VoodooGPIO::intel_gpio_to_pin(UInt32 offset, * @param mask Whether to mask or unmask. */ void VoodooGPIO::intel_gpio_irq_mask_unmask(unsigned pin, bool mask) { - const struct intel_community *community = intel_get_community(pin); + struct intel_community *community = intel_get_community(pin); if (community) { - const struct intel_padgroup *padgrp = intel_community_get_padgroup(community, pin); + struct intel_padgroup *padgrp = intel_community_get_padgroup(community, pin); if (!padgrp) return; @@ -379,9 +379,9 @@ bool VoodooGPIO::intel_pinctrl_add_padgroups(intel_community *community) { ngpps = community->ngpps; else ngpps = DIV_ROUND_UP(community->npins, community->gpp_size); - - gpps = (struct intel_padgroup *)IOMalloc(ngpps * sizeof(struct intel_padgroup)); - + + gpps = IONew(intel_padgroup, ngpps); + for (int i = 0; i < ngpps; i++) { if (community->gpps) { gpps[i] = community->gpps[i]; @@ -436,43 +436,39 @@ bool VoodooGPIO::intel_pinctrl_should_save(unsigned pin) { */ if (community->pinInterruptActionOwners[communityidx]) return true; + return false; } void VoodooGPIO::intel_pinctrl_pm_init() { - context.pads = (struct intel_pad_context *)IOMalloc(npins * sizeof(struct intel_pad_context)); - memset(context.pads, 0, npins * sizeof(struct intel_pad_context)); - - context.communities = (struct intel_community_context *)IOMalloc(ncommunities * sizeof(struct intel_community_context)); - memset(context.communities, 0, ncommunities * sizeof(struct intel_community_context)); - + context.pads = IONew(intel_pad_context, npins); + memset(context.pads, 0, npins * sizeof(intel_pad_context)); + + context.communities = IONew(intel_community_context, ncommunities); + memset(context.communities, 0, ncommunities * sizeof(intel_community_context)); + for (int i = 0; i < ncommunities; i++) { - struct intel_community *community = &communities[i]; - UInt32 *intmask = (UInt32 *)IOMalloc(community->ngpps * sizeof(UInt32)); - - context.communities[i].intmask = intmask; + intel_community *community = &communities[i]; + + context.communities[i].intmask = IONew(UInt32, community->ngpps);; } } void VoodooGPIO::intel_pinctrl_pm_release() { for (int i = 0; i < ncommunities; i++) { - struct intel_community *community = &communities[i]; - IOFree(context.communities[i].intmask, community->ngpps * sizeof(UInt32)); - - context.communities[i].intmask = NULL; + intel_community *community = &communities[i]; + IOSafeDeleteNULL(context.communities[i].intmask, UInt32, community->ngpps); } - - IOFree(context.communities, ncommunities * sizeof(struct intel_community_context)); - context.communities = NULL; - - IOFree(context.pads, npins * sizeof(intel_pad_context)); - context.pads = NULL; + + IOSafeDeleteNULL(context.communities, intel_community_context, ncommunities); + + IOSafeDeleteNULL(context.pads, intel_pad_context, npins); } void VoodooGPIO::intel_pinctrl_suspend() { struct intel_pad_context *pads = context.pads; for (int i = 0; i < npins; i++) { - const struct pinctrl_pin_desc *desc = &pins[i]; + struct pinctrl_pin_desc *desc = &pins[i]; IOVirtualAddress padcfg; uint32_t val; @@ -519,7 +515,7 @@ void VoodooGPIO::intel_pinctrl_resume() { struct intel_pad_context *pads = context.pads; for (int i = 0; i < npins; i++) { - const struct pinctrl_pin_desc *desc = &pins[i]; + struct pinctrl_pin_desc *desc = &pins[i]; IOVirtualAddress padcfg; uint32_t val; @@ -559,6 +555,15 @@ void VoodooGPIO::intel_pinctrl_resume() { } } +bool VoodooGPIO::init(OSDictionary* properties) { + if (!IOService::init(properties)) + return false; + + memset(&(this->context), 0, sizeof(intel_pinctrl_context)); + + return true; +} + bool VoodooGPIO::start(IOService *provider) { if (!npins || !ngroups || !nfunctions || !ncommunities) { IOLog("%s::Missing Platform Data! Aborting!\n", getName()); @@ -577,7 +582,7 @@ bool VoodooGPIO::start(IOService *provider) { return false; } workLoop->retain(); - + interruptSource = IOInterruptEventSource::interruptEventSource(this, OSMemberFunctionCast(IOInterruptEventAction, this, &VoodooGPIO::InterruptOccurred), provider); if (!interruptSource) { IOLog("%s::Failed to get GPIO Controller interrupt!\n", getName()); @@ -587,7 +592,7 @@ bool VoodooGPIO::start(IOService *provider) { workLoop->addEventSource(interruptSource); interruptSource->enable(); - + command_gate = IOCommandGate::commandGate(this); if (!command_gate || (workLoop->addEventSource(command_gate) != kIOReturnSuccess)) { IOLog("%s Could not open command gate\n", getName()); @@ -640,7 +645,7 @@ bool VoodooGPIO::start(IOService *provider) { size_t sz = sizeof(OSObject *) * communities[i].npins; communities[i].pinInterruptActionOwners = (OSObject **)IOMalloc(sz); memset(communities[i].pinInterruptActionOwners, 0, sz); - + sz = sizeof(IOInterruptAction) * communities[i].npins; communities[i].pinInterruptAction = (IOInterruptAction *)IOMalloc(sz); memset(communities[i].pinInterruptAction, 0, sz); @@ -659,7 +664,7 @@ bool VoodooGPIO::start(IOService *provider) { controllerIsAwake = true; registerService(); - + // Declare an array of two IOPMPowerState structures (kMyNumberOfStates = 2). #define kMyNumberOfStates 2 @@ -692,20 +697,13 @@ void VoodooGPIO::stop(IOService *provider) { for (int i = 0; i < ncommunities; i++) { if (communities[i].gpps_alloc) { - IOFree((void *)communities[i].gpps, communities[i].ngpps * sizeof(struct intel_padgroup)); - communities[i].gpps = NULL; - } - - for (int j = 0; j < communities[i].npins; j++) { - communities[i].pinInterruptActionOwners[j] = NULL; - communities[i].pinInterruptAction[j] = NULL; - communities[i].interruptTypes[j] = 0; - communities[i].pinInterruptRefcons[j] = NULL; + IOSafeDeleteNULL(communities[i].gpps, intel_padgroup, communities[i].ngpps); } - IOFree(communities[i].pinInterruptActionOwners, sizeof(OSObject *) * communities[i].npins); - IOFree(communities[i].pinInterruptAction, sizeof(IOInterruptAction) * communities[i].npins); - IOFree(communities[i].interruptTypes, sizeof(unsigned) * communities[i].npins); - IOFree(communities[i].pinInterruptRefcons, sizeof(void *) * communities[i].npins); + + IOSafeDeleteNULL(communities[i].pinInterruptActionOwners, OSObject*, communities[i].npins); + IOSafeDeleteNULL(communities[i].pinInterruptAction, IOInterruptAction, communities[i].npins); + IOSafeDeleteNULL(communities[i].interruptTypes, unsigned, communities[i].npins); + IOSafeDeleteNULL(communities[i].pinInterruptRefcons, void*, communities[i].npins); } if (interruptSource) { @@ -718,11 +716,8 @@ void VoodooGPIO::stop(IOService *provider) { workLoop->removeEventSource(command_gate); OSSafeReleaseNULL(command_gate); } - - if (workLoop) { - workLoop->release(); - workLoop = NULL; - } + + OSSafeReleaseNULL(workLoop); PMstop(); @@ -771,8 +766,8 @@ IOReturn VoodooGPIO::setPowerState(unsigned long powerState, IOService *whatDevi void VoodooGPIO::intel_gpio_community_irq_handler(struct intel_community *community) { for (int gpp = 0; gpp < community->ngpps; gpp++) { - const struct intel_padgroup *padgrp = &community->gpps[gpp]; - + struct intel_padgroup *padgrp = &community->gpps[gpp]; + unsigned long pending, enabled; pending = readl(community->regs + GPI_IS + padgrp->reg_num * 4); @@ -785,7 +780,7 @@ void VoodooGPIO::intel_gpio_community_irq_handler(struct intel_community *commun unsigned padno = padgrp->base - community->pin_base; if (padno >= community->npins) break; - + for (int i = 0; i < 32; i++) { bool isPin = (pending >> i) & 0x1; if (isPin) { @@ -823,7 +818,7 @@ IOReturn VoodooGPIO::getInterruptType(int pin, int *interruptType) { * @param pin 'Software' pin number (i.e. GpioInt). */ IOReturn VoodooGPIO::registerInterrupt(int pin, OSObject *target, IOInterruptAction handler, void *refcon) { - const struct intel_community *community; + struct intel_community *community; SInt32 hw_pin = intel_gpio_to_pin(pin, &community, nullptr); if (hw_pin < 0) return kIOReturnNoInterrupt; @@ -845,7 +840,7 @@ IOReturn VoodooGPIO::registerInterrupt(int pin, OSObject *target, IOInterruptAct * @param pin 'Software' pin number (i.e. GpioInt). */ IOReturn VoodooGPIO::unregisterInterrupt(int pin) { - const struct intel_community *community; + struct intel_community *community; SInt32 hw_pin = intel_gpio_to_pin(pin, &community, nullptr); if (hw_pin < 0) return kIOReturnNoInterrupt; @@ -864,7 +859,7 @@ IOReturn VoodooGPIO::unregisterInterrupt(int pin) { * @param pin 'Software' pin number (i.e. GpioInt). */ IOReturn VoodooGPIO::enableInterrupt(int pin) { - const struct intel_community *community; + struct intel_community *community; SInt32 hw_pin = intel_gpio_to_pin(pin, &community, nullptr); if (hw_pin < 0) return kIOReturnNoInterrupt; @@ -895,7 +890,7 @@ IOReturn VoodooGPIO::disableInterrupt(int pin) { * @param type Interrupt type to set for specified pin. */ IOReturn VoodooGPIO::setInterruptTypeForPin(int pin, int type) { - const struct intel_community *community; + struct intel_community *community; SInt32 hw_pin = intel_gpio_to_pin(pin, &community, nullptr); if (hw_pin < 0) return kIOReturnNoInterrupt; diff --git a/VoodooGPIO/VoodooGPIO.hpp b/VoodooGPIO/VoodooGPIO.hpp index 6dbd42c..f0673b2 100644 --- a/VoodooGPIO/VoodooGPIO.hpp +++ b/VoodooGPIO/VoodooGPIO.hpp @@ -18,6 +18,17 @@ #ifndef VoodooGPIO_h #define VoodooGPIO_h +// Exists in the macOS 10.15 SDK +#ifndef IOSafeDeleteNULL +#define IOSafeDeleteNULL(ptr, type, count) \ + do { \ + if (NULL != (ptr)) { \ + IODelete((ptr), type, count); \ + (ptr) = NULL; \ + } \ + } while (0) +#endif + struct pinctrl_pin_desc { unsigned number; char *name; @@ -117,7 +128,7 @@ struct intel_community { unsigned gpp_num_padown_regs; size_t npins; unsigned features; - const struct intel_padgroup *gpps; + struct intel_padgroup *gpps; size_t ngpps; bool gpps_alloc; /* Reserved for the core driver */ @@ -184,7 +195,7 @@ class VoodooGPIO : public IOService { protected: struct pinctrl_pin_desc *pins; size_t npins; - const struct intel_pingroup *groups; + struct intel_pingroup *groups; size_t ngroups; struct intel_function *functions; size_t nfunctions; @@ -196,9 +207,9 @@ class VoodooGPIO : public IOService { bool controllerIsAwake; - IOWorkLoop *workLoop; - IOInterruptEventSource *interruptSource; - IOCommandGate* command_gate; + IOWorkLoop *workLoop = nullptr; + IOInterruptEventSource *interruptSource = nullptr; + IOCommandGate* command_gate = nullptr; UInt32 readl(IOVirtualAddress addr); void writel(UInt32 b, IOVirtualAddress addr); @@ -206,7 +217,7 @@ class VoodooGPIO : public IOService { IOWorkLoop* getWorkLoop(); struct intel_community *intel_get_community(unsigned pin); - const struct intel_padgroup *intel_community_get_padgroup(const struct intel_community *community, unsigned pin); + struct intel_padgroup *intel_community_get_padgroup(struct intel_community *community, unsigned pin); IOVirtualAddress intel_get_padcfg(unsigned pin, unsigned reg); bool intel_pad_owned_by_host(unsigned pin); @@ -215,8 +226,8 @@ class VoodooGPIO : public IOService { bool intel_pad_is_unlocked(unsigned int pin); SInt32 intel_gpio_to_pin(UInt32 offset, - const struct intel_community **community, - const struct intel_padgroup **padgrp); + struct intel_community **community, + struct intel_padgroup **padgrp); void intel_gpio_irq_mask_unmask(unsigned pin, bool mask); bool intel_gpio_irq_set_type(unsigned pin, unsigned type); @@ -244,6 +255,8 @@ class VoodooGPIO : public IOService { IOReturn setInterruptTypeForPin(int pin, int type); + bool init(OSDictionary* properties) override; + bool start(IOService *provider) override; void stop(IOService *provider) override;