Re: [PATCH v5 3/5] input: misc: pm8941-pwrkey: add support for PON GEN3 base addresses

[Anjelique Melendez <quic_amelende@quicinc.com>]

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[] = {
> 
>