| Message ID | 20220411200506.22891-1-quic_amelende@quicinc.com |
|---|---|
| Headers | show |
| Series | Extend pm8941-pwrkey driver | expand |
On Mon, Apr 11, 2022 at 01:05:03PM -0700, Anjelique Melendez wrote: > From: David Collins <quic_collinsd@quicinc.com> > > Update the description of "reg" property to add the PON_PBS base > address along with PON_HLOS base address. Also add "reg-names" > property description. > > Signed-off-by: David Collins <quic_collinsd@quicinc.com> > Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com> > --- > .../bindings/power/reset/qcom,pon.yaml | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml > index 353f155d..542200b2 100644 > --- a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml > +++ b/Documentation/bindings/power/reset/qcom,pon.yaml > @@ -26,7 +26,25 @@ properties: > - qcom,pm8998-pon > > reg: > - maxItems: 1 > + description: | > + Specifies the SPMI base address for the PON (power-on) peripheral. For > + PMICs that have the PON peripheral (GEN3) split into PON_HLOS and PON_PBS > + (e.g. PMK8350), this can hold addresses of both PON_HLOS and PON_PBS > + peripherals. In that case, the PON_PBS address needs to be specified to > + facilitate software debouncing on some PMICs. > + minItems: 1 > + maxItems: 2 > + > + reg-names: > + description: | > + For PON GEN1 and GEN2, it should be "pon". For PON GEN3 it should include > + "pon_hlos" and optionally "pon_pbs". > + minItems: 1 > + maxItems: 2 > + items: > + - const: pon_hlos > + - const: pon_pbs > + - const: pon This says there are 3 entries, but you limited to 2. The schema also doesn't match what the description says. Entries should be extended by adding new entries to the end and keeping optional entries last. So like this: minItems: 1 items: - const: pon - const: pon_hlos - const: pon_pbs Rob
On 11/04/2022 23:05, Anjelique Melendez wrote: > Currently, PON address is read from the "reg" property. For PON GEN3, > which starts with PMK8350, the "reg" property will have both the PON > HLOS and PON PBS addesses defined. Add support so that all PON > generations can be configured. > > Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com> > Reviewed-by: Stephen Boyd <swboyd@chromium.org> > --- > drivers/input/misc/pm8941-pwrkey.c | 31 +++++++++++++++++++++++------- > 1 file changed, 24 insertions(+), 7 deletions(-) > > diff --git a/drivers/input/misc/pm8941-pwrkey.c b/drivers/input/misc/pm8941-pwrkey.c > index e0240db12d4f..43106e4cfd23 100644 > --- a/drivers/input/misc/pm8941-pwrkey.c > +++ b/drivers/input/misc/pm8941-pwrkey.c > @@ -12,6 +12,7 @@ > #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> > @@ -44,6 +45,7 @@ struct pm8941_data { > unsigned int status_bit; > bool supports_ps_hold_poff_config; > bool supports_debounce_config; > + bool has_pon_pbs; > const char *name; > const char *phys; > }; > @@ -52,6 +54,7 @@ struct pm8941_pwrkey { > struct device *dev; > int irq; > u32 baseaddr; > + u32 pon_pbs_baseaddr; > struct regmap *regmap; > struct input_dev *input; > > @@ -167,6 +170,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 +193,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 +207,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_cpup(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_cpup(addr); > } > - if (error) > - return error; > > pwrkey->irq = platform_get_irq(pdev, 0); > if (pwrkey->irq < 0) > @@ -316,6 +329,7 @@ static const struct pm8941_data pwrkey_data = { > .phys = "pm8941_pwrkey/input0", > .supports_ps_hold_poff_config = true, > .supports_debounce_config = true, > + .has_pon_pbs = false, > }; > > static const struct pm8941_data resin_data = { > @@ -325,6 +339,7 @@ static const struct pm8941_data resin_data = { > .phys = "pm8941_resin/input0", > .supports_ps_hold_poff_config = true, > .supports_debounce_config = true, > + .has_pon_pbs = false, No need to declare that a field is false. Just skip this completely. > }; > > static const struct pm8941_data pon_gen3_pwrkey_data = { > @@ -333,6 +348,7 @@ static const struct pm8941_data pon_gen3_pwrkey_data = { > .phys = "pmic_pwrkey/input0", > .supports_ps_hold_poff_config = false, > .supports_debounce_config = false, > + .has_pon_pbs = true, > }; > > static const struct pm8941_data pon_gen3_resin_data = { > @@ -341,6 +357,7 @@ static const struct pm8941_data pon_gen3_resin_data = { > .phys = "pmic_resin/input0", > .supports_ps_hold_poff_config = false, > .supports_debounce_config = false, > + .has_pon_pbs = true, > }; > > static const struct of_device_id pm8941_pwr_key_id_table[] = {
On 4/14/2022 8:34 AM, Rob Herring wrote: > On Mon, Apr 11, 2022 at 01:05:03PM -0700, Anjelique Melendez wrote: >> From: David Collins <quic_collinsd@quicinc.com> >> >> Update the description of "reg" property to add the PON_PBS base >> address along with PON_HLOS base address. Also add "reg-names" >> property description. >> >> Signed-off-by: David Collins <quic_collinsd@quicinc.com> >> Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com> >> --- >> .../bindings/power/reset/qcom,pon.yaml | 20 +++++++++++++++++++- >> 1 file changed, 19 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml >> index 353f155d..542200b2 100644 >> --- a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml >> +++ b/Documentation/bindings/power/reset/qcom,pon.yaml >> @@ -26,7 +26,25 @@ properties: >> - qcom,pm8998-pon >> >> reg: >> - maxItems: 1 >> + description: | >> + Specifies the SPMI base address for the PON (power-on) peripheral. For >> + PMICs that have the PON peripheral (GEN3) split into PON_HLOS and PON_PBS >> + (e.g. PMK8350), this can hold addresses of both PON_HLOS and PON_PBS >> + peripherals. In that case, the PON_PBS address needs to be specified to >> + facilitate software debouncing on some PMICs. >> + minItems: 1 >> + maxItems: 2 >> + >> + reg-names: >> + description: | >> + For PON GEN1 and GEN2, it should be "pon". For PON GEN3 it should include >> + "pon_hlos" and optionally "pon_pbs". >> + minItems: 1 >> + maxItems: 2 >> + items: >> + - const: pon_hlos >> + - const: pon_pbs >> + - const: pon > > This says there are 3 entries, but you limited to 2. The schema also > doesn't match what the description says. Entries should be extended by > adding new entries to the end and keeping optional entries last. So like > this: > > minItems: 1 > items: > - const: pon > - const: pon_hlos > - const: pon_pbs > > RobWill update in next patch. Thanks
On 4/14/2022 2:18 PM, Dmitry Baryshkov wrote: > On 11/04/2022 23:05, Anjelique Melendez wrote: >> Currently, PON address is read from the "reg" property. For PON GEN3, >> which starts with PMK8350, the "reg" property will have both the PON >> HLOS and PON PBS addesses defined. Add support so that all PON >> generations can be configured. >> >> Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com> >> Reviewed-by: Stephen Boyd <swboyd@chromium.org> >> --- >> drivers/input/misc/pm8941-pwrkey.c | 31 +++++++++++++++++++++++------- >> 1 file changed, 24 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/input/misc/pm8941-pwrkey.c b/drivers/input/misc/pm8941-pwrkey.c >> index e0240db12d4f..43106e4cfd23 100644 >> --- a/drivers/input/misc/pm8941-pwrkey.c >> +++ b/drivers/input/misc/pm8941-pwrkey.c >> @@ -12,6 +12,7 @@ >> #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> >> @@ -44,6 +45,7 @@ struct pm8941_data { >> unsigned int status_bit; >> bool supports_ps_hold_poff_config; >> bool supports_debounce_config; >> + bool has_pon_pbs; >> const char *name; >> const char *phys; >> }; >> @@ -52,6 +54,7 @@ struct pm8941_pwrkey { >> struct device *dev; >> int irq; >> u32 baseaddr; >> + u32 pon_pbs_baseaddr; >> struct regmap *regmap; >> struct input_dev *input; >> @@ -167,6 +170,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 +193,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 +207,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_cpup(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_cpup(addr); >> } >> - if (error) >> - return error; >> pwrkey->irq = platform_get_irq(pdev, 0); >> if (pwrkey->irq < 0) >> @@ -316,6 +329,7 @@ static const struct pm8941_data pwrkey_data = { >> .phys = "pm8941_pwrkey/input0", >> .supports_ps_hold_poff_config = true, >> .supports_debounce_config = true, >> + .has_pon_pbs = false, >> }; >> static const struct pm8941_data resin_data = { >> @@ -325,6 +339,7 @@ static const struct pm8941_data resin_data = { >> .phys = "pm8941_resin/input0", >> .supports_ps_hold_poff_config = true, >> .supports_debounce_config = true, >> + .has_pon_pbs = false, > > No need to declare that a field is false. Just skip this completely. ACK. Will get rid in next version. > >> }; >> static const struct pm8941_data pon_gen3_pwrkey_data = { >> @@ -333,6 +348,7 @@ static const struct pm8941_data pon_gen3_pwrkey_data = { >> .phys = "pmic_pwrkey/input0", >> .supports_ps_hold_poff_config = false, >> .supports_debounce_config = false, >> + .has_pon_pbs = true, >> }; >> static const struct pm8941_data pon_gen3_resin_data = { >> @@ -341,6 +357,7 @@ static const struct pm8941_data pon_gen3_resin_data = { >> .phys = "pmic_resin/input0", >> .supports_ps_hold_poff_config = false, >> .supports_debounce_config = false, >> + .has_pon_pbs = true, >> }; >> static const struct of_device_id pm8941_pwr_key_id_table[] = { > >
Changes from v4: - Added new dt-binding patch as 1/5 Changes from v3: - Fixed dereference issue in 2/4 - Added Stephen's reviewed by tag for 2/4 Changes from v2: - Addressed Stephen's comments - Add Stephen's reviewed by tag for 1/4 - Fixed style for 2/4 - Corrected function call to use correct function for 3/4 Changes from v1: - Removed Change-Id from all patches - Updated subject line of cover letter - Addressed Stephen's comments for v1 1/3 - Separated error message fix to own patch (v2 1/4) - Separated PON GEN3 base address changes to own patch (v2 2/4) - Added new variables and functions to make code more readable - Removed v1 3/3 as per Bjorn's comments Anjelique Melendez (2): input: misc: pm8941-pwrkey: fix error message input: misc: pm8941-pwrkey: add support for PON GEN3 base addresses David Collins (3): dt-bindings: power: reset: qcom-pon: update "reg" property details input: misc: pm8941-pwrkey: add software key press debouncing support input: misc: pm8941-pwrkey: simulate missed key press events Documentation/devicetree/bindings/power/reset/qcom,pon.yaml | 20 +++++++++++++++++++- drivers/input/misc/pm8941-pwrkey.c | 124 +++++++++++++++++++++++++---- 2 files changed, 129 insertions(+), 15 deletions(-)