* [PATCH 1/2 v2] device-tree: nexus7-flo: Remove power gpio key entry and use pmic8xxx-pwrkey @ 2016-04-14 21:07 John Stultz 2016-04-14 21:07 ` [PATCH 2/2] device-tree: nexus7: Add bq27541 battery interface to dts John Stultz ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: John Stultz @ 2016-04-14 21:07 UTC (permalink / raw) To: lkml Cc: John Stultz, Rob Herring, Arnd Bergmann, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Andy Gross, Vinay Simha BN, Bjorn Andersson, Stephen Boyd, linux-arm-msm, devicetree Since the pmic8xxx-pwrkey driver is already supported in the qcom-apq8064.dtsi, and the pmic8xxx-pwrkey supports logic to configure proper device shutdown when ps_hold goes low, it is better to use that driver then a generic gpio button. Thus this patch remove the gpio power key entry here, so we don't get double input events from having two drivers enabled. The one gotcha with the pmic8xxx-pwrkey is it has a fairly long debounce delay, which we shorten here to make the button behave as expected. Cc: Rob Herring <robh+dt@kernel.org> Cc: Arnd Bergmann <arnd.bergmann@linaro.org> Cc: Pawel Moll <pawel.moll@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk> Cc: Kumar Gala <galak@codeaurora.org> Cc: Andy Gross <agross@codeaurora.org> Cc: Vinay Simha BN <simhavcs@gmail.com> Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Cc: Stephen Boyd <stephen.boyd@linaro.org> Cc: linux-arm-msm@vger.kernel.org Cc: devicetree@vger.kernel.org Signed-off-by: John Stultz <john.stultz@linaro.org> --- v2: - Add wakeup-source entry as suggested by Sudeep Holla <sudeep.holla@arm.com> arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts index c535b3f..15da084 100644 --- a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts +++ b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts @@ -29,12 +29,6 @@ gpio-keys { compatible = "gpio-keys"; - power { - label = "Power"; - gpios = <&tlmm_pinmux 26 GPIO_ACTIVE_LOW>; - linux,code = <KEY_POWER>; - gpio-key,wakeup; - }; volume_up { label = "Volume Up"; gpios = <&pm8921_gpio 4 GPIO_ACTIVE_HIGH>; @@ -190,6 +184,16 @@ }; }; + /* override default debounce for power-key */ + qcom,ssbi@500000 { + pmic@0 { + pwrkey@1c { + debounce = <1>; + wakeup-source; + }; + }; + }; + gsbi@16200000 { status = "okay"; qcom,mode = <GSBI_PROT_I2C>; -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] device-tree: nexus7: Add bq27541 battery interface to dts 2016-04-14 21:07 [PATCH 1/2 v2] device-tree: nexus7-flo: Remove power gpio key entry and use pmic8xxx-pwrkey John Stultz @ 2016-04-14 21:07 ` John Stultz [not found] ` <1460668031-12384-2-git-send-email-john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2016-04-15 17:23 ` Bjorn Andersson 2016-04-15 16:57 ` [PATCH 1/2 v2] device-tree: nexus7-flo: Remove power gpio key entry and use pmic8xxx-pwrkey Rob Herring 2016-04-15 17:22 ` Bjorn Andersson 2 siblings, 2 replies; 10+ messages in thread From: John Stultz @ 2016-04-14 21:07 UTC (permalink / raw) To: lkml Cc: John Stultz, Rob Herring, Arnd Bergmann, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Andy Gross, Vinay Simha BN, Bjorn Andersson, Stephen Boyd, linux-arm-msm, devicetree Add support for battery level reading on the Nexus7 by enabling the bq27541 driver in the nexus7 dts Cc: Rob Herring <robh+dt@kernel.org> Cc: Arnd Bergmann <arnd.bergmann@linaro.org> Cc: Pawel Moll <pawel.moll@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk> Cc: Kumar Gala <galak@codeaurora.org> Cc: Andy Gross <agross@codeaurora.org> Cc: Vinay Simha BN <simhavcs@gmail.com> Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Cc: Stephen Boyd <stephen.boyd@linaro.org> Cc: linux-arm-msm@vger.kernel.org Cc: devicetree@vger.kernel.org Signed-off-by: John Stultz <john.stultz@linaro.org> --- arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts index 15da084..b9028ab 100644 --- a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts +++ b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts @@ -228,6 +228,12 @@ reg = <0x52>; pagesize = <32>; }; + + bq27541@55 { + compatible = "ti,bq27541"; + reg = <0x55>; + }; + }; }; -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <1460668031-12384-2-git-send-email-john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 2/2] device-tree: nexus7: Add bq27541 battery interface to dts [not found] ` <1460668031-12384-2-git-send-email-john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> @ 2016-04-15 16:57 ` Rob Herring 0 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2016-04-15 16:57 UTC (permalink / raw) To: John Stultz Cc: lkml, Arnd Bergmann, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Andy Gross, Vinay Simha BN, Bjorn Andersson, Stephen Boyd, linux-arm-msm, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On Thu, Apr 14, 2016 at 4:07 PM, John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote: > Add support for battery level reading on the Nexus7 by > enabling the bq27541 driver in the nexus7 dts > > Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > Cc: Arnd Bergmann <arnd.bergmann-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org> > Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> > Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org> > Cc: Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> > Cc: Andy Gross <agross-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> > Cc: Vinay Simha BN <simhavcs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > Cc: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > Cc: Stephen Boyd <stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > Signed-off-by: John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > --- > arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts | 6 ++++++ > 1 file changed, 6 insertions(+) Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] device-tree: nexus7: Add bq27541 battery interface to dts 2016-04-14 21:07 ` [PATCH 2/2] device-tree: nexus7: Add bq27541 battery interface to dts John Stultz [not found] ` <1460668031-12384-2-git-send-email-john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> @ 2016-04-15 17:23 ` Bjorn Andersson 1 sibling, 0 replies; 10+ messages in thread From: Bjorn Andersson @ 2016-04-15 17:23 UTC (permalink / raw) To: John Stultz Cc: lkml, Rob Herring, Arnd Bergmann, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Andy Gross, Vinay Simha BN, Stephen Boyd, linux-arm-msm, devicetree On Thu 14 Apr 14:07 PDT 2016, John Stultz wrote: > Add support for battery level reading on the Nexus7 by > enabling the bq27541 driver in the nexus7 dts > [..] Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2 v2] device-tree: nexus7-flo: Remove power gpio key entry and use pmic8xxx-pwrkey 2016-04-14 21:07 [PATCH 1/2 v2] device-tree: nexus7-flo: Remove power gpio key entry and use pmic8xxx-pwrkey John Stultz 2016-04-14 21:07 ` [PATCH 2/2] device-tree: nexus7: Add bq27541 battery interface to dts John Stultz @ 2016-04-15 16:57 ` Rob Herring 2016-04-15 17:22 ` Bjorn Andersson 2 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2016-04-15 16:57 UTC (permalink / raw) To: John Stultz Cc: lkml, Arnd Bergmann, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Andy Gross, Vinay Simha BN, Bjorn Andersson, Stephen Boyd, linux-arm-msm, devicetree@vger.kernel.org On Thu, Apr 14, 2016 at 4:07 PM, John Stultz <john.stultz@linaro.org> wrote: > Since the pmic8xxx-pwrkey driver is already supported in the > qcom-apq8064.dtsi, and the pmic8xxx-pwrkey supports logic to > configure proper device shutdown when ps_hold goes low, it is > better to use that driver then a generic gpio button. > > Thus this patch remove the gpio power key entry here, so we > don't get double input events from having two drivers enabled. > > The one gotcha with the pmic8xxx-pwrkey is it has a fairly > long debounce delay, which we shorten here to make the button > behave as expected. > > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Arnd Bergmann <arnd.bergmann@linaro.org> > Cc: Pawel Moll <pawel.moll@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk> > Cc: Kumar Gala <galak@codeaurora.org> > Cc: Andy Gross <agross@codeaurora.org> > Cc: Vinay Simha BN <simhavcs@gmail.com> > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Stephen Boyd <stephen.boyd@linaro.org> > Cc: linux-arm-msm@vger.kernel.org > Cc: devicetree@vger.kernel.org > Signed-off-by: John Stultz <john.stultz@linaro.org> > --- > v2: > - Add wakeup-source entry as suggested by > Sudeep Holla <sudeep.holla@arm.com> > > arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) Acked-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2 v2] device-tree: nexus7-flo: Remove power gpio key entry and use pmic8xxx-pwrkey 2016-04-14 21:07 [PATCH 1/2 v2] device-tree: nexus7-flo: Remove power gpio key entry and use pmic8xxx-pwrkey John Stultz 2016-04-14 21:07 ` [PATCH 2/2] device-tree: nexus7: Add bq27541 battery interface to dts John Stultz 2016-04-15 16:57 ` [PATCH 1/2 v2] device-tree: nexus7-flo: Remove power gpio key entry and use pmic8xxx-pwrkey Rob Herring @ 2016-04-15 17:22 ` Bjorn Andersson 2016-04-15 18:59 ` Stephen Boyd 2 siblings, 1 reply; 10+ messages in thread From: Bjorn Andersson @ 2016-04-15 17:22 UTC (permalink / raw) To: John Stultz, Stephen Boyd Cc: lkml, Rob Herring, Arnd Bergmann, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Andy Gross, Vinay Simha BN, linux-arm-msm, devicetree On Thu 14 Apr 14:07 PDT 2016, John Stultz wrote: > Since the pmic8xxx-pwrkey driver is already supported in the > qcom-apq8064.dtsi, and the pmic8xxx-pwrkey supports logic to > configure proper device shutdown when ps_hold goes low, it is > better to use that driver then a generic gpio button. > > Thus this patch remove the gpio power key entry here, so we > don't get double input events from having two drivers enabled. > This part has my ack. > The one gotcha with the pmic8xxx-pwrkey is it has a fairly > long debounce delay, which we shorten here to make the button > behave as expected. > It's set to 15ms, so this sounds like a bug. > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Arnd Bergmann <arnd.bergmann@linaro.org> > Cc: Pawel Moll <pawel.moll@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk> > Cc: Kumar Gala <galak@codeaurora.org> > Cc: Andy Gross <agross@codeaurora.org> > Cc: Vinay Simha BN <simhavcs@gmail.com> > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Stephen Boyd <stephen.boyd@linaro.org> > Cc: linux-arm-msm@vger.kernel.org > Cc: devicetree@vger.kernel.org You don't have to mention everyone here... > Signed-off-by: John Stultz <john.stultz@linaro.org> > --- > v2: > - Add wakeup-source entry as suggested by > Sudeep Holla <sudeep.holla@arm.com> > > arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts [..] > @@ -190,6 +184,16 @@ > }; > }; > > + /* override default debounce for power-key */ > + qcom,ssbi@500000 { > + pmic@0 { > + pwrkey@1c { > + debounce = <1>; The debounce is specified in microseconds, so if the 15625us that's specified in the dtsi is too much for you we have a bug in the driver. Further, comparing the math with downstream indicates that we're quite off. Could you please try the downstream calculation of "delay", by changing pmic8xxx_pwrkey_probe() to include: delay = (kpb_delay << 6) / USEC_PER_SEC; delay = ilog2(delay); Unfortunately I don't have the register documentation for this pmic. Stephen, can you shed some light on the trig-delay (what I presume is the bark timer in later versions) bits in PON_CNTRL_1 (0x1c) on PM8921. > + wakeup-source; The driver already enables wakeup on itself, so I don't think this should be necessary (i.e. you should be able to drop this entire node from the dts). > + }; > + }; > + }; > + Regards, Bjorn ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2 v2] device-tree: nexus7-flo: Remove power gpio key entry and use pmic8xxx-pwrkey 2016-04-15 17:22 ` Bjorn Andersson @ 2016-04-15 18:59 ` Stephen Boyd 2016-04-15 21:58 ` John Stultz 2016-04-15 22:01 ` Bjorn Andersson 0 siblings, 2 replies; 10+ messages in thread From: Stephen Boyd @ 2016-04-15 18:59 UTC (permalink / raw) To: Bjorn Andersson, Dmitry Torokhov Cc: John Stultz, Stephen Boyd, lkml, Rob Herring, Arnd Bergmann, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Andy Gross, Vinay Simha BN, linux-arm-msm, devicetree, linux-input On 04/15, Bjorn Andersson wrote: > On Thu 14 Apr 14:07 PDT 2016, John Stultz wrote: > > > Signed-off-by: John Stultz <john.stultz@linaro.org> > > --- > > v2: > > - Add wakeup-source entry as suggested by > > Sudeep Holla <sudeep.holla@arm.com> > > > > arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts | 16 ++++++++++------ > > 1 file changed, 10 insertions(+), 6 deletions(-) > > > > diff --git a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts > [..] > > @@ -190,6 +184,16 @@ > > }; > > }; > > > > + /* override default debounce for power-key */ > > + qcom,ssbi@500000 { > > + pmic@0 { > > + pwrkey@1c { > > + debounce = <1>; > > The debounce is specified in microseconds, so if the 15625us that's > specified in the dtsi is too much for you we have a bug in the driver. > > Further, comparing the math with downstream indicates that we're quite > off. > > Could you please try the downstream calculation of "delay", by changing > pmic8xxx_pwrkey_probe() to include: > > delay = (kpb_delay << 6) / USEC_PER_SEC; > delay = ilog2(delay); > > Unfortunately I don't have the register documentation for this pmic. > > Stephen, can you shed some light on the trig-delay (what I presume is > the bark timer in later versions) bits in PON_CNTRL_1 (0x1c) on PM8921. It looks like this driver needs some love! qcom has layered these patches on top of the upstream submission (hashes from msm-3.10 branch): 0fba556b7b95 input: pmic8xxx-pwrkey: Support sysfs interface for debounce f042aa6efec6 input: pm8xxx-pwrkey: Update key press status during probe 0b58468eb5ca input: pwrkey: Handle out-of-order press and release interrupts 2099d2368cb5 input: pmic8xxx-pwrkey: Keep pdata after probe 4ecfcdf235de input: pm8xxx-pwrkey: Move from threaded irq to any-context irq dfed28404288 input: pmic8xxx-pwrkey: Change algorithm on converting trigger delay That last one is the one you want here, although f042aa6efec6 looks interesting too. Anyway, the equation for the lower 3 bits for the trigger delay is: delay (Seconds) = (1 / 1024) * 2 ^ (x + 4) where x is the 3 bits. With so few bits we only have 8 delays, ranging from 2 seconds to 1/64 seconds (16/1024 == 1/64). Funny thing, I looked back on some older PMICs and the documentation usually has the same equation. Except for this one document that seems to have messed up the formatting for the power of 2 part so the equation looks like: delay (Seconds) = (1 / 1024) * 2 x + 4 Maybe the driver was written to this equation, but I'm willing to bet that the documentation is wrong here. I seriously doubt the power key would change like this! Alright, here's the patch which should make the debounce actually work. ----8<----- From: Stephen Boyd <sboyd@codeaurora.org> Subject: [PATCH] Input: pmic8xxx-pwrkey: Fix algorithm for converting trigger delay The trigger delay algorithm that converts from microseconds to the register value looks incorrect. According to most of the PMIC documentation, the equation is delay (Seconds) = (1 / 1024) * 2 ^ (x + 4) except for one case where the documentation looks to have a formatting issue and the equation looks like delay (Seconds) = (1 / 1024) * 2 x + 4 Most likely this driver was written with the improper documentation to begin with. According to the downstream sources the valid delays are from 2 seconds to 1/64 second, and the latter equation just doesn't make sense for that. Let's fix the algorithm and the range check to match the documentation and the downstream sources. Reported-by: Bjorn Andersson <bjorn.andersson@linaro.org> Cc: John Stultz <john.stultz@linaro.org> Fixes: 92d57a73e410 ("input: Add support for Qualcomm PMIC8XXX power key") Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/input/misc/pmic8xxx-pwrkey.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/input/misc/pmic8xxx-pwrkey.c b/drivers/input/misc/pmic8xxx-pwrkey.c index 3f02e0e03d12..67aab86048ad 100644 --- a/drivers/input/misc/pmic8xxx-pwrkey.c +++ b/drivers/input/misc/pmic8xxx-pwrkey.c @@ -353,7 +353,8 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev) if (of_property_read_u32(pdev->dev.of_node, "debounce", &kpd_delay)) kpd_delay = 15625; - if (kpd_delay > 62500 || kpd_delay == 0) { + /* Valid range of pwr key trigger delay is 1/64 sec to 2 seconds. */ + if (kpd_delay > USEC_PER_SEC * 2 || kpd_delay < USEC_PER_SEC / 64) { dev_err(&pdev->dev, "invalid power key trigger delay\n"); return -EINVAL; } @@ -385,8 +386,8 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev) pwr->name = "pmic8xxx_pwrkey"; pwr->phys = "pmic8xxx_pwrkey/input0"; - delay = (kpd_delay << 10) / USEC_PER_SEC; - delay = 1 + ilog2(delay); + delay = (kpd_delay << 6) / USEC_PER_SEC; + delay = ilog2(delay); err = regmap_read(regmap, PON_CNTL_1, &pon_cntl); if (err < 0) { -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2 v2] device-tree: nexus7-flo: Remove power gpio key entry and use pmic8xxx-pwrkey 2016-04-15 18:59 ` Stephen Boyd @ 2016-04-15 21:58 ` John Stultz 2016-04-15 22:01 ` Bjorn Andersson 1 sibling, 0 replies; 10+ messages in thread From: John Stultz @ 2016-04-15 21:58 UTC (permalink / raw) To: Stephen Boyd Cc: Bjorn Andersson, Dmitry Torokhov, Stephen Boyd, lkml, Rob Herring, Arnd Bergmann, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Andy Gross, Vinay Simha BN, linux-arm-msm@vger.kernel.org, devicetree, linux-input On Fri, Apr 15, 2016 at 11:59 AM, Stephen Boyd <sboyd@codeaurora.org> wrote: > From: Stephen Boyd <sboyd@codeaurora.org> > Subject: [PATCH] Input: pmic8xxx-pwrkey: Fix algorithm for converting trigger > delay > > The trigger delay algorithm that converts from microseconds to > the register value looks incorrect. According to most of the PMIC > documentation, the equation is > > delay (Seconds) = (1 / 1024) * 2 ^ (x + 4) > > except for one case where the documentation looks to have a > formatting issue and the equation looks like > > delay (Seconds) = (1 / 1024) * 2 x + 4 > > Most likely this driver was written with the improper > documentation to begin with. According to the downstream sources > the valid delays are from 2 seconds to 1/64 second, and the > latter equation just doesn't make sense for that. Let's fix the > algorithm and the range check to match the documentation and the > downstream sources. > > Reported-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: John Stultz <john.stultz@linaro.org> > Fixes: 92d57a73e410 ("input: Add support for Qualcomm PMIC8XXX power key") > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> This works great for me, and lets me zap the pwrkey node in my dts. Tested-by: John Stultz <john.stultz@linaro.org> I'll also send an updated variant of my patch that only removes the gpio key. thanks! -john ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2 v2] device-tree: nexus7-flo: Remove power gpio key entry and use pmic8xxx-pwrkey 2016-04-15 18:59 ` Stephen Boyd 2016-04-15 21:58 ` John Stultz @ 2016-04-15 22:01 ` Bjorn Andersson 2016-04-17 12:24 ` Dmitry Torokhov 1 sibling, 1 reply; 10+ messages in thread From: Bjorn Andersson @ 2016-04-15 22:01 UTC (permalink / raw) To: Stephen Boyd Cc: Dmitry Torokhov, John Stultz, Stephen Boyd, lkml, Rob Herring, Arnd Bergmann, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Andy Gross, Vinay Simha BN, linux-arm-msm, devicetree, linux-input On Fri 15 Apr 11:59 PDT 2016, Stephen Boyd wrote: [..] > ----8<----- > From: Stephen Boyd <sboyd@codeaurora.org> > Subject: [PATCH] Input: pmic8xxx-pwrkey: Fix algorithm for converting trigger > delay > > The trigger delay algorithm that converts from microseconds to > the register value looks incorrect. According to most of the PMIC > documentation, the equation is > > delay (Seconds) = (1 / 1024) * 2 ^ (x + 4) > > except for one case where the documentation looks to have a > formatting issue and the equation looks like > > delay (Seconds) = (1 / 1024) * 2 x + 4 > > Most likely this driver was written with the improper > documentation to begin with. According to the downstream sources > the valid delays are from 2 seconds to 1/64 second, and the > latter equation just doesn't make sense for that. Let's fix the > algorithm and the range check to match the documentation and the > downstream sources. > > Reported-by: Bjorn Andersson <bjorn.andersson@linaro.org> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: John Stultz <john.stultz@linaro.org> > Fixes: 92d57a73e410 ("input: Add support for Qualcomm PMIC8XXX power key") > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> > --- > drivers/input/misc/pmic8xxx-pwrkey.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/input/misc/pmic8xxx-pwrkey.c b/drivers/input/misc/pmic8xxx-pwrkey.c > index 3f02e0e03d12..67aab86048ad 100644 > --- a/drivers/input/misc/pmic8xxx-pwrkey.c > +++ b/drivers/input/misc/pmic8xxx-pwrkey.c > @@ -353,7 +353,8 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev) > if (of_property_read_u32(pdev->dev.of_node, "debounce", &kpd_delay)) > kpd_delay = 15625; > > - if (kpd_delay > 62500 || kpd_delay == 0) { > + /* Valid range of pwr key trigger delay is 1/64 sec to 2 seconds. */ > + if (kpd_delay > USEC_PER_SEC * 2 || kpd_delay < USEC_PER_SEC / 64) { > dev_err(&pdev->dev, "invalid power key trigger delay\n"); > return -EINVAL; > } > @@ -385,8 +386,8 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev) > pwr->name = "pmic8xxx_pwrkey"; > pwr->phys = "pmic8xxx_pwrkey/input0"; > > - delay = (kpd_delay << 10) / USEC_PER_SEC; > - delay = 1 + ilog2(delay); > + delay = (kpd_delay << 6) / USEC_PER_SEC; > + delay = ilog2(delay); Regards, Bjorn ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2 v2] device-tree: nexus7-flo: Remove power gpio key entry and use pmic8xxx-pwrkey 2016-04-15 22:01 ` Bjorn Andersson @ 2016-04-17 12:24 ` Dmitry Torokhov 0 siblings, 0 replies; 10+ messages in thread From: Dmitry Torokhov @ 2016-04-17 12:24 UTC (permalink / raw) To: Bjorn Andersson Cc: Stephen Boyd, John Stultz, Stephen Boyd, lkml, Rob Herring, Arnd Bergmann, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Andy Gross, Vinay Simha BN, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA On Fri, Apr 15, 2016 at 03:01:06PM -0700, Bjorn Andersson wrote: > On Fri 15 Apr 11:59 PDT 2016, Stephen Boyd wrote: > > [..] > > > ----8<----- > > From: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> > > Subject: [PATCH] Input: pmic8xxx-pwrkey: Fix algorithm for converting trigger > > delay > > > > The trigger delay algorithm that converts from microseconds to > > the register value looks incorrect. According to most of the PMIC > > documentation, the equation is > > > > delay (Seconds) = (1 / 1024) * 2 ^ (x + 4) > > > > except for one case where the documentation looks to have a > > formatting issue and the equation looks like > > > > delay (Seconds) = (1 / 1024) * 2 x + 4 > > > > Most likely this driver was written with the improper > > documentation to begin with. According to the downstream sources > > the valid delays are from 2 seconds to 1/64 second, and the > > latter equation just doesn't make sense for that. Let's fix the > > algorithm and the range check to match the documentation and the > > downstream sources. > > > > Reported-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > > Acked-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Applied, thank you. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-04-17 12:24 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-14 21:07 [PATCH 1/2 v2] device-tree: nexus7-flo: Remove power gpio key entry and use pmic8xxx-pwrkey John Stultz 2016-04-14 21:07 ` [PATCH 2/2] device-tree: nexus7: Add bq27541 battery interface to dts John Stultz [not found] ` <1460668031-12384-2-git-send-email-john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2016-04-15 16:57 ` Rob Herring 2016-04-15 17:23 ` Bjorn Andersson 2016-04-15 16:57 ` [PATCH 1/2 v2] device-tree: nexus7-flo: Remove power gpio key entry and use pmic8xxx-pwrkey Rob Herring 2016-04-15 17:22 ` Bjorn Andersson 2016-04-15 18:59 ` Stephen Boyd 2016-04-15 21:58 ` John Stultz 2016-04-15 22:01 ` Bjorn Andersson 2016-04-17 12:24 ` Dmitry Torokhov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).