diff mbox series

[v3,02/10] pinctrl: Ingenic: Add support for read the pin configuration of X1830.

Message ID 1615975084-68203-3-git-send-email-zhouyanjie@wanyeetech.com
State New
Headers show
Series Fix bugs and add support for new Ingenic SoCs. | expand

Commit Message

Zhou Yanjie March 17, 2021, 9:57 a.m. UTC
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@wanyeetech.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(-)

Comments

Paul Cercueil March 22, 2021, 5:58 p.m. UTC | #1
Le mer. 17 mars 2021 à 17:57, 周琰杰 (Zhou Yanjie) 
<zhouyanjie@wanyeetech.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@wanyeetech.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.

With that:

Reviewed-by: Paul Cercueil <paul@crapouillou.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
>
Zhou Yanjie March 25, 2021, 7:38 a.m. UTC | #2
Hi,

On 2021/3/23 上午1:58, Paul Cercueil wrote:
>
>
> Le mer. 17 mars 2021 à 17:57, 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.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@wanyeetech.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@crapouillou.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
>>
>
diff mbox series

Patch

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;
+
+		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;