| Message ID | 4b57167184fa9aa4d1c183d4c8df6e6a600a7dbd.1498087915.git.sathyanarayanan.kuppuswamy@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Jun 26, 2017 at 7:37 PM, <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > > According to Whiskey Cove PMIC GPIO controller specification, for GPIO > pins 0-12, GPIO input and output register control address range from, > > 0x4e44-0x4e50 for GPIO outputs control register > > 0x4e51-0x4e5d for GPIO input control register > > But, currently when calculating the GPIO register offsets in to_reg() > function, all GPIO pins in the same bank uses the same GPIO control > register address. This logic is incorrect. This patch fixes this > issue. > > This patch also adds support to selectively skip register modification > for virtual GPIOs. > > In case of Whiskey Cove PMIC, ACPI code may use up 94 virtual GPIOs. > These virtual GPIOs are used by the ACPI code as means to access various > non GPIO bits of PMIC. So for these virtual GPIOs, we don't need to > manipulate the physical GPIO pin register. A similar patch has been > merged recently by Hans for Crystal Cove PMIC GPIO driver. You can > find more details about it in Commit 9a752b4c9ab9 ("gpio: crystalcove: > Do not write regular gpio registers for virtual GPIOs") So where can I get a handle on the people inside Intel who are obviously using ACPI GPIO class for shoehorning what we in the linux kernel call syscon or register bit misc access into the GPIO ACPI container just because they feel it is convenient? They need to invent a NEW ACPI four-character thing and call that "misc register bit" (_MRB?) or whatever and have it bind to syscon. This is not working. It feels like I am starting to maintain Intel's swiss army knife for misc register manipulation, and that should not be done by "virtual GPIO" because just look at it: General-purpose input/output - yeah that sounds like something going in/out of the system right? And something that is used by electronics outside the provider. Like a LED. Or a key. "Virtual GPIO" - those are not input/outputs at all in most cases, because they are internally routed, and they are not general purpose at all because mostly they are for a specific purpose. Neither are they virtual because the signals are very real. Calling something "virtual GPIO" is just one big confusion for the brain. Looking at Rusty Russell's API design manifesto: http://sweng.the-davies.net/Home/rustys-api-design-manifesto this is just screwing things up. > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > Reported-by: Jukka Laitinen <jukka.laitinen@intel.com> I have applied the patch because I think you know this better than me, just including Mika and Andy as a side note. The patch is fine because it makes the system run despite the above mentioned violation of ACPI which is not your fault. Would you mind signing yourself up as maintainer in the MAINTAINERS file for this driver? Yours, Linus Walleij -- 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
+Cc: Hans On Mon, Jun 26, 2017 at 8:37 PM, <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > > According to Whiskey Cove PMIC GPIO controller specification, for GPIO > pins 0-12, GPIO input and output register control address range from, > > 0x4e44-0x4e50 for GPIO outputs control register > > 0x4e51-0x4e5d for GPIO input control register > > But, currently when calculating the GPIO register offsets in to_reg() > function, all GPIO pins in the same bank uses the same GPIO control > register address. This logic is incorrect. This patch fixes this > issue. > > This patch also adds support to selectively skip register modification > for virtual GPIOs. > > In case of Whiskey Cove PMIC, ACPI code may use up 94 virtual GPIOs. > These virtual GPIOs are used by the ACPI code as means to access various > non GPIO bits of PMIC. So for these virtual GPIOs, we don't need to > manipulate the physical GPIO pin register. A similar patch has been > merged recently by Hans for Crystal Cove PMIC GPIO driver. You can > find more details about it in Commit 9a752b4c9ab9 ("gpio: crystalcove: > Do not write regular gpio registers for virtual GPIOs") For me (disregards to content of the patch) the question is: did we ever have a *working* solution looking to the bug fixes on this driver?! I would suggest to stop applying patches on Intel PMICs without Tested-by tag from independent testers. Hans, do you have anything to add / comment on this?
Hi, On 29-06-17 14:30, Andy Shevchenko wrote: > +Cc: Hans > > On Mon, Jun 26, 2017 at 8:37 PM, > <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: >> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> >> >> According to Whiskey Cove PMIC GPIO controller specification, for GPIO >> pins 0-12, GPIO input and output register control address range from, >> >> 0x4e44-0x4e50 for GPIO outputs control register >> >> 0x4e51-0x4e5d for GPIO input control register >> >> But, currently when calculating the GPIO register offsets in to_reg() >> function, all GPIO pins in the same bank uses the same GPIO control >> register address. This logic is incorrect. This patch fixes this >> issue. > >> >> This patch also adds support to selectively skip register modification >> for virtual GPIOs. >> >> In case of Whiskey Cove PMIC, ACPI code may use up 94 virtual GPIOs. >> These virtual GPIOs are used by the ACPI code as means to access various >> non GPIO bits of PMIC. So for these virtual GPIOs, we don't need to >> manipulate the physical GPIO pin register. A similar patch has been >> merged recently by Hans for Crystal Cove PMIC GPIO driver. You can >> find more details about it in Commit 9a752b4c9ab9 ("gpio: crystalcove: >> Do not write regular gpio registers for virtual GPIOs") > > For me (disregards to content of the patch) the question is: did we > ever have a *working* solution looking to the bug fixes on this > driver?! > > I would suggest to stop applying patches on Intel PMICs without > Tested-by tag from independent testers. > > Hans, do you have anything to add / comment on this? Yes, I noticed the driver .name = "bxt_wcove_gpio", I myself have a device with a Cherry Trail Whiskey Cove variant. For those reading along here which SoC / platform a PMIC is used on (Cherry Trail vs Broxton) may seem unrelevant. But Intel has a per platform variant of its Crystal Cove and Whiskey Cove PMICs and the platform variants are really just completely different PMICs, using incompatible registermaps for one. So I would really like us to stop referring to these devices as Whiskey Cove (or wcove) and instead name them "Cherry Trail Whiskey Cove" or cht_wc, "Bay Trail Crystal Cove" / byt_crc, etc. and do so consistently! E.g. I've recently learned that there are Cherry Trail devices with Crystal Cove PMICs (Dell Venue 8 pro 5855) and enabling the Crystal Cove PMIC ACPI Opregion on those wrecks havoc because it causes the wrong registers to get modified. Specifically the regulator control registers have slightly different addresses so we are modifying the wrong regulators! <sarcasm> Which is not a problem, right ? </sarcasm> Anyways back to the topic. Kuppuswamy do you have access to *Cherry Trail* Whiskey Cove documentation and can you check that the GPIO ctrl and irq registers are the same there ? IOW can you check if we can re-use this driver for the Cherry Trail Whiskey Cove PMIC ? If not then the .c file should really be renamed to drivers/gpio/gpio-bxt-wcove.c And future patches should also use gpio-bxt-wcove in their subject. With that said, the patch looks good to me. Regards, Hans -- 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 Linus, On 06/29/2017 05:17 AM, Linus Walleij wrote: > On Mon, Jun 26, 2017 at 7:37 PM, > <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > >> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> >> >> According to Whiskey Cove PMIC GPIO controller specification, for GPIO >> pins 0-12, GPIO input and output register control address range from, >> >> 0x4e44-0x4e50 for GPIO outputs control register >> >> 0x4e51-0x4e5d for GPIO input control register >> >> But, currently when calculating the GPIO register offsets in to_reg() >> function, all GPIO pins in the same bank uses the same GPIO control >> register address. This logic is incorrect. This patch fixes this >> issue. >> >> This patch also adds support to selectively skip register modification >> for virtual GPIOs. >> >> In case of Whiskey Cove PMIC, ACPI code may use up 94 virtual GPIOs. >> These virtual GPIOs are used by the ACPI code as means to access various >> non GPIO bits of PMIC. So for these virtual GPIOs, we don't need to >> manipulate the physical GPIO pin register. A similar patch has been >> merged recently by Hans for Crystal Cove PMIC GPIO driver. You can >> find more details about it in Commit 9a752b4c9ab9 ("gpio: crystalcove: >> Do not write regular gpio registers for virtual GPIOs") > So where can I get a handle on the people inside Intel who are obviously > using ACPI GPIO class for shoehorning what we in the linux kernel call > syscon or register bit misc access into the GPIO ACPI container just > because they feel it is convenient? Found the patch from where it all started. It looks like the origin of this hack is from Crystal Cove PMIC. This was added to handle the side-effects of windows specific BIOS fix . After that instead of finding the proper BIOS fix, the same hack has been adapted to other Intel family PMIC devices in ACPI tables. commit dcdc3018d6357c35eae7d80b323e10bd72253cb7 Author: Aaron Lu <aaron.lu@intel.com> Date: Thu Sep 25 10:57:26 2014 +0800 gpio: crystalcove: support virtual GPIO The virtual GPIO introduced in ACPI table of Baytrail-T based system is used to solve a problem under Windows. We do not have such problems under Linux so we do not actually need them. But we have to tell GPIO library that the Crystal Cove GPIO chip has this many GPIO pins or the common GPIO handler will refuse any access to those high number GPIO pins, which will resulted in a failure evaluation of every ACPI control method that is used to turn on/off power resource and/or report sensor temperatures. Signed-off-by: Aaron Lu <aaron.lu@intel.com> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> [changed vgpio number from 0x5e to 94] Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > They need to invent a NEW ACPI four-character thing and call that > "misc register bit" (_MRB?) or whatever and have it bind to syscon. > This is not working. At this point I am not sure whether this BIOS fix is a hack or actual requirement. Mike or Aaron might have more info on this windows issue mentioned in the above commit message. If this is really a requirement then I can send these details to Rafael or Bob Moore for more ACPI specific review. > > It feels like I am starting to maintain Intel's swiss army knife for misc > register manipulation, and that should not be done by "virtual GPIO" > because just look at it: Sorry about that. When I submitted this patch, I also noticed that this "Virtual GPIO" concept has nothing to do with GPIO subsystem. But since this hack is already merged and has tight dependency on BIOS, I can't fix it cleanly any more. Only solution is, to prevent this hack being adapted to any future Intel PMIC GPIO drivers. > > General-purpose input/output - yeah that sounds like something > going in/out of the system right? And something that is used by electronics > outside the provider. Like a LED. Or a key. > > "Virtual GPIO" - those are not input/outputs at all in most cases, > because they are internally routed, and they are not general purpose > at all because mostly they are for a specific purpose. Neither are they > virtual because the signals are very real. > > Calling something "virtual GPIO" is just one big confusion for the brain. > > Looking at Rusty Russell's API design manifesto: > http://sweng.the-davies.net/Home/rustys-api-design-manifesto > this is just screwing things up. > >> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> >> Reported-by: Jukka Laitinen <jukka.laitinen@intel.com> > I have applied the patch because I think you know this better than > me, just including Mika and Andy as a side note. The patch is fine > because it makes the system run despite the above mentioned > violation of ACPI which is not your fault. Thanks. I have also tested it and it seem to run fine. Without this fix, I am getting Interrupt storms in USB Type-C device. > > Would you mind signing yourself up as maintainer in the MAINTAINERS > file for this driver? I don't mind. I can sign-up for it. > > Yours, > Linus Walleij >
> So where can I get a handle on the people inside Intel who are obviously > using ACPI GPIO class for shoehorning what we in the linux kernel call > syscon or register bit misc access into the GPIO ACPI container just > because they feel it is convenient? It's a Windowsism and since Windows is the primary OS shipped the vendors of client platforms do what is needed to make Windows work nicely. > They need to invent a NEW ACPI four-character thing and call that > "misc register bit" (_MRB?) or whatever and have it bind to syscon. > This is not working. Short of Microsoft adopting such a standard I don't think it would make any difference (beyond making life worse because you'd have a new _MRB that wasn't used by Windows so nobody ever tested). > It feels like I am starting to maintain Intel's swiss army knife for misc > register manipulation, and that should not be done by "virtual GPIO" > because just look at it: It's an ACPIism not an Intelism. I expect it's there on other vendors devices and the same things will pop up as Windows/ARM platforms with ACPI appear. > General-purpose input/output - yeah that sounds like something > going in/out of the system right? It's become an ACPI interface for controlling all sorts of system state in a way that works nicely in Windows. Rightly or wrongly that's the situation and we are still the tail not the dog. Alan -- 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 Andy, On 06/29/2017 05:30 AM, Andy Shevchenko wrote: > +Cc: Hans > > On Mon, Jun 26, 2017 at 8:37 PM, > <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: >> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> >> >> According to Whiskey Cove PMIC GPIO controller specification, for GPIO >> pins 0-12, GPIO input and output register control address range from, >> >> 0x4e44-0x4e50 for GPIO outputs control register >> >> 0x4e51-0x4e5d for GPIO input control register >> >> But, currently when calculating the GPIO register offsets in to_reg() >> function, all GPIO pins in the same bank uses the same GPIO control >> register address. This logic is incorrect. This patch fixes this >> issue. >> This patch also adds support to selectively skip register modification >> for virtual GPIOs. >> >> In case of Whiskey Cove PMIC, ACPI code may use up 94 virtual GPIOs. >> These virtual GPIOs are used by the ACPI code as means to access various >> non GPIO bits of PMIC. So for these virtual GPIOs, we don't need to >> manipulate the physical GPIO pin register. A similar patch has been >> merged recently by Hans for Crystal Cove PMIC GPIO driver. You can >> find more details about it in Commit 9a752b4c9ab9 ("gpio: crystalcove: >> Do not write regular gpio registers for virtual GPIOs") > For me (disregards to content of the patch) the question is: did we > ever have a *working* solution looking to the bug fixes on this > driver?! We recently enabled this driver to handle some some requirements in IOT ref kit board and started seeing all these issues. Given that I am seeing issues related to IRQ handling, register mapping, I can safely say that this driver was never tested before. > > I would suggest to stop applying patches on Intel PMICs without > Tested-by tag from independent testers. For the patches I have sent, I have tested it in Apollo lake reference board and also sent this patch to testing team for verification. Only after confirming that it works, I sent this patch to upstream for review. > > Hans, do you have anything to add / comment on this? >
On Thursday, June 29, 2017 08:28:57 PM Alan Cox wrote: > > So where can I get a handle on the people inside Intel who are obviously > > using ACPI GPIO class for shoehorning what we in the linux kernel call > > syscon or register bit misc access into the GPIO ACPI container just > > because they feel it is convenient? > > It's a Windowsism and since Windows is the primary OS shipped the vendors > of client platforms do what is needed to make Windows work nicely. Yes, we are pretty much on the receiving end here and unfortunately we need to deal with stuff already manufactured and shipped. Thanks, Rafael -- 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 Hans, On 06/29/2017 06:24 AM, Hans de Goede wrote: > Hi, > > On 29-06-17 14:30, Andy Shevchenko wrote: >> +Cc: Hans >> >> On Mon, Jun 26, 2017 at 8:37 PM, >> <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: >>> From: Kuppuswamy Sathyanarayanan >>> <sathyanarayanan.kuppuswamy@linux.intel.com> >>> >>> According to Whiskey Cove PMIC GPIO controller specification, for GPIO >>> pins 0-12, GPIO input and output register control address range from, >>> >>> 0x4e44-0x4e50 for GPIO outputs control register >>> >>> 0x4e51-0x4e5d for GPIO input control register >>> >>> But, currently when calculating the GPIO register offsets in to_reg() >>> function, all GPIO pins in the same bank uses the same GPIO control >>> register address. This logic is incorrect. This patch fixes this >>> issue. >> >>> >>> This patch also adds support to selectively skip register modification >>> for virtual GPIOs. >>> >>> In case of Whiskey Cove PMIC, ACPI code may use up 94 virtual GPIOs. >>> These virtual GPIOs are used by the ACPI code as means to access >>> various >>> non GPIO bits of PMIC. So for these virtual GPIOs, we don't need to >>> manipulate the physical GPIO pin register. A similar patch has been >>> merged recently by Hans for Crystal Cove PMIC GPIO driver. You can >>> find more details about it in Commit 9a752b4c9ab9 ("gpio: crystalcove: >>> Do not write regular gpio registers for virtual GPIOs") >> >> For me (disregards to content of the patch) the question is: did we >> ever have a *working* solution looking to the bug fixes on this >> driver?! >> >> I would suggest to stop applying patches on Intel PMICs without >> Tested-by tag from independent testers. >> >> Hans, do you have anything to add / comment on this? > > Yes, I noticed the driver .name = "bxt_wcove_gpio", I myself have > a device with a Cherry Trail Whiskey Cove variant. For those reading > along here which SoC / platform a PMIC is used on (Cherry Trail vs > Broxton) may seem unrelevant. But Intel has a per platform variant > of its Crystal Cove and Whiskey Cove PMICs and the platform variants > are really just completely different PMICs, using incompatible > registermaps for one. So I would really like us to stop referring > to these devices as Whiskey Cove (or wcove) and instead name them > "Cherry Trail Whiskey Cove" or cht_wc, "Bay Trail Crystal Cove" / > byt_crc, etc. and do so consistently! > > E.g. I've recently learned that there are Cherry Trail devices > with Crystal Cove PMICs (Dell Venue 8 pro 5855) and enabling > the Crystal Cove PMIC ACPI Opregion on those wrecks havoc because > it causes the wrong registers to get modified. Specifically > the regulator control registers have slightly different addresses > so we are modifying the wrong regulators! <sarcasm> Which is not a > problem, right ? </sarcasm> > > Anyways back to the topic. Kuppuswamy do you have access to > *Cherry Trail* Whiskey Cove documentation and can you check that > the GPIO ctrl and irq registers are the same there ? IOW can > you check if we can re-use this driver for the > Cherry Trail Whiskey Cove PMIC ? You are right. I just checked the spec documents and the GPIO controller register map for CHT PMIC is different compared to BXT. In BXT Whiskey Cove, we have 3 GPIO banks. But in CHT, we have only two. Also they are different alignment in register map. So this driver will not work with CHT Whiskey Cove PMIC. > If not then the .c file > should really be renamed to drivers/gpio/gpio-bxt-wcove.c I also agree with this point. If Linus is also fine with rename, I can submit a patch for it. > > And future patches should also use gpio-bxt-wcove in their > subject. > > With that said, the patch looks good to me. > > Regards, > > Hans >
On Thu, Jun 29, 2017 at 9:28 PM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote: >> General-purpose input/output - yeah that sounds like something >> going in/out of the system right? > > It's become an ACPI interface for controlling all sorts of system state > in a way that works nicely in Windows. Rightly or wrongly that's the > situation and we are still the tail not the dog. I see. Well I maintain it right now and I want these systems to work so I guess I just have to carry it forward like this, sigh. Thanks for the detailed explanation. Yours, Linus Walleij -- 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
diff --git a/drivers/gpio/gpio-wcove.c b/drivers/gpio/gpio-wcove.c index 7b1bc20..37c103e 100644 --- a/drivers/gpio/gpio-wcove.c +++ b/drivers/gpio/gpio-wcove.c @@ -108,19 +108,14 @@ struct wcove_gpio { static inline unsigned int to_reg(int gpio, enum ctrl_register reg_type) { unsigned int reg; - int bank; - if (gpio < BANK0_NR_PINS) - bank = 0; - else if (gpio < BANK0_NR_PINS + BANK1_NR_PINS) - bank = 1; - else - bank = 2; + if (gpio >= WCOVE_GPIO_NUM) + return -EOPNOTSUPP; if (reg_type == CTRL_IN) - reg = GPIO_IN_CTRL_BASE + bank; + reg = GPIO_IN_CTRL_BASE + gpio; else - reg = GPIO_OUT_CTRL_BASE + bank; + reg = GPIO_OUT_CTRL_BASE + gpio; return reg; } @@ -145,7 +140,10 @@ static void wcove_update_irq_mask(struct wcove_gpio *wg, int gpio) static void wcove_update_irq_ctrl(struct wcove_gpio *wg, int gpio) { - unsigned int reg = to_reg(gpio, CTRL_IN); + int reg = to_reg(gpio, CTRL_IN); + + if (reg < 0) + return; regmap_update_bits(wg->regmap, reg, CTLI_INTCNT_BE, wg->intcnt); } @@ -153,27 +151,36 @@ static void wcove_update_irq_ctrl(struct wcove_gpio *wg, int gpio) static int wcove_gpio_dir_in(struct gpio_chip *chip, unsigned int gpio) { struct wcove_gpio *wg = gpiochip_get_data(chip); + int reg = to_reg(gpio, CTRL_OUT); + + if (reg < 0) + return 0; - return regmap_write(wg->regmap, to_reg(gpio, CTRL_OUT), - CTLO_INPUT_SET); + return regmap_write(wg->regmap, reg, CTLO_INPUT_SET); } static int wcove_gpio_dir_out(struct gpio_chip *chip, unsigned int gpio, int value) { struct wcove_gpio *wg = gpiochip_get_data(chip); + int reg = to_reg(gpio, CTRL_OUT); - return regmap_write(wg->regmap, to_reg(gpio, CTRL_OUT), - CTLO_OUTPUT_SET | value); + if (reg < 0) + return 0; + + return regmap_write(wg->regmap, reg, CTLO_OUTPUT_SET | value); } static int wcove_gpio_get_direction(struct gpio_chip *chip, unsigned int gpio) { struct wcove_gpio *wg = gpiochip_get_data(chip); unsigned int val; - int ret; + int ret, reg = to_reg(gpio, CTRL_OUT); + + if (reg < 0) + return 0; - ret = regmap_read(wg->regmap, to_reg(gpio, CTRL_OUT), &val); + ret = regmap_read(wg->regmap, reg, &val); if (ret) return ret; @@ -184,9 +191,12 @@ static int wcove_gpio_get(struct gpio_chip *chip, unsigned int gpio) { struct wcove_gpio *wg = gpiochip_get_data(chip); unsigned int val; - int ret; + int ret, reg = to_reg(gpio, CTRL_IN); + + if (reg < 0) + return 0; - ret = regmap_read(wg->regmap, to_reg(gpio, CTRL_IN), &val); + ret = regmap_read(wg->regmap, reg, &val); if (ret) return ret; @@ -197,25 +207,33 @@ static void wcove_gpio_set(struct gpio_chip *chip, unsigned int gpio, int value) { struct wcove_gpio *wg = gpiochip_get_data(chip); + int reg = to_reg(gpio, CTRL_OUT); + + if (reg < 0) + return; if (value) - regmap_update_bits(wg->regmap, to_reg(gpio, CTRL_OUT), 1, 1); + regmap_update_bits(wg->regmap, reg, 1, 1); else - regmap_update_bits(wg->regmap, to_reg(gpio, CTRL_OUT), 1, 0); + regmap_update_bits(wg->regmap, reg, 1, 0); } static int wcove_gpio_set_config(struct gpio_chip *chip, unsigned int gpio, unsigned long config) { struct wcove_gpio *wg = gpiochip_get_data(chip); + int reg = to_reg(gpio, CTRL_OUT); + + if (reg < 0) + return 0; switch (pinconf_to_config_param(config)) { case PIN_CONFIG_DRIVE_OPEN_DRAIN: - return regmap_update_bits(wg->regmap, to_reg(gpio, CTRL_OUT), - CTLO_DRV_MASK, CTLO_DRV_OD); + return regmap_update_bits(wg->regmap, reg, CTLO_DRV_MASK, + CTLO_DRV_OD); case PIN_CONFIG_DRIVE_PUSH_PULL: - return regmap_update_bits(wg->regmap, to_reg(gpio, CTRL_OUT), - CTLO_DRV_MASK, CTLO_DRV_CMOS); + return regmap_update_bits(wg->regmap, reg, CTLO_DRV_MASK, + CTLO_DRV_CMOS); default: break; } @@ -228,6 +246,9 @@ static int wcove_irq_type(struct irq_data *data, unsigned int type) struct gpio_chip *chip = irq_data_get_irq_chip_data(data); struct wcove_gpio *wg = gpiochip_get_data(chip); + if (data->hwirq >= WCOVE_GPIO_NUM) + return 0; + switch (type) { case IRQ_TYPE_NONE: wg->intcnt = CTLI_INTCNT_DIS; @@ -278,6 +299,9 @@ static void wcove_irq_unmask(struct irq_data *data) struct gpio_chip *chip = irq_data_get_irq_chip_data(data); struct wcove_gpio *wg = gpiochip_get_data(chip); + if (data->hwirq >= WCOVE_GPIO_NUM) + return; + wg->set_irq_mask = false; wg->update |= UPDATE_IRQ_MASK; } @@ -287,6 +311,9 @@ static void wcove_irq_mask(struct irq_data *data) struct gpio_chip *chip = irq_data_get_irq_chip_data(data); struct wcove_gpio *wg = gpiochip_get_data(chip); + if (data->hwirq >= WCOVE_GPIO_NUM) + return; + wg->set_irq_mask = true; wg->update |= UPDATE_IRQ_MASK; }