pxa27x and pinctrl-single

pxa27x and pinctrl-single

[Mike Dunn]

Hi,

I'd like to start converting to device tree usage some of the old pxa27x
platforms I'm fond of, starting with adding pinctrl support.  If I'm not
mistaken, this will clear the way for some ongoing updates to the pinctrl/gpio
code used by newer Marvell arches.

I noticed that Haojian pulled the pinctrl-pxa driver from the kernel, in favor
of using pinctrl-single, so I turned my attention to that.  From what I can
tell, pinctrl-single is currently inadequate for pxa27x because:

(1) On the pxa27x, setting the mux for a pin involves configuring both the alt
function register *and* the direction register, requiring the ability to specify
in the device tree multiple reg/value pairs for each pin.

(2) Some of the pinctrl-single code still assumes one register-per-pin; see for
example the functions pcs_allocate_pin_table() and pcs_get_pin_by_offset().
Curiously, the recently added pcs_request_gpio() function also suffers from
this, unless I'm mistaken.

So I'm wondering if it's appropriate to patch pinctrl-single to make it suitable
for pxa27x, or should there be a separate driver for pxa27x?

Normally I'd just submit a patch for consideration, but I think that work is
ongoing, so maybe I better ask first in case other plans are in progress.  Plus
I'm new to pinctrl and device tree.  Any comments, suggestions, hints, etc
appreciated.

Thanks,
Mike

Re: pxa27x and pinctrl-single

[Haojian Zhuang]

On 6 June 2013 01:23, Mike Dunn <mikedunn-kFrNdAxtuftBDgjK7y7TUQ@public.gmane.org> wrote:
> Hi,
>
> I'd like to start converting to device tree usage some of the old pxa27x
> platforms I'm fond of, starting with adding pinctrl support.  If I'm not
> mistaken, this will clear the way for some ongoing updates to the pinctrl/gpio
> code used by newer Marvell arches.
>
> I noticed that Haojian pulled the pinctrl-pxa driver from the kernel, in favor
> of using pinctrl-single, so I turned my attention to that.  From what I can
> tell, pinctrl-single is currently inadequate for pxa27x because:
>
> (1) On the pxa27x, setting the mux for a pin involves configuring both the alt
> function register *and* the direction register, requiring the ability to specify
> in the device tree multiple reg/value pairs for each pin.
In PXA27x, GPIO controller control both GPIO & pin alternate function. The
alternate function is covered by GAFRx registers.

At first, we need to move GAFRx from gpio-pxa driver. Then we can support
it by pinctrl-single driver since pinctrl-single driver could support mulitple
pins in one pin registers. There's "bits-per-mux" property in pinctrl-single
driver.
>
> (2) Some of the pinctrl-single code still assumes one register-per-pin; see for
> example the functions pcs_allocate_pin_table() and pcs_get_pin_by_offset().
> Curiously, the recently added pcs_request_gpio() function also suffers from
> this, unless I'm mistaken.
>
Not exactly. pinctrl-single driver could support multiple pins in one register.
Now Manjunathappa also fixed some issue in pinctrl-single for supporting
multiple pins. (https://lkml.org/lkml/2013/5/21/226) I suggest you to develop
based on his patches.

Regards
Haojian

Re: pxa27x and pinctrl-single

[Mike Dunn]

Thanks for the reply Haojian.

On 06/05/2013 05:43 PM, Haojian Zhuang wrote:
> On 6 June 2013 01:23, Mike Dunn <mikedunn@newsguy.com> wrote:
>> Hi,
>>
>> I'd like to start converting to device tree usage some of the old pxa27x
>> platforms I'm fond of, starting with adding pinctrl support.  If I'm not
>> mistaken, this will clear the way for some ongoing updates to the pinctrl/gpio
>> code used by newer Marvell arches.
>>
>> I noticed that Haojian pulled the pinctrl-pxa driver from the kernel, in favor
>> of using pinctrl-single, so I turned my attention to that.  From what I can
>> tell, pinctrl-single is currently inadequate for pxa27x because:
>>
>> (1) On the pxa27x, setting the mux for a pin involves configuring both the alt
>> function register *and* the direction register, requiring the ability to specify
>> in the device tree multiple reg/value pairs for each pin.
> In PXA27x, GPIO controller control both GPIO & pin alternate function. The
> alternate function is covered by GAFRx registers.
> 
> At first, we need to move GAFRx from gpio-pxa driver. Then we can support


Thanks... I wondered about division of labor between pinctrl and gpio.  Pinctrl
did seem to be the appropriate place.


> it by pinctrl-single driver since pinctrl-single driver could support mulitple
> pins in one pin registers. There's "bits-per-mux" property in pinctrl-single
> driver.


Yes, but currently pinctrl-single only supports writing one register for a given
pin (with multiple pins sharing a register if bit-per-mux==true).  On pxa27x, a
pin's alt function is determined by the values written to both the GAFRx and the
GPDRx registers, so I think that pinctrl-single may need to allow a device tree
to specify multiple reg/value/mask sets for any one pin.  I don't have a
pxa3xx/mmp developer's manual handy, but from the code it appears that on these
arches the direction register is irrelevant to the mux setting.


>>
>> (2) Some of the pinctrl-single code still assumes one register-per-pin; see for
>> example the functions pcs_allocate_pin_table() and pcs_get_pin_by_offset().
>> Curiously, the recently added pcs_request_gpio() function also suffers from
>> this, unless I'm mistaken.
>>
> Not exactly. pinctrl-single driver could support multiple pins in one register.
> Now Manjunathappa also fixed some issue in pinctrl-single for supporting
> multiple pins. (https://lkml.org/lkml/2013/5/21/226) I suggest you to develop
> based on his patches.


Thanks for the pointer.  I was away from the linux-arm-kernel ML until
yesterday, so I didn't see the patch.  Yes, it looks like bits_per_pin corrects
the problem.

I guess I'll take a stab at this.  Any other comments greatly appreciated.

Thanks again,
Mike

Re: pxa27x and pinctrl-single

[Haojian Zhuang]

On 7 June 2013 01:33, Mike Dunn <mikedunn-kFrNdAxtuftBDgjK7y7TUQ@public.gmane.org> wrote:
> Thanks for the reply Haojian.
>
> On 06/05/2013 05:43 PM, Haojian Zhuang wrote:
>> On 6 June 2013 01:23, Mike Dunn <mikedunn-kFrNdAxtuftBDgjK7y7TUQ@public.gmane.org> wrote:
>>> Hi,
>>>
>>> I'd like to start converting to device tree usage some of the old pxa27x
>>> platforms I'm fond of, starting with adding pinctrl support.  If I'm not
>>> mistaken, this will clear the way for some ongoing updates to the pinctrl/gpio
>>> code used by newer Marvell arches.
>>>
>>> I noticed that Haojian pulled the pinctrl-pxa driver from the kernel, in favor
>>> of using pinctrl-single, so I turned my attention to that.  From what I can
>>> tell, pinctrl-single is currently inadequate for pxa27x because:
>>>
>>> (1) On the pxa27x, setting the mux for a pin involves configuring both the alt
>>> function register *and* the direction register, requiring the ability to specify
>>> in the device tree multiple reg/value pairs for each pin.
>> In PXA27x, GPIO controller control both GPIO & pin alternate function. The
>> alternate function is covered by GAFRx registers.
>>
>> At first, we need to move GAFRx from gpio-pxa driver. Then we can support
>
>
> Thanks... I wondered about division of labor between pinctrl and gpio.  Pinctrl
> did seem to be the appropriate place.
>
>
>> it by pinctrl-single driver since pinctrl-single driver could support mulitple
>> pins in one pin registers. There's "bits-per-mux" property in pinctrl-single
>> driver.
>
>
> Yes, but currently pinctrl-single only supports writing one register for a given
> pin (with multiple pins sharing a register if bit-per-mux==true).  On pxa27x, a
> pin's alt function is determined by the values written to both the GAFRx and the
> GPDRx registers, so I think that pinctrl-single may need to allow a device tree
> to specify multiple reg/value/mask sets for any one pin.  I don't have a
> pxa3xx/mmp developer's manual handy, but from the code it appears that on these
> arches the direction register is irrelevant to the mux setting.
>

No, it's not need to access both GAFRx & GPDRx. Let's focus on PXA27x first.

GPIO ‘x’ Alternate Function Select Bits (where x = 112 through 120)
A bit-pair in this register determines the corresponding GPIO pin’s
functionality as one of the alternate functions that is mapped to it or as a
generic GPIO pin.

0b00 = The corresponding GPIO pin (GPIO<x>) is used as a general-
purpose I/O.
0b01 = The corresponding GPIO pin (GPIO<x>) is used for its alternate
function 1.
0b10 = The corresponding GPIO pin (GPIO<x>) is used for its alternate
function 2.
0b11 = The corresponding GPIO pin (GPIO<x>) is used for its alternate
function 3.

We can see that GAFRx configures the pinmux. That's all. We can use
pinctrl-single
driver to cover this.

I know that you're also talking the implementation in gpio-pxa driver.
__gpio_is_occupied() checks both GAFRx & GPDRx. Because this GPIO
pin should be configured as GPIO input. Actually we could also discard the
checking.

So there's no relationship between GAFRx and GPDRx. And "inverted" GPIO
pins only exists in PXA26x. This "inverted" means that GPDR bits should
also be inverted.

Regards
Haojian

Re: pxa27x and pinctrl-single

[Mike Dunn]

On 06/06/2013 04:58 PM, Haojian Zhuang wrote:
> On 7 June 2013 01:33, Mike Dunn <mikedunn@newsguy.com> wrote:
>>


[...]


>>
>> Yes, but currently pinctrl-single only supports writing one register for a given
>> pin (with multiple pins sharing a register if bit-per-mux==true).  On pxa27x, a
>> pin's alt function is determined by the values written to both the GAFRx and the
>> GPDRx registers, so I think that pinctrl-single may need to allow a device tree
>> to specify multiple reg/value/mask sets for any one pin.  I don't have a
>> pxa3xx/mmp developer's manual handy, but from the code it appears that on these
>> arches the direction register is irrelevant to the mux setting.
>>
> 
> No, it's not need to access both GAFRx & GPDRx. Let's focus on PXA27x first.
> 
> GPIO ‘x’ Alternate Function Select Bits (where x = 112 through 120)
> A bit-pair in this register determines the corresponding GPIO pin’s
> functionality as one of the alternate functions that is mapped to it or as a
> generic GPIO pin.
> 
> 0b00 = The corresponding GPIO pin (GPIO<x>) is used as a general-
> purpose I/O.
> 0b01 = The corresponding GPIO pin (GPIO<x>) is used for its alternate
> function 1.
> 0b10 = The corresponding GPIO pin (GPIO<x>) is used for its alternate
> function 2.
> 0b11 = The corresponding GPIO pin (GPIO<x>) is used for its alternate
> function 3.
> 
> We can see that GAFRx configures the pinmux. That's all. We can use
> pinctrl-single
> driver to cover this.


But on the pxa27x there are usually *two* meanings for any one of the 3 alt
functions, depending on the direction.  As a random example... gpio28 alt fn
0b01 is AC97_BITCLK if the direction is input, but is I2S_BITCLK if the
direction is output (from PXA27x Developer's Manual, Table 24-2).


> 
> I know that you're also talking the implementation in gpio-pxa driver.
> __gpio_is_occupied() checks both GAFRx & GPDRx. Because this GPIO
> pin should be configured as GPIO input. Actually we could also discard the
> checking.
> 
> So there's no relationship between GAFRx and GPDRx.


But I am thinking on pxa27x there is... see my example above.  Perhaps this is
not true for the later Marvell arches and you are missing this point?  Or else I
am stating the obvious to you and I am missing your point.  In either case,
thanks for your patience.

Thanks,
Mike

Re: pxa27x and pinctrl-single

[Haojian Zhuang]

On Fri, Jun 7, 2013 at 8:48 AM, Mike Dunn <mikedunn-kFrNdAxtuftBDgjK7y7TUQ@public.gmane.org> wrote:
> On 06/06/2013 04:58 PM, Haojian Zhuang wrote:
>> On 7 June 2013 01:33, Mike Dunn <mikedunn-kFrNdAxtuftBDgjK7y7TUQ@public.gmane.org> wrote:
>>>
>
>
> [...]
>
>
>>>
>>> Yes, but currently pinctrl-single only supports writing one register for a given
>>> pin (with multiple pins sharing a register if bit-per-mux==true).  On pxa27x, a
>>> pin's alt function is determined by the values written to both the GAFRx and the
>>> GPDRx registers, so I think that pinctrl-single may need to allow a device tree
>>> to specify multiple reg/value/mask sets for any one pin.  I don't have a
>>> pxa3xx/mmp developer's manual handy, but from the code it appears that on these
>>> arches the direction register is irrelevant to the mux setting.
>>>
>>
>> No, it's not need to access both GAFRx & GPDRx. Let's focus on PXA27x first.
>>
>> GPIO ‘x’ Alternate Function Select Bits (where x = 112 through 120)
>> A bit-pair in this register determines the corresponding GPIO pin’s
>> functionality as one of the alternate functions that is mapped to it or as a
>> generic GPIO pin.
>>
>> 0b00 = The corresponding GPIO pin (GPIO<x>) is used as a general-
>> purpose I/O.
>> 0b01 = The corresponding GPIO pin (GPIO<x>) is used for its alternate
>> function 1.
>> 0b10 = The corresponding GPIO pin (GPIO<x>) is used for its alternate
>> function 2.
>> 0b11 = The corresponding GPIO pin (GPIO<x>) is used for its alternate
>> function 3.
>>
>> We can see that GAFRx configures the pinmux. That's all. We can use
>> pinctrl-single
>> driver to cover this.
>
>
> But on the pxa27x there are usually *two* meanings for any one of the 3 alt
> functions, depending on the direction.  As a random example... gpio28 alt fn
> 0b01 is AC97_BITCLK if the direction is input, but is I2S_BITCLK if the
> direction is output (from PXA27x Developer's Manual, Table 24-2).
>

You didn't understand the alternate function. GPIO mode is only one mode
of alternate functions. Because GPIO means that user can program the
pin, you also need to configure the direction for GPIO usage.

Most of pins are either input or output. If the alternate function is
AC97_BITCLK,
input mode is configured by hardware automatically. You shouldn't configure
it in GPDR register. Since it's working in AC97 mode, not GPIO mode. Configuring
in GPDR register can't work.

>
>>
>> I know that you're also talking the implementation in gpio-pxa driver.
>> __gpio_is_occupied() checks both GAFRx & GPDRx. Because this GPIO
>> pin should be configured as GPIO input. Actually we could also discard the
>> checking.
>>
>> So there's no relationship between GAFRx and GPDRx.
>
>
> But I am thinking on pxa27x there is... see my example above.  Perhaps this is
> not true for the later Marvell arches and you are missing this point?  Or else I
> am stating the obvious to you and I am missing your point.  In either case,
> thanks for your patience.
>
I explained this above.

Regards
Haojian

Re: pxa27x and pinctrl-single

[Mike Dunn]

On 06/06/2013 06:21 PM, Haojian Zhuang wrote:
> On Fri, Jun 7, 2013 at 8:48 AM, Mike Dunn <mikedunn@newsguy.com> wrote:
>> On 06/06/2013 04:58 PM, Haojian Zhuang wrote:
>>> On 7 June 2013 01:33, Mike Dunn <mikedunn@newsguy.com> wrote:
>>>>
>>
>>
>> [...]
>>
>>
>>>>
>>>> Yes, but currently pinctrl-single only supports writing one register for a given
>>>> pin (with multiple pins sharing a register if bit-per-mux==true).  On pxa27x, a
>>>> pin's alt function is determined by the values written to both the GAFRx and the
>>>> GPDRx registers, so I think that pinctrl-single may need to allow a device tree
>>>> to specify multiple reg/value/mask sets for any one pin.  I don't have a
>>>> pxa3xx/mmp developer's manual handy, but from the code it appears that on these
>>>> arches the direction register is irrelevant to the mux setting.
>>>>
>>>
>>> No, it's not need to access both GAFRx & GPDRx. Let's focus on PXA27x first.
>>>
>>> GPIO ‘x’ Alternate Function Select Bits (where x = 112 through 120)
>>> A bit-pair in this register determines the corresponding GPIO pin’s
>>> functionality as one of the alternate functions that is mapped to it or as a
>>> generic GPIO pin.
>>>
>>> 0b00 = The corresponding GPIO pin (GPIO<x>) is used as a general-
>>> purpose I/O.
>>> 0b01 = The corresponding GPIO pin (GPIO<x>) is used for its alternate
>>> function 1.
>>> 0b10 = The corresponding GPIO pin (GPIO<x>) is used for its alternate
>>> function 2.
>>> 0b11 = The corresponding GPIO pin (GPIO<x>) is used for its alternate
>>> function 3.
>>>
>>> We can see that GAFRx configures the pinmux. That's all. We can use
>>> pinctrl-single
>>> driver to cover this.
>>
>>
>> But on the pxa27x there are usually *two* meanings for any one of the 3 alt
>> functions, depending on the direction.  As a random example... gpio28 alt fn
>> 0b01 is AC97_BITCLK if the direction is input, but is I2S_BITCLK if the
>> direction is output (from PXA27x Developer's Manual, Table 24-2).
>>
> 
> You didn't understand the alternate function. GPIO mode is only one mode
> of alternate functions. Because GPIO means that user can program the
> pin, you also need to configure the direction for GPIO usage.
> 
> Most of pins are either input or output. If the alternate function is
> AC97_BITCLK,
> input mode is configured by hardware automatically. You shouldn't configure
> it in GPDR register. Since it's working in AC97 mode, not GPIO mode. Configuring
> in GPDR register can't work.


This is where you are wrong Haojian.  Now I am sure that this peculiarity is
unique to pxa2xx.  From pxa27x developer's manual (section 24.4.2)...

GPIO pins can have as many as three alternate input and three alternate output
functions.

And in the comments above the __gpio_is_occupied() function in gpio-pxa.c you
mentioned earlier...

/*
 * On PXA25x and PXA27x, GAFRx and GPDRx together decide the alternate
 * function of a GPIO, and GPDRx cannot be altered once configured. It


I'll try to state it another way... on the old Intel pxa arches, the direction
bit is effectively a third alt function bit, as "alt function" is defined by the
newer Marvell processors.

I'll be happy to send you the pxa27x developer's manual if you still have
doubts.  In the gpio chapter you will see a table with a row for each gpio pin,
each row containing 6 columns labeled

alt fn 1 (in)
alt fn 2 (in)
alt fn 3 (in)
alt fn 1 (out)
alt fn 2 (out)
alt fn 3 (out)

I hope that makes sense now.

Thanks again,
Mike

Re: pxa27x and pinctrl-single

[Haojian Zhuang]

On 7 June 2013 22:50, Mike Dunn <mikedunn-kFrNdAxtuftBDgjK7y7TUQ@public.gmane.org> wrote:
> On 06/06/2013 06:21 PM, Haojian Zhuang wrote:
>> On Fri, Jun 7, 2013 at 8:48 AM, Mike Dunn <mikedunn-kFrNdAxtuftBDgjK7y7TUQ@public.gmane.org> wrote:
>>> On 06/06/2013 04:58 PM, Haojian Zhuang wrote:
>>>> On 7 June 2013 01:33, Mike Dunn <mikedunn-kFrNdAxtuftBDgjK7y7TUQ@public.gmane.org> wrote:
>>>>>
>>>
>>>
>>> [...]
>>>
>>>
>>>>>
>>>>> Yes, but currently pinctrl-single only supports writing one register for a given
>>>>> pin (with multiple pins sharing a register if bit-per-mux==true).  On pxa27x, a
>>>>> pin's alt function is determined by the values written to both the GAFRx and the
>>>>> GPDRx registers, so I think that pinctrl-single may need to allow a device tree
>>>>> to specify multiple reg/value/mask sets for any one pin.  I don't have a
>>>>> pxa3xx/mmp developer's manual handy, but from the code it appears that on these
>>>>> arches the direction register is irrelevant to the mux setting.
>>>>>
>>>>
>>>> No, it's not need to access both GAFRx & GPDRx. Let's focus on PXA27x first.
>>>>
>>>> GPIO ‘x’ Alternate Function Select Bits (where x = 112 through 120)
>>>> A bit-pair in this register determines the corresponding GPIO pin’s
>>>> functionality as one of the alternate functions that is mapped to it or as a
>>>> generic GPIO pin.
>>>>
>>>> 0b00 = The corresponding GPIO pin (GPIO<x>) is used as a general-
>>>> purpose I/O.
>>>> 0b01 = The corresponding GPIO pin (GPIO<x>) is used for its alternate
>>>> function 1.
>>>> 0b10 = The corresponding GPIO pin (GPIO<x>) is used for its alternate
>>>> function 2.
>>>> 0b11 = The corresponding GPIO pin (GPIO<x>) is used for its alternate
>>>> function 3.
>>>>
>>>> We can see that GAFRx configures the pinmux. That's all. We can use
>>>> pinctrl-single
>>>> driver to cover this.
>>>
>>>
>>> But on the pxa27x there are usually *two* meanings for any one of the 3 alt
>>> functions, depending on the direction.  As a random example... gpio28 alt fn
>>> 0b01 is AC97_BITCLK if the direction is input, but is I2S_BITCLK if the
>>> direction is output (from PXA27x Developer's Manual, Table 24-2).
>>>
>>
>> You didn't understand the alternate function. GPIO mode is only one mode
>> of alternate functions. Because GPIO means that user can program the
>> pin, you also need to configure the direction for GPIO usage.
>>
>> Most of pins are either input or output. If the alternate function is
>> AC97_BITCLK,
>> input mode is configured by hardware automatically. You shouldn't configure
>> it in GPDR register. Since it's working in AC97 mode, not GPIO mode. Configuring
>> in GPDR register can't work.
>
>
> This is where you are wrong Haojian.  Now I am sure that this peculiarity is
> unique to pxa2xx.  From pxa27x developer's manual (section 24.4.2)...
>
> GPIO pins can have as many as three alternate input and three alternate output
> functions.
>
> And in the comments above the __gpio_is_occupied() function in gpio-pxa.c you
> mentioned earlier...
>
> /*
>  * On PXA25x and PXA27x, GAFRx and GPDRx together decide the alternate
>  * function of a GPIO, and GPDRx cannot be altered once configured. It
>
>
> I'll try to state it another way... on the old Intel pxa arches, the direction
> bit is effectively a third alt function bit, as "alt function" is defined by the
> newer Marvell processors.
>
> I'll be happy to send you the pxa27x developer's manual if you still have
> doubts.  In the gpio chapter you will see a table with a row for each gpio pin,
> each row containing 6 columns labeled
>
> alt fn 1 (in)
> alt fn 2 (in)
> alt fn 3 (in)
> alt fn 1 (out)
> alt fn 2 (out)
> alt fn 3 (out)
>
> I hope that makes sense now.
>
> Thanks again,
> Mike

Thank you for pointing this to me. I didn't realize the GPDR needs to
be configured
in PXA2xx. It's totally different from PXA3xx.

Since you need to configure both GPDRx and GAFRx. If we are talking
pinctrl-single
as reference, we can make it work by this way.

We need to define two pinmux controllers. One is for GPDRx, and the
other is for GAFRx.
Both of them need to support pinctrl-single,bits property. For any
alternate pins in DTS,
we could include these two pins from two pinmux controllers. What's
your opinion?

Regards
Haojian

Re: pxa27x and pinctrl-single

[Mike Dunn]

On 06/07/2013 08:16 AM, Haojian Zhuang wrote:
> 

[...]


> Since you need to configure both GPDRx and GAFRx. If we are talking
> pinctrl-single
> as reference, we can make it work by this way.
> 
> We need to define two pinmux controllers. One is for GPDRx, and the
> other is for GAFRx.
> Both of them need to support pinctrl-single,bits property. For any
> alternate pins in DTS,
> we could include these two pins from two pinmux controllers. What's
> your opinion?


Are you are suggesting that the dts file can be defined such that the desired
values are written to GAFR and GPDR, without having to make any changes to the
pinctrl-single driver code?  If I understand correctly, we would be defining two
"pins" in the device tree for each actual pin.  That seems very ugly.

I was thinking that pinctrl-single could be modified to support multiple
reg/value/mask pairs for each pin listed in the pinctrl-single,bits property.
There is a comment at the top of pcs_parse_one_pinctrl_entry() that seems to
suggest the possibility...

 * Note that this binding currently supports only sets of one register + value.

I am still learning pinctrl and devicetree and studying the driver, so I am not
adament about anything at this point.  Guidance appreciated!

Thanks,
Mike

Re: pxa27x and pinctrl-single

[Haojian Zhuang]

On 8 June 2013 01:41, Mike Dunn <mikedunn-kFrNdAxtuftBDgjK7y7TUQ@public.gmane.org> wrote:
> On 06/07/2013 08:16 AM, Haojian Zhuang wrote:
>>
>
> [...]
>
>
>> Since you need to configure both GPDRx and GAFRx. If we are talking
>> pinctrl-single
>> as reference, we can make it work by this way.
>>
>> We need to define two pinmux controllers. One is for GPDRx, and the
>> other is for GAFRx.
>> Both of them need to support pinctrl-single,bits property. For any
>> alternate pins in DTS,
>> we could include these two pins from two pinmux controllers. What's
>> your opinion?
>
>
> Are you are suggesting that the dts file can be defined such that the desired
> values are written to GAFR and GPDR, without having to make any changes to the
> pinctrl-single driver code?  If I understand correctly, we would be defining two
> "pins" in the device tree for each actual pin.  That seems very ugly.
>
> I was thinking that pinctrl-single could be modified to support multiple
> reg/value/mask pairs for each pin listed in the pinctrl-single,bits property.
> There is a comment at the top of pcs_parse_one_pinctrl_entry() that seems to
> suggest the possibility...

We only support continuous register offset in pinctrl-single driver.
GPDRx is in range of 0x40e0000c~0x40e0010c, GAFRx is in range of
0x40e00054~0x40e00070.

So I suggest you to split them as two pinmux controller. If you define them into
one pinmux controler, it's also OK. But they are still two pins in the on pinmux
controller. And you should avoid to access those spare pins in the middle.

I don't suggest you to support multiple reg/value/mask pairs for each pin in
pinctrl-single driver. It's too complex. It already exceeds the design scope
of the pinctrl-single driver.

Regards
Haojian
>
>  * Note that this binding currently supports only sets of one register + value.
>
> I am still learning pinctrl and devicetree and studying the driver, so I am not
> adament about anything at this point.  Guidance appreciated!
>
> Thanks,
> Mike

Re: pxa27x and pinctrl-single

[Mike Dunn]

On 06/07/2013 06:20 PM, Haojian Zhuang wrote:
> On 8 June 2013 01:41, Mike Dunn <mikedunn@newsguy.com> wrote:
>> On 06/07/2013 08:16 AM, Haojian Zhuang wrote:
>>>
>>
>> [...]
>>
>>
>>> Since you need to configure both GPDRx and GAFRx. If we are talking
>>> pinctrl-single
>>> as reference, we can make it work by this way.
>>>
>>> We need to define two pinmux controllers. One is for GPDRx, and the
>>> other is for GAFRx.
>>> Both of them need to support pinctrl-single,bits property. For any
>>> alternate pins in DTS,
>>> we could include these two pins from two pinmux controllers. What's
>>> your opinion?
>>
>>
>> Are you are suggesting that the dts file can be defined such that the desired
>> values are written to GAFR and GPDR, without having to make any changes to the
>> pinctrl-single driver code?  If I understand correctly, we would be defining two
>> "pins" in the device tree for each actual pin.  That seems very ugly.
>>
>> I was thinking that pinctrl-single could be modified to support multiple
>> reg/value/mask pairs for each pin listed in the pinctrl-single,bits property.
>> There is a comment at the top of pcs_parse_one_pinctrl_entry() that seems to
>> suggest the possibility...
> 
> We only support continuous register offset in pinctrl-single driver.
> GPDRx is in range of 0x40e0000c~0x40e0010c, GAFRx is in range of
> 0x40e00054~0x40e00070.


Ah, I see...


> 
> So I suggest you to split them as two pinmux controller. If you define them into
> one pinmux controler, it's also OK. But they are still two pins in the on pinmux
> controller. And you should avoid to access those spare pins in the middle.


I'm still not smart enough to parse this.  Do you mean create a separate driver,
or two instances of pinctrl-single?


> 
> I don't suggest you to support multiple reg/value/mask pairs for each pin in
> pinctrl-single driver. It's too complex. It already exceeds the design scope
> of the pinctrl-single driver.


Yes, this I understand.  BTW, I see there are more patches for pinctrl-single
coming in.

Thanks for the advice Haojian.

Mike