| Message ID | 20220120204132.17875-2-quic_amelende@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Add support for pm8941-pwrkey.c | expand |
Quoting Anjelique Melendez (2022-01-20 12:41:33) > From: David Collins <collinsd@codeaurora.org> > > On certain PMICs, an unexpected assertion of KPDPWR_DEB (the > positive logic hardware debounced power key signal) may be seen > during the falling edge of KPDPWR_N (i.e. a power key press) when > it occurs close to the rising edge of SLEEP_CLK. This then > triggers a spurious KPDPWR interrupt. > > Handle this issue by adding software debouncing support to ignore > key events that occur within the hardware debounce delay after the > most recent key release event. > > Change-Id: Ifa3809935c01aab9078ba2302397bc9ebf390021 Please remove change-id when upstreaming. > Signed-off-by: David Collins <collinsd@codeaurora.org> > Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com> > --- > diff --git a/drivers/input/misc/pm8941-pwrkey.c b/drivers/input/misc/pm8941-pwrkey.c > index 33609603245d..b912ce00ce1c 100644 > --- a/drivers/input/misc/pm8941-pwrkey.c > +++ b/drivers/input/misc/pm8941-pwrkey.c > @@ -126,19 +144,65 @@ static irqreturn_t pm8941_pwrkey_irq(int irq, void *_data) > struct pm8941_pwrkey *pwrkey = _data; > unsigned int sts; > int error; > + u64 elapsed_us; > + > + if (pwrkey->sw_debounce_time_us) { > + elapsed_us = ktime_us_delta(ktime_get(), > + pwrkey->last_release_time); > + if (elapsed_us < pwrkey->sw_debounce_time_us) { Perhaps storing last_release_time + sw_debounce_time_us via ktime_add_us() in the struct would be better. Then this line would be if (ktime_before(debounce_end, ktime_get())) and we'd avoid a division when converting to microseconds to compare time. > + dev_dbg(pwrkey->dev, "ignoring key event received after %llu us, debounce time=%u us\n", > + elapsed_us, pwrkey->sw_debounce_time_us); > + return IRQ_HANDLED; > + } > + } > > error = regmap_read(pwrkey->regmap, > pwrkey->baseaddr + PON_RT_STS, &sts); Nitpick: I'd prefer this be on one line. And 'ret' or 'err' be used as it's shorter. > if (error) > return IRQ_HANDLED; > > - input_report_key(pwrkey->input, pwrkey->code, > - sts & pwrkey->data->status_bit); > + sts &= pwrkey->data->status_bit; > + > + if (pwrkey->sw_debounce_time_us && !sts) > + pwrkey->last_release_time = ktime_get(); > + > + input_report_key(pwrkey->input, pwrkey->code, sts); > input_sync(pwrkey->input); > > return IRQ_HANDLED; > } > > +static int pm8941_pwrkey_sw_debounce_init(struct pm8941_pwrkey *pwrkey) > +{ > + unsigned int val, addr; > + int error; > + > + if (pwrkey->data->has_pon_pbs && !pwrkey->pon_pbs_baseaddr) { > + dev_err(pwrkey->dev, "PON_PBS address missing, can't read HW debounce time\n"); > + return 0; > + } > + > + if (pwrkey->pon_pbs_baseaddr) > + addr = pwrkey->pon_pbs_baseaddr + PON_DBC_CTL; > + else > + addr = pwrkey->baseaddr + PON_DBC_CTL; > + error = regmap_read(pwrkey->regmap, addr, &val); > + if (error) > + return error; > + > + if (pwrkey->subtype >= PON_SUBTYPE_GEN2_PRIMARY) > + pwrkey->sw_debounce_time_us = 2 * USEC_PER_SEC / > + (1 << (0xf - (val & 0xf))); > + else > + pwrkey->sw_debounce_time_us = 2 * USEC_PER_SEC / > + (1 << (0x7 - (val & 0x7))); Can we have one more local variable like 'mask' or 'offset'. Then the code would be easier to read if (pwrkey->subtype >= PON_SUBTYPE_GEN2_PRIMARY) mask = 0xf; else mask = 0x7 pwrkey->sw_debounce_time_us = 2 * USEC_PER_SEC / (1 << mask - (val & 0x7)); > + > + dev_dbg(pwrkey->dev, "SW debounce time = %u us\n", > + pwrkey->sw_debounce_time_us); > + > + return 0; > +} > + > static int __maybe_unused pm8941_pwrkey_suspend(struct device *dev) > { > struct pm8941_pwrkey *pwrkey = dev_get_drvdata(dev); > @@ -167,6 +231,8 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) > struct pm8941_pwrkey *pwrkey; > bool pull_up; > struct device *parent; > + struct device_node *regmap_node; > + const __be32 *addr; > u32 req_delay; > int error; > > @@ -188,8 +254,10 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) > pwrkey->data = of_device_get_match_data(&pdev->dev); > > parent = pdev->dev.parent; > + regmap_node = pdev->dev.of_node; > pwrkey->regmap = dev_get_regmap(parent, NULL); > if (!pwrkey->regmap) { > + regmap_node = parent->of_node; > /* > * We failed to get regmap for parent. Let's see if we are > * a child of pon node and read regmap and reg from its > @@ -200,15 +268,21 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) > dev_err(&pdev->dev, "failed to locate regmap\n"); > return -ENODEV; > } > + } > > - error = of_property_read_u32(parent->of_node, > - "reg", &pwrkey->baseaddr); > - } else { > - error = of_property_read_u32(pdev->dev.of_node, "reg", > - &pwrkey->baseaddr); > + addr = of_get_address(regmap_node, 0, NULL, NULL); > + if (!addr) { > + dev_err(&pdev->dev, "reg property missing\n"); > + return -EINVAL; > + } > + pwrkey->baseaddr = be32_to_cpu(*addr); Can this hunk be split off? A new API is used and it doesn't look relevant to this patch. > + > + if (pwrkey->data->has_pon_pbs) { > + /* PON_PBS base address is optional */ > + addr = of_get_address(regmap_node, 1, NULL, NULL); > + if (addr) > + pwrkey->pon_pbs_baseaddr = be32_to_cpu(*addr); > } > - if (error) > - return error; > > pwrkey->irq = platform_get_irq(pdev, 0); > if (pwrkey->irq < 0) > @@ -217,7 +291,14 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) > error = regmap_read(pwrkey->regmap, pwrkey->baseaddr + PON_REV2, > &pwrkey->revision); > if (error) { > - dev_err(&pdev->dev, "failed to set debounce: %d\n", error); > + dev_err(&pdev->dev, "failed to read revision: %d\n", error); Nice error message fix! > + return error; > + } > + > + error = regmap_read(pwrkey->regmap, pwrkey->baseaddr + PON_SUBTYPE, > + &pwrkey->subtype); > + if (error) { > + dev_err(&pdev->dev, "failed to read subtype: %d\n", error); > return error; > } > > @@ -255,6 +336,12 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) > } > } > > + if (pwrkey->data->needs_sw_debounce) { > + error = pm8941_pwrkey_sw_debounce_init(pwrkey); > + if (error) > + return error; > + } > + > if (pwrkey->data->pull_up_bit) { > error = regmap_update_bits(pwrkey->regmap, > pwrkey->baseaddr + PON_PULL_CTL, > @@ -316,6 +403,8 @@ static const struct pm8941_data pwrkey_data = { > .phys = "pm8941_pwrkey/input0", > .supports_ps_hold_poff_config = true, > .supports_debounce_config = true, > + .needs_sw_debounce = true, needs_sw_debounce is always true? Why is it even an option then? > + .has_pon_pbs = false, > }; > > static const struct pm8941_data resin_data = {
On 1/20/2022 8:08 PM, Stephen Boyd wrote: > Quoting Anjelique Melendez (2022-01-20 12:41:33) >> From: David Collins <collinsd@codeaurora.org> >> >> On certain PMICs, an unexpected assertion of KPDPWR_DEB (the >> positive logic hardware debounced power key signal) may be seen >> during the falling edge of KPDPWR_N (i.e. a power key press) when >> it occurs close to the rising edge of SLEEP_CLK. This then >> triggers a spurious KPDPWR interrupt. >> >> Handle this issue by adding software debouncing support to ignore >> key events that occur within the hardware debounce delay after the >> most recent key release event. >> >> Change-Id: Ifa3809935c01aab9078ba2302397bc9ebf390021 > Please remove change-id when upstreaming. Will remove change-id from other 2 patches as well. > >> Signed-off-by: David Collins <collinsd@codeaurora.org> >> Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com> >> --- >> diff --git a/drivers/input/misc/pm8941-pwrkey.c b/drivers/input/misc/pm8941-pwrkey.c >> index 33609603245d..b912ce00ce1c 100644 >> --- a/drivers/input/misc/pm8941-pwrkey.c >> +++ b/drivers/input/misc/pm8941-pwrkey.c >> @@ -126,19 +144,65 @@ static irqreturn_t pm8941_pwrkey_irq(int irq, void *_data) >> struct pm8941_pwrkey *pwrkey = _data; >> unsigned int sts; >> int error; >> + u64 elapsed_us; >> + >> + if (pwrkey->sw_debounce_time_us) { >> + elapsed_us = ktime_us_delta(ktime_get(), >> + pwrkey->last_release_time); >> + if (elapsed_us < pwrkey->sw_debounce_time_us) { > Perhaps storing last_release_time + sw_debounce_time_us via > ktime_add_us() in the struct would be better. Then this line would be > > if (ktime_before(debounce_end, ktime_get())) > > and we'd avoid a division when converting to microseconds to compare > time. Sure this can be done! >> + dev_dbg(pwrkey->dev, "ignoring key event received after %llu us, debounce time=%u us\n", >> + elapsed_us, pwrkey->sw_debounce_time_us); >> + return IRQ_HANDLED; >> + } >> + } >> >> error = regmap_read(pwrkey->regmap, >> pwrkey->baseaddr + PON_RT_STS, &sts); > Nitpick: I'd prefer this be on one line. And 'ret' or 'err' be used as > it's shorter. ACK > >> if (error) >> return IRQ_HANDLED; >> >> - input_report_key(pwrkey->input, pwrkey->code, >> - sts & pwrkey->data->status_bit); >> + sts &= pwrkey->data->status_bit; >> + >> + if (pwrkey->sw_debounce_time_us && !sts) >> + pwrkey->last_release_time = ktime_get(); >> + >> + input_report_key(pwrkey->input, pwrkey->code, sts); >> input_sync(pwrkey->input); >> >> return IRQ_HANDLED; >> } >> >> +static int pm8941_pwrkey_sw_debounce_init(struct pm8941_pwrkey *pwrkey) >> +{ >> + unsigned int val, addr; >> + int error; >> + >> + if (pwrkey->data->has_pon_pbs && !pwrkey->pon_pbs_baseaddr) { >> + dev_err(pwrkey->dev, "PON_PBS address missing, can't read HW debounce time\n"); >> + return 0; >> + } >> + >> + if (pwrkey->pon_pbs_baseaddr) >> + addr = pwrkey->pon_pbs_baseaddr + PON_DBC_CTL; >> + else >> + addr = pwrkey->baseaddr + PON_DBC_CTL; >> + error = regmap_read(pwrkey->regmap, addr, &val); >> + if (error) >> + return error; >> + >> + if (pwrkey->subtype >= PON_SUBTYPE_GEN2_PRIMARY) >> + pwrkey->sw_debounce_time_us = 2 * USEC_PER_SEC / >> + (1 << (0xf - (val & 0xf))); >> + else >> + pwrkey->sw_debounce_time_us = 2 * USEC_PER_SEC / >> + (1 << (0x7 - (val & 0x7))); > Can we have one more local variable like 'mask' or 'offset'. Then the > code would be easier to read > > if (pwrkey->subtype >= PON_SUBTYPE_GEN2_PRIMARY) > mask = 0xf; > else > mask = 0x7 > > pwrkey->sw_debounce_time_us = 2 * USEC_PER_SEC / (1 << mask - (val & 0x7)); Sure not a problem! >> + >> + dev_dbg(pwrkey->dev, "SW debounce time = %u us\n", >> + pwrkey->sw_debounce_time_us); >> + >> + return 0; >> +} >> + >> static int __maybe_unused pm8941_pwrkey_suspend(struct device *dev) >> { >> struct pm8941_pwrkey *pwrkey = dev_get_drvdata(dev); >> @@ -167,6 +231,8 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) >> struct pm8941_pwrkey *pwrkey; >> bool pull_up; >> struct device *parent; >> + struct device_node *regmap_node; >> + const __be32 *addr; >> u32 req_delay; >> int error; >> >> @@ -188,8 +254,10 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) >> pwrkey->data = of_device_get_match_data(&pdev->dev); >> >> parent = pdev->dev.parent; >> + regmap_node = pdev->dev.of_node; >> pwrkey->regmap = dev_get_regmap(parent, NULL); >> if (!pwrkey->regmap) { >> + regmap_node = parent->of_node; >> /* >> * We failed to get regmap for parent. Let's see if we are >> * a child of pon node and read regmap and reg from its >> @@ -200,15 +268,21 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) >> dev_err(&pdev->dev, "failed to locate regmap\n"); >> return -ENODEV; >> } >> + } >> >> - error = of_property_read_u32(parent->of_node, >> - "reg", &pwrkey->baseaddr); >> - } else { >> - error = of_property_read_u32(pdev->dev.of_node, "reg", >> - &pwrkey->baseaddr); >> + addr = of_get_address(regmap_node, 0, NULL, NULL); >> + if (!addr) { >> + dev_err(&pdev->dev, "reg property missing\n"); >> + return -EINVAL; >> + } >> + pwrkey->baseaddr = be32_to_cpu(*addr); > Can this hunk be split off? A new API is used and it doesn't look > relevant to this patch. In PMK8350 and following chips the reg property will have the pon hlos address first, followed by a second pon pbs address. This change is needed so that both the older chipsets and the newer can be used regardless of how many reg addresses are being used. > >> + >> + if (pwrkey->data->has_pon_pbs) { >> + /* PON_PBS base address is optional */ >> + addr = of_get_address(regmap_node, 1, NULL, NULL); >> + if (addr) >> + pwrkey->pon_pbs_baseaddr = be32_to_cpu(*addr); >> } >> - if (error) >> - return error; >> >> pwrkey->irq = platform_get_irq(pdev, 0); >> if (pwrkey->irq < 0) >> @@ -217,7 +291,14 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) >> error = regmap_read(pwrkey->regmap, pwrkey->baseaddr + PON_REV2, >> &pwrkey->revision); >> if (error) { >> - dev_err(&pdev->dev, "failed to set debounce: %d\n", error); >> + dev_err(&pdev->dev, "failed to read revision: %d\n", error); > Nice error message fix! > >> + return error; >> + } >> + >> + error = regmap_read(pwrkey->regmap, pwrkey->baseaddr + PON_SUBTYPE, >> + &pwrkey->subtype); >> + if (error) { >> + dev_err(&pdev->dev, "failed to read subtype: %d\n", error); >> return error; >> } >> >> @@ -255,6 +336,12 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) >> } >> } >> >> + if (pwrkey->data->needs_sw_debounce) { >> + error = pm8941_pwrkey_sw_debounce_init(pwrkey); >> + if (error) >> + return error; >> + } >> + >> if (pwrkey->data->pull_up_bit) { >> error = regmap_update_bits(pwrkey->regmap, >> pwrkey->baseaddr + PON_PULL_CTL, >> @@ -316,6 +403,8 @@ static const struct pm8941_data pwrkey_data = { >> .phys = "pm8941_pwrkey/input0", >> .supports_ps_hold_poff_config = true, >> .supports_debounce_config = true, >> + .needs_sw_debounce = true, > needs_sw_debounce is always true? Why is it even an option then? As of right now the "needs_sw_debounce" property is being used for a sw work around for a hw problem. We anticipate that chips in the future will fix this hw problem and we would then have devices where needs_sw_debounce would be set to false. > >> + .has_pon_pbs = false, >> }; >> >> static const struct pm8941_data resin_data = {
Quoting Anjelique Melendez (2022-01-21 16:04:13) > > > On 1/20/2022 8:08 PM, Stephen Boyd wrote: > > Quoting Anjelique Melendez (2022-01-20 12:41:33) > >> @@ -200,15 +268,21 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) > >> dev_err(&pdev->dev, "failed to locate regmap\n"); > >> return -ENODEV; > >> } > >> + } > >> > >> - error = of_property_read_u32(parent->of_node, > >> - "reg", &pwrkey->baseaddr); > >> - } else { > >> - error = of_property_read_u32(pdev->dev.of_node, "reg", > >> - &pwrkey->baseaddr); > >> + addr = of_get_address(regmap_node, 0, NULL, NULL); > >> + if (!addr) { > >> + dev_err(&pdev->dev, "reg property missing\n"); > >> + return -EINVAL; > >> + } > >> + pwrkey->baseaddr = be32_to_cpu(*addr); > > Can this hunk be split off? A new API is used and it doesn't look > > relevant to this patch. > > In PMK8350 and following chips the reg property will have the pon hlos address first, > followed by a second pon pbs address. This change is needed so that both the older chipsets > and the newer can be used regardless of how many reg addresses are being used. Got it, but do we ned to change to of_get_address() in this patch? I was suggesting that the change to the new API be done first so that it's clearer what's going on with the change in address location. > > > > >> + > >> + if (pwrkey->data->has_pon_pbs) { > >> + /* PON_PBS base address is optional */ > >> + addr = of_get_address(regmap_node, 1, NULL, NULL); > >> + if (addr) > >> + pwrkey->pon_pbs_baseaddr = be32_to_cpu(*addr); > >> } > >> - if (error) > >> - return error; > >> > >> pwrkey->irq = platform_get_irq(pdev, 0); > >> if (pwrkey->irq < 0) > >> @@ -217,7 +291,14 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) > >> error = regmap_read(pwrkey->regmap, pwrkey->baseaddr + PON_REV2, > >> &pwrkey->revision); > >> if (error) { > >> - dev_err(&pdev->dev, "failed to set debounce: %d\n", error); > >> + dev_err(&pdev->dev, "failed to read revision: %d\n", error); > > Nice error message fix! This can be split off to a different patch as well. > > > >> + return error; > >> + } > >> + > >> + error = regmap_read(pwrkey->regmap, pwrkey->baseaddr + PON_SUBTYPE, > >> + &pwrkey->subtype); > >> + if (error) { > >> + dev_err(&pdev->dev, "failed to read subtype: %d\n", error); > >> return error; > >> } > >> > >> @@ -255,6 +336,12 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) > >> } > >> } > >> > >> + if (pwrkey->data->needs_sw_debounce) { > >> + error = pm8941_pwrkey_sw_debounce_init(pwrkey); > >> + if (error) > >> + return error; > >> + } > >> + > >> if (pwrkey->data->pull_up_bit) { > >> error = regmap_update_bits(pwrkey->regmap, > >> pwrkey->baseaddr + PON_PULL_CTL, > >> @@ -316,6 +403,8 @@ static const struct pm8941_data pwrkey_data = { > >> .phys = "pm8941_pwrkey/input0", > >> .supports_ps_hold_poff_config = true, > >> .supports_debounce_config = true, > >> + .needs_sw_debounce = true, > > needs_sw_debounce is always true? Why is it even an option then? > > As of right now the "needs_sw_debounce" property is being used for a sw work around for a hw > problem. We anticipate that chips in the future will fix this hw problem and we would then have > devices where needs_sw_debounce would be set to false. Hmm ok. Why can't future chips be supported in this series? What happens if nobody ever adds support for the new chips? We're left with this condition that looks like dead code.
On 1/24/2022 11:33 AM, Stephen Boyd wrote: > Quoting Anjelique Melendez (2022-01-21 16:04:13) >> >> On 1/20/2022 8:08 PM, Stephen Boyd wrote: >>> Quoting Anjelique Melendez (2022-01-20 12:41:33) >>>> @@ -200,15 +268,21 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) >>>> dev_err(&pdev->dev, "failed to locate regmap\n"); >>>> return -ENODEV; >>>> } >>>> + } >>>> >>>> - error = of_property_read_u32(parent->of_node, >>>> - "reg", &pwrkey->baseaddr); >>>> - } else { >>>> - error = of_property_read_u32(pdev->dev.of_node, "reg", >>>> - &pwrkey->baseaddr); >>>> + addr = of_get_address(regmap_node, 0, NULL, NULL); >>>> + if (!addr) { >>>> + dev_err(&pdev->dev, "reg property missing\n"); >>>> + return -EINVAL; >>>> + } >>>> + pwrkey->baseaddr = be32_to_cpu(*addr); >>> Can this hunk be split off? A new API is used and it doesn't look >>> relevant to this patch. >> In PMK8350 and following chips the reg property will have the pon hlos address first, >> followed by a second pon pbs address. This change is needed so that both the older chipsets >> and the newer can be used regardless of how many reg addresses are being used. > Got it, but do we ned to change to of_get_address() in this patch? I was > suggesting that the change to the new API be done first so that it's > clearer what's going on with the change in address location. Ok, makes sense. Will separate this into it's own patch for v2. >>>> + >>>> + if (pwrkey->data->has_pon_pbs) { >>>> + /* PON_PBS base address is optional */ >>>> + addr = of_get_address(regmap_node, 1, NULL, NULL); >>>> + if (addr) >>>> + pwrkey->pon_pbs_baseaddr = be32_to_cpu(*addr); >>>> } >>>> - if (error) >>>> - return error; >>>> >>>> pwrkey->irq = platform_get_irq(pdev, 0); >>>> if (pwrkey->irq < 0) >>>> @@ -217,7 +291,14 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) >>>> error = regmap_read(pwrkey->regmap, pwrkey->baseaddr + PON_REV2, >>>> &pwrkey->revision); >>>> if (error) { >>>> - dev_err(&pdev->dev, "failed to set debounce: %d\n", error); >>>> + dev_err(&pdev->dev, "failed to read revision: %d\n", error); >>> Nice error message fix! > This can be split off to a different patch as well. Will do. >>>> + return error; >>>> + } >>>> + >>>> + error = regmap_read(pwrkey->regmap, pwrkey->baseaddr + PON_SUBTYPE, >>>> + &pwrkey->subtype); >>>> + if (error) { >>>> + dev_err(&pdev->dev, "failed to read subtype: %d\n", error); >>>> return error; >>>> } >>>> >>>> @@ -255,6 +336,12 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) >>>> } >>>> } >>>> >>>> + if (pwrkey->data->needs_sw_debounce) { >>>> + error = pm8941_pwrkey_sw_debounce_init(pwrkey); >>>> + if (error) >>>> + return error; >>>> + } >>>> + >>>> if (pwrkey->data->pull_up_bit) { >>>> error = regmap_update_bits(pwrkey->regmap, >>>> pwrkey->baseaddr + PON_PULL_CTL, >>>> @@ -316,6 +403,8 @@ static const struct pm8941_data pwrkey_data = { >>>> .phys = "pm8941_pwrkey/input0", >>>> .supports_ps_hold_poff_config = true, >>>> .supports_debounce_config = true, >>>> + .needs_sw_debounce = true, >>> needs_sw_debounce is always true? Why is it even an option then? >> As of right now the "needs_sw_debounce" property is being used for a sw work around for a hw >> problem. We anticipate that chips in the future will fix this hw problem and we would then have >> devices where needs_sw_debounce would be set to false. > Hmm ok. Why can't future chips be supported in this series? What happens > if nobody ever adds support for the new chips? We're left with this > condition that looks like dead code. Sure, makes sense. Will remove "needs_sw_debounce" property for V2.
diff --git a/drivers/input/misc/pm8941-pwrkey.c b/drivers/input/misc/pm8941-pwrkey.c index 33609603245d..b912ce00ce1c 100644 --- a/drivers/input/misc/pm8941-pwrkey.c +++ b/drivers/input/misc/pm8941-pwrkey.c @@ -9,9 +9,11 @@ #include <linux/input.h> #include <linux/interrupt.h> #include <linux/kernel.h> +#include <linux/ktime.h> #include <linux/log2.h> #include <linux/module.h> #include <linux/of.h> +#include <linux/of_address.h> #include <linux/of_device.h> #include <linux/platform_device.h> #include <linux/reboot.h> @@ -19,6 +21,16 @@ #define PON_REV2 0x01 +#define PON_SUBTYPE 0x05 + +#define PON_SUBTYPE_PRIMARY 0x01 +#define PON_SUBTYPE_SECONDARY 0x02 +#define PON_SUBTYPE_1REG 0x03 +#define PON_SUBTYPE_GEN2_PRIMARY 0x04 +#define PON_SUBTYPE_GEN2_SECONDARY 0x05 +#define PON_SUBTYPE_GEN3_PBS 0x08 +#define PON_SUBTYPE_GEN3_HLOS 0x09 + #define PON_RT_STS 0x10 #define PON_KPDPWR_N_SET BIT(0) #define PON_RESIN_N_SET BIT(1) @@ -44,6 +56,8 @@ struct pm8941_data { unsigned int status_bit; bool supports_ps_hold_poff_config; bool supports_debounce_config; + bool needs_sw_debounce; + bool has_pon_pbs; const char *name; const char *phys; }; @@ -52,13 +66,17 @@ struct pm8941_pwrkey { struct device *dev; int irq; u32 baseaddr; + u32 pon_pbs_baseaddr; struct regmap *regmap; struct input_dev *input; unsigned int revision; + unsigned int subtype; struct notifier_block reboot_notifier; u32 code; + u32 sw_debounce_time_us; + ktime_t last_release_time; const struct pm8941_data *data; }; @@ -126,19 +144,65 @@ static irqreturn_t pm8941_pwrkey_irq(int irq, void *_data) struct pm8941_pwrkey *pwrkey = _data; unsigned int sts; int error; + u64 elapsed_us; + + if (pwrkey->sw_debounce_time_us) { + elapsed_us = ktime_us_delta(ktime_get(), + pwrkey->last_release_time); + if (elapsed_us < pwrkey->sw_debounce_time_us) { + dev_dbg(pwrkey->dev, "ignoring key event received after %llu us, debounce time=%u us\n", + elapsed_us, pwrkey->sw_debounce_time_us); + return IRQ_HANDLED; + } + } error = regmap_read(pwrkey->regmap, pwrkey->baseaddr + PON_RT_STS, &sts); if (error) return IRQ_HANDLED; - input_report_key(pwrkey->input, pwrkey->code, - sts & pwrkey->data->status_bit); + sts &= pwrkey->data->status_bit; + + if (pwrkey->sw_debounce_time_us && !sts) + pwrkey->last_release_time = ktime_get(); + + input_report_key(pwrkey->input, pwrkey->code, sts); input_sync(pwrkey->input); return IRQ_HANDLED; } +static int pm8941_pwrkey_sw_debounce_init(struct pm8941_pwrkey *pwrkey) +{ + unsigned int val, addr; + int error; + + if (pwrkey->data->has_pon_pbs && !pwrkey->pon_pbs_baseaddr) { + dev_err(pwrkey->dev, "PON_PBS address missing, can't read HW debounce time\n"); + return 0; + } + + if (pwrkey->pon_pbs_baseaddr) + addr = pwrkey->pon_pbs_baseaddr + PON_DBC_CTL; + else + addr = pwrkey->baseaddr + PON_DBC_CTL; + error = regmap_read(pwrkey->regmap, addr, &val); + if (error) + return error; + + if (pwrkey->subtype >= PON_SUBTYPE_GEN2_PRIMARY) + pwrkey->sw_debounce_time_us = 2 * USEC_PER_SEC / + (1 << (0xf - (val & 0xf))); + else + pwrkey->sw_debounce_time_us = 2 * USEC_PER_SEC / + (1 << (0x7 - (val & 0x7))); + + dev_dbg(pwrkey->dev, "SW debounce time = %u us\n", + pwrkey->sw_debounce_time_us); + + return 0; +} + static int __maybe_unused pm8941_pwrkey_suspend(struct device *dev) { struct pm8941_pwrkey *pwrkey = dev_get_drvdata(dev); @@ -167,6 +231,8 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) struct pm8941_pwrkey *pwrkey; bool pull_up; struct device *parent; + struct device_node *regmap_node; + const __be32 *addr; u32 req_delay; int error; @@ -188,8 +254,10 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) pwrkey->data = of_device_get_match_data(&pdev->dev); parent = pdev->dev.parent; + regmap_node = pdev->dev.of_node; pwrkey->regmap = dev_get_regmap(parent, NULL); if (!pwrkey->regmap) { + regmap_node = parent->of_node; /* * We failed to get regmap for parent. Let's see if we are * a child of pon node and read regmap and reg from its @@ -200,15 +268,21 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) dev_err(&pdev->dev, "failed to locate regmap\n"); return -ENODEV; } + } - error = of_property_read_u32(parent->of_node, - "reg", &pwrkey->baseaddr); - } else { - error = of_property_read_u32(pdev->dev.of_node, "reg", - &pwrkey->baseaddr); + addr = of_get_address(regmap_node, 0, NULL, NULL); + if (!addr) { + dev_err(&pdev->dev, "reg property missing\n"); + return -EINVAL; + } + pwrkey->baseaddr = be32_to_cpu(*addr); + + if (pwrkey->data->has_pon_pbs) { + /* PON_PBS base address is optional */ + addr = of_get_address(regmap_node, 1, NULL, NULL); + if (addr) + pwrkey->pon_pbs_baseaddr = be32_to_cpu(*addr); } - if (error) - return error; pwrkey->irq = platform_get_irq(pdev, 0); if (pwrkey->irq < 0) @@ -217,7 +291,14 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) error = regmap_read(pwrkey->regmap, pwrkey->baseaddr + PON_REV2, &pwrkey->revision); if (error) { - dev_err(&pdev->dev, "failed to set debounce: %d\n", error); + dev_err(&pdev->dev, "failed to read revision: %d\n", error); + return error; + } + + error = regmap_read(pwrkey->regmap, pwrkey->baseaddr + PON_SUBTYPE, + &pwrkey->subtype); + if (error) { + dev_err(&pdev->dev, "failed to read subtype: %d\n", error); return error; } @@ -255,6 +336,12 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev) } } + if (pwrkey->data->needs_sw_debounce) { + error = pm8941_pwrkey_sw_debounce_init(pwrkey); + if (error) + return error; + } + if (pwrkey->data->pull_up_bit) { error = regmap_update_bits(pwrkey->regmap, pwrkey->baseaddr + PON_PULL_CTL, @@ -316,6 +403,8 @@ static const struct pm8941_data pwrkey_data = { .phys = "pm8941_pwrkey/input0", .supports_ps_hold_poff_config = true, .supports_debounce_config = true, + .needs_sw_debounce = true, + .has_pon_pbs = false, }; static const struct pm8941_data resin_data = { @@ -325,6 +414,8 @@ static const struct pm8941_data resin_data = { .phys = "pm8941_resin/input0", .supports_ps_hold_poff_config = true, .supports_debounce_config = true, + .needs_sw_debounce = true, + .has_pon_pbs = false, }; static const struct pm8941_data pon_gen3_pwrkey_data = { @@ -333,6 +424,8 @@ static const struct pm8941_data pon_gen3_pwrkey_data = { .phys = "pmic_pwrkey/input0", .supports_ps_hold_poff_config = false, .supports_debounce_config = false, + .needs_sw_debounce = true, + .has_pon_pbs = true, }; static const struct pm8941_data pon_gen3_resin_data = { @@ -341,6 +434,8 @@ static const struct pm8941_data pon_gen3_resin_data = { .phys = "pmic_resin/input0", .supports_ps_hold_poff_config = false, .supports_debounce_config = false, + .needs_sw_debounce = true, + .has_pon_pbs = true, }; static const struct of_device_id pm8941_pwr_key_id_table[] = {