* PATCH v2 @ 2017-02-22 15:34 robert.middleton 2017-02-22 15:34 ` [PATCH v2] gpio:mcp23s08 Fixed missing interrupts robert.middleton 2017-03-09 16:37 ` PATCH v2 Robert Middleton 0 siblings, 2 replies; 10+ messages in thread From: robert.middleton @ 2017-02-22 15:34 UTC (permalink / raw) To: linux-gpio; +Cc: linus.walleij, gnurou This v2 patch changes the MCP23S08 interrupt logic to ensure that if multiple inputs are changed at the same time, the kernel will be notified of all the pins that changed. I added several variables to the irq handler, however this means that there are now quite a few local variables(possibly too many, as the kernel coding standard says that more than 5-10 is probably too many: https://www.kernel.org/doc/Documentation/process/coding-style.rst). I'm not sure what the best way to fix this is, or if it has to be fixed. -Robert Middleton ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] gpio:mcp23s08 Fixed missing interrupts 2017-02-22 15:34 PATCH v2 robert.middleton @ 2017-02-22 15:34 ` robert.middleton 2017-03-14 13:19 ` Linus Walleij 2017-03-09 16:37 ` PATCH v2 Robert Middleton 1 sibling, 1 reply; 10+ messages in thread From: robert.middleton @ 2017-02-22 15:34 UTC (permalink / raw) To: linux-gpio; +Cc: linus.walleij, gnurou, Robert Middleton From: Robert Middleton <robert.middleton@rm5248.com> When an interrupt occurs on an MCP23S08 chip, the INTF register will only contain one bit as causing the interrupt. If more than two pins change at the same time on the chip, this causes one of the pins to not be reported. This patch fixes the logic for checking if a pin has changed, so that multiple pins will always cause more than one change. Signed-off-by: Robert Middleton <robert.middleton@rm5248.com> --- drivers/gpio/gpio-mcp23s08.c | 66 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 61 insertions(+), 5 deletions(-) diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/gpio/gpio-mcp23s08.c index 99d37b5..4e03919 100644 --- a/drivers/gpio/gpio-mcp23s08.c +++ b/drivers/gpio/gpio-mcp23s08.c @@ -337,8 +337,10 @@ mcp23s08_direction_output(struct gpio_chip *chip, unsigned offset, int value) static irqreturn_t mcp23s08_irq(int irq, void *data) { struct mcp23s08 *mcp = data; - int intcap, intf, i; + int intcap, intf, i, gpio, gpio_orig, intcap_mask; unsigned int child_irq; + bool intf_set, intcap_changed, gpio_bit_changed, + defval_changed, gpio_set; mutex_lock(&mcp->lock); intf = mcp->ops->read(mcp, MCP_INTF); @@ -356,14 +358,68 @@ static irqreturn_t mcp23s08_irq(int irq, void *data) } mcp->cache[MCP_INTCAP] = intcap; + + /* This clears the interrupt(configurable on S18) */ + gpio = mcp->ops->read(mcp, MCP_GPIO); + if (gpio < 0) { + mutex_unlock(&mcp->lock); + return IRQ_HANDLED; + } + gpio_orig = mcp->cache[MCP_GPIO]; + mcp->cache[MCP_GPIO] = gpio; mutex_unlock(&mcp->lock); + if (mcp->cache[MCP_INTF] == 0) { + /* There is no interrupt pending */ + return IRQ_HANDLED; + } + + dev_dbg(mcp->chip.parent, + "intcap 0x%04X intf 0x%04X gpio_orig 0x%04X gpio 0x%04X\n", + intcap, intf, gpio_orig, gpio); for (i = 0; i < mcp->chip.ngpio; i++) { - if ((BIT(i) & mcp->cache[MCP_INTF]) && - ((BIT(i) & intcap & mcp->irq_rise) || - (mcp->irq_fall & ~intcap & BIT(i)) || - (BIT(i) & mcp->cache[MCP_INTCON]))) { + /* We must check all of the inputs on the chip, + * otherwise we may not notice a change on >=2 pins. + * + * On at least the mcp23s17, INTCAP is only updated + * one byte at a time(INTCAPA and INTCAPB are + * not written to at the same time - only on a per-bank + * basis). + * + * INTF only contains the single bit that caused the + * interrupt per-bank. On the mcp23s17, there is + * INTFA and INTFB. If two pins are changed on the A + * side at the same time, INTF will only have one bit + * set. If one pin on the A side and one pin on the B + * side are changed at the same time, INTF will have + * two bits set. Thus, INTF can't be the only check + * to see if the input has changed. + */ + + intf_set = BIT(i) & mcp->cache[MCP_INTF]; + if (i < 8 && intf_set) + intcap_mask = 0x00FF; + else if (i >= 8 && intf_set + intcap_mask = 0xFF00; + else + intcap_mask = 0x00; + + intcap_changed = (intcap_mask & + (BIT(i) & mcp->cache[MCP_INTCAP])) != + (intcap_mask & (BIT(i) & gpio_orig)); + gpio_set = BIT(i) & mcp->cache[MCP_GPIO]; + gpio_bit_changed = (BIT(i) & gpio_orig) != + (BIT(i) & mcp->cache[MCP_GPIO]); + defval_changed = (BIT(i) & mcp->cache[MCP_INTCON]) && + ((BIT(i) & mcp->cache[MCP_GPIO]) != + (BIT(i) & mcp->cache[MCP_DEFVAL])); + + if (((gpio_bit_changed || intcap_changed) && + (BIT(i) & mcp->irq_rise) && gpio_set) || + ((gpio_bit_changed || intcap_changed) && + (BIT(i) & mcp->irq_fall) && !gpio_set) || + defval_changed) { child_irq = irq_find_mapping(mcp->chip.irqdomain, i); handle_nested_irq(child_irq); } -- 2.1.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] gpio:mcp23s08 Fixed missing interrupts 2017-02-22 15:34 ` [PATCH v2] gpio:mcp23s08 Fixed missing interrupts robert.middleton @ 2017-03-14 13:19 ` Linus Walleij 2017-03-14 17:41 ` Robert Middleton 0 siblings, 1 reply; 10+ messages in thread From: Linus Walleij @ 2017-03-14 13:19 UTC (permalink / raw) To: robert.middleton; +Cc: linux-gpio, Alexandre Courbot On Wed, Feb 22, 2017 at 4:34 PM, <robert.middleton@rm5248.com> wrote: > From: Robert Middleton <robert.middleton@rm5248.com> > > When an interrupt occurs on an MCP23S08 chip, the INTF register will only > contain one bit as causing the interrupt. If more than two pins change at > the same time on the chip, this causes one of the pins to not be reported. > This patch fixes the logic for checking if a pin has changed, so that > multiple pins will always cause more than one change. > > Signed-off-by: Robert Middleton <robert.middleton@rm5248.com> OMG that looks complicated. But good commit with proper comments, so patch applied for fixes. I had to do some fuzzing patch -p1 < foo.patch so please check the result. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] gpio:mcp23s08 Fixed missing interrupts 2017-03-14 13:19 ` Linus Walleij @ 2017-03-14 17:41 ` Robert Middleton 2017-03-14 21:16 ` Linus Walleij 0 siblings, 1 reply; 10+ messages in thread From: Robert Middleton @ 2017-03-14 17:41 UTC (permalink / raw) To: Linus Walleij; +Cc: linux-gpio, Alexandre Courbot On Tue, Mar 14, 2017 at 9:19 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Wed, Feb 22, 2017 at 4:34 PM, <robert.middleton@rm5248.com> wrote: > >> From: Robert Middleton <robert.middleton@rm5248.com> >> >> When an interrupt occurs on an MCP23S08 chip, the INTF register will only >> contain one bit as causing the interrupt. If more than two pins change at >> the same time on the chip, this causes one of the pins to not be reported. >> This patch fixes the logic for checking if a pin has changed, so that >> multiple pins will always cause more than one change. >> >> Signed-off-by: Robert Middleton <robert.middleton@rm5248.com> > > OMG that looks complicated. But good commit with proper comments, > so patch applied for fixes. I had to do some fuzzing patch -p1 < foo.patch > so please check the result. > > Yours, > Linus Walleij Linux, Thanks, I will check it out once that gets pushed(I'm assuming it will be in the linux-gpio repository at https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/) -Robert Middleton ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] gpio:mcp23s08 Fixed missing interrupts 2017-03-14 17:41 ` Robert Middleton @ 2017-03-14 21:16 ` Linus Walleij 2017-03-15 1:56 ` Robert Middleton 0 siblings, 1 reply; 10+ messages in thread From: Linus Walleij @ 2017-03-14 21:16 UTC (permalink / raw) To: Robert Middleton; +Cc: linux-gpio, Alexandre Courbot On Tue, Mar 14, 2017 at 6:41 PM, Robert Middleton <robert.middleton@rm5248.com> wrote: > On Tue, Mar 14, 2017 at 9:19 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >> On Wed, Feb 22, 2017 at 4:34 PM, <robert.middleton@rm5248.com> wrote: >> >>> From: Robert Middleton <robert.middleton@rm5248.com> >>> >>> When an interrupt occurs on an MCP23S08 chip, the INTF register will only >>> contain one bit as causing the interrupt. If more than two pins change at >>> the same time on the chip, this causes one of the pins to not be reported. >>> This patch fixes the logic for checking if a pin has changed, so that >>> multiple pins will always cause more than one change. >>> >>> Signed-off-by: Robert Middleton <robert.middleton@rm5248.com> >> >> OMG that looks complicated. But good commit with proper comments, >> so patch applied for fixes. I had to do some fuzzing patch -p1 < foo.patch >> so please check the result. >> >> Yours, >> Linus Walleij > > Linux, > > Thanks, I will check it out once that gets pushed(I'm assuming it will > be in the linux-gpio repository at > https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/) Unfortunately it explodes in the build servers due to changes in the driver. Can you rebase and test on v4.11-rc1 and resend? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] gpio:mcp23s08 Fixed missing interrupts 2017-03-14 21:16 ` Linus Walleij @ 2017-03-15 1:56 ` Robert Middleton 2017-03-15 21:01 ` Robert Middleton 0 siblings, 1 reply; 10+ messages in thread From: Robert Middleton @ 2017-03-15 1:56 UTC (permalink / raw) To: Linus Walleij; +Cc: linux-gpio, Alexandre Courbot On Tue, Mar 14, 2017 at 5:16 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Tue, Mar 14, 2017 at 6:41 PM, Robert Middleton > <robert.middleton@rm5248.com> wrote: >> On Tue, Mar 14, 2017 at 9:19 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >>> On Wed, Feb 22, 2017 at 4:34 PM, <robert.middleton@rm5248.com> wrote: >>> >>>> From: Robert Middleton <robert.middleton@rm5248.com> >>>> >>>> When an interrupt occurs on an MCP23S08 chip, the INTF register will only >>>> contain one bit as causing the interrupt. If more than two pins change at >>>> the same time on the chip, this causes one of the pins to not be reported. >>>> This patch fixes the logic for checking if a pin has changed, so that >>>> multiple pins will always cause more than one change. >>>> >>>> Signed-off-by: Robert Middleton <robert.middleton@rm5248.com> >>> >>> OMG that looks complicated. But good commit with proper comments, >>> so patch applied for fixes. I had to do some fuzzing patch -p1 < foo.patch >>> so please check the result. >>> >>> Yours, >>> Linus Walleij >> >> Linux, >> >> Thanks, I will check it out once that gets pushed(I'm assuming it will >> be in the linux-gpio repository at >> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/) > > Unfortunately it explodes in the build servers due to changes in > the driver. > > Can you rebase and test on v4.11-rc1 and resend? > > Yours, > Linus Walleij Sure, that will take me a day or two but I can get that done. -Robert Middleton ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] gpio:mcp23s08 Fixed missing interrupts 2017-03-15 1:56 ` Robert Middleton @ 2017-03-15 21:01 ` Robert Middleton 0 siblings, 0 replies; 10+ messages in thread From: Robert Middleton @ 2017-03-15 21:01 UTC (permalink / raw) To: Linus Walleij; +Cc: linux-gpio, Alexandre Courbot On Tue, Mar 14, 2017 at 9:56 PM, Robert Middleton <robert.middleton@rm5248.com> wrote: > On Tue, Mar 14, 2017 at 5:16 PM, Linus Walleij <linus.walleij@linaro.org> wrote: >> On Tue, Mar 14, 2017 at 6:41 PM, Robert Middleton >> <robert.middleton@rm5248.com> wrote: >>> On Tue, Mar 14, 2017 at 9:19 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >>>> On Wed, Feb 22, 2017 at 4:34 PM, <robert.middleton@rm5248.com> wrote: >>>> >>>>> From: Robert Middleton <robert.middleton@rm5248.com> >>>>> >>>>> When an interrupt occurs on an MCP23S08 chip, the INTF register will only >>>>> contain one bit as causing the interrupt. If more than two pins change at >>>>> the same time on the chip, this causes one of the pins to not be reported. >>>>> This patch fixes the logic for checking if a pin has changed, so that >>>>> multiple pins will always cause more than one change. >>>>> >>>>> Signed-off-by: Robert Middleton <robert.middleton@rm5248.com> >>>> >>>> OMG that looks complicated. But good commit with proper comments, >>>> so patch applied for fixes. I had to do some fuzzing patch -p1 < foo.patch >>>> so please check the result. >>>> >>>> Yours, >>>> Linus Walleij >>> >>> Linux, >>> >>> Thanks, I will check it out once that gets pushed(I'm assuming it will >>> be in the linux-gpio repository at >>> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/) >> >> Unfortunately it explodes in the build servers due to changes in >> the driver. >> >> Can you rebase and test on v4.11-rc1 and resend? >> >> Yours, >> Linus Walleij > > Sure, that will take me a day or two but I can get that done. > > -Robert Middleton Patch v3 has been sent. Looking at the v2 patch, the biggest problem was that I had somehow forgot a ), which was causing a parse error in gcc. I'm not quite sure how that happened. I tested with Torvalds' 4.11-rc1 kernel, everything still worked properly. -Robert Middleton ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PATCH v2 2017-02-22 15:34 PATCH v2 robert.middleton 2017-02-22 15:34 ` [PATCH v2] gpio:mcp23s08 Fixed missing interrupts robert.middleton @ 2017-03-09 16:37 ` Robert Middleton 2017-03-10 0:04 ` Phil Reid 1 sibling, 1 reply; 10+ messages in thread From: Robert Middleton @ 2017-03-09 16:37 UTC (permalink / raw) To: linux-gpio; +Cc: Linus Walleij, Alexandre Courbot On Wed, Feb 22, 2017 at 10:34 AM, <robert.middleton@rm5248.com> wrote: > > This v2 patch changes the MCP23S08 interrupt logic to ensure that if multiple > inputs are changed at the same time, the kernel will be notified of all > the pins that changed. > > I added several variables to the irq handler, however this means that there > are now quite a few local variables(possibly too many, as the kernel coding > standard says that more than 5-10 is probably too many: > https://www.kernel.org/doc/Documentation/process/coding-style.rst). I'm > not sure what the best way to fix this is, or if it has to be fixed. > > -Robert Middleton > -- > To unsubscribe from this list: send the line "unsubscribe linux-gpio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Hi, I would like to try and get some verification here on if this patch is a good fix or not. I have done testing on my side, and was wondering if there was something else that I had to do in order to have this patched upstream. -Robert Middleton ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PATCH v2 2017-03-09 16:37 ` PATCH v2 Robert Middleton @ 2017-03-10 0:04 ` Phil Reid 0 siblings, 0 replies; 10+ messages in thread From: Phil Reid @ 2017-03-10 0:04 UTC (permalink / raw) To: Robert Middleton, linux-gpio; +Cc: Linus Walleij, Alexandre Courbot On 10/03/2017 00:37, Robert Middleton wrote: > On Wed, Feb 22, 2017 at 10:34 AM, <robert.middleton@rm5248.com> wrote: >> >> This v2 patch changes the MCP23S08 interrupt logic to ensure that if multiple >> inputs are changed at the same time, the kernel will be notified of all >> the pins that changed. >> >> I added several variables to the irq handler, however this means that there >> are now quite a few local variables(possibly too many, as the kernel coding >> standard says that more than 5-10 is probably too many: >> https://www.kernel.org/doc/Documentation/process/coding-style.rst). I'm >> not sure what the best way to fix this is, or if it has to be fixed. >> >> -Robert Middleton >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > Hi, I would like to try and get some verification here on if this patch > is a good fix or not. I have done testing on my side, and was > wondering if there was something else that I had to do in order to have > this patched upstream. > G'day Rob, I was going to test it on my setup, just haven't got to it this week so far. Probably Monday for me at this rate. -- Regards Phil Reid ^ permalink raw reply [flat|nested] 10+ messages in thread
* PATCH v2 @ 2021-06-19 15:35 Linyu Yuan 0 siblings, 0 replies; 10+ messages in thread From: Linyu Yuan @ 2021-06-19 15:35 UTC (permalink / raw) To: Felipe Balbi, Thinh Nguyen, Greg Kroah-Hartman Cc: linux-usb, linux-kernel, Jack Pham Subject: [PATCH v2 0/2] usb: udc: indroduce more api for lower gadget driver introdue following udc apis which will forbid lower gadget driver operate upper layer usb gadget driver directly. void usb_gadget_udc_disconnect(struct usb_gadget *); void usb_gadget_udc_suspend(struct usb_gadget *); void usb_gadget_udc_resume(struct usb_gadget *); int usb_gadget_udc_setup(struct usb_gadget *, const struct usb_ctrlrequest *); dwc3 is first driver to use these apis. Linyu Yuan (2): usb: udc: core: hide struct usb_gadget_driver to gadget driver usb: dwc3: fix race of usb_gadget_driver operation drivers/usb/dwc3/core.h | 2 -- drivers/usb/dwc3/ep0.c | 6 +--- drivers/usb/dwc3/gadget.c | 53 +++++++++-------------------------- drivers/usb/gadget/udc/core.c | 47 ++++++++++++++++++++++++++++++- include/linux/usb/gadget.h | 6 ++++ 5 files changed, 66 insertions(+), 48 deletions(-) ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-06-19 15:36 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-22 15:34 PATCH v2 robert.middleton 2017-02-22 15:34 ` [PATCH v2] gpio:mcp23s08 Fixed missing interrupts robert.middleton 2017-03-14 13:19 ` Linus Walleij 2017-03-14 17:41 ` Robert Middleton 2017-03-14 21:16 ` Linus Walleij 2017-03-15 1:56 ` Robert Middleton 2017-03-15 21:01 ` Robert Middleton 2017-03-09 16:37 ` PATCH v2 Robert Middleton 2017-03-10 0:04 ` Phil Reid 2021-06-19 15:35 Linyu Yuan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.