[PATCH] pinctrl: single: support GPIO for bits pinctrl

[PATCH] pinctrl: single: support GPIO for bits pinctrl

[Jun Nie]

Support GPIO for one register control multiple pins case
with calculating register offset first, then bit offset.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 drivers/pinctrl/pinctrl-single.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 13b45f2..bd69d9a 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -494,7 +494,7 @@ static int pcs_request_gpio(struct pinctrl_dev *pctldev,
 	struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
 	struct pcs_gpiofunc_range *frange = NULL;
 	struct list_head *pos, *tmp;
-	int mux_bytes = 0;
+	int offset, mux_bytes = 0;
 	unsigned data;
 
 	/* If function mask is null, return directly. */
@@ -507,9 +507,23 @@ static int pcs_request_gpio(struct pinctrl_dev *pctldev,
 			|| pin < frange->offset)
 			continue;
 		mux_bytes = pcs->width / BITS_PER_BYTE;
-		data = pcs->read(pcs->base + pin * mux_bytes) & ~pcs->fmask;
-		data |= frange->gpiofunc;
-		pcs->write(data, pcs->base + pin * mux_bytes);
+		if (pcs->bits_per_mux) {
+			int pin_pos, byte_num, num_pins_in_register;
+
+			num_pins_in_register = pcs->width / pcs->bits_per_pin;
+			byte_num = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
+			offset = (byte_num / mux_bytes) * mux_bytes;
+			pin_pos = pin % num_pins_in_register;
+			pin_pos *= pcs->bits_per_pin;
+			data = pcs->read(pcs->base + offset) &
+				~(pcs->fmask << pin_pos);
+			data |= frange->gpiofunc << pin_pos;
+		} else {
+			offset = pin * mux_bytes;
+			data = pcs->read(pcs->base + offset) & ~pcs->fmask;
+			data |= frange->gpiofunc;
+		}
+		pcs->write(data, pcs->base + offset);
 		break;
 	}
 	return 0;
-- 
1.9.1

Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl

[Tony Lindgren]

* Jun Nie <jun.nie@linaro.org> [150616 18:58]:
> Support GPIO for one register control multiple pins case
> with calculating register offset first, then bit offset.
> 
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  drivers/pinctrl/pinctrl-single.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> index 13b45f2..bd69d9a 100644
> --- a/drivers/pinctrl/pinctrl-single.c
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -494,7 +494,7 @@ static int pcs_request_gpio(struct pinctrl_dev *pctldev,
>  	struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
>  	struct pcs_gpiofunc_range *frange = NULL;
>  	struct list_head *pos, *tmp;
> -	int mux_bytes = 0;
> +	int offset, mux_bytes = 0;
>  	unsigned data;
>  
>  	/* If function mask is null, return directly. */
> @@ -507,9 +507,23 @@ static int pcs_request_gpio(struct pinctrl_dev *pctldev,
>  			|| pin < frange->offset)
>  			continue;
>  		mux_bytes = pcs->width / BITS_PER_BYTE;
> -		data = pcs->read(pcs->base + pin * mux_bytes) & ~pcs->fmask;
> -		data |= frange->gpiofunc;
> -		pcs->write(data, pcs->base + pin * mux_bytes);
> +		if (pcs->bits_per_mux) {
> +			int pin_pos, byte_num, num_pins_in_register;
> +
> +			num_pins_in_register = pcs->width / pcs->bits_per_pin;
> +			byte_num = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
> +			offset = (byte_num / mux_bytes) * mux_bytes;
> +			pin_pos = pin % num_pins_in_register;
> +			pin_pos *= pcs->bits_per_pin;
> +			data = pcs->read(pcs->base + offset) &
> +				~(pcs->fmask << pin_pos);

Should you check the pcs->fmask here too in case some bits are reserved?

> +			data |= frange->gpiofunc << pin_pos;
> +		} else {
> +			offset = pin * mux_bytes;
> +			data = pcs->read(pcs->base + offset) & ~pcs->fmask;
> +			data |= frange->gpiofunc;
> +		}
> +		pcs->write(data, pcs->base + offset);
>  		break;
>  	}
>  	return 0;

Other than that looks OK to me, would be good to also wait for Haojian's
comments here.

Regards,

Tony

Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl

[Haojian Zhuang]

On 17 June 2015 at 15:17, Tony Lindgren <tony@atomide.com> wrote:
> * Jun Nie <jun.nie@linaro.org> [150616 18:58]:
>> Support GPIO for one register control multiple pins case
>> with calculating register offset first, then bit offset.
>>
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>>  drivers/pinctrl/pinctrl-single.c | 22 ++++++++++++++++++----
>>  1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
>> index 13b45f2..bd69d9a 100644
>> --- a/drivers/pinctrl/pinctrl-single.c
>> +++ b/drivers/pinctrl/pinctrl-single.c
>> @@ -494,7 +494,7 @@ static int pcs_request_gpio(struct pinctrl_dev *pctldev,
>>       struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
>>       struct pcs_gpiofunc_range *frange = NULL;
>>       struct list_head *pos, *tmp;
>> -     int mux_bytes = 0;
>> +     int offset, mux_bytes = 0;
>>       unsigned data;
>>
>>       /* If function mask is null, return directly. */
>> @@ -507,9 +507,23 @@ static int pcs_request_gpio(struct pinctrl_dev *pctldev,
>>                       || pin < frange->offset)
>>                       continue;
>>               mux_bytes = pcs->width / BITS_PER_BYTE;
>> -             data = pcs->read(pcs->base + pin * mux_bytes) & ~pcs->fmask;
>> -             data |= frange->gpiofunc;
>> -             pcs->write(data, pcs->base + pin * mux_bytes);
>> +             if (pcs->bits_per_mux) {
>> +                     int pin_pos, byte_num, num_pins_in_register;
>> +
>> +                     num_pins_in_register = pcs->width / pcs->bits_per_pin;
>> +                     byte_num = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
>> +                     offset = (byte_num / mux_bytes) * mux_bytes;
>> +                     pin_pos = pin % num_pins_in_register;
>> +                     pin_pos *= pcs->bits_per_pin;
>> +                     data = pcs->read(pcs->base + offset) &
>> +                             ~(pcs->fmask << pin_pos);
>
> Should you check the pcs->fmask here too in case some bits are reserved?
>
>> +                     data |= frange->gpiofunc << pin_pos;
>> +             } else {
>> +                     offset = pin * mux_bytes;
>> +                     data = pcs->read(pcs->base + offset) & ~pcs->fmask;
>> +                     data |= frange->gpiofunc;
>> +             }
>> +             pcs->write(data, pcs->base + offset);
>>               break;
>>       }
>>       return 0;
>
> Other than that looks OK to me, would be good to also wait for Haojian's
> comments here.
>

I'm fine on this.

Reviewed-by: Haojian Zhuang <haojian.zhuang@linaro.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in

Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl

[Jun Nie]

2015-06-17 15:17 GMT+08:00 Tony Lindgren <tony@atomide.com>:
> * Jun Nie <jun.nie@linaro.org> [150616 18:58]:
>> Support GPIO for one register control multiple pins case
>> with calculating register offset first, then bit offset.
>>
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>>  drivers/pinctrl/pinctrl-single.c | 22 ++++++++++++++++++----
>>  1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
>> index 13b45f2..bd69d9a 100644
>> --- a/drivers/pinctrl/pinctrl-single.c
>> +++ b/drivers/pinctrl/pinctrl-single.c
>> @@ -494,7 +494,7 @@ static int pcs_request_gpio(struct pinctrl_dev *pctldev,
>>       struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
>>       struct pcs_gpiofunc_range *frange = NULL;
>>       struct list_head *pos, *tmp;
>> -     int mux_bytes = 0;
>> +     int offset, mux_bytes = 0;
>>       unsigned data;
>>
>>       /* If function mask is null, return directly. */
>> @@ -507,9 +507,23 @@ static int pcs_request_gpio(struct pinctrl_dev *pctldev,
>>                       || pin < frange->offset)
>>                       continue;
>>               mux_bytes = pcs->width / BITS_PER_BYTE;
>> -             data = pcs->read(pcs->base + pin * mux_bytes) & ~pcs->fmask;
>> -             data |= frange->gpiofunc;
>> -             pcs->write(data, pcs->base + pin * mux_bytes);
>> +             if (pcs->bits_per_mux) {
>> +                     int pin_pos, byte_num, num_pins_in_register;
>> +
>> +                     num_pins_in_register = pcs->width / pcs->bits_per_pin;
>> +                     byte_num = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
>> +                     offset = (byte_num / mux_bytes) * mux_bytes;
>> +                     pin_pos = pin % num_pins_in_register;
>> +                     pin_pos *= pcs->bits_per_pin;
>> +                     data = pcs->read(pcs->base + offset) &
>> +                             ~(pcs->fmask << pin_pos);
>
> Should you check the pcs->fmask here too in case some bits are reserved?
>
Did not catch your idea? Those bits set in fmask are dedicated for one
pin mux control and should be clear before set as desired value per my
understanding. Do you mean some bits may be reserved and not for any
function?

>> +                     data |= frange->gpiofunc << pin_pos;
>> +             } else {
>> +                     offset = pin * mux_bytes;
>> +                     data = pcs->read(pcs->base + offset) & ~pcs->fmask;
>> +                     data |= frange->gpiofunc;
>> +             }
>> +             pcs->write(data, pcs->base + offset);
>>               break;
>>       }
>>       return 0;
>
> Other than that looks OK to me, would be good to also wait for Haojian's
> comments here.
>
> Regards,
>
> Tony

Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl

[Tony Lindgren]

* Jun Nie <jun.nie@linaro.org> [150623 02:56]:
> 2015-06-17 15:17 GMT+08:00 Tony Lindgren <tony@atomide.com>:
> > * Jun Nie <jun.nie@linaro.org> [150616 18:58]:
> >> +             if (pcs->bits_per_mux) {
> >> +                     int pin_pos, byte_num, num_pins_in_register;
> >> +
> >> +                     num_pins_in_register = pcs->width / pcs->bits_per_pin;
> >> +                     byte_num = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
> >> +                     offset = (byte_num / mux_bytes) * mux_bytes;
> >> +                     pin_pos = pin % num_pins_in_register;
> >> +                     pin_pos *= pcs->bits_per_pin;
> >> +                     data = pcs->read(pcs->base + offset) &
> >> +                             ~(pcs->fmask << pin_pos);
> >
> > Should you check the pcs->fmask here too in case some bits are reserved?
> >
> Did not catch your idea? Those bits set in fmask are dedicated for one
> pin mux control and should be clear before set as desired value per my
> understanding. Do you mean some bits may be reserved and not for any
> function?

Right, can you please check that we don't try to write to reserved
bits in the hardawre if the mask is set?

Regards,

Tony

Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl

[Jun Nie]

2015-06-23 18:14 GMT+08:00 Tony Lindgren <tony@atomide.com>:
> * Jun Nie <jun.nie@linaro.org> [150623 02:56]:
>> 2015-06-17 15:17 GMT+08:00 Tony Lindgren <tony@atomide.com>:
>> > * Jun Nie <jun.nie@linaro.org> [150616 18:58]:
>> >> +             if (pcs->bits_per_mux) {
>> >> +                     int pin_pos, byte_num, num_pins_in_register;
>> >> +
>> >> +                     num_pins_in_register = pcs->width / pcs->bits_per_pin;
>> >> +                     byte_num = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
>> >> +                     offset = (byte_num / mux_bytes) * mux_bytes;
>> >> +                     pin_pos = pin % num_pins_in_register;
>> >> +                     pin_pos *= pcs->bits_per_pin;
>> >> +                     data = pcs->read(pcs->base + offset) &
>> >> +                             ~(pcs->fmask << pin_pos);
>> >
>> > Should you check the pcs->fmask here too in case some bits are reserved?
>> >
>> Did not catch your idea? Those bits set in fmask are dedicated for one
>> pin mux control and should be clear before set as desired value per my
>> understanding. Do you mean some bits may be reserved and not for any
>> function?
>
> Right, can you please check that we don't try to write to reserved
> bits in the hardawre if the mask is set?
Then I have question that how can I know what bits is for function
mask, what bits are for reserved? Do we have any other value to
indicate it? I did not find it in one register for one pin mux case.

>
> Regards,
>
> Tony

Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl

[Jun Nie]

2015-06-23 18:18 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
> 2015-06-23 18:14 GMT+08:00 Tony Lindgren <tony@atomide.com>:
>> * Jun Nie <jun.nie@linaro.org> [150623 02:56]:
>>> 2015-06-17 15:17 GMT+08:00 Tony Lindgren <tony@atomide.com>:
>>> > * Jun Nie <jun.nie@linaro.org> [150616 18:58]:
>>> >> +             if (pcs->bits_per_mux) {
>>> >> +                     int pin_pos, byte_num, num_pins_in_register;
>>> >> +
>>> >> +                     num_pins_in_register = pcs->width / pcs->bits_per_pin;
>>> >> +                     byte_num = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
>>> >> +                     offset = (byte_num / mux_bytes) * mux_bytes;
>>> >> +                     pin_pos = pin % num_pins_in_register;
>>> >> +                     pin_pos *= pcs->bits_per_pin;
>>> >> +                     data = pcs->read(pcs->base + offset) &
>>> >> +                             ~(pcs->fmask << pin_pos);
>>> >
>>> > Should you check the pcs->fmask here too in case some bits are reserved?
>>> >
>>> Did not catch your idea? Those bits set in fmask are dedicated for one
>>> pin mux control and should be clear before set as desired value per my
>>> understanding. Do you mean some bits may be reserved and not for any
>>> function?
>>
>> Right, can you please check that we don't try to write to reserved
>> bits in the hardawre if the mask is set?
> Then I have question that how can I know what bits is for function
> mask, what bits are for reserved? Do we have any other value to
> indicate it? I did not find it in one register for one pin mux case.

Tony,

Could you help elaborate this? Thanks!
>
>>
>> Regards,
>>
>> Tony

Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl

[Tony Lindgren]

* Jun Nie <jun.nie@linaro.org> [150706 01:43]:
> 2015-06-23 18:18 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
> > 2015-06-23 18:14 GMT+08:00 Tony Lindgren <tony@atomide.com>:
> >> * Jun Nie <jun.nie@linaro.org> [150623 02:56]:
> >>> 2015-06-17 15:17 GMT+08:00 Tony Lindgren <tony@atomide.com>:
> >>> > * Jun Nie <jun.nie@linaro.org> [150616 18:58]:
> >>> >> +             if (pcs->bits_per_mux) {
> >>> >> +                     int pin_pos, byte_num, num_pins_in_register;
> >>> >> +
> >>> >> +                     num_pins_in_register = pcs->width / pcs->bits_per_pin;
> >>> >> +                     byte_num = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
> >>> >> +                     offset = (byte_num / mux_bytes) * mux_bytes;
> >>> >> +                     pin_pos = pin % num_pins_in_register;
> >>> >> +                     pin_pos *= pcs->bits_per_pin;
> >>> >> +                     data = pcs->read(pcs->base + offset) &
> >>> >> +                             ~(pcs->fmask << pin_pos);
> >>> >
> >>> > Should you check the pcs->fmask here too in case some bits are reserved?
> >>> >
> >>> Did not catch your idea? Those bits set in fmask are dedicated for one
> >>> pin mux control and should be clear before set as desired value per my
> >>> understanding. Do you mean some bits may be reserved and not for any
> >>> function?
> >>
> >> Right, can you please check that we don't try to write to reserved
> >> bits in the hardawre if the mask is set?
>
> > Then I have question that how can I know what bits is for function
> > mask, what bits are for reserved? Do we have any other value to
> > indicate it? I did not find it in one register for one pin mux case.
> 
> Could you help elaborate this? Thanks!

We can only write to the bits specified in pinctrl-single,function-mask.

Regards,

Tony

Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl

[Jun Nie]

2015-07-06 17:03 GMT+08:00 Tony Lindgren <tony@atomide.com>:
> * Jun Nie <jun.nie@linaro.org> [150706 01:43]:
>> 2015-06-23 18:18 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
>> > 2015-06-23 18:14 GMT+08:00 Tony Lindgren <tony@atomide.com>:
>> >> * Jun Nie <jun.nie@linaro.org> [150623 02:56]:
>> >>> 2015-06-17 15:17 GMT+08:00 Tony Lindgren <tony@atomide.com>:
>> >>> > * Jun Nie <jun.nie@linaro.org> [150616 18:58]:
>> >>> >> +             if (pcs->bits_per_mux) {
>> >>> >> +                     int pin_pos, byte_num, num_pins_in_register;
>> >>> >> +
>> >>> >> +                     num_pins_in_register = pcs->width / pcs->bits_per_pin;
>> >>> >> +                     byte_num = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
>> >>> >> +                     offset = (byte_num / mux_bytes) * mux_bytes;
>> >>> >> +                     pin_pos = pin % num_pins_in_register;
>> >>> >> +                     pin_pos *= pcs->bits_per_pin;
>> >>> >> +                     data = pcs->read(pcs->base + offset) &
>> >>> >> +                             ~(pcs->fmask << pin_pos);
>> >>> >
>> >>> > Should you check the pcs->fmask here too in case some bits are reserved?
>> >>> >
>> >>> Did not catch your idea? Those bits set in fmask are dedicated for one
>> >>> pin mux control and should be clear before set as desired value per my
>> >>> understanding. Do you mean some bits may be reserved and not for any
>> >>> function?
>> >>
>> >> Right, can you please check that we don't try to write to reserved
>> >> bits in the hardawre if the mask is set?
>>
>> > Then I have question that how can I know what bits is for function
>> > mask, what bits are for reserved? Do we have any other value to
>> > indicate it? I did not find it in one register for one pin mux case.
>>
>> Could you help elaborate this? Thanks!
>
> We can only write to the bits specified in pinctrl-single,function-mask.
>
I see, you want below mask to make sure gpiofunc value does not exceed
expected bits though it should be safe if dts data is correct. Right?
+                       data = pcs->read(pcs->base + offset) &
+                               ~(pcs->fmask << pin_pos);
+                       data |= (pcs->fmask & frange->gpiofunc) << pin_pos;

> Regards,
>
> Tony

Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl

[Tony Lindgren]

* Jun Nie <jun.nie@linaro.org> [150706 02:21]:
> 2015-07-06 17:03 GMT+08:00 Tony Lindgren <tony@atomide.com>:
> > * Jun Nie <jun.nie@linaro.org> [150706 01:43]:
> >> 2015-06-23 18:18 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
> >> > 2015-06-23 18:14 GMT+08:00 Tony Lindgren <tony@atomide.com>:
> >> >> * Jun Nie <jun.nie@linaro.org> [150623 02:56]:
> >> >>> 2015-06-17 15:17 GMT+08:00 Tony Lindgren <tony@atomide.com>:
> >> >>> > * Jun Nie <jun.nie@linaro.org> [150616 18:58]:
> >> >>> >> +             if (pcs->bits_per_mux) {
> >> >>> >> +                     int pin_pos, byte_num, num_pins_in_register;
> >> >>> >> +
> >> >>> >> +                     num_pins_in_register = pcs->width / pcs->bits_per_pin;
> >> >>> >> +                     byte_num = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
> >> >>> >> +                     offset = (byte_num / mux_bytes) * mux_bytes;
> >> >>> >> +                     pin_pos = pin % num_pins_in_register;
> >> >>> >> +                     pin_pos *= pcs->bits_per_pin;
> >> >>> >> +                     data = pcs->read(pcs->base + offset) &
> >> >>> >> +                             ~(pcs->fmask << pin_pos);
> >> >>> >
> >> >>> > Should you check the pcs->fmask here too in case some bits are reserved?
> >> >>> >
> >> >>> Did not catch your idea? Those bits set in fmask are dedicated for one
> >> >>> pin mux control and should be clear before set as desired value per my
> >> >>> understanding. Do you mean some bits may be reserved and not for any
> >> >>> function?
> >> >>
> >> >> Right, can you please check that we don't try to write to reserved
> >> >> bits in the hardawre if the mask is set?
> >>
> >> > Then I have question that how can I know what bits is for function
> >> > mask, what bits are for reserved? Do we have any other value to
> >> > indicate it? I did not find it in one register for one pin mux case.
> >>
> >> Could you help elaborate this? Thanks!
> >
> > We can only write to the bits specified in pinctrl-single,function-mask.
> >
> I see, you want below mask to make sure gpiofunc value does not exceed
> expected bits though it should be safe if dts data is correct. Right?
> +                       data = pcs->read(pcs->base + offset) &
> +                               ~(pcs->fmask << pin_pos);
> +                       data |= (pcs->fmask & frange->gpiofunc) << pin_pos;

Yes please :)

Tony

[PATCH] pinctrl: single: support GPIO for bits pinctrl

[Jun Nie]

Support GPIO for one register control multiple pins case
with calculating register offset first, then bit offset.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 drivers/pinctrl/pinctrl-single.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 13b45f2..bd69d9a 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -494,7 +494,7 @@ static int pcs_request_gpio(struct pinctrl_dev *pctldev,
 	struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
 	struct pcs_gpiofunc_range *frange = NULL;
 	struct list_head *pos, *tmp;
-	int mux_bytes = 0;
+	int offset, mux_bytes = 0;
 	unsigned data;
 
 	/* If function mask is null, return directly. */
@@ -507,9 +507,23 @@ static int pcs_request_gpio(struct pinctrl_dev *pctldev,
 			|| pin < frange->offset)
 			continue;
 		mux_bytes = pcs->width / BITS_PER_BYTE;
-		data = pcs->read(pcs->base + pin * mux_bytes) & ~pcs->fmask;
-		data |= frange->gpiofunc;
-		pcs->write(data, pcs->base + pin * mux_bytes);
+		if (pcs->bits_per_mux) {
+			int pin_pos, byte_num, num_pins_in_register;
+
+			num_pins_in_register = pcs->width / pcs->bits_per_pin;
+			byte_num = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
+			offset = (byte_num / mux_bytes) * mux_bytes;
+			pin_pos = pin % num_pins_in_register;
+			pin_pos *= pcs->bits_per_pin;
+			data = pcs->read(pcs->base + offset) &
+				~(pcs->fmask << pin_pos);
+			data |= frange->gpiofunc << pin_pos;
+		} else {
+			offset = pin * mux_bytes;
+			data = pcs->read(pcs->base + offset) & ~pcs->fmask;
+			data |= frange->gpiofunc;
+		}
+		pcs->write(data, pcs->base + offset);
 		break;
 	}
 	return 0;
-- 
1.9.1

Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl

[Linus Walleij]

On Fri, Jun 12, 2015 at 10:19 AM, Jun Nie <jun.nie@linaro.org> wrote:

> Support GPIO for one register control multiple pins case
> with calculating register offset first, then bit offset.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>

You need to send this patch to Tony Lindgren for review.

Yours,
Linus Walleij

Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl

[Tony Lindgren]

* Linus Walleij <linus.walleij@linaro.org> [150616 02:19]:
> On Fri, Jun 12, 2015 at 10:19 AM, Jun Nie <jun.nie@linaro.org> wrote:
> 
> > Support GPIO for one register control multiple pins case
> > with calculating register offset first, then bit offset.
> >
> > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> 
> You need to send this patch to Tony Lindgren for review.

Yes please repost, and also add Cc Haojian Zhuang
<haojian.zhuang@linaro.org> as he added the GPIO support
to the driver.

Regards,

Tony