linux-kernel - Re: [PATCH v3 02/10] pinctrl: Ingenic: Add support for read the pin configuration of X1830.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a278847d-da2d-eeec-98d0-4bbca0b90cee@wanyeetech.com>
Date:   Thu, 25 Mar 2021 15:38:28 +0800
From:   Zhou Yanjie <zhouyanjie@...yeetech.com>
To:     Paul Cercueil <paul@...pouillou.net>
Cc:     linus.walleij@...aro.org, robh+dt@...nel.org,
        linux-mips@...r.kernel.org, linux-gpio@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        hns@...delico.com, paul@...die.org.uk, andy.shevchenko@...il.com,
        dongsheng.qiu@...enic.com, aric.pzqi@...enic.com,
        sernia.zhou@...mail.com
Subject: Re: [PATCH v3 02/10] pinctrl: Ingenic: Add support for read the pin
 configuration of X1830.

Hi,

On 2021/3/23 上午1:58, Paul Cercueil wrote:
>
>
> Le mer. 17 mars 2021 à 17:57, 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@...yeetech.com> a écrit :
>> Add X1830 support in "ingenic_pinconf_get()", so that it can read the
>> configuration of X1830 SoC correctly.
>>
>> Fixes: d7da2a1e4e08 ("pinctrl: Ingenic: Add pinctrl driver for X1830.")
>>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@...yeetech.com>
>> ---
>>
>> Notes:
>>     v2:
>>     New patch.
>>
>>     v2->v3:
>>     1.Add fixes tag.
>>     2.Adjust the code, simplify the ingenic_pinconf_get() function.
>>
>>  drivers/pinctrl/pinctrl-ingenic.c | 38 
>> ++++++++++++++++++++++++++++++--------
>>  1 file changed, 30 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-ingenic.c 
>> b/drivers/pinctrl/pinctrl-ingenic.c
>> index 05dfa0a..1d43b98 100644
>> --- a/drivers/pinctrl/pinctrl-ingenic.c
>> +++ b/drivers/pinctrl/pinctrl-ingenic.c
>> @@ -2109,26 +2109,48 @@ static int ingenic_pinconf_get(struct 
>> pinctrl_dev *pctldev,
>>      enum pin_config_param param = pinconf_to_config_param(*config);
>>      unsigned int idx = pin % PINS_PER_GPIO_CHIP;
>>      unsigned int offt = pin / PINS_PER_GPIO_CHIP;
>> -    bool pull;
>> +    unsigned int bias;
>> +    bool pull, pullup, pulldown;
>>
>> -    if (jzpc->info->version >= ID_JZ4770)
>> -        pull = !ingenic_get_pin_config(jzpc, pin, JZ4770_GPIO_PEN);
>> -    else
>> -        pull = !ingenic_get_pin_config(jzpc, pin, 
>> JZ4740_GPIO_PULL_DIS);
>> +    if (jzpc->info->version >= ID_X1830) {
>> +        unsigned int half = PINS_PER_GPIO_CHIP / 2;
>> +        unsigned int idxh = pin % half * 2;
>
> I had to look up operator precedence in C, '*' and '%' have the same 
> priority so this reads left-to-right.
>
> I'd suggest adding parentheses around the '%' to make it more obvious.
>

Sure.


> With that:
>
> Reviewed-by: Paul Cercueil <paul@...pouillou.net>
>
> Cheers,
> -Paul
>
>> +
>> +        if (idx < half)
>> +            regmap_read(jzpc->map, offt * jzpc->info->reg_offset +
>> +                    X1830_GPIO_PEL, &bias);
>> +        else
>> +            regmap_read(jzpc->map, offt * jzpc->info->reg_offset +
>> +                    X1830_GPIO_PEH, &bias);
>> +
>> +        bias = (bias >> idxh) & (GPIO_PULL_UP | GPIO_PULL_DOWN);
>> +
>> +        pullup = (bias == GPIO_PULL_UP) && 
>> (jzpc->info->pull_ups[offt] & BIT(idx));
>> +        pulldown = (bias == GPIO_PULL_DOWN) && 
>> (jzpc->info->pull_downs[offt] & BIT(idx));
>> +
>> +    } else {
>> +        if (jzpc->info->version >= ID_JZ4770)
>> +            pull = !ingenic_get_pin_config(jzpc, pin, JZ4770_GPIO_PEN);
>> +        else
>> +            pull = !ingenic_get_pin_config(jzpc, pin, 
>> JZ4740_GPIO_PULL_DIS);
>> +
>> +        pullup = pull && (jzpc->info->pull_ups[offt] & BIT(idx));
>> +        pulldown = pull && (jzpc->info->pull_downs[offt] & BIT(idx));
>> +    }
>>
>>      switch (param) {
>>      case PIN_CONFIG_BIAS_DISABLE:
>> -        if (pull)
>> +        if (pullup || pulldown)
>>              return -EINVAL;
>>          break;
>>
>>      case PIN_CONFIG_BIAS_PULL_UP:
>> -        if (!pull || !(jzpc->info->pull_ups[offt] & BIT(idx)))
>> +        if (!pullup)
>>              return -EINVAL;
>>          break;
>>
>>      case PIN_CONFIG_BIAS_PULL_DOWN:
>> -        if (!pull || !(jzpc->info->pull_downs[offt] & BIT(idx)))
>> +        if (!pulldown)
>>              return -EINVAL;
>>          break;
>>
>> -- 
>> 2.7.4
>>
>