| Message ID | 20220120204132.17875-2-quic_amelende@quicinc.com |
|---|---|
| State | New |
| 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 = {
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[] = {