Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver

[Haojian Zhuang]

On 8 June 2013 03:18, Stephen Warren <swarren@xxxxxxxxxxxxx> wrote:
> On 06/06/2013 09:30 AM, Christian Ruppert wrote:
>> On Thu, Jun 06, 2013 at 10:32:21PM +0800, Haojian Zhuang wrote:
>>> On 6 June 2013 22:11, Christian Ruppert <christian.ruppert@xxxxxxxxxx> wrote:
>>>> On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
>>>>> On 3 June 2013 20:30, Christian Ruppert <christian.ruppert@xxxxxxxxxx> wrote:
>>>>>> OK, here's a simplified example of what we would like to do (this seems
>>>>>> pretty common so I suppose there is a way I haven't understood). Our
>>>>>> situation is slightly more complex but for the purpose of discussion
>>>>>> let's assume a chip with 8 pins which can be configured for the
>>>>>> following functions:
>>>>>>
>>>>>> Pin   GPIO-A    I2C    SPI0     SPI1
>>>>>> ------------------------------------
>>>>>>  1    GPIOA0    SDA             MISO1
>>>>>>  2    GPIOA1    SCL             MOSI1
>>>>>>  3    GPIOA2                    SS1_B
>>>>>>  4    GPIOA3                    SCLK1
>>>>>>  5    GPIOA4           MISO0
>>>>>>  6    GPIOA5           MOSI0
>>>>>>  7    GPIOA6           SS0_B
>>>>>>  8    GPIOA7           SCLK0
>>>>>>
>>>>>> We can now define the following pinctrl-single:
>>>>>>
>>>>>> pinmux: pinmux@0xFFEE0000 {
>>>>>>         compatible = "pinctrl-single";
>>>>>>         reg = <0xFFEE0000 0x8>;
>>>>>>         #address-cells = <1>;
>>>>>>         #size-cells = <0>;
>>>>>>         #gpio-range-cells = <3>;
>>>>>>         pinctrl-single,register-width = <32>;
>>>>>>         pinctrl-single,function-mask = <0xffffffff>;
>>>>>>         pinctrl-single,gpio-range = <&range 1 8 0>;
>>>>>>         gpioa_pins: pinmux_gpioa_pins {
>>>>>>                 pinctrl-single,pins = <0x0 0 0x4 0>
>>>>>>         };
>>>>>>         i2c_pins: pinmux_i2c_pins {
>>>>>>                 pinctrl-single,pins = <0x0 1>
>>>>>>         };
>>>>>>         spi0_pins: pinmux_spi0_pins {
>>>>>>                 pinctrl-single,pins = <0x1 1>
>>>>> <0x1 1>?
>>>>>
>>>>> If each pinmux register is only for one pin in your SoC.
>>>>> I think that your definitions are wrong above. We use
>>>>> register offset as the first argument, not pin number.
>>>>> And the second argument should be pin function number.
>>>>
>>>> In our case each pinmux register (bit field) actually controls an entire
>>>> group of pins.
>>>>
>>>>> If multiple pins are sharing one register with different bits,
>>>>> you need to enable "pinctrl-single,bit-per-mux".
>>>>
>>>> Multiple pins are sharing the same bits in the same register. Do you
>>>> think this prevents us from using pinctrl-single?
>>>>
>>> Could you give me your register definition? Then I can understand you
>>> better.
>>
>> In our example, the register map would look a bit like the following.
>> Note that every register configures four pins at a time.
>>
>> Register 0x0:
>>  Mode  GPIO-A    I2C    SPI1
>>  Value 0x0       0x1    0x2
>>  ---------------------------
>>  Pin1  GPIOA0    SDA    MISO1
>>  Pin2  GPIOA1    SCL    MOSI1
>>  Pin3  GPIOA2           SS1_B
>>  Pin4  GPIOA3           SCLK1
>>
>> Register 0x4:
>>  Mode  GPIO-A    SPI0
>>  Value 0x0       0x1
>>  ---------------------
>>  Pin5  GPIOA4    MISO0
>>  Pin6  GPIOA5    MOSI0
>>  Pin7  GPIOA6    SS0_B
>>  Pin8  GPIOA7    SCLK0
>
> My suggestion here is that pinctrl-single isn't appropriate. The only
> way it could work is if you pretend that each group-of-pins is actually
> a single pin.
>
> However, then the correlation between these pretend pins (i.e. really
> the groups) and GPIOs won't work, because each "pin" is really 4 pins,
> and hence 4 GPIOs, and hence you won't be able to gpio_get() more than 1
> GPIO per pin group, I think.

Actually we can get each GPIO in the SoC. But we need to do some workaround.

1. As we discussed, we need to pretend a pin group as a single pin.

2. In DTS, we need to define "gpio-ranges" in gpio node and
"pinctrl-single,gpio-range"
in pinmux node as below.

    gpio {
             /* gpio offset, pin offset, nr pins */
             /* skip GPIOA1 & GPIOA3, PIN0 means pin1/pin2, PIN1 means
pin3/pin4 */
            gpio-ranges = <&pmx 0 0 1 &pmx 2 1 1>;
    };

    pmx {
              /* pin offset, nr pins, gpio function */
             pinctrl-single,gpio-range = <&range 0 1 0 &range 1 1 0>
    };
    range {
             #pinctrl-single,gpio-range-cells = <3>;
    };

Because we pretend pin1/pin2 as one single pin (PIN1), we skip to define it
in gpio-ranges. This range is only help you to find right pinmux controller.

Yes, I agree that pinctrl-single driver isn't 100% appropriate. But it
could work.
I verified it.

Regards
Haojian

>
> It's not hard (although possibly data intensive depending on your SoC)
> to represent your HW just fine with a native pinctrl driver; pinctrl
> itself has the ability to separate the concepts of pins, groups-of-pins,
> and the mux-functions-that-are-assigned-to-groups. If any of your HW
> registers actually do control only a single pin, you can simply create
> both a pin and a group that contains only that one pin. This is all very
> similar to how Tegra works, although it sounds like your registers may
> be a bit more regular than Tegras - Tegra has a very variable number of
> pins in each grop, and even some overlap between groups (mux function
> groups and pin configuration groups aren't aligned).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/