All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pinctrl: single: support GPIO for bits pinctrl
@ 2015-06-17  1:56 Jun Nie
  2015-06-17  7:17 ` Tony Lindgren
  0 siblings, 1 reply; 13+ messages in thread
From: Jun Nie @ 2015-06-17  1:56 UTC (permalink / raw)
  To: tony, haojian.zhuang, linus.walleij, linux-gpio
  Cc: shawn.guo, wan.zhijun, jason.liu, 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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl
  2015-06-17  1:56 [PATCH] pinctrl: single: support GPIO for bits pinctrl Jun Nie
@ 2015-06-17  7:17 ` Tony Lindgren
  2015-06-19 14:55   ` Haojian Zhuang
  2015-06-23  9:54   ` Jun Nie
  0 siblings, 2 replies; 13+ messages in thread
From: Tony Lindgren @ 2015-06-17  7:17 UTC (permalink / raw)
  To: Jun Nie
  Cc: haojian.zhuang, linus.walleij, linux-gpio, shawn.guo, wan.zhijun,
	jason.liu

* 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl
  2015-06-17  7:17 ` Tony Lindgren
@ 2015-06-19 14:55   ` Haojian Zhuang
  2015-06-23  9:54   ` Jun Nie
  1 sibling, 0 replies; 13+ messages in thread
From: Haojian Zhuang @ 2015-06-19 14:55 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Jun Nie, Linus Walleij, linux-gpio, Shawn Guo, wan.zhijun, Jason Liu

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl
  2015-06-17  7:17 ` Tony Lindgren
  2015-06-19 14:55   ` Haojian Zhuang
@ 2015-06-23  9:54   ` Jun Nie
  2015-06-23 10:14     ` Tony Lindgren
  1 sibling, 1 reply; 13+ messages in thread
From: Jun Nie @ 2015-06-23  9:54 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Haojian Zhuang, Linus Walleij, linux-gpio, Shawn Guo, wan.zhijun,
	Jason Liu

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl
  2015-06-23  9:54   ` Jun Nie
@ 2015-06-23 10:14     ` Tony Lindgren
  2015-06-23 10:18       ` Jun Nie
  0 siblings, 1 reply; 13+ messages in thread
From: Tony Lindgren @ 2015-06-23 10:14 UTC (permalink / raw)
  To: Jun Nie
  Cc: Haojian Zhuang, Linus Walleij, linux-gpio, Shawn Guo, wan.zhijun,
	Jason Liu

* 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl
  2015-06-23 10:14     ` Tony Lindgren
@ 2015-06-23 10:18       ` Jun Nie
  2015-07-06  8:40         ` Jun Nie
  0 siblings, 1 reply; 13+ messages in thread
From: Jun Nie @ 2015-06-23 10:18 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Haojian Zhuang, Linus Walleij, linux-gpio, Shawn Guo, wan.zhijun,
	Jason Liu

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl
  2015-06-23 10:18       ` Jun Nie
@ 2015-07-06  8:40         ` Jun Nie
  2015-07-06  9:03           ` Tony Lindgren
  0 siblings, 1 reply; 13+ messages in thread
From: Jun Nie @ 2015-07-06  8:40 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Haojian Zhuang, Linus Walleij, linux-gpio, Shawn Guo, wan.zhijun,
	Jason Liu

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl
  2015-07-06  8:40         ` Jun Nie
@ 2015-07-06  9:03           ` Tony Lindgren
  2015-07-06  9:19             ` Jun Nie
  0 siblings, 1 reply; 13+ messages in thread
From: Tony Lindgren @ 2015-07-06  9:03 UTC (permalink / raw)
  To: Jun Nie
  Cc: Haojian Zhuang, Linus Walleij, linux-gpio, Shawn Guo, wan.zhijun,
	Jason Liu

* 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl
  2015-07-06  9:03           ` Tony Lindgren
@ 2015-07-06  9:19             ` Jun Nie
  2015-07-06 10:40               ` Tony Lindgren
  0 siblings, 1 reply; 13+ messages in thread
From: Jun Nie @ 2015-07-06  9:19 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Haojian Zhuang, Linus Walleij, linux-gpio, Shawn Guo, wan.zhijun,
	Jason Liu

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl
  2015-07-06  9:19             ` Jun Nie
@ 2015-07-06 10:40               ` Tony Lindgren
  0 siblings, 0 replies; 13+ messages in thread
From: Tony Lindgren @ 2015-07-06 10:40 UTC (permalink / raw)
  To: Jun Nie
  Cc: Haojian Zhuang, Linus Walleij, linux-gpio, Shawn Guo, wan.zhijun,
	Jason Liu

* 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl
  2015-06-16  9:17 ` Linus Walleij
@ 2015-06-16 11:28   ` Tony Lindgren
  0 siblings, 0 replies; 13+ messages in thread
From: Tony Lindgren @ 2015-06-16 11:28 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Jun Nie, linux-gpio, Shawn Guo, wan.zhijun, jason.liu

* 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] pinctrl: single: support GPIO for bits pinctrl
  2015-06-12  8:19 Jun Nie
@ 2015-06-16  9:17 ` Linus Walleij
  2015-06-16 11:28   ` Tony Lindgren
  0 siblings, 1 reply; 13+ messages in thread
From: Linus Walleij @ 2015-06-16  9:17 UTC (permalink / raw)
  To: Jun Nie, Tony Lindgren; +Cc: linux-gpio, Shawn Guo, wan.zhijun, jason.liu

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH] pinctrl: single: support GPIO for bits pinctrl
@ 2015-06-12  8:19 Jun Nie
  2015-06-16  9:17 ` Linus Walleij
  0 siblings, 1 reply; 13+ messages in thread
From: Jun Nie @ 2015-06-12  8:19 UTC (permalink / raw)
  To: linus.walleij, linux-gpio; +Cc: shawn.guo, wan.zhijun, jason.liu, 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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-07-06 10:40 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-17  1:56 [PATCH] pinctrl: single: support GPIO for bits pinctrl Jun Nie
2015-06-17  7:17 ` Tony Lindgren
2015-06-19 14:55   ` Haojian Zhuang
2015-06-23  9:54   ` Jun Nie
2015-06-23 10:14     ` Tony Lindgren
2015-06-23 10:18       ` Jun Nie
2015-07-06  8:40         ` Jun Nie
2015-07-06  9:03           ` Tony Lindgren
2015-07-06  9:19             ` Jun Nie
2015-07-06 10:40               ` Tony Lindgren
  -- strict thread matches above, loose matches on Subject: below --
2015-06-12  8:19 Jun Nie
2015-06-16  9:17 ` Linus Walleij
2015-06-16 11:28   ` Tony Lindgren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.