Messages in this thread | | | Subject | Re: [PATCH v1 1/1] gpio: gpio-crystalcove: Skip IRQ CTRL register update for virtual GPIOs | From | "Kuppuswamy, Sathyanarayanan" <> | Date | Tue, 11 Jul 2017 10:20:50 -0700 |
| |
Hi Hans,
On 7/11/2017 2:47 AM, Hans de Goede wrote: > Hi, > > On 11-07-17 01:35, sathyanarayanan kuppuswamy wrote: >> Hi Hans, >> >> Do you have any comments on this patch ? It kind of fixes your patch, >> so would prefer to get your comments. > > Sorry I did not notice this patch before, did you Cc me ? > > As for the patch, I deliberately did not add the check > to crystalcove_update_irq_ctrl, crystalcove_update_irq_ctrl > only gets called from crystalcove_bus_sync_unlock if > UPDATE_IRQ_TYPE > > UPDATE_IRQ_TYPE only gets set from crystalcove_irq_type > which at the top contains: > > if (data->hwirq >= CRYSTALCOVE_GPIO_NUM) > return 0; > > So crystalcove_update_irq_ctrl will never get called for > virtual GPIOs. > > TL;DR: your patch is not necessary. Thanks for the clarification. > > Regards, > > Hans > > >> >> >> On 06/15/2017 02:45 PM, sathyanarayanan kuppuswamy wrote: >>> Hi Andy, >>> >>> >>> On 06/15/2017 02:19 AM, Andy Shevchenko wrote: >>>> On Thu, Jun 15, 2017 at 2:21 AM, >>>> <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: >>>>> From: Kuppuswamy Sathyanarayanan >>>>> <sathyanarayanan.kuppuswamy@linux.intel.com> >>>>> >>>>> Commit 9a752b4c9ab9 ("gpio: crystalcove: Do not write regular gpio >>>>> registers for virtual GPIOs") added support to skip GPIO register >>>>> update for virtual GPIOs, but it missed to add skip logic in >>>>> crystalcove_update_irq_ctrl() function. This patch fixes it. >>>>> @@ -134,6 +134,9 @@ static void crystalcove_update_irq_ctrl(struct >>>>> crystalcove_gpio *cg, int gpio) >>>>> { >>>>> int reg = to_reg(gpio, CTRL_IN); >>>>> >>>>> + if (reg < 0) >>>>> + return; >>>>> + >>>>> regmap_update_bits(cg->regmap, reg, CTLI_INTCNT_BE, >>>>> cg->intcnt_value); >>>>> } >>>> Shouldn't it have been done using irq_valid_mask flag in the first >>>> place? >>> Agree. Setting irq_valid_mask would be the proper approach to skip >>> IRQ for some GPIO pins. But commit 9a752b4c9ab9 added the GPIO index >>> based checks in other IRQ set/unset functions in this driver and >>> missed to add it only in this update_irq_ctrl() function. May be I >>> can submit a separate patch to clean it up and use logic based on >>> setting irq_valid_mask. >>> >>> Even if we do the cleanup patch, I would still prefer to have this >>> check. Since to_reg() can return value < 0, its good to check it >>> before passing it to regmap_* functions. Let me know your comments. >>> >>>> >>> >>
| |