Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Tue, Jun 11, 2013 at 9:27 AM, Christian Ruppert wrote: > On Fri, Jun 07, 2013 at 01:18:35PM -0600, Stephen Warren wrote: >> 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. > > I agree: Unluckily, pinctrl-single doesn't seem to be what we are looking > for. I agree on this conclusion. > We already have a draft for a native pinctrl driver (see original post) > but there was some disagreement about exposing kernel internal pin > numbering to device tree users. But that is an orthogonal issue I believe. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Tue, Jun 11, 2013 at 9:27 AM, Christian Ruppert christian.rupp...@abilis.com wrote: On Fri, Jun 07, 2013 at 01:18:35PM -0600, Stephen Warren wrote: 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. I agree: Unluckily, pinctrl-single doesn't seem to be what we are looking for. I agree on this conclusion. We already have a draft for a native pinctrl driver (see original post) but there was some disagreement about exposing kernel internal pin numbering to device tree users. But that is an orthogonal issue I believe. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Fri, Jun 07, 2013 at 01:18:35PM -0600, Stephen Warren 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
> >> wrote:
> >>> On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
> On 3 June 2013 20:30, Christian Ruppert
> 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-AI2CSPI0 SPI1
> >
> > 1GPIOA0SDA MISO1
> > 2GPIOA1SCL MOSI1
> > 3GPIOA2SS1_B
> > 4GPIOA3SCLK1
> > 5GPIOA4 MISO0
> > 6GPIOA5 MOSI0
> > 7GPIOA6 SS0_B
> > 8GPIOA7 SCLK0
> >
> > We can now define the following pinctrl-single:
> >
> > pinmux: pinmux@0xFFEE {
> > compatible = "pinctrl-single";
> > reg = <0xFFEE 0x8>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > #gpio-range-cells = <3>;
> > pinctrl-single,register-width = <32>;
> > pinctrl-single,function-mask = <0x>;
> > pinctrl-single,gpio-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-AI2CSPI1
> > Value 0x0 0x10x2
> > ---
> > Pin1 GPIOA0SDAMISO1
> > Pin2 GPIOA1SCLMOSI1
> > Pin3 GPIOA2 SS1_B
> > Pin4 GPIOA3 SCLK1
> >
> > Register 0x4:
> > Mode GPIO-ASPI0
> > Value 0x0 0x1
> > -
> > Pin5 GPIOA4MISO0
> > Pin6 GPIOA5MOSI0
> > Pin7 GPIOA6SS0_B
> > Pin8 GPIOA7SCLK0
>
> 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.
I agree: Unluckily, pinctrl-single doesn't seem to be what we are looking
for.
> 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).
Actually, our registers are quite a bit more complex than the above
example (interdependencies between pin groups, overlapping
functionalities etc.) but I set the example to understand the concepts
which could then possibly be extended/applied to our real case.
We already have a draft for a native pinctrl driver (see original post)
but there was some disagreement about exposing kernel internal pin
numbering to device tree users. Now that it's pretty sure that we cannot
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Fri, Jun 07, 2013 at 01:18:35PM -0600, Stephen Warren 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.rupp...@abilis.com
wrote:
On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
On 3 June 2013 20:30, Christian Ruppert christian.rupp...@abilis.com
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-AI2CSPI0 SPI1
1GPIOA0SDA MISO1
2GPIOA1SCL MOSI1
3GPIOA2SS1_B
4GPIOA3SCLK1
5GPIOA4 MISO0
6GPIOA5 MOSI0
7GPIOA6 SS0_B
8GPIOA7 SCLK0
We can now define the following pinctrl-single:
pinmux: pinmux@0xFFEE {
compatible = pinctrl-single;
reg = 0xFFEE 0x8;
#address-cells = 1;
#size-cells = 0;
#gpio-range-cells = 3;
pinctrl-single,register-width = 32;
pinctrl-single,function-mask = 0x;
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-AI2CSPI1
Value 0x0 0x10x2
---
Pin1 GPIOA0SDAMISO1
Pin2 GPIOA1SCLMOSI1
Pin3 GPIOA2 SS1_B
Pin4 GPIOA3 SCLK1
Register 0x4:
Mode GPIO-ASPI0
Value 0x0 0x1
-
Pin5 GPIOA4MISO0
Pin6 GPIOA5MOSI0
Pin7 GPIOA6SS0_B
Pin8 GPIOA7SCLK0
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.
I agree: Unluckily, pinctrl-single doesn't seem to be what we are looking
for.
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).
Actually, our registers are quite a bit more complex than the above
example (interdependencies between pin groups, overlapping
functionalities etc.) but I set the example to understand the concepts
which could then possibly be extended/applied to our real case.
We already have a draft for a native pinctrl driver (see original post)
but there was some disagreement about exposing kernel internal pin
numbering to device tree users. Now that it's pretty sure that we cannot
reasonably use pinctrl-single I'll take up that work again and make a
proposal based on 1) in https://lkml.org/lkml/2013/5/22/207.
Greetings,
Christian
--
Christian Ruppert , christian.rupp...@abilis.com
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 06/08/2013 02:31 AM, Haojian Zhuang wrote:
> On 8 June 2013 03:18, Stephen Warren 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
wrote:
> On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
>> On 3 June 2013 20:30, Christian Ruppert
>> 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-AI2CSPI0 SPI1
>>>
>>> 1GPIOA0SDA MISO1
>>> 2GPIOA1SCL MOSI1
>>> 3GPIOA2SS1_B
>>> 4GPIOA3SCLK1
>>> 5GPIOA4 MISO0
>>> 6GPIOA5 MOSI0
>>> 7GPIOA6 SS0_B
>>> 8GPIOA7 SCLK0
>>>
>>> We can now define the following pinctrl-single:
>>>
>>> pinmux: pinmux@0xFFEE {
>>> compatible = "pinctrl-single";
>>> reg = <0xFFEE 0x8>;
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>> #gpio-range-cells = <3>;
>>> pinctrl-single,register-width = <32>;
>>> pinctrl-single,function-mask = <0x>;
>>> pinctrl-single,gpio-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-AI2CSPI1
>>> Value 0x0 0x10x2
>>> ---
>>> Pin1 GPIOA0SDAMISO1
>>> Pin2 GPIOA1SCLMOSI1
>>> Pin3 GPIOA2 SS1_B
>>> Pin4 GPIOA3 SCLK1
>>>
>>> Register 0x4:
>>> Mode GPIO-ASPI0
>>> Value 0x0 0x1
>>> -
>>> Pin5 GPIOA4MISO0
>>> Pin6 GPIOA5MOSI0
>>> Pin7 GPIOA6SS0_B
>>> Pin8 GPIOA7SCLK0
>>
>> 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 = < 0 0 1 2 1 1>;
> };
>
> pmx {
> /* pin offset, nr pins, gpio function */
> pinctrl-single,gpio-range = < 0 1 0 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.
Yeah, that sounds pretty horrible, sorry.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 8 June 2013 03:18, Stephen Warren 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
>>> wrote:
On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
> On 3 June 2013 20:30, Christian Ruppert
> 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-AI2CSPI0 SPI1
>>
>> 1GPIOA0SDA MISO1
>> 2GPIOA1SCL MOSI1
>> 3GPIOA2SS1_B
>> 4GPIOA3SCLK1
>> 5GPIOA4 MISO0
>> 6GPIOA5 MOSI0
>> 7GPIOA6 SS0_B
>> 8GPIOA7 SCLK0
>>
>> We can now define the following pinctrl-single:
>>
>> pinmux: pinmux@0xFFEE {
>> compatible = "pinctrl-single";
>> reg = <0xFFEE 0x8>;
>> #address-cells = <1>;
>> #size-cells = <0>;
>> #gpio-range-cells = <3>;
>> pinctrl-single,register-width = <32>;
>> pinctrl-single,function-mask = <0x>;
>> pinctrl-single,gpio-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-AI2CSPI1
>> Value 0x0 0x10x2
>> ---
>> Pin1 GPIOA0SDAMISO1
>> Pin2 GPIOA1SCLMOSI1
>> Pin3 GPIOA2 SS1_B
>> Pin4 GPIOA3 SCLK1
>>
>> Register 0x4:
>> Mode GPIO-ASPI0
>> Value 0x0 0x1
>> -
>> Pin5 GPIOA4MISO0
>> Pin6 GPIOA5MOSI0
>> Pin7 GPIOA6SS0_B
>> Pin8 GPIOA7SCLK0
>
> 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 = < 0 0 1 2 1 1>;
};
pmx {
/* pin offset, nr pins, gpio function */
pinctrl-single,gpio-range = < 0 1 0 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
>
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 8 June 2013 03:18, Stephen Warren swar...@wwwdotorg.org 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.rupp...@abilis.com
wrote:
On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
On 3 June 2013 20:30, Christian Ruppert christian.rupp...@abilis.com
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-AI2CSPI0 SPI1
1GPIOA0SDA MISO1
2GPIOA1SCL MOSI1
3GPIOA2SS1_B
4GPIOA3SCLK1
5GPIOA4 MISO0
6GPIOA5 MOSI0
7GPIOA6 SS0_B
8GPIOA7 SCLK0
We can now define the following pinctrl-single:
pinmux: pinmux@0xFFEE {
compatible = pinctrl-single;
reg = 0xFFEE 0x8;
#address-cells = 1;
#size-cells = 0;
#gpio-range-cells = 3;
pinctrl-single,register-width = 32;
pinctrl-single,function-mask = 0x;
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-AI2CSPI1
Value 0x0 0x10x2
---
Pin1 GPIOA0SDAMISO1
Pin2 GPIOA1SCLMOSI1
Pin3 GPIOA2 SS1_B
Pin4 GPIOA3 SCLK1
Register 0x4:
Mode GPIO-ASPI0
Value 0x0 0x1
-
Pin5 GPIOA4MISO0
Pin6 GPIOA5MOSI0
Pin7 GPIOA6SS0_B
Pin8 GPIOA7SCLK0
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
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 06/08/2013 02:31 AM, Haojian Zhuang wrote:
On 8 June 2013 03:18, Stephen Warren swar...@wwwdotorg.org 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.rupp...@abilis.com
wrote:
On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
On 3 June 2013 20:30, Christian Ruppert christian.rupp...@abilis.com
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-AI2CSPI0 SPI1
1GPIOA0SDA MISO1
2GPIOA1SCL MOSI1
3GPIOA2SS1_B
4GPIOA3SCLK1
5GPIOA4 MISO0
6GPIOA5 MOSI0
7GPIOA6 SS0_B
8GPIOA7 SCLK0
We can now define the following pinctrl-single:
pinmux: pinmux@0xFFEE {
compatible = pinctrl-single;
reg = 0xFFEE 0x8;
#address-cells = 1;
#size-cells = 0;
#gpio-range-cells = 3;
pinctrl-single,register-width = 32;
pinctrl-single,function-mask = 0x;
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-AI2CSPI1
Value 0x0 0x10x2
---
Pin1 GPIOA0SDAMISO1
Pin2 GPIOA1SCLMOSI1
Pin3 GPIOA2 SS1_B
Pin4 GPIOA3 SCLK1
Register 0x4:
Mode GPIO-ASPI0
Value 0x0 0x1
-
Pin5 GPIOA4MISO0
Pin6 GPIOA5MOSI0
Pin7 GPIOA6SS0_B
Pin8 GPIOA7SCLK0
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.
Yeah, that sounds pretty horrible, sorry.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
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 wrote:
>>> On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
On 3 June 2013 20:30, Christian Ruppert
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-AI2CSPI0 SPI1
>
> 1GPIOA0SDA MISO1
> 2GPIOA1SCL MOSI1
> 3GPIOA2SS1_B
> 4GPIOA3SCLK1
> 5GPIOA4 MISO0
> 6GPIOA5 MOSI0
> 7GPIOA6 SS0_B
> 8GPIOA7 SCLK0
>
> We can now define the following pinctrl-single:
>
> pinmux: pinmux@0xFFEE {
> compatible = "pinctrl-single";
> reg = <0xFFEE 0x8>;
> #address-cells = <1>;
> #size-cells = <0>;
> #gpio-range-cells = <3>;
> pinctrl-single,register-width = <32>;
> pinctrl-single,function-mask = <0x>;
> pinctrl-single,gpio-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-AI2CSPI1
> Value 0x0 0x10x2
> ---
> Pin1 GPIOA0SDAMISO1
> Pin2 GPIOA1SCLMOSI1
> Pin3 GPIOA2 SS1_B
> Pin4 GPIOA3 SCLK1
>
> Register 0x4:
> Mode GPIO-ASPI0
> Value 0x0 0x1
> -
> Pin5 GPIOA4MISO0
> Pin6 GPIOA5MOSI0
> Pin7 GPIOA6SS0_B
> Pin8 GPIOA7SCLK0
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.
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 7 June 2013 19:32, Christian Ruppert wrote:
> On Fri, Jun 07, 2013 at 08:00:57AM +0800, Haojian Zhuang wrote:
>> On 6 June 2013 23:30, Christian Ruppert wrote:
>> > On Thu, Jun 06, 2013 at 10:32:21PM +0800, Haojian Zhuang wrote:
>> >> On 6 June 2013 22:11, Christian Ruppert
>> >> wrote:
>> >> > On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
>> >> >> On 3 June 2013 20:30, Christian Ruppert
>> >> >> 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-AI2CSPI0 SPI1
>> >> >> >
>> >> >> > 1GPIOA0SDA MISO1
>> >> >> > 2GPIOA1SCL MOSI1
>> >> >> > 3GPIOA2SS1_B
>> >> >> > 4GPIOA3SCLK1
>> >> >> > 5GPIOA4 MISO0
>> >> >> > 6GPIOA5 MOSI0
>> >> >> > 7GPIOA6 SS0_B
>> >> >> > 8GPIOA7 SCLK0
>> >> >> >
>> >> >> > We can now define the following pinctrl-single:
>> >> >> >
>> >> >> > pinmux: pinmux@0xFFEE {
>> >> >> > compatible = "pinctrl-single";
>> >> >> > reg = <0xFFEE 0x8>;
>> >> >> > #address-cells = <1>;
>> >> >> > #size-cells = <0>;
>> >> >> > #gpio-range-cells = <3>;
>> >> >> > pinctrl-single,register-width = <32>;
>> >> >> > pinctrl-single,function-mask = <0x>;
>> >> >> > pinctrl-single,gpio-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-AI2CSPI1
>> > Value 0x0 0x10x2
>> > ---
>> > Pin1 GPIOA0SDAMISO1
>> > Pin2 GPIOA1SCLMOSI1
>> > Pin3 GPIOA2 SS1_B
>> > Pin4 GPIOA3 SCLK1
>> >
>> > Register 0x4:
>> > Mode GPIO-ASPI0
>> > Value 0x0 0x1
>> > -
>> > Pin5 GPIOA4MISO0
>> > Pin6 GPIOA5MOSI0
>> > Pin7 GPIOA6SS0_B
>> > Pin8 GPIOA7SCLK0
>> >
>>
>> You said "Multiple pins are sharing the same bits in the same register.".
>> I need to understand which bits you're talking about in your register.
>
> In the above example, bits 0 and 1 of register 0x0 control pins 1
> through 4 and bit 0 of register 0x4 controls pins 5 through 8. The
> moment you write a new value in either of those registers, all four pins
> will change functionality simultaneously. There is no way to control the
> functionality of each pin individually.
>
Oh. So some bits in the same register control multiple pins.
Yeah, I also meet this in Hisilicon SoC. My solution is to only define the
pinmux register for one pin, and skip other pins.
1. You're using GPIOA0 & GPIOA1 in two different driver.
You only need to define GPIOA0 or GPIOA1 in one of driver. Don't define
them at the same time.
2. You're using GPIOA0 & GPIOA1 in the same driver.
You only need to define GPIOA0 or GPIOA1.
3. If you're using SPI or any other function, it's same as GPIO function.
4. There's no #4. Since you won't use GPIOA0 with SCL pin together.
Regards
Haojian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Fri, Jun 07, 2013 at 01:36:16PM +0200, Linus Walleij wrote: > On Mon, Jun 3, 2013 at 11:42 AM, Christian Ruppert > wrote: > > > Ease of use is also the reason why I added the gpio-base property to the > > original driver: Finding out the global GPIO number to use in > > /sys/class/gpio for a given GPIO of a given bank seems to be a recurring > > headache for our customers and the definition of the bank's base number > > in the device tree is an attempt to improve this situation. > > What you need to do in that case is to find a way to name the pins > in sysfs (creating symbolic links with the GPIO pin name) so they > can use these names in sysfs instead. > > There is no ambition from my side to try to correlate the > GPIO sysfs interface and device trees. This is because the GPIO > sysfs is not universally liked. And when you say you want to make the > sysfs ABI easy to use that lights a big red light on my panel. I will > explain why. > > What is very important is that your customers understand that > the GPIO sysfs shall not be used for things like: > > - LEDs > - Switches > - Regulators > - Camera muxes > - etc > > From the kernel community we have tried (or atleast I have tried) > that this kind of hardware shall be handled by the apropriate linux > subsystems, and not by obscure userspace code. I totally agree with that and we already declare the LEDs etc. on our PCBs in the device trees. However, we have at least one customer who has a user space driver for some peripheral on i2c which needs to be reset through GPIO. I'm not sure which framework to use for this sort of applications. Greetings, Christian -- Christian Ruppert , /| Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri _// | bilis Systems CH-1228 Plan-les-Ouates -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Mon, Jun 3, 2013 at 11:42 AM, Christian Ruppert
wrote:
> Ease of use is also the reason why I added the gpio-base property to the
> original driver: Finding out the global GPIO number to use in
> /sys/class/gpio for a given GPIO of a given bank seems to be a recurring
> headache for our customers and the definition of the bank's base number
> in the device tree is an attempt to improve this situation.
What you need to do in that case is to find a way to name the pins
in sysfs (creating symbolic links with the GPIO pin name) so they
can use these names in sysfs instead.
There is no ambition from my side to try to correlate the
GPIO sysfs interface and device trees. This is because the GPIO
sysfs is not universally liked. And when you say you want to make the
sysfs ABI easy to use that lights a big red light on my panel. I will
explain why.
What is very important is that your customers understand that
the GPIO sysfs shall not be used for things like:
- LEDs
- Switches
- Regulators
- Camera muxes
- etc
>From the kernel community we have tried (or atleast I have tried)
that this kind of hardware shall be handled by the apropriate linux
subsystems, and not by obscure userspace code.
In a fight between device tree and GPIO sysfs device tree
*wins*.
Consider this example from the Snowball device tree:
en_3v3_reg: en_3v3 {
compatible = "regulator-fixed";
regulator-name = "en-3v3-fixed-supply";
regulator-min-microvolt = <330>;
regulator-max-microvolt = <330>;
gpios = < 26 0x4>; // 26
startup-delay-us = <5000>;
enable-active-high;
};
gpio_keys {
compatible = "gpio-keys";
#address-cells = <1>;
#size-cells = <0>;
button@1 {
debounce_interval = <50>;
wakeup = <1>;
linux,code = <2>;
label = "userpb";
gpios = < 0 0x4>;
};
button@2 {
debounce_interval = <50>;
wakeup = <1>;
linux,code = <3>;
label = "extkb1";
gpios = < 23 0x4>;
};
button@3 {
debounce_interval = <50>;
wakeup = <1>;
linux,code = <4>;
label = "extkb2";
gpios = < 24 0x4>;
};
button@4 {
debounce_interval = <50>;
wakeup = <1>;
linux,code = <5>;
label = "extkb3";
gpios = < 1 0x4>;
};
button@5 {
debounce_interval = <50>;
wakeup = <1>;
linux,code = <6>;
label = "extkb4";
gpios = < 2 0x4>;
};
};
leds {
compatible = "gpio-leds";
used-led {
label = "user_led";
gpios = < 14 0x4>;
default-state = "on";
linux,default-trigger = "heartbeat";
};
};
As you immediately realize, if people don't know how to specify
the above and start writing a userspace daemon to do the same
thing by hammering on sysfs files, they are doing the wrong thing.
What you need to make sure is that before your customers start
to do userspace tricks in GPIO sysfs they need to answer the
question "why?".
If the GPIO sysfs is encouraging people to do things like the
above from userspace, it needs to be actively discouraged,
because that is hurting the people doing that.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Fri, Jun 07, 2013 at 08:00:57AM +0800, Haojian Zhuang wrote:
> On 6 June 2013 23:30, Christian Ruppert wrote:
> > On Thu, Jun 06, 2013 at 10:32:21PM +0800, Haojian Zhuang wrote:
> >> On 6 June 2013 22:11, Christian Ruppert
> >> wrote:
> >> > On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
> >> >> On 3 June 2013 20:30, Christian Ruppert
> >> >> 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-AI2CSPI0 SPI1
> >> >> >
> >> >> > 1GPIOA0SDA MISO1
> >> >> > 2GPIOA1SCL MOSI1
> >> >> > 3GPIOA2SS1_B
> >> >> > 4GPIOA3SCLK1
> >> >> > 5GPIOA4 MISO0
> >> >> > 6GPIOA5 MOSI0
> >> >> > 7GPIOA6 SS0_B
> >> >> > 8GPIOA7 SCLK0
> >> >> >
> >> >> > We can now define the following pinctrl-single:
> >> >> >
> >> >> > pinmux: pinmux@0xFFEE {
> >> >> > compatible = "pinctrl-single";
> >> >> > reg = <0xFFEE 0x8>;
> >> >> > #address-cells = <1>;
> >> >> > #size-cells = <0>;
> >> >> > #gpio-range-cells = <3>;
> >> >> > pinctrl-single,register-width = <32>;
> >> >> > pinctrl-single,function-mask = <0x>;
> >> >> > pinctrl-single,gpio-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-AI2CSPI1
> > Value 0x0 0x10x2
> > ---
> > Pin1 GPIOA0SDAMISO1
> > Pin2 GPIOA1SCLMOSI1
> > Pin3 GPIOA2 SS1_B
> > Pin4 GPIOA3 SCLK1
> >
> > Register 0x4:
> > Mode GPIO-ASPI0
> > Value 0x0 0x1
> > -
> > Pin5 GPIOA4MISO0
> > Pin6 GPIOA5MOSI0
> > Pin7 GPIOA6SS0_B
> > Pin8 GPIOA7SCLK0
> >
>
> You said "Multiple pins are sharing the same bits in the same register.".
> I need to understand which bits you're talking about in your register.
In the above example, bits 0 and 1 of register 0x0 control pins 1
through 4 and bit 0 of register 0x4 controls pins 5 through 8. The
moment you write a new value in either of those registers, all four pins
will change functionality simultaneously. There is no way to control the
functionality of each pin individually.
Greetings,
Christian
--
Christian Ruppert ,
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
_// | bilis Systems CH-1228 Plan-les-Ouates
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 7 June 2013 19:32, Christian Ruppert christian.rupp...@abilis.com wrote:
On Fri, Jun 07, 2013 at 08:00:57AM +0800, Haojian Zhuang wrote:
On 6 June 2013 23:30, Christian Ruppert christian.rupp...@abilis.com wrote:
On Thu, Jun 06, 2013 at 10:32:21PM +0800, Haojian Zhuang wrote:
On 6 June 2013 22:11, Christian Ruppert christian.rupp...@abilis.com
wrote:
On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
On 3 June 2013 20:30, Christian Ruppert christian.rupp...@abilis.com
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-AI2CSPI0 SPI1
1GPIOA0SDA MISO1
2GPIOA1SCL MOSI1
3GPIOA2SS1_B
4GPIOA3SCLK1
5GPIOA4 MISO0
6GPIOA5 MOSI0
7GPIOA6 SS0_B
8GPIOA7 SCLK0
We can now define the following pinctrl-single:
pinmux: pinmux@0xFFEE {
compatible = pinctrl-single;
reg = 0xFFEE 0x8;
#address-cells = 1;
#size-cells = 0;
#gpio-range-cells = 3;
pinctrl-single,register-width = 32;
pinctrl-single,function-mask = 0x;
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-AI2CSPI1
Value 0x0 0x10x2
---
Pin1 GPIOA0SDAMISO1
Pin2 GPIOA1SCLMOSI1
Pin3 GPIOA2 SS1_B
Pin4 GPIOA3 SCLK1
Register 0x4:
Mode GPIO-ASPI0
Value 0x0 0x1
-
Pin5 GPIOA4MISO0
Pin6 GPIOA5MOSI0
Pin7 GPIOA6SS0_B
Pin8 GPIOA7SCLK0
You said Multiple pins are sharing the same bits in the same register..
I need to understand which bits you're talking about in your register.
In the above example, bits 0 and 1 of register 0x0 control pins 1
through 4 and bit 0 of register 0x4 controls pins 5 through 8. The
moment you write a new value in either of those registers, all four pins
will change functionality simultaneously. There is no way to control the
functionality of each pin individually.
Oh. So some bits in the same register control multiple pins.
Yeah, I also meet this in Hisilicon SoC. My solution is to only define the
pinmux register for one pin, and skip other pins.
1. You're using GPIOA0 GPIOA1 in two different driver.
You only need to define GPIOA0 or GPIOA1 in one of driver. Don't define
them at the same time.
2. You're using GPIOA0 GPIOA1 in the same driver.
You only need to define GPIOA0 or GPIOA1.
3. If you're using SPI or any other function, it's same as GPIO function.
4. There's no #4. Since you won't use GPIOA0 with SCL pin together.
Regards
Haojian
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
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.rupp...@abilis.com wrote:
On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
On 3 June 2013 20:30, Christian Ruppert christian.rupp...@abilis.com
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-AI2CSPI0 SPI1
1GPIOA0SDA MISO1
2GPIOA1SCL MOSI1
3GPIOA2SS1_B
4GPIOA3SCLK1
5GPIOA4 MISO0
6GPIOA5 MOSI0
7GPIOA6 SS0_B
8GPIOA7 SCLK0
We can now define the following pinctrl-single:
pinmux: pinmux@0xFFEE {
compatible = pinctrl-single;
reg = 0xFFEE 0x8;
#address-cells = 1;
#size-cells = 0;
#gpio-range-cells = 3;
pinctrl-single,register-width = 32;
pinctrl-single,function-mask = 0x;
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-AI2CSPI1
Value 0x0 0x10x2
---
Pin1 GPIOA0SDAMISO1
Pin2 GPIOA1SCLMOSI1
Pin3 GPIOA2 SS1_B
Pin4 GPIOA3 SCLK1
Register 0x4:
Mode GPIO-ASPI0
Value 0x0 0x1
-
Pin5 GPIOA4MISO0
Pin6 GPIOA5MOSI0
Pin7 GPIOA6SS0_B
Pin8 GPIOA7SCLK0
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.
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Fri, Jun 07, 2013 at 08:00:57AM +0800, Haojian Zhuang wrote:
On 6 June 2013 23:30, Christian Ruppert christian.rupp...@abilis.com wrote:
On Thu, Jun 06, 2013 at 10:32:21PM +0800, Haojian Zhuang wrote:
On 6 June 2013 22:11, Christian Ruppert christian.rupp...@abilis.com
wrote:
On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
On 3 June 2013 20:30, Christian Ruppert christian.rupp...@abilis.com
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-AI2CSPI0 SPI1
1GPIOA0SDA MISO1
2GPIOA1SCL MOSI1
3GPIOA2SS1_B
4GPIOA3SCLK1
5GPIOA4 MISO0
6GPIOA5 MOSI0
7GPIOA6 SS0_B
8GPIOA7 SCLK0
We can now define the following pinctrl-single:
pinmux: pinmux@0xFFEE {
compatible = pinctrl-single;
reg = 0xFFEE 0x8;
#address-cells = 1;
#size-cells = 0;
#gpio-range-cells = 3;
pinctrl-single,register-width = 32;
pinctrl-single,function-mask = 0x;
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-AI2CSPI1
Value 0x0 0x10x2
---
Pin1 GPIOA0SDAMISO1
Pin2 GPIOA1SCLMOSI1
Pin3 GPIOA2 SS1_B
Pin4 GPIOA3 SCLK1
Register 0x4:
Mode GPIO-ASPI0
Value 0x0 0x1
-
Pin5 GPIOA4MISO0
Pin6 GPIOA5MOSI0
Pin7 GPIOA6SS0_B
Pin8 GPIOA7SCLK0
You said Multiple pins are sharing the same bits in the same register..
I need to understand which bits you're talking about in your register.
In the above example, bits 0 and 1 of register 0x0 control pins 1
through 4 and bit 0 of register 0x4 controls pins 5 through 8. The
moment you write a new value in either of those registers, all four pins
will change functionality simultaneously. There is no way to control the
functionality of each pin individually.
Greetings,
Christian
--
Christian Ruppert , christian.rupp...@abilis.com
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
_// | bilis Systems CH-1228 Plan-les-Ouates
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Mon, Jun 3, 2013 at 11:42 AM, Christian Ruppert
christian.rupp...@abilis.com wrote:
Ease of use is also the reason why I added the gpio-base property to the
original driver: Finding out the global GPIO number to use in
/sys/class/gpio for a given GPIO of a given bank seems to be a recurring
headache for our customers and the definition of the bank's base number
in the device tree is an attempt to improve this situation.
What you need to do in that case is to find a way to name the pins
in sysfs (creating symbolic links with the GPIO pin name) so they
can use these names in sysfs instead.
There is no ambition from my side to try to correlate the
GPIO sysfs interface and device trees. This is because the GPIO
sysfs is not universally liked. And when you say you want to make the
sysfs ABI easy to use that lights a big red light on my panel. I will
explain why.
What is very important is that your customers understand that
the GPIO sysfs shall not be used for things like:
- LEDs
- Switches
- Regulators
- Camera muxes
- etc
From the kernel community we have tried (or atleast I have tried)
that this kind of hardware shall be handled by the apropriate linux
subsystems, and not by obscure userspace code.
In a fight between device tree and GPIO sysfs device tree
*wins*.
Consider this example from the Snowball device tree:
en_3v3_reg: en_3v3 {
compatible = regulator-fixed;
regulator-name = en-3v3-fixed-supply;
regulator-min-microvolt = 330;
regulator-max-microvolt = 330;
gpios = gpio0 26 0x4; // 26
startup-delay-us = 5000;
enable-active-high;
};
gpio_keys {
compatible = gpio-keys;
#address-cells = 1;
#size-cells = 0;
button@1 {
debounce_interval = 50;
wakeup = 1;
linux,code = 2;
label = userpb;
gpios = gpio1 0 0x4;
};
button@2 {
debounce_interval = 50;
wakeup = 1;
linux,code = 3;
label = extkb1;
gpios = gpio4 23 0x4;
};
button@3 {
debounce_interval = 50;
wakeup = 1;
linux,code = 4;
label = extkb2;
gpios = gpio4 24 0x4;
};
button@4 {
debounce_interval = 50;
wakeup = 1;
linux,code = 5;
label = extkb3;
gpios = gpio5 1 0x4;
};
button@5 {
debounce_interval = 50;
wakeup = 1;
linux,code = 6;
label = extkb4;
gpios = gpio5 2 0x4;
};
};
leds {
compatible = gpio-leds;
used-led {
label = user_led;
gpios = gpio4 14 0x4;
default-state = on;
linux,default-trigger = heartbeat;
};
};
As you immediately realize, if people don't know how to specify
the above and start writing a userspace daemon to do the same
thing by hammering on sysfs files, they are doing the wrong thing.
What you need to make sure is that before your customers start
to do userspace tricks in GPIO sysfs they need to answer the
question why?.
If the GPIO sysfs is encouraging people to do things like the
above from userspace, it needs to be actively discouraged,
because that is hurting the people doing that.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Fri, Jun 07, 2013 at 01:36:16PM +0200, Linus Walleij wrote: On Mon, Jun 3, 2013 at 11:42 AM, Christian Ruppert christian.rupp...@abilis.com wrote: Ease of use is also the reason why I added the gpio-base property to the original driver: Finding out the global GPIO number to use in /sys/class/gpio for a given GPIO of a given bank seems to be a recurring headache for our customers and the definition of the bank's base number in the device tree is an attempt to improve this situation. What you need to do in that case is to find a way to name the pins in sysfs (creating symbolic links with the GPIO pin name) so they can use these names in sysfs instead. There is no ambition from my side to try to correlate the GPIO sysfs interface and device trees. This is because the GPIO sysfs is not universally liked. And when you say you want to make the sysfs ABI easy to use that lights a big red light on my panel. I will explain why. What is very important is that your customers understand that the GPIO sysfs shall not be used for things like: - LEDs - Switches - Regulators - Camera muxes - etc From the kernel community we have tried (or atleast I have tried) that this kind of hardware shall be handled by the apropriate linux subsystems, and not by obscure userspace code. I totally agree with that and we already declare the LEDs etc. on our PCBs in the device trees. However, we have at least one customer who has a user space driver for some peripheral on i2c which needs to be reset through GPIO. I'm not sure which framework to use for this sort of applications. Greetings, Christian -- Christian Ruppert , christian.rupp...@abilis.com /| Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri _// | bilis Systems CH-1228 Plan-les-Ouates -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 6 June 2013 23:30, Christian Ruppert wrote:
> On Thu, Jun 06, 2013 at 10:32:21PM +0800, Haojian Zhuang wrote:
>> On 6 June 2013 22:11, Christian Ruppert wrote:
>> > On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
>> >> On 3 June 2013 20:30, Christian Ruppert
>> >> 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-AI2CSPI0 SPI1
>> >> >
>> >> > 1GPIOA0SDA MISO1
>> >> > 2GPIOA1SCL MOSI1
>> >> > 3GPIOA2SS1_B
>> >> > 4GPIOA3SCLK1
>> >> > 5GPIOA4 MISO0
>> >> > 6GPIOA5 MOSI0
>> >> > 7GPIOA6 SS0_B
>> >> > 8GPIOA7 SCLK0
>> >> >
>> >> > We can now define the following pinctrl-single:
>> >> >
>> >> > pinmux: pinmux@0xFFEE {
>> >> > compatible = "pinctrl-single";
>> >> > reg = <0xFFEE 0x8>;
>> >> > #address-cells = <1>;
>> >> > #size-cells = <0>;
>> >> > #gpio-range-cells = <3>;
>> >> > pinctrl-single,register-width = <32>;
>> >> > pinctrl-single,function-mask = <0x>;
>> >> > pinctrl-single,gpio-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-AI2CSPI1
> Value 0x0 0x10x2
> ---
> Pin1 GPIOA0SDAMISO1
> Pin2 GPIOA1SCLMOSI1
> Pin3 GPIOA2 SS1_B
> Pin4 GPIOA3 SCLK1
>
> Register 0x4:
> Mode GPIO-ASPI0
> Value 0x0 0x1
> -
> Pin5 GPIOA4MISO0
> Pin6 GPIOA5MOSI0
> Pin7 GPIOA6SS0_B
> Pin8 GPIOA7SCLK0
>
You said "Multiple pins are sharing the same bits in the same register.".
I need to understand which bits you're talking about in your register.
Regards
Haojian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Thu, Jun 06, 2013 at 10:32:21PM +0800, Haojian Zhuang wrote:
> On 6 June 2013 22:11, Christian Ruppert wrote:
> > On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
> >> On 3 June 2013 20:30, Christian Ruppert
> >> 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-AI2CSPI0 SPI1
> >> >
> >> > 1GPIOA0SDA MISO1
> >> > 2GPIOA1SCL MOSI1
> >> > 3GPIOA2SS1_B
> >> > 4GPIOA3SCLK1
> >> > 5GPIOA4 MISO0
> >> > 6GPIOA5 MOSI0
> >> > 7GPIOA6 SS0_B
> >> > 8GPIOA7 SCLK0
> >> >
> >> > We can now define the following pinctrl-single:
> >> >
> >> > pinmux: pinmux@0xFFEE {
> >> > compatible = "pinctrl-single";
> >> > reg = <0xFFEE 0x8>;
> >> > #address-cells = <1>;
> >> > #size-cells = <0>;
> >> > #gpio-range-cells = <3>;
> >> > pinctrl-single,register-width = <32>;
> >> > pinctrl-single,function-mask = <0x>;
> >> > pinctrl-single,gpio-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-AI2CSPI1
Value 0x0 0x10x2
---
Pin1 GPIOA0SDAMISO1
Pin2 GPIOA1SCLMOSI1
Pin3 GPIOA2 SS1_B
Pin4 GPIOA3 SCLK1
Register 0x4:
Mode GPIO-ASPI0
Value 0x0 0x1
-
Pin5 GPIOA4MISO0
Pin6 GPIOA5MOSI0
Pin7 GPIOA6SS0_B
Pin8 GPIOA7SCLK0
Greetings,
Christian
--
Christian Ruppert ,
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
_// | bilis Systems CH-1228 Plan-les-Ouates
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 6 June 2013 22:11, Christian Ruppert wrote:
> On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
>> On 3 June 2013 20:30, Christian Ruppert 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-AI2CSPI0 SPI1
>> >
>> > 1GPIOA0SDA MISO1
>> > 2GPIOA1SCL MOSI1
>> > 3GPIOA2SS1_B
>> > 4GPIOA3SCLK1
>> > 5GPIOA4 MISO0
>> > 6GPIOA5 MOSI0
>> > 7GPIOA6 SS0_B
>> > 8GPIOA7 SCLK0
>> >
>> > We can now define the following pinctrl-single:
>> >
>> > pinmux: pinmux@0xFFEE {
>> > compatible = "pinctrl-single";
>> > reg = <0xFFEE 0x8>;
>> > #address-cells = <1>;
>> > #size-cells = <0>;
>> > #gpio-range-cells = <3>;
>> > pinctrl-single,register-width = <32>;
>> > pinctrl-single,function-mask = <0x>;
>> > pinctrl-single,gpio-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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
> On 3 June 2013 20:30, Christian Ruppert 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-AI2CSPI0 SPI1
> >
> > 1GPIOA0SDA MISO1
> > 2GPIOA1SCL MOSI1
> > 3GPIOA2SS1_B
> > 4GPIOA3SCLK1
> > 5GPIOA4 MISO0
> > 6GPIOA5 MOSI0
> > 7GPIOA6 SS0_B
> > 8GPIOA7 SCLK0
> >
> > We can now define the following pinctrl-single:
> >
> > pinmux: pinmux@0xFFEE {
> > compatible = "pinctrl-single";
> > reg = <0xFFEE 0x8>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > #gpio-range-cells = <3>;
> > pinctrl-single,register-width = <32>;
> > pinctrl-single,function-mask = <0x>;
> > pinctrl-single,gpio-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?
> > };
> > spi1_pins: pinmux_spi1_pins {
> > pinctrl-single,pins = <0x0 2>
> > };
> > range: gpio-range {
> > #pinctrl-single,gpio-range-cells = <3>;
> > };
> > };
> > gpioa: gpio_a {
> > /* ... */
> > gpio-controller;
> > gpio-ranges = < 0 0 8>;
> > };
> >
> > How do I tell pinctrl-single that:
> > 1. I2C and SPI1 cannot be selected at the same time?
> You needn't. If the pin is gotten by I2C driver, SPI driver can't get
> this pin any more.
OK
> > 2. In case I2C is selected, GPIOA0 and GPIOA1 cannot be requested but
> >GPIOA2 and GPIOA3 are available?
> I think that you have your own GPIO driver. At first, you need to
> define .request()
> & .free() in gpio chip.
>
> If I2C function is only configured by bootloader & the pin isn't
> controlled by any
> driver, you can use gpio_request() directly to request this GPIO pin.
>
> If the pin is controlled by I2C driver & you want to declare it as
> GPIO function in
> I2C driver, you can use gpio_request() directly. Then the pin function becomes
> GPIO.
>
> If the pin is controlled by I2C driver & you want to declare it as
> GPIO function in
> other driver, you'll meet failure on requesting this pin.
This seems to be an issue for us. I think we'll keep pinctrl-single in
mind for the evaluation phase of future chips but in production we
definitely need to be able to use e.g. GPIOA2 from other drivers than
the I2C bus driver.
> > 3. In case SPI1 is selected GPIOA0-GPIOA3 are not available?
> Of course, you can set one pin with two function mode at the same time.
>
> But you can switch it between SPI and GPIO mode in the same driver. I
> mentioned
> it in #2 by details.
OK
> > 4. In case SPI0 is selected GPIOA4-GPIOA7 are not available?
> Same #3.
OK
> > Exactly. This is what I'm wondering about in the example above. What
> > must I do to get a clear error message in case someone by mistake tries
> > the following in the above example:
> >
> > spi0: tb10x_spi0 {
> > /* ... */
> > pinctrl-names = "default";
> > pinctrl-0 = <_pins>;
> > };
> > i2c: tb10x_i2c {
> > /* ... */
> > pinctrl-names = "default";
> > pinctrl-0 = <_pins>;
> > };
> >
> > And the following:
> >
> > i2c: tb10x_i2c {
> > /* ... */
> > pinctrl-names = "default";
> > pinctrl-0 = <_pins>;
> > };
> > some_external_component: ext_comp {
> > /* ... */
> > gpios = < 0>;
> > };
>
> Which kind of error message do you need? If you're concerning on pin conflict,
> you'll get it while kernel is running.
OK
--
Christian Ruppert ,
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
_// | bilis Systems CH-1228
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
On 3 June 2013 20:30, Christian Ruppert christian.rupp...@abilis.com 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-AI2CSPI0 SPI1
1GPIOA0SDA MISO1
2GPIOA1SCL MOSI1
3GPIOA2SS1_B
4GPIOA3SCLK1
5GPIOA4 MISO0
6GPIOA5 MOSI0
7GPIOA6 SS0_B
8GPIOA7 SCLK0
We can now define the following pinctrl-single:
pinmux: pinmux@0xFFEE {
compatible = pinctrl-single;
reg = 0xFFEE 0x8;
#address-cells = 1;
#size-cells = 0;
#gpio-range-cells = 3;
pinctrl-single,register-width = 32;
pinctrl-single,function-mask = 0x;
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?
};
spi1_pins: pinmux_spi1_pins {
pinctrl-single,pins = 0x0 2
};
range: gpio-range {
#pinctrl-single,gpio-range-cells = 3;
};
};
gpioa: gpio_a {
/* ... */
gpio-controller;
gpio-ranges = pinmux 0 0 8;
};
How do I tell pinctrl-single that:
1. I2C and SPI1 cannot be selected at the same time?
You needn't. If the pin is gotten by I2C driver, SPI driver can't get
this pin any more.
OK
2. In case I2C is selected, GPIOA0 and GPIOA1 cannot be requested but
GPIOA2 and GPIOA3 are available?
I think that you have your own GPIO driver. At first, you need to
define .request()
.free() in gpio chip.
If I2C function is only configured by bootloader the pin isn't
controlled by any
driver, you can use gpio_request() directly to request this GPIO pin.
If the pin is controlled by I2C driver you want to declare it as
GPIO function in
I2C driver, you can use gpio_request() directly. Then the pin function becomes
GPIO.
If the pin is controlled by I2C driver you want to declare it as
GPIO function in
other driver, you'll meet failure on requesting this pin.
This seems to be an issue for us. I think we'll keep pinctrl-single in
mind for the evaluation phase of future chips but in production we
definitely need to be able to use e.g. GPIOA2 from other drivers than
the I2C bus driver.
3. In case SPI1 is selected GPIOA0-GPIOA3 are not available?
Of course, you can set one pin with two function mode at the same time.
But you can switch it between SPI and GPIO mode in the same driver. I
mentioned
it in #2 by details.
OK
4. In case SPI0 is selected GPIOA4-GPIOA7 are not available?
Same #3.
OK
Exactly. This is what I'm wondering about in the example above. What
must I do to get a clear error message in case someone by mistake tries
the following in the above example:
spi0: tb10x_spi0 {
/* ... */
pinctrl-names = default;
pinctrl-0 = spi0_pins;
};
i2c: tb10x_i2c {
/* ... */
pinctrl-names = default;
pinctrl-0 = i2c_pins;
};
And the following:
i2c: tb10x_i2c {
/* ... */
pinctrl-names = default;
pinctrl-0 = i2c_pins;
};
some_external_component: ext_comp {
/* ... */
gpios = gpioa 0;
};
Which kind of error message do you need? If you're concerning on pin conflict,
you'll get it while kernel is running.
OK
--
Christian Ruppert , christian.rupp...@abilis.com
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
_// | bilis Systems CH-1228 Plan-les-Ouates
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 6 June 2013 22:11, Christian Ruppert christian.rupp...@abilis.com wrote:
On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
On 3 June 2013 20:30, Christian Ruppert christian.rupp...@abilis.com 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-AI2CSPI0 SPI1
1GPIOA0SDA MISO1
2GPIOA1SCL MOSI1
3GPIOA2SS1_B
4GPIOA3SCLK1
5GPIOA4 MISO0
6GPIOA5 MOSI0
7GPIOA6 SS0_B
8GPIOA7 SCLK0
We can now define the following pinctrl-single:
pinmux: pinmux@0xFFEE {
compatible = pinctrl-single;
reg = 0xFFEE 0x8;
#address-cells = 1;
#size-cells = 0;
#gpio-range-cells = 3;
pinctrl-single,register-width = 32;
pinctrl-single,function-mask = 0x;
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.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Thu, Jun 06, 2013 at 10:32:21PM +0800, Haojian Zhuang wrote:
On 6 June 2013 22:11, Christian Ruppert christian.rupp...@abilis.com wrote:
On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
On 3 June 2013 20:30, Christian Ruppert christian.rupp...@abilis.com
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-AI2CSPI0 SPI1
1GPIOA0SDA MISO1
2GPIOA1SCL MOSI1
3GPIOA2SS1_B
4GPIOA3SCLK1
5GPIOA4 MISO0
6GPIOA5 MOSI0
7GPIOA6 SS0_B
8GPIOA7 SCLK0
We can now define the following pinctrl-single:
pinmux: pinmux@0xFFEE {
compatible = pinctrl-single;
reg = 0xFFEE 0x8;
#address-cells = 1;
#size-cells = 0;
#gpio-range-cells = 3;
pinctrl-single,register-width = 32;
pinctrl-single,function-mask = 0x;
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-AI2CSPI1
Value 0x0 0x10x2
---
Pin1 GPIOA0SDAMISO1
Pin2 GPIOA1SCLMOSI1
Pin3 GPIOA2 SS1_B
Pin4 GPIOA3 SCLK1
Register 0x4:
Mode GPIO-ASPI0
Value 0x0 0x1
-
Pin5 GPIOA4MISO0
Pin6 GPIOA5MOSI0
Pin7 GPIOA6SS0_B
Pin8 GPIOA7SCLK0
Greetings,
Christian
--
Christian Ruppert , christian.rupp...@abilis.com
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
_// | bilis Systems CH-1228 Plan-les-Ouates
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 6 June 2013 23:30, Christian Ruppert christian.rupp...@abilis.com wrote:
On Thu, Jun 06, 2013 at 10:32:21PM +0800, Haojian Zhuang wrote:
On 6 June 2013 22:11, Christian Ruppert christian.rupp...@abilis.com wrote:
On Wed, Jun 05, 2013 at 09:44:27AM +0800, Haojian Zhuang wrote:
On 3 June 2013 20:30, Christian Ruppert christian.rupp...@abilis.com
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-AI2CSPI0 SPI1
1GPIOA0SDA MISO1
2GPIOA1SCL MOSI1
3GPIOA2SS1_B
4GPIOA3SCLK1
5GPIOA4 MISO0
6GPIOA5 MOSI0
7GPIOA6 SS0_B
8GPIOA7 SCLK0
We can now define the following pinctrl-single:
pinmux: pinmux@0xFFEE {
compatible = pinctrl-single;
reg = 0xFFEE 0x8;
#address-cells = 1;
#size-cells = 0;
#gpio-range-cells = 3;
pinctrl-single,register-width = 32;
pinctrl-single,function-mask = 0x;
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-AI2CSPI1
Value 0x0 0x10x2
---
Pin1 GPIOA0SDAMISO1
Pin2 GPIOA1SCLMOSI1
Pin3 GPIOA2 SS1_B
Pin4 GPIOA3 SCLK1
Register 0x4:
Mode GPIO-ASPI0
Value 0x0 0x1
-
Pin5 GPIOA4MISO0
Pin6 GPIOA5MOSI0
Pin7 GPIOA6SS0_B
Pin8 GPIOA7SCLK0
You said Multiple pins are sharing the same bits in the same register..
I need to understand which bits you're talking about in your register.
Regards
Haojian
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 3 June 2013 20:30, Christian Ruppert 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-AI2CSPI0 SPI1
>
> 1GPIOA0SDA MISO1
> 2GPIOA1SCL MOSI1
> 3GPIOA2SS1_B
> 4GPIOA3SCLK1
> 5GPIOA4 MISO0
> 6GPIOA5 MOSI0
> 7GPIOA6 SS0_B
> 8GPIOA7 SCLK0
>
> We can now define the following pinctrl-single:
>
> pinmux: pinmux@0xFFEE {
> compatible = "pinctrl-single";
> reg = <0xFFEE 0x8>;
> #address-cells = <1>;
> #size-cells = <0>;
> #gpio-range-cells = <3>;
> pinctrl-single,register-width = <32>;
> pinctrl-single,function-mask = <0x>;
> pinctrl-single,gpio-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.
If multiple pins are sharing one register with different bits,
you need to enable "pinctrl-single,bit-per-mux".
> };
> spi1_pins: pinmux_spi1_pins {
> pinctrl-single,pins = <0x0 2>
> };
> range: gpio-range {
> #pinctrl-single,gpio-range-cells = <3>;
> };
> };
> gpioa: gpio_a {
> /* ... */
> gpio-controller;
> gpio-ranges = < 0 0 8>;
> };
>
> How do I tell pinctrl-single that:
> 1. I2C and SPI1 cannot be selected at the same time?
You needn't. If the pin is gotten by I2C driver, SPI driver can't get
this pin any more.
> 2. In case I2C is selected, GPIOA0 and GPIOA1 cannot be requested but
>GPIOA2 and GPIOA3 are available?
I think that you have your own GPIO driver. At first, you need to
define .request()
& .free() in gpio chip.
If I2C function is only configured by bootloader & the pin isn't
controlled by any
driver, you can use gpio_request() directly to request this GPIO pin.
If the pin is controlled by I2C driver & you want to declare it as
GPIO function in
I2C driver, you can use gpio_request() directly. Then the pin function becomes
GPIO.
If the pin is controlled by I2C driver & you want to declare it as
GPIO function in
other driver, you'll meet failure on requesting this pin.
> 3. In case SPI1 is selected GPIOA0-GPIOA3 are not available?
Of course, you can set one pin with two function mode at the same time.
But you can switch it between SPI and GPIO mode in the same driver. I mentioned
it in #2 by details.
> 4. In case SPI0 is selected GPIOA4-GPIOA7 are not available?
Same #3.
>
>
> Exactly. This is what I'm wondering about in the example above. What
> must I do to get a clear error message in case someone by mistake tries
> the following in the above example:
>
> spi0: tb10x_spi0 {
> /* ... */
> pinctrl-names = "default";
> pinctrl-0 = <_pins>;
> };
> i2c: tb10x_i2c {
> /* ... */
> pinctrl-names = "default";
> pinctrl-0 = <_pins>;
> };
>
> And the following:
>
> i2c: tb10x_i2c {
> /* ... */
> pinctrl-names = "default";
> pinctrl-0 = <_pins>;
> };
> some_external_component: ext_comp {
> /* ... */
> gpios = < 0>;
> };
Which kind of error message do you need? If you're concerning on pin conflict,
you'll get it while kernel is running.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 3 June 2013 20:30, Christian Ruppert christian.rupp...@abilis.com 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-AI2CSPI0 SPI1
1GPIOA0SDA MISO1
2GPIOA1SCL MOSI1
3GPIOA2SS1_B
4GPIOA3SCLK1
5GPIOA4 MISO0
6GPIOA5 MOSI0
7GPIOA6 SS0_B
8GPIOA7 SCLK0
We can now define the following pinctrl-single:
pinmux: pinmux@0xFFEE {
compatible = pinctrl-single;
reg = 0xFFEE 0x8;
#address-cells = 1;
#size-cells = 0;
#gpio-range-cells = 3;
pinctrl-single,register-width = 32;
pinctrl-single,function-mask = 0x;
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.
If multiple pins are sharing one register with different bits,
you need to enable pinctrl-single,bit-per-mux.
};
spi1_pins: pinmux_spi1_pins {
pinctrl-single,pins = 0x0 2
};
range: gpio-range {
#pinctrl-single,gpio-range-cells = 3;
};
};
gpioa: gpio_a {
/* ... */
gpio-controller;
gpio-ranges = pinmux 0 0 8;
};
How do I tell pinctrl-single that:
1. I2C and SPI1 cannot be selected at the same time?
You needn't. If the pin is gotten by I2C driver, SPI driver can't get
this pin any more.
2. In case I2C is selected, GPIOA0 and GPIOA1 cannot be requested but
GPIOA2 and GPIOA3 are available?
I think that you have your own GPIO driver. At first, you need to
define .request()
.free() in gpio chip.
If I2C function is only configured by bootloader the pin isn't
controlled by any
driver, you can use gpio_request() directly to request this GPIO pin.
If the pin is controlled by I2C driver you want to declare it as
GPIO function in
I2C driver, you can use gpio_request() directly. Then the pin function becomes
GPIO.
If the pin is controlled by I2C driver you want to declare it as
GPIO function in
other driver, you'll meet failure on requesting this pin.
3. In case SPI1 is selected GPIOA0-GPIOA3 are not available?
Of course, you can set one pin with two function mode at the same time.
But you can switch it between SPI and GPIO mode in the same driver. I mentioned
it in #2 by details.
4. In case SPI0 is selected GPIOA4-GPIOA7 are not available?
Same #3.
Exactly. This is what I'm wondering about in the example above. What
must I do to get a clear error message in case someone by mistake tries
the following in the above example:
spi0: tb10x_spi0 {
/* ... */
pinctrl-names = default;
pinctrl-0 = spi0_pins;
};
i2c: tb10x_i2c {
/* ... */
pinctrl-names = default;
pinctrl-0 = i2c_pins;
};
And the following:
i2c: tb10x_i2c {
/* ... */
pinctrl-names = default;
pinctrl-0 = i2c_pins;
};
some_external_component: ext_comp {
/* ... */
gpios = gpioa 0;
};
Which kind of error message do you need? If you're concerning on pin conflict,
you'll get it while kernel is running.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Sun, May 26, 2013 at 11:49:20PM +0800, Haojian Zhuang wrote:
> On 24 May 2013 19:50, Christian Ruppert wrote:
> > Hello Haojian,
> >
> > On Thu, May 23, 2013 at 03:43:27PM +0800, Haojian Zhuang wrote:
> >> On 22 May 2013 22:28, Christian Ruppert
> >> wrote:
> >> >
> >> > On Mon, May 20, 2013 at 10:10:33AM +0200, Linus Walleij wrote:
> >> > > On Thu, May 16, 2013 at 2:12 AM, Stephen Warren
> >> > > wrote:
> >> > > > On 05/10/2013 02:25 AM, Christian Ruppert wrote:
> [...]
> >> I think that you want to keep the logic simple. If so, I prefer you can
> >> check pinctrl-single driver first. All pins are defined in DTS instead.
> >
> > Thanks for the hint. I haven't understood how to associate GPIOs to
> > other functions, however: Our hardware pin controller makes GPIO pins
> > available depending on the configuration of the non-GPIO interfaces.
> > This means that in many configurations, GPIO banks are only partially
>
> They're multiple function pins. You can find those pins in most modern SoCs.
>
> > available because some pins are used for other purposes. We can't expect
> > our customers to manually change the pin assignments in the device tree
> > in order to take this into account for every PCB.
>
> Yes, you need to define the gpio-range & pinctrl-single,gpio-range.
> If you define them in your DTSI file, customer only need to include it.
> If you define them in your DTS file, customer only need to copy them into
> their DTS file.
>
> If you don't have the requirements of routing multiple pins to the same GPIO,
> I suggest you to define them in your DTSI file.
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-AI2CSPI0 SPI1
1GPIOA0SDA MISO1
2GPIOA1SCL MOSI1
3GPIOA2SS1_B
4GPIOA3SCLK1
5GPIOA4 MISO0
6GPIOA5 MOSI0
7GPIOA6 SS0_B
8GPIOA7 SCLK0
We can now define the following pinctrl-single:
pinmux: pinmux@0xFFEE {
compatible = "pinctrl-single";
reg = <0xFFEE 0x8>;
#address-cells = <1>;
#size-cells = <0>;
#gpio-range-cells = <3>;
pinctrl-single,register-width = <32>;
pinctrl-single,function-mask = <0x>;
pinctrl-single,gpio-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>
};
spi1_pins: pinmux_spi1_pins {
pinctrl-single,pins = <0x0 2>
};
range: gpio-range {
#pinctrl-single,gpio-range-cells = <3>;
};
};
gpioa: gpio_a {
/* ... */
gpio-controller;
gpio-ranges = < 0 0 8>;
};
How do I tell pinctrl-single that:
1. I2C and SPI1 cannot be selected at the same time?
2. In case I2C is selected, GPIOA0 and GPIOA1 cannot be requested but
GPIOA2 and GPIOA3 are available?
3. In case SPI1 is selected GPIOA0-GPIOA3 are not available?
4. In case SPI0 is selected GPIOA4-GPIOA7 are not available?
> > Is there a way to make different pinmux functions mutually exclusive in
> > pinctrl-single, e.g. a pin is either a GPIO or part of an SPI interface?
>
> gpio_request() could help you to request GPIO if the pin isn't used yet.
> And even your pin is in I2C mode without usage.
>
> But if you want to the mutually exclusive, you could do by this way.
>
> 1) Function is only in GPIO mode.
> In uart node, GPIO is function 1.
> pinctrl-0 = <_pmx_func>;
> uart1_pmx_func: uart1_pmx_func {
> pinctrl-single,pins = <0x104 0x1>;
> };
>
> Then you can't switch to SPI interface, unless you want to switch pin state.
>
> 2) Function is only in UART mode.
> You can hack gpio function in pinctrl-single,gpio-range property. i.e. you
> always set gpio function as UART mode by hack.
>
> But I still don't understand why you need this feature.
>
> > Can the same thing be done for example to mux either SPI or I2C on the
> > same pins?
>
> Are you using the develop board that one pin may be routed to multiple
> functions? And you can choose SPI or I2C by switch.
We do have development boards to which the customer can connect their
own peripherals (and adapt the device tree accordingly). We are also
looking for a comprehensive way to configure the I/Os even for customers
designing their own boards.
> It means that the sames pin are shared between SPI and I2C driver. Either
> SPI driver gets this pin, or I2C driver gets this pin. Only one driver could
> get the
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Wed, May 29, 2013 at 02:21:05PM +0200, Linus Walleij wrote: > On Fri, May 24, 2013 at 1:50 PM, Christian Ruppert > wrote: > > > I haven't understood how to associate GPIOs to > > other functions, however: Our hardware pin controller makes GPIO pins > > available depending on the configuration of the non-GPIO interfaces. > > This means that in many configurations, GPIO banks are only partially > > available because some pins are used for other purposes. We can't expect > > our customers to manually change the pin assignments in the device tree > > in order to take this into account for every PCB. > > But what is it your customers do when customizing a board then? Ideally, they just select the pin functions they would like to use on their PCB using the phandle mechanism outlined in Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt. We will prepare the nodes to point to in our SOC .dtsi files. Obviously, customers will look at these files, in particular the nodes they point to. The simpler these nodes are (and the easier to understand) the better. > Part of the promise of the device tree is to make it easy for downstream > users to customize the kernel, *especially* for different boards/systems > using the same SoC, for example. It is very much intended as a > customization tools for embedded developers getting boards from > a chipset vendor. Yes, this is our understanding as well. This is why we would like to avoid confusion through the exposure of obscure number spaces, even if a customer is not supposed to modify them. Ease of use is also the reason why I added the gpio-base property to the original driver: Finding out the global GPIO number to use in /sys/class/gpio for a given GPIO of a given bank seems to be a recurring headache for our customers and the definition of the bank's base number in the device tree is an attempt to improve this situation. I am looking forward to the patch you said Alexandre is working on which will probably be a much better solution. > Maybe I don't understand what is meant by changing the pin > assignments above ... Every pin can have exactly one function at a time and is thus assigned to that function. Ideally, conflicts are cleanly managed in the pinmux driver and an error message is generated so customers can understand why something doesn't work and how to fix it. Greetings, Christian -- Christian Ruppert , /| Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri _// | bilis Systems CH-1228 Plan-les-Ouates -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Wed, May 29, 2013 at 02:21:05PM +0200, Linus Walleij wrote: On Fri, May 24, 2013 at 1:50 PM, Christian Ruppert christian.rupp...@abilis.com wrote: I haven't understood how to associate GPIOs to other functions, however: Our hardware pin controller makes GPIO pins available depending on the configuration of the non-GPIO interfaces. This means that in many configurations, GPIO banks are only partially available because some pins are used for other purposes. We can't expect our customers to manually change the pin assignments in the device tree in order to take this into account for every PCB. But what is it your customers do when customizing a board then? Ideally, they just select the pin functions they would like to use on their PCB using the phandle mechanism outlined in Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt. We will prepare the nodes to point to in our SOC .dtsi files. Obviously, customers will look at these files, in particular the nodes they point to. The simpler these nodes are (and the easier to understand) the better. Part of the promise of the device tree is to make it easy for downstream users to customize the kernel, *especially* for different boards/systems using the same SoC, for example. It is very much intended as a customization tools for embedded developers getting boards from a chipset vendor. Yes, this is our understanding as well. This is why we would like to avoid confusion through the exposure of obscure number spaces, even if a customer is not supposed to modify them. Ease of use is also the reason why I added the gpio-base property to the original driver: Finding out the global GPIO number to use in /sys/class/gpio for a given GPIO of a given bank seems to be a recurring headache for our customers and the definition of the bank's base number in the device tree is an attempt to improve this situation. I am looking forward to the patch you said Alexandre is working on which will probably be a much better solution. Maybe I don't understand what is meant by changing the pin assignments above ... Every pin can have exactly one function at a time and is thus assigned to that function. Ideally, conflicts are cleanly managed in the pinmux driver and an error message is generated so customers can understand why something doesn't work and how to fix it. Greetings, Christian -- Christian Ruppert , christian.rupp...@abilis.com /| Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri _// | bilis Systems CH-1228 Plan-les-Ouates -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Sun, May 26, 2013 at 11:49:20PM +0800, Haojian Zhuang wrote:
On 24 May 2013 19:50, Christian Ruppert christian.rupp...@abilis.com wrote:
Hello Haojian,
On Thu, May 23, 2013 at 03:43:27PM +0800, Haojian Zhuang wrote:
On 22 May 2013 22:28, Christian Ruppert christian.rupp...@abilis.com
wrote:
On Mon, May 20, 2013 at 10:10:33AM +0200, Linus Walleij wrote:
On Thu, May 16, 2013 at 2:12 AM, Stephen Warren
swar...@wwwdotorg.org wrote:
On 05/10/2013 02:25 AM, Christian Ruppert wrote:
[...]
I think that you want to keep the logic simple. If so, I prefer you can
check pinctrl-single driver first. All pins are defined in DTS instead.
Thanks for the hint. I haven't understood how to associate GPIOs to
other functions, however: Our hardware pin controller makes GPIO pins
available depending on the configuration of the non-GPIO interfaces.
This means that in many configurations, GPIO banks are only partially
They're multiple function pins. You can find those pins in most modern SoCs.
available because some pins are used for other purposes. We can't expect
our customers to manually change the pin assignments in the device tree
in order to take this into account for every PCB.
Yes, you need to define the gpio-range pinctrl-single,gpio-range.
If you define them in your DTSI file, customer only need to include it.
If you define them in your DTS file, customer only need to copy them into
their DTS file.
If you don't have the requirements of routing multiple pins to the same GPIO,
I suggest you to define them in your DTSI file.
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-AI2CSPI0 SPI1
1GPIOA0SDA MISO1
2GPIOA1SCL MOSI1
3GPIOA2SS1_B
4GPIOA3SCLK1
5GPIOA4 MISO0
6GPIOA5 MOSI0
7GPIOA6 SS0_B
8GPIOA7 SCLK0
We can now define the following pinctrl-single:
pinmux: pinmux@0xFFEE {
compatible = pinctrl-single;
reg = 0xFFEE 0x8;
#address-cells = 1;
#size-cells = 0;
#gpio-range-cells = 3;
pinctrl-single,register-width = 32;
pinctrl-single,function-mask = 0x;
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
};
spi1_pins: pinmux_spi1_pins {
pinctrl-single,pins = 0x0 2
};
range: gpio-range {
#pinctrl-single,gpio-range-cells = 3;
};
};
gpioa: gpio_a {
/* ... */
gpio-controller;
gpio-ranges = pinmux 0 0 8;
};
How do I tell pinctrl-single that:
1. I2C and SPI1 cannot be selected at the same time?
2. In case I2C is selected, GPIOA0 and GPIOA1 cannot be requested but
GPIOA2 and GPIOA3 are available?
3. In case SPI1 is selected GPIOA0-GPIOA3 are not available?
4. In case SPI0 is selected GPIOA4-GPIOA7 are not available?
Is there a way to make different pinmux functions mutually exclusive in
pinctrl-single, e.g. a pin is either a GPIO or part of an SPI interface?
gpio_request() could help you to request GPIO if the pin isn't used yet.
And even your pin is in I2C mode without usage.
But if you want to the mutually exclusive, you could do by this way.
1) Function is only in GPIO mode.
In uart node, GPIO is function 1.
pinctrl-0 = uart1_pmx_func;
uart1_pmx_func: uart1_pmx_func {
pinctrl-single,pins = 0x104 0x1;
};
Then you can't switch to SPI interface, unless you want to switch pin state.
2) Function is only in UART mode.
You can hack gpio function in pinctrl-single,gpio-range property. i.e. you
always set gpio function as UART mode by hack.
But I still don't understand why you need this feature.
Can the same thing be done for example to mux either SPI or I2C on the
same pins?
Are you using the develop board that one pin may be routed to multiple
functions? And you can choose SPI or I2C by switch.
We do have development boards to which the customer can connect their
own peripherals (and adapt the device tree accordingly). We are also
looking for a comprehensive way to configure the I/Os even for customers
designing their own boards.
It means that the sames pin are shared between SPI and I2C driver. Either
SPI driver gets this pin, or I2C driver gets this pin. Only one driver could
get the pins even you don't use pinctrl-single driver.
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Fri, May 24, 2013 at 1:50 PM, Christian Ruppert wrote: > I haven't understood how to associate GPIOs to > other functions, however: Our hardware pin controller makes GPIO pins > available depending on the configuration of the non-GPIO interfaces. > This means that in many configurations, GPIO banks are only partially > available because some pins are used for other purposes. We can't expect > our customers to manually change the pin assignments in the device tree > in order to take this into account for every PCB. But what is it your customers do when customizing a board then? Part of the promise of the device tree is to make it easy for downstream users to customize the kernel, *especially* for different boards/systems using the same SoC, for example. It is very much intended as a customization tools for embedded developers getting boards from a chipset vendor. Maybe I don't understand what is meant by changing the pin assignments above ... Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Fri, May 24, 2013 at 1:50 PM, Christian Ruppert christian.rupp...@abilis.com wrote: I haven't understood how to associate GPIOs to other functions, however: Our hardware pin controller makes GPIO pins available depending on the configuration of the non-GPIO interfaces. This means that in many configurations, GPIO banks are only partially available because some pins are used for other purposes. We can't expect our customers to manually change the pin assignments in the device tree in order to take this into account for every PCB. But what is it your customers do when customizing a board then? Part of the promise of the device tree is to make it easy for downstream users to customize the kernel, *especially* for different boards/systems using the same SoC, for example. It is very much intended as a customization tools for embedded developers getting boards from a chipset vendor. Maybe I don't understand what is meant by changing the pin assignments above ... Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 24 May 2013 19:50, Christian Ruppert wrote:
> Hello Haojian,
>
> On Thu, May 23, 2013 at 03:43:27PM +0800, Haojian Zhuang wrote:
>> On 22 May 2013 22:28, Christian Ruppert wrote:
>> >
>> > On Mon, May 20, 2013 at 10:10:33AM +0200, Linus Walleij wrote:
>> > > On Thu, May 16, 2013 at 2:12 AM, Stephen Warren
>> > > wrote:
>> > > > On 05/10/2013 02:25 AM, Christian Ruppert wrote:
>> > >
>> > > >> (*1) TB100 GPIO ranges are defined as a phandle to the I/O function
>> > > >> which provides all pins of a given GPIO port. This function is
>> > > >> not
>> > > >> necessarily requested from pinctrl and GPIO ports may overlap
>> > > >> with
>> > > >> other functions. The pin controller knows about this and provides
>> > > >> whatever GPIO pin is available after mapping other requested
>> > > >> functions.
>> > > >> (*2) Here, the entire GPIOB port is explicitly requested by the GPIO
>> > > >> module, i.e. all pins of the port are made available as GPIOs.
>> > > >
>> > > > So I think all you're looking for is a way in DT to represent GPIO
>> > > > ranges? I don't think that should be by string name, but rather
>> > > > numbers:
>> > > >
>> > > > (actually, doesn't pinctrl-simple already have this?)
>> > >
>> > > Now I'm ever more confused ... we already have this :-)
>> > >
>> > > It's not even pinctrl-simple-centric it is completely generic.
>> > > The code is in drivers/gpio/gpiolib-of.c.
>> > >
>> > > It was written by Shiraz Hashin and Haojian Zhuang.
>> > > At the time I augmented the core code quite a bit to make
>> > > a good fit.
>> >
>> > I agree. Unluckily, it uses pinctrl-internal pin numbering which we
>> > would have to make coherent with the physical pin numbers of the
>> > individual packaging variants of the chip in order to expose them to
>> > customers (see my previous mail at https://lkml.org/lkml/2013/5/22/207).
>> >
>> > Adapting the kernel-internal pin numbering in function of the product
>> > variant doesn't seem such a good idea to me: All pin groups etc. will
>> > have to be redefined for every product, a huge source of bugs and
>> > unnecessary static data within the drivers.
>> >I review two methods you mentioned in this mail.
>>
>> I think that you want to keep the logic simple. If so, I prefer you can
>> check pinctrl-single driver first. All pins are defined in DTS instead.
>
> Thanks for the hint. I haven't understood how to associate GPIOs to
> other functions, however: Our hardware pin controller makes GPIO pins
> available depending on the configuration of the non-GPIO interfaces.
> This means that in many configurations, GPIO banks are only partially
They're multiple function pins. You can find those pins in most modern SoCs.
> available because some pins are used for other purposes. We can't expect
> our customers to manually change the pin assignments in the device tree
> in order to take this into account for every PCB.
Yes, you need to define the gpio-range & pinctrl-single,gpio-range.
If you define them in your DTSI file, customer only need to include it.
If you define them in your DTS file, customer only need to copy them into
their DTS file.
If you don't have the requirements of routing multiple pins to the same GPIO,
I suggest you to define them in your DTSI file.
>
> Is there a way to make different pinmux functions mutually exclusive in
> pinctrl-single, e.g. a pin is either a GPIO or part of an SPI interface?
gpio_request() could help you to request GPIO if the pin isn't used yet.
And even your pin is in I2C mode without usage.
But if you want to the mutually exclusive, you could do by this way.
1) Function is only in GPIO mode.
In uart node, GPIO is function 1.
pinctrl-0 = <_pmx_func>;
uart1_pmx_func: uart1_pmx_func {
pinctrl-single,pins = <0x104 0x1>;
};
Then you can't switch to SPI interface, unless you want to switch pin state.
2) Function is only in UART mode.
You can hack gpio function in pinctrl-single,gpio-range property. i.e. you
always set gpio function as UART mode by hack.
But I still don't understand why you need this feature.
> Can the same thing be done for example to mux either SPI or I2C on the
> same pins?
Are you using the develop board that one pin may be routed to multiple
functions? And you can choose SPI or I2C by switch.
It means that the sames pin are shared between SPI and I2C driver. Either
SPI driver gets this pin, or I2C driver gets this pin. Only one driver could
get the pins even you don't use pinctrl-single driver. It likes GPIO.
SPI & I2C driver are always enabled in your kernel image. So you're enabling
different devices with different hardware configuration, and you need to
prepare two DTS files. Your boot loader should find which
hardware configuration is enabled & loaded the right DTS file.
So different pinmux settings are written in these two DTS files.
>
>> [...]
>
> Greetings,
> Christian
>
> --
> Christian Ruppert ,
>
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 24 May 2013 19:50, Christian Ruppert christian.rupp...@abilis.com wrote:
Hello Haojian,
On Thu, May 23, 2013 at 03:43:27PM +0800, Haojian Zhuang wrote:
On 22 May 2013 22:28, Christian Ruppert christian.rupp...@abilis.com wrote:
On Mon, May 20, 2013 at 10:10:33AM +0200, Linus Walleij wrote:
On Thu, May 16, 2013 at 2:12 AM, Stephen Warren swar...@wwwdotorg.org
wrote:
On 05/10/2013 02:25 AM, Christian Ruppert wrote:
(*1) TB100 GPIO ranges are defined as a phandle to the I/O function
which provides all pins of a given GPIO port. This function is
not
necessarily requested from pinctrl and GPIO ports may overlap
with
other functions. The pin controller knows about this and provides
whatever GPIO pin is available after mapping other requested
functions.
(*2) Here, the entire GPIOB port is explicitly requested by the GPIO
module, i.e. all pins of the port are made available as GPIOs.
So I think all you're looking for is a way in DT to represent GPIO
ranges? I don't think that should be by string name, but rather
numbers:
(actually, doesn't pinctrl-simple already have this?)
Now I'm ever more confused ... we already have this :-)
It's not even pinctrl-simple-centric it is completely generic.
The code is in drivers/gpio/gpiolib-of.c.
It was written by Shiraz Hashin and Haojian Zhuang.
At the time I augmented the core code quite a bit to make
a good fit.
I agree. Unluckily, it uses pinctrl-internal pin numbering which we
would have to make coherent with the physical pin numbers of the
individual packaging variants of the chip in order to expose them to
customers (see my previous mail at https://lkml.org/lkml/2013/5/22/207).
Adapting the kernel-internal pin numbering in function of the product
variant doesn't seem such a good idea to me: All pin groups etc. will
have to be redefined for every product, a huge source of bugs and
unnecessary static data within the drivers.
I review two methods you mentioned in this mail.
I think that you want to keep the logic simple. If so, I prefer you can
check pinctrl-single driver first. All pins are defined in DTS instead.
Thanks for the hint. I haven't understood how to associate GPIOs to
other functions, however: Our hardware pin controller makes GPIO pins
available depending on the configuration of the non-GPIO interfaces.
This means that in many configurations, GPIO banks are only partially
They're multiple function pins. You can find those pins in most modern SoCs.
available because some pins are used for other purposes. We can't expect
our customers to manually change the pin assignments in the device tree
in order to take this into account for every PCB.
Yes, you need to define the gpio-range pinctrl-single,gpio-range.
If you define them in your DTSI file, customer only need to include it.
If you define them in your DTS file, customer only need to copy them into
their DTS file.
If you don't have the requirements of routing multiple pins to the same GPIO,
I suggest you to define them in your DTSI file.
Is there a way to make different pinmux functions mutually exclusive in
pinctrl-single, e.g. a pin is either a GPIO or part of an SPI interface?
gpio_request() could help you to request GPIO if the pin isn't used yet.
And even your pin is in I2C mode without usage.
But if you want to the mutually exclusive, you could do by this way.
1) Function is only in GPIO mode.
In uart node, GPIO is function 1.
pinctrl-0 = uart1_pmx_func;
uart1_pmx_func: uart1_pmx_func {
pinctrl-single,pins = 0x104 0x1;
};
Then you can't switch to SPI interface, unless you want to switch pin state.
2) Function is only in UART mode.
You can hack gpio function in pinctrl-single,gpio-range property. i.e. you
always set gpio function as UART mode by hack.
But I still don't understand why you need this feature.
Can the same thing be done for example to mux either SPI or I2C on the
same pins?
Are you using the develop board that one pin may be routed to multiple
functions? And you can choose SPI or I2C by switch.
It means that the sames pin are shared between SPI and I2C driver. Either
SPI driver gets this pin, or I2C driver gets this pin. Only one driver could
get the pins even you don't use pinctrl-single driver. It likes GPIO.
SPI I2C driver are always enabled in your kernel image. So you're enabling
different devices with different hardware configuration, and you need to
prepare two DTS files. Your boot loader should find which
hardware configuration is enabled loaded the right DTS file.
So different pinmux settings are written in these two DTS files.
[...]
Greetings,
Christian
--
Christian Ruppert , christian.rupp...@abilis.com
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Fri, May 24, 2013 at 11:20:31AM +0200, Linus Walleij wrote: > On Wed, May 22, 2013 at 4:28 PM, Christian Ruppert > wrote: > > On Mon, May 20, 2013 at 10:10:33AM +0200, Linus Walleij wrote: > > >> It's not even pinctrl-simple-centric it is completely generic. > >> The code is in drivers/gpio/gpiolib-of.c. > >> > >> It was written by Shiraz Hashin and Haojian Zhuang. > >> At the time I augmented the core code quite a bit to make > >> a good fit. > > > > I agree. Unluckily, it uses pinctrl-internal pin numbering which we > > would have to make coherent with the physical pin numbers of the > > individual packaging variants of the chip in order to expose them to > > customers (see my previous mail at https://lkml.org/lkml/2013/5/22/207). > > Again, the Linux pin numberspace is sparse, you can use whatever > pin number you like as long as it fits in a u32 and does not overlap > with other pins. > > If you have a problem that the physical pin numbers and the > bank offsets or bits you need to poke or something doesn't > match, that is an issue for the *driver* not for the pin control > subsystem. However the pin control core may provide > cross-mapping helpers as discussed elsewhere, but the > pin control internal numbering is *not* an issue. Those are > just "some number" on a certain pin controller, use the number > from the datasheet if you like. Understood. What I'm looking for is a way to support different products with different physical pin numbering, ideally without changing Linux internal pin numbers from one product variant under the constraints highlighted in the thread you call "elsewhere". Greetings, Christian -- Christian Ruppert , /| Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri _// | bilis Systems CH-1228 Plan-les-Ouates -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
Hello Haojian, On Thu, May 23, 2013 at 03:43:27PM +0800, Haojian Zhuang wrote: > On 22 May 2013 22:28, Christian Ruppert wrote: > > > > On Mon, May 20, 2013 at 10:10:33AM +0200, Linus Walleij wrote: > > > On Thu, May 16, 2013 at 2:12 AM, Stephen Warren > > > wrote: > > > > On 05/10/2013 02:25 AM, Christian Ruppert wrote: > > > > > > >> (*1) TB100 GPIO ranges are defined as a phandle to the I/O function > > > >> which provides all pins of a given GPIO port. This function is not > > > >> necessarily requested from pinctrl and GPIO ports may overlap with > > > >> other functions. The pin controller knows about this and provides > > > >> whatever GPIO pin is available after mapping other requested > > > >> functions. > > > >> (*2) Here, the entire GPIOB port is explicitly requested by the GPIO > > > >> module, i.e. all pins of the port are made available as GPIOs. > > > > > > > > So I think all you're looking for is a way in DT to represent GPIO > > > > ranges? I don't think that should be by string name, but rather numbers: > > > > > > > > (actually, doesn't pinctrl-simple already have this?) > > > > > > Now I'm ever more confused ... we already have this :-) > > > > > > It's not even pinctrl-simple-centric it is completely generic. > > > The code is in drivers/gpio/gpiolib-of.c. > > > > > > It was written by Shiraz Hashin and Haojian Zhuang. > > > At the time I augmented the core code quite a bit to make > > > a good fit. > > > > I agree. Unluckily, it uses pinctrl-internal pin numbering which we > > would have to make coherent with the physical pin numbers of the > > individual packaging variants of the chip in order to expose them to > > customers (see my previous mail at https://lkml.org/lkml/2013/5/22/207). > > > > Adapting the kernel-internal pin numbering in function of the product > > variant doesn't seem such a good idea to me: All pin groups etc. will > > have to be redefined for every product, a huge source of bugs and > > unnecessary static data within the drivers. > >I review two methods you mentioned in this mail. > > I think that you want to keep the logic simple. If so, I prefer you can > check pinctrl-single driver first. All pins are defined in DTS instead. Thanks for the hint. I haven't understood how to associate GPIOs to other functions, however: Our hardware pin controller makes GPIO pins available depending on the configuration of the non-GPIO interfaces. This means that in many configurations, GPIO banks are only partially available because some pins are used for other purposes. We can't expect our customers to manually change the pin assignments in the device tree in order to take this into account for every PCB. Is there a way to make different pinmux functions mutually exclusive in pinctrl-single, e.g. a pin is either a GPIO or part of an SPI interface? Can the same thing be done for example to mux either SPI or I2C on the same pins? If the answer to both questions is yes, we could predefine all functions as we currently do and our pinctrl driver could be entirely replaced with pinctrl-single. > [...] Greetings, Christian -- Christian Ruppert , /| Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri _// | bilis Systems CH-1228 Plan-les-Ouates -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Wed, May 22, 2013 at 4:28 PM, Christian Ruppert wrote: > On Mon, May 20, 2013 at 10:10:33AM +0200, Linus Walleij wrote: >> It's not even pinctrl-simple-centric it is completely generic. >> The code is in drivers/gpio/gpiolib-of.c. >> >> It was written by Shiraz Hashin and Haojian Zhuang. >> At the time I augmented the core code quite a bit to make >> a good fit. > > I agree. Unluckily, it uses pinctrl-internal pin numbering which we > would have to make coherent with the physical pin numbers of the > individual packaging variants of the chip in order to expose them to > customers (see my previous mail at https://lkml.org/lkml/2013/5/22/207). Again, the Linux pin numberspace is sparse, you can use whatever pin number you like as long as it fits in a u32 and does not overlap with other pins. If you have a problem that the physical pin numbers and the bank offsets or bits you need to poke or something doesn't match, that is an issue for the *driver* not for the pin control subsystem. However the pin control core may provide cross-mapping helpers as discussed elsewhere, but the pin control internal numbering is *not* an issue. Those are just "some number" on a certain pin controller, use the number from the datasheet if you like. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Wed, May 22, 2013 at 4:28 PM, Christian Ruppert christian.rupp...@abilis.com wrote: On Mon, May 20, 2013 at 10:10:33AM +0200, Linus Walleij wrote: It's not even pinctrl-simple-centric it is completely generic. The code is in drivers/gpio/gpiolib-of.c. It was written by Shiraz Hashin and Haojian Zhuang. At the time I augmented the core code quite a bit to make a good fit. I agree. Unluckily, it uses pinctrl-internal pin numbering which we would have to make coherent with the physical pin numbers of the individual packaging variants of the chip in order to expose them to customers (see my previous mail at https://lkml.org/lkml/2013/5/22/207). Again, the Linux pin numberspace is sparse, you can use whatever pin number you like as long as it fits in a u32 and does not overlap with other pins. If you have a problem that the physical pin numbers and the bank offsets or bits you need to poke or something doesn't match, that is an issue for the *driver* not for the pin control subsystem. However the pin control core may provide cross-mapping helpers as discussed elsewhere, but the pin control internal numbering is *not* an issue. Those are just some number on a certain pin controller, use the number from the datasheet if you like. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
Hello Haojian, On Thu, May 23, 2013 at 03:43:27PM +0800, Haojian Zhuang wrote: On 22 May 2013 22:28, Christian Ruppert christian.rupp...@abilis.com wrote: On Mon, May 20, 2013 at 10:10:33AM +0200, Linus Walleij wrote: On Thu, May 16, 2013 at 2:12 AM, Stephen Warren swar...@wwwdotorg.org wrote: On 05/10/2013 02:25 AM, Christian Ruppert wrote: (*1) TB100 GPIO ranges are defined as a phandle to the I/O function which provides all pins of a given GPIO port. This function is not necessarily requested from pinctrl and GPIO ports may overlap with other functions. The pin controller knows about this and provides whatever GPIO pin is available after mapping other requested functions. (*2) Here, the entire GPIOB port is explicitly requested by the GPIO module, i.e. all pins of the port are made available as GPIOs. So I think all you're looking for is a way in DT to represent GPIO ranges? I don't think that should be by string name, but rather numbers: (actually, doesn't pinctrl-simple already have this?) Now I'm ever more confused ... we already have this :-) It's not even pinctrl-simple-centric it is completely generic. The code is in drivers/gpio/gpiolib-of.c. It was written by Shiraz Hashin and Haojian Zhuang. At the time I augmented the core code quite a bit to make a good fit. I agree. Unluckily, it uses pinctrl-internal pin numbering which we would have to make coherent with the physical pin numbers of the individual packaging variants of the chip in order to expose them to customers (see my previous mail at https://lkml.org/lkml/2013/5/22/207). Adapting the kernel-internal pin numbering in function of the product variant doesn't seem such a good idea to me: All pin groups etc. will have to be redefined for every product, a huge source of bugs and unnecessary static data within the drivers. I review two methods you mentioned in this mail. I think that you want to keep the logic simple. If so, I prefer you can check pinctrl-single driver first. All pins are defined in DTS instead. Thanks for the hint. I haven't understood how to associate GPIOs to other functions, however: Our hardware pin controller makes GPIO pins available depending on the configuration of the non-GPIO interfaces. This means that in many configurations, GPIO banks are only partially available because some pins are used for other purposes. We can't expect our customers to manually change the pin assignments in the device tree in order to take this into account for every PCB. Is there a way to make different pinmux functions mutually exclusive in pinctrl-single, e.g. a pin is either a GPIO or part of an SPI interface? Can the same thing be done for example to mux either SPI or I2C on the same pins? If the answer to both questions is yes, we could predefine all functions as we currently do and our pinctrl driver could be entirely replaced with pinctrl-single. [...] Greetings, Christian -- Christian Ruppert , christian.rupp...@abilis.com /| Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri _// | bilis Systems CH-1228 Plan-les-Ouates -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Fri, May 24, 2013 at 11:20:31AM +0200, Linus Walleij wrote: On Wed, May 22, 2013 at 4:28 PM, Christian Ruppert christian.rupp...@abilis.com wrote: On Mon, May 20, 2013 at 10:10:33AM +0200, Linus Walleij wrote: It's not even pinctrl-simple-centric it is completely generic. The code is in drivers/gpio/gpiolib-of.c. It was written by Shiraz Hashin and Haojian Zhuang. At the time I augmented the core code quite a bit to make a good fit. I agree. Unluckily, it uses pinctrl-internal pin numbering which we would have to make coherent with the physical pin numbers of the individual packaging variants of the chip in order to expose them to customers (see my previous mail at https://lkml.org/lkml/2013/5/22/207). Again, the Linux pin numberspace is sparse, you can use whatever pin number you like as long as it fits in a u32 and does not overlap with other pins. If you have a problem that the physical pin numbers and the bank offsets or bits you need to poke or something doesn't match, that is an issue for the *driver* not for the pin control subsystem. However the pin control core may provide cross-mapping helpers as discussed elsewhere, but the pin control internal numbering is *not* an issue. Those are just some number on a certain pin controller, use the number from the datasheet if you like. Understood. What I'm looking for is a way to support different products with different physical pin numbering, ideally without changing Linux internal pin numbers from one product variant under the constraints highlighted in the thread you call elsewhere. Greetings, Christian -- Christian Ruppert , christian.rupp...@abilis.com /| Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri _// | bilis Systems CH-1228 Plan-les-Ouates -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 22 May 2013 22:28, Christian Ruppert wrote:
>
> On Mon, May 20, 2013 at 10:10:33AM +0200, Linus Walleij wrote:
> > On Thu, May 16, 2013 at 2:12 AM, Stephen Warren
> > wrote:
> > > On 05/10/2013 02:25 AM, Christian Ruppert wrote:
> >
> > >> (*1) TB100 GPIO ranges are defined as a phandle to the I/O function
> > >> which provides all pins of a given GPIO port. This function is not
> > >> necessarily requested from pinctrl and GPIO ports may overlap with
> > >> other functions. The pin controller knows about this and provides
> > >> whatever GPIO pin is available after mapping other requested
> > >> functions.
> > >> (*2) Here, the entire GPIOB port is explicitly requested by the GPIO
> > >> module, i.e. all pins of the port are made available as GPIOs.
> > >
> > > So I think all you're looking for is a way in DT to represent GPIO
> > > ranges? I don't think that should be by string name, but rather numbers:
> > >
> > > (actually, doesn't pinctrl-simple already have this?)
> >
> > Now I'm ever more confused ... we already have this :-)
> >
> > It's not even pinctrl-simple-centric it is completely generic.
> > The code is in drivers/gpio/gpiolib-of.c.
> >
> > It was written by Shiraz Hashin and Haojian Zhuang.
> > At the time I augmented the core code quite a bit to make
> > a good fit.
>
> I agree. Unluckily, it uses pinctrl-internal pin numbering which we
> would have to make coherent with the physical pin numbers of the
> individual packaging variants of the chip in order to expose them to
> customers (see my previous mail at https://lkml.org/lkml/2013/5/22/207).
>
> Adapting the kernel-internal pin numbering in function of the product
> variant doesn't seem such a good idea to me: All pin groups etc. will
> have to be redefined for every product, a huge source of bugs and
> unnecessary static data within the drivers.
>I review two methods you mentioned in this mail.
I think that you want to keep the logic simple. If so, I prefer you can
check pinctrl-single driver first. All pins are defined in DTS instead.
Here's the sample on Hi3620 SoC. And some pins even share one pinctrl
registers.
gpio controller:
gpio-ranges = < 3 94 1 7 96 1>;
Each gpio pins in gpio controller could trace to the pin controller.
pin controller:
pinctrl-single,gpio-range = < 0 3 0 3 9 1
12 1 0 13 29 1
43 1 0 44 49 1
94 1 1 96 2 1>;
range: gpio-range {
#pinctrl-single,gpio-range-cells = <3>;
};
If the pin could be configured as GPIO mode, we could trace to the
gpio controller by this table. We can see those pins are not continuous,
and it doesn't matter. And it could handle the relations between
multiple gpio controllers and multiple pin controllers.
These pin numbers are only used to bind GPIO number. Customers
needn't to know the details. The device driver only set the pins to
right mode (GPIO or any other) with right configuration. The device
driver even needn't know the GPIO number and the pin number.
Because everything is already handled. They only need the pinctrl
array. I prefer you to check drivers/tty/serial/amba-pl011.c.
In some SoC, both pin26 & pin104 could be configured as GPIO3.
We only need to define one in your DTS because only one could
be effect in the real world.
And we abstract pin number from the register offset. You also needn't
worry the pin name such as AA1, AB1. Those names are used to
communicate with hardware guys, not software guys.
If you want to redefine the pin groups because they're used as LCD pins,
not MMC pins any more. You needn't have to modify the pin arrays in
your pinctrl driver anymore. Because pinctrl-single driver don't define
any pin arrays. You can only define pin arrays in your DTS file.
Is it helpful to you?
Regards
Haojian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 22 May 2013 22:28, Christian Ruppert christian.rupp...@abilis.com wrote:
On Mon, May 20, 2013 at 10:10:33AM +0200, Linus Walleij wrote:
On Thu, May 16, 2013 at 2:12 AM, Stephen Warren swar...@wwwdotorg.org
wrote:
On 05/10/2013 02:25 AM, Christian Ruppert wrote:
(*1) TB100 GPIO ranges are defined as a phandle to the I/O function
which provides all pins of a given GPIO port. This function is not
necessarily requested from pinctrl and GPIO ports may overlap with
other functions. The pin controller knows about this and provides
whatever GPIO pin is available after mapping other requested
functions.
(*2) Here, the entire GPIOB port is explicitly requested by the GPIO
module, i.e. all pins of the port are made available as GPIOs.
So I think all you're looking for is a way in DT to represent GPIO
ranges? I don't think that should be by string name, but rather numbers:
(actually, doesn't pinctrl-simple already have this?)
Now I'm ever more confused ... we already have this :-)
It's not even pinctrl-simple-centric it is completely generic.
The code is in drivers/gpio/gpiolib-of.c.
It was written by Shiraz Hashin and Haojian Zhuang.
At the time I augmented the core code quite a bit to make
a good fit.
I agree. Unluckily, it uses pinctrl-internal pin numbering which we
would have to make coherent with the physical pin numbers of the
individual packaging variants of the chip in order to expose them to
customers (see my previous mail at https://lkml.org/lkml/2013/5/22/207).
Adapting the kernel-internal pin numbering in function of the product
variant doesn't seem such a good idea to me: All pin groups etc. will
have to be redefined for every product, a huge source of bugs and
unnecessary static data within the drivers.
I review two methods you mentioned in this mail.
I think that you want to keep the logic simple. If so, I prefer you can
check pinctrl-single driver first. All pins are defined in DTS instead.
Here's the sample on Hi3620 SoC. And some pins even share one pinctrl
registers.
gpio controller:
gpio-ranges = pmx0 3 94 1 pmx0 7 96 1;
Each gpio pins in gpio controller could trace to the pin controller.
pin controller:
pinctrl-single,gpio-range = range 0 3 0 range 3 9 1
range 12 1 0 range 13 29 1
range 43 1 0 range 44 49 1
range 94 1 1 range 96 2 1;
range: gpio-range {
#pinctrl-single,gpio-range-cells = 3;
};
If the pin could be configured as GPIO mode, we could trace to the
gpio controller by this table. We can see those pins are not continuous,
and it doesn't matter. And it could handle the relations between
multiple gpio controllers and multiple pin controllers.
These pin numbers are only used to bind GPIO number. Customers
needn't to know the details. The device driver only set the pins to
right mode (GPIO or any other) with right configuration. The device
driver even needn't know the GPIO number and the pin number.
Because everything is already handled. They only need the pinctrl
array. I prefer you to check drivers/tty/serial/amba-pl011.c.
In some SoC, both pin26 pin104 could be configured as GPIO3.
We only need to define one in your DTS because only one could
be effect in the real world.
And we abstract pin number from the register offset. You also needn't
worry the pin name such as AA1, AB1. Those names are used to
communicate with hardware guys, not software guys.
If you want to redefine the pin groups because they're used as LCD pins,
not MMC pins any more. You needn't have to modify the pin arrays in
your pinctrl driver anymore. Because pinctrl-single driver don't define
any pin arrays. You can only define pin arrays in your DTS file.
Is it helpful to you?
Regards
Haojian
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Mon, May 20, 2013 at 10:10:33AM +0200, Linus Walleij wrote:
> On Thu, May 16, 2013 at 2:12 AM, Stephen Warren wrote:
> > On 05/10/2013 02:25 AM, Christian Ruppert wrote:
>
> >> (*1) TB100 GPIO ranges are defined as a phandle to the I/O function
> >> which provides all pins of a given GPIO port. This function is not
> >> necessarily requested from pinctrl and GPIO ports may overlap with
> >> other functions. The pin controller knows about this and provides
> >> whatever GPIO pin is available after mapping other requested
> >> functions.
> >> (*2) Here, the entire GPIOB port is explicitly requested by the GPIO
> >> module, i.e. all pins of the port are made available as GPIOs.
> >
> > So I think all you're looking for is a way in DT to represent GPIO
> > ranges? I don't think that should be by string name, but rather numbers:
> >
> > (actually, doesn't pinctrl-simple already have this?)
>
> Now I'm ever more confused ... we already have this :-)
>
> It's not even pinctrl-simple-centric it is completely generic.
> The code is in drivers/gpio/gpiolib-of.c.
>
> It was written by Shiraz Hashin and Haojian Zhuang.
> At the time I augmented the core code quite a bit to make
> a good fit.
I agree. Unluckily, it uses pinctrl-internal pin numbering which we
would have to make coherent with the physical pin numbers of the
individual packaging variants of the chip in order to expose them to
customers (see my previous mail at https://lkml.org/lkml/2013/5/22/207).
Adapting the kernel-internal pin numbering in function of the product
variant doesn't seem such a good idea to me: All pin groups etc. will
have to be redefined for every product, a huge source of bugs and
unnecessary static data within the drivers.
> This is from:
> Documentation/devicetree/bindings/gpio/gpio.txt
>
> 2.1) gpio-controller and pinctrl subsystem
> --
>
> gpio-controller on a SOC might be tightly coupled with the pinctrl
> subsystem, in the sense that the pins can be used by other functions
> together with optional gpio feature.
>
> While the pin allocation is totally managed by the pin ctrl subsystem,
> gpio (under gpiolib) is still maintained by gpio drivers. It may happen
> that different pin ranges in a SoC is managed by different gpio drivers.
>
> This makes it logical to let gpio drivers announce their pin ranges to
> the pin ctrl subsystem and call 'pinctrl_request_gpio' in order to
> request the corresponding pin before any gpio usage.
>
> For this, the gpio controller can use a pinctrl phandle and pins to
> announce the pinrange to the pin ctrl subsystem. For example,
>
> qe_pio_e: gpio-controller@1460 {
> #gpio-cells = <2>;
> compatible = "fsl,qe-pario-bank-e", "fsl,qe-pario-bank";
> reg = <0x1460 0x18>;
> gpio-controller;
> gpio-ranges = < 0 20 10>, < 10 50 20>;
>
> }
>
> where,
> and is the phandle to the pinctrl DT node.
>
>Next values specify the base pin and number of pins for the range
>handled by 'qe_pio_e' gpio. In the given example from base pin 20 to
>pin 29 under pinctrl1 with gpio offset 0 and pin 50 to pin 69 under
>pinctrl2 with gpio offset 10 is handled by this gpio controller.
>
> The pinctrl node must have "#gpio-range-cells" property to show number of
> arguments to pass with phandle from gpio controllers node.
>
> Yours,
> Linus Walleij
--
Christian Ruppert ,
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
_// | bilis Systems CH-1228 Plan-les-Ouates
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Mon, May 20, 2013 at 10:03:24AM +0200, Linus Walleij wrote:
> On Wed, May 15, 2013 at 11:41 AM, Christian Ruppert
> wrote:
> > On Tue, May 14, 2013 at 02:29:46PM +0200, Linus Walleij wrote:
>
> >> Look at for example
> >> drivers/pinctrl/pinctrl-abx500.c:
> >>
> >> static u8 abx500_get_mode(struct pinctrl_dev *pctldev, struct gpio_chip
> >> *chip,
> >> unsigned gpio)
> >> {
> >> (...)
> >> /* on ABx5xx, there is no GPIO0, so adjust the offset */
> >> unsigned offset = gpio - 1;
> >>
> >> As you can see, this driver, which does not use device tree,
> >> is working around the same problem. Here the problem is that
> >> the pins are numbered starting at 1 instead of 0, a very trivial
> >> numberspace shuffleing.
> >
> > Let me see if I understand this right:
> > In ABx5xx, the pin numbering is offset by 1 wrt. GPIO numbering?
>
> No, sorry that the code is strange... The variable is named "gpio"
> because that is the numbering used in the data sheet.
>
> Think of "gpio" as "the number in the data sheet".
>
> Then offset is calculated to start from 0.
OK, seems like I thoroughly misunderstood what you were saying and drew
some wrong conclusions from that. In order to avoid confusion I'll
delete everything related to this misunderstanding below.
> [...]
> If you mean that you try to map the global GPIO number space
> 1:1 on top of what your datasheet has, just *don't do that*.
> Because we want to get rid of this global GPIO numberspace.
> Alexandre is already working hard on this!
Thanks for the hint. Defining a global GPIO number space in the data
sheet was one of our ideas but I agree it would be preferable to do away
with this completely and rather deal with GPIOs on a bank level.
> > - The custom logic inside many pinctrl drivers would be confined in one
> >translation function the driver provides instead of being spread out
> >all over the driver.
> > - "Sparse GPIO ranges" are easy to implement if required by the
> >platform/driver.
>
> This looks good.
I think these advantages could be maintained through two alternative
methods:
1) Extend the GPIO ranges mechanism to allow for pin groups in addition
to contiguous pin ranges. This would be my preferred method since no
pin numbers are exposed to the user at all (neither kernel internal
nor physical). All pin numbering remains inside the driver. This
would be a clean version of what I did in the original patch.
2) Add a physical-to-Linux-internal pin number translation callback to
the pinmux driver core interface. Map 1-to-1 if this callback is not
defined to maintain backwards compatibility. I don't like this
solution as much as 1) for the following reasons:
. It adds yet another number space to confuse things even more.
. Managing contiguous ranges becomes more difficult because ranges
contiguous in one number space (physical pins, Linux pins, GPIOs)
are not necessarily contiguous in the others, requiring more
complex translation mechanisms.
. Some chips have matrix like number schemes (A1, A2, ..., B1, B1,
...) etc. which cannot be directly addressed with this and would
need to introduce yet another layer of indirection.
. In cases where different package options need to be supported, a
translation table is required for each inside the driver. For
modern SOCs, each of these tables can easily be a few kB in size
and I don't know if we want to add that amount of static data to
the kernel, esp. because all but one of the tables are unused.
I don't know ACPI and one of the two solutions might not be applicable
there.
> [...]
> We need to iterate this discussion a few turns until I actually
> understand your problem I think.
I agree. Let me resume my high level constraints again:
- Align customer visible interfaces (such as device tree and
sys/class/gpio) with our hardware documentation.
- Documentation can be augmented (e.g. a global GPIO number space
_could_ be added) but fundamentals (such as physical pin numbering,
our GPIO bank structure) must not be changed.
- Several distinct naming schemes for the same thing (such as pins) are
only allowed if it is obvious to which naming scheme a given name
belongs. E.g. pins may have both a name (string) and a number (and
they actually have) but it is not allowed to have two different
numbers designating the same pin.
Greetings,
Christian
--
Christian Ruppert ,
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
_// | bilis Systems CH-1228 Plan-les-Ouates
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Mon, May 20, 2013 at 10:03:24AM +0200, Linus Walleij wrote:
On Wed, May 15, 2013 at 11:41 AM, Christian Ruppert
christian.rupp...@abilis.com wrote:
On Tue, May 14, 2013 at 02:29:46PM +0200, Linus Walleij wrote:
Look at for example
drivers/pinctrl/pinctrl-abx500.c:
static u8 abx500_get_mode(struct pinctrl_dev *pctldev, struct gpio_chip
*chip,
unsigned gpio)
{
(...)
/* on ABx5xx, there is no GPIO0, so adjust the offset */
unsigned offset = gpio - 1;
As you can see, this driver, which does not use device tree,
is working around the same problem. Here the problem is that
the pins are numbered starting at 1 instead of 0, a very trivial
numberspace shuffleing.
Let me see if I understand this right:
In ABx5xx, the pin numbering is offset by 1 wrt. GPIO numbering?
No, sorry that the code is strange... The variable is named gpio
because that is the numbering used in the data sheet.
Think of gpio as the number in the data sheet.
Then offset is calculated to start from 0.
OK, seems like I thoroughly misunderstood what you were saying and drew
some wrong conclusions from that. In order to avoid confusion I'll
delete everything related to this misunderstanding below.
[...]
If you mean that you try to map the global GPIO number space
1:1 on top of what your datasheet has, just *don't do that*.
Because we want to get rid of this global GPIO numberspace.
Alexandre is already working hard on this!
Thanks for the hint. Defining a global GPIO number space in the data
sheet was one of our ideas but I agree it would be preferable to do away
with this completely and rather deal with GPIOs on a bank level.
- The custom logic inside many pinctrl drivers would be confined in one
translation function the driver provides instead of being spread out
all over the driver.
- Sparse GPIO ranges are easy to implement if required by the
platform/driver.
This looks good.
I think these advantages could be maintained through two alternative
methods:
1) Extend the GPIO ranges mechanism to allow for pin groups in addition
to contiguous pin ranges. This would be my preferred method since no
pin numbers are exposed to the user at all (neither kernel internal
nor physical). All pin numbering remains inside the driver. This
would be a clean version of what I did in the original patch.
2) Add a physical-to-Linux-internal pin number translation callback to
the pinmux driver core interface. Map 1-to-1 if this callback is not
defined to maintain backwards compatibility. I don't like this
solution as much as 1) for the following reasons:
. It adds yet another number space to confuse things even more.
. Managing contiguous ranges becomes more difficult because ranges
contiguous in one number space (physical pins, Linux pins, GPIOs)
are not necessarily contiguous in the others, requiring more
complex translation mechanisms.
. Some chips have matrix like number schemes (A1, A2, ..., B1, B1,
...) etc. which cannot be directly addressed with this and would
need to introduce yet another layer of indirection.
. In cases where different package options need to be supported, a
translation table is required for each inside the driver. For
modern SOCs, each of these tables can easily be a few kB in size
and I don't know if we want to add that amount of static data to
the kernel, esp. because all but one of the tables are unused.
I don't know ACPI and one of the two solutions might not be applicable
there.
[...]
We need to iterate this discussion a few turns until I actually
understand your problem I think.
I agree. Let me resume my high level constraints again:
- Align customer visible interfaces (such as device tree and
sys/class/gpio) with our hardware documentation.
- Documentation can be augmented (e.g. a global GPIO number space
_could_ be added) but fundamentals (such as physical pin numbering,
our GPIO bank structure) must not be changed.
- Several distinct naming schemes for the same thing (such as pins) are
only allowed if it is obvious to which naming scheme a given name
belongs. E.g. pins may have both a name (string) and a number (and
they actually have) but it is not allowed to have two different
numbers designating the same pin.
Greetings,
Christian
--
Christian Ruppert , christian.rupp...@abilis.com
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
_// | bilis Systems CH-1228 Plan-les-Ouates
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Mon, May 20, 2013 at 10:10:33AM +0200, Linus Walleij wrote:
On Thu, May 16, 2013 at 2:12 AM, Stephen Warren swar...@wwwdotorg.org wrote:
On 05/10/2013 02:25 AM, Christian Ruppert wrote:
(*1) TB100 GPIO ranges are defined as a phandle to the I/O function
which provides all pins of a given GPIO port. This function is not
necessarily requested from pinctrl and GPIO ports may overlap with
other functions. The pin controller knows about this and provides
whatever GPIO pin is available after mapping other requested
functions.
(*2) Here, the entire GPIOB port is explicitly requested by the GPIO
module, i.e. all pins of the port are made available as GPIOs.
So I think all you're looking for is a way in DT to represent GPIO
ranges? I don't think that should be by string name, but rather numbers:
(actually, doesn't pinctrl-simple already have this?)
Now I'm ever more confused ... we already have this :-)
It's not even pinctrl-simple-centric it is completely generic.
The code is in drivers/gpio/gpiolib-of.c.
It was written by Shiraz Hashin and Haojian Zhuang.
At the time I augmented the core code quite a bit to make
a good fit.
I agree. Unluckily, it uses pinctrl-internal pin numbering which we
would have to make coherent with the physical pin numbers of the
individual packaging variants of the chip in order to expose them to
customers (see my previous mail at https://lkml.org/lkml/2013/5/22/207).
Adapting the kernel-internal pin numbering in function of the product
variant doesn't seem such a good idea to me: All pin groups etc. will
have to be redefined for every product, a huge source of bugs and
unnecessary static data within the drivers.
This is from:
Documentation/devicetree/bindings/gpio/gpio.txt
2.1) gpio-controller and pinctrl subsystem
--
gpio-controller on a SOC might be tightly coupled with the pinctrl
subsystem, in the sense that the pins can be used by other functions
together with optional gpio feature.
While the pin allocation is totally managed by the pin ctrl subsystem,
gpio (under gpiolib) is still maintained by gpio drivers. It may happen
that different pin ranges in a SoC is managed by different gpio drivers.
This makes it logical to let gpio drivers announce their pin ranges to
the pin ctrl subsystem and call 'pinctrl_request_gpio' in order to
request the corresponding pin before any gpio usage.
For this, the gpio controller can use a pinctrl phandle and pins to
announce the pinrange to the pin ctrl subsystem. For example,
qe_pio_e: gpio-controller@1460 {
#gpio-cells = 2;
compatible = fsl,qe-pario-bank-e, fsl,qe-pario-bank;
reg = 0x1460 0x18;
gpio-controller;
gpio-ranges = pinctrl1 0 20 10, pinctrl2 10 50 20;
}
where,
pinctrl1 and pinctrl2 is the phandle to the pinctrl DT node.
Next values specify the base pin and number of pins for the range
handled by 'qe_pio_e' gpio. In the given example from base pin 20 to
pin 29 under pinctrl1 with gpio offset 0 and pin 50 to pin 69 under
pinctrl2 with gpio offset 10 is handled by this gpio controller.
The pinctrl node must have #gpio-range-cells property to show number of
arguments to pass with phandle from gpio controllers node.
Yours,
Linus Walleij
--
Christian Ruppert , christian.rupp...@abilis.com
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
_// | bilis Systems CH-1228 Plan-les-Ouates
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Thu, May 16, 2013 at 2:12 AM, Stephen Warren wrote:
> On 05/10/2013 02:25 AM, Christian Ruppert wrote:
>> (*1) TB100 GPIO ranges are defined as a phandle to the I/O function
>> which provides all pins of a given GPIO port. This function is not
>> necessarily requested from pinctrl and GPIO ports may overlap with
>> other functions. The pin controller knows about this and provides
>> whatever GPIO pin is available after mapping other requested
>> functions.
>> (*2) Here, the entire GPIOB port is explicitly requested by the GPIO
>> module, i.e. all pins of the port are made available as GPIOs.
>
> So I think all you're looking for is a way in DT to represent GPIO
> ranges? I don't think that should be by string name, but rather numbers:
>
> (actually, doesn't pinctrl-simple already have this?)
Now I'm ever more confused ... we already have this :-)
It's not even pinctrl-simple-centric it is completely generic.
The code is in drivers/gpio/gpiolib-of.c.
It was written by Shiraz Hashin and Haojian Zhuang.
At the time I augmented the core code quite a bit to make
a good fit.
This is from:
Documentation/devicetree/bindings/gpio/gpio.txt
2.1) gpio-controller and pinctrl subsystem
--
gpio-controller on a SOC might be tightly coupled with the pinctrl
subsystem, in the sense that the pins can be used by other functions
together with optional gpio feature.
While the pin allocation is totally managed by the pin ctrl subsystem,
gpio (under gpiolib) is still maintained by gpio drivers. It may happen
that different pin ranges in a SoC is managed by different gpio drivers.
This makes it logical to let gpio drivers announce their pin ranges to
the pin ctrl subsystem and call 'pinctrl_request_gpio' in order to
request the corresponding pin before any gpio usage.
For this, the gpio controller can use a pinctrl phandle and pins to
announce the pinrange to the pin ctrl subsystem. For example,
qe_pio_e: gpio-controller@1460 {
#gpio-cells = <2>;
compatible = "fsl,qe-pario-bank-e", "fsl,qe-pario-bank";
reg = <0x1460 0x18>;
gpio-controller;
gpio-ranges = < 0 20 10>, < 10 50 20>;
}
where,
and is the phandle to the pinctrl DT node.
Next values specify the base pin and number of pins for the range
handled by 'qe_pio_e' gpio. In the given example from base pin 20 to
pin 29 under pinctrl1 with gpio offset 0 and pin 50 to pin 69 under
pinctrl2 with gpio offset 10 is handled by this gpio controller.
The pinctrl node must have "#gpio-range-cells" property to show number of
arguments to pass with phandle from gpio controllers node.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Wed, May 15, 2013 at 11:41 AM, Christian Ruppert
wrote:
> On Tue, May 14, 2013 at 02:29:46PM +0200, Linus Walleij wrote:
>> Look at for example
>> drivers/pinctrl/pinctrl-abx500.c:
>>
>> static u8 abx500_get_mode(struct pinctrl_dev *pctldev, struct gpio_chip
>> *chip,
>> unsigned gpio)
>> {
>> (...)
>> /* on ABx5xx, there is no GPIO0, so adjust the offset */
>> unsigned offset = gpio - 1;
>>
>> As you can see, this driver, which does not use device tree,
>> is working around the same problem. Here the problem is that
>> the pins are numbered starting at 1 instead of 0, a very trivial
>> numberspace shuffleing.
>
> Let me see if I understand this right:
> In ABx5xx, the pin numbering is offset by 1 wrt. GPIO numbering?
No, sorry that the code is strange... The variable is named "gpio"
because that is the numbering used in the data sheet.
Think of "gpio" as "the number in the data sheet".
Then offset is calculated to start from 0.
>> I'd be open to this approach if you:
>>
>> - Make it generic for all pinctrl drivers, i.e. add the translation
>> to the core so it does not just apply to devices using device tree.
>
> Do you mean what would be required here is a generic way to translate
> pin numbers to GPIO numbers in the pin controller?
No that is already done by the GPIO ranges.
I thought this discussion was about shuffleing around/translating
the numberings of the pins themselves.
> This translation is currently achieved by the gpio_range mechanism.
Yep.
> Internals of gpio_range leak into the pinctrl drivers and are bypassed
> in many cases. E.g. pinctrl-abx500.c uses an internal reimplementation
> of gpio ranges and the offset by one instead of the information in the
> pinctrl_gpio_range structure.
It has abx500_pinrange to associate some information with certain
ranges of pins. It has nothing to do with GPIO ranges if you look
closer at it.
The offset by one is achieved not by this range type, but by hard-coding
a subtraction with one at every entry point, as illustrated. This
is orthogonal to all range concepts.
> Mapping pin numbers to GPIO numbers in the pin controller would have the
> following advantages:
> - It makes GPIO numbering well defined which is clearly an advantage
>for the /sys/class/gpio interface: GPIO numbering is now controlled
>by the pinctrl driver author and no longer needs to be kernel
>internal.
> - The device tree/acpi issue is solved since a GPIO controller could
>now define its range in GPIO number space (which becomes public)
>rather than kernel-internal pin number space. At least for our
>products, GPIO numbering generally doesn't change between different
>variants of the same chip and the /sys/class/gpio customer
>documentation would apply to device also.
This is getting very confused so I can't follow it.
For the GPIO and pin control subsystems there exist three things:
- Local offsets on the GPIO controller, such as if there is a
GPIO controller for 32 lines represented by 32 bits, offset
0 .. 31.
- The global GPIO number space, which is a big array with
some roof, where the GPIO numbers are shoehorned in,
trying not to collide.
- Local offsets on the pin controller, which work the same way
as GPIO local offsets.
I think you are talking about something completely different
here, and that might be the numbering scheme used in the
data sheet, or device tree is this correct? Please call that
the data sheet or device tree numbering system in that
case, or it will be very confusing for me. To me all
"GPIO numbers" are pure kernel concepts.
If you mean that you try to map the global GPIO number space
1:1 on top of what your datasheet has, just *don't do that*.
Because we want to get rid of this global GPIO numberspace.
Alexandre is already working hard on this!
Can you please try to be very specific on what is
going on here?
> - The custom logic inside many pinctrl drivers would be confined in one
>translation function the driver provides instead of being spread out
>all over the driver.
> - "Sparse GPIO ranges" are easy to implement if required by the
>platform/driver.
This looks good.
>> - Augment the pinctrl-abx500.c driver to show how this simplifies
>> that driver. (Does not need to be perfect, I'll help out finalizing it
>> for sure.)
>
> The issue is that such a change is quite fundamental, all pinctrl
> drivers would have to be upgraded (not just pinctrl-abx500.c) and struct
> pinctrl_gpio_range would have to be removed from the gpio_request_enable
> callback and friends in favour of some generic translation mechanism.
Doing large refactorings is a normal part of kernel life.
See: Documentation/stable_api_nonsense.txt
> I am also afraid that the custom logic in many drivers could only be
> rewritten with the help of the respective driver's author.
If you're patching their drivers they are obliged to help out
by reviewing and
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Wed, May 15, 2013 at 11:41 AM, Christian Ruppert
christian.rupp...@abilis.com wrote:
On Tue, May 14, 2013 at 02:29:46PM +0200, Linus Walleij wrote:
Look at for example
drivers/pinctrl/pinctrl-abx500.c:
static u8 abx500_get_mode(struct pinctrl_dev *pctldev, struct gpio_chip
*chip,
unsigned gpio)
{
(...)
/* on ABx5xx, there is no GPIO0, so adjust the offset */
unsigned offset = gpio - 1;
As you can see, this driver, which does not use device tree,
is working around the same problem. Here the problem is that
the pins are numbered starting at 1 instead of 0, a very trivial
numberspace shuffleing.
Let me see if I understand this right:
In ABx5xx, the pin numbering is offset by 1 wrt. GPIO numbering?
No, sorry that the code is strange... The variable is named gpio
because that is the numbering used in the data sheet.
Think of gpio as the number in the data sheet.
Then offset is calculated to start from 0.
I'd be open to this approach if you:
- Make it generic for all pinctrl drivers, i.e. add the translation
to the core so it does not just apply to devices using device tree.
Do you mean what would be required here is a generic way to translate
pin numbers to GPIO numbers in the pin controller?
No that is already done by the GPIO ranges.
I thought this discussion was about shuffleing around/translating
the numberings of the pins themselves.
This translation is currently achieved by the gpio_range mechanism.
Yep.
Internals of gpio_range leak into the pinctrl drivers and are bypassed
in many cases. E.g. pinctrl-abx500.c uses an internal reimplementation
of gpio ranges and the offset by one instead of the information in the
pinctrl_gpio_range structure.
It has abx500_pinrange to associate some information with certain
ranges of pins. It has nothing to do with GPIO ranges if you look
closer at it.
The offset by one is achieved not by this range type, but by hard-coding
a subtraction with one at every entry point, as illustrated. This
is orthogonal to all range concepts.
Mapping pin numbers to GPIO numbers in the pin controller would have the
following advantages:
- It makes GPIO numbering well defined which is clearly an advantage
for the /sys/class/gpio interface: GPIO numbering is now controlled
by the pinctrl driver author and no longer needs to be kernel
internal.
- The device tree/acpi issue is solved since a GPIO controller could
now define its range in GPIO number space (which becomes public)
rather than kernel-internal pin number space. At least for our
products, GPIO numbering generally doesn't change between different
variants of the same chip and the /sys/class/gpio customer
documentation would apply to device also.
This is getting very confused so I can't follow it.
For the GPIO and pin control subsystems there exist three things:
- Local offsets on the GPIO controller, such as if there is a
GPIO controller for 32 lines represented by 32 bits, offset
0 .. 31.
- The global GPIO number space, which is a big array with
some roof, where the GPIO numbers are shoehorned in,
trying not to collide.
- Local offsets on the pin controller, which work the same way
as GPIO local offsets.
I think you are talking about something completely different
here, and that might be the numbering scheme used in the
data sheet, or device tree is this correct? Please call that
the data sheet or device tree numbering system in that
case, or it will be very confusing for me. To me all
GPIO numbers are pure kernel concepts.
If you mean that you try to map the global GPIO number space
1:1 on top of what your datasheet has, just *don't do that*.
Because we want to get rid of this global GPIO numberspace.
Alexandre is already working hard on this!
Can you please try to be very specific on what is
going on here?
- The custom logic inside many pinctrl drivers would be confined in one
translation function the driver provides instead of being spread out
all over the driver.
- Sparse GPIO ranges are easy to implement if required by the
platform/driver.
This looks good.
- Augment the pinctrl-abx500.c driver to show how this simplifies
that driver. (Does not need to be perfect, I'll help out finalizing it
for sure.)
The issue is that such a change is quite fundamental, all pinctrl
drivers would have to be upgraded (not just pinctrl-abx500.c) and struct
pinctrl_gpio_range would have to be removed from the gpio_request_enable
callback and friends in favour of some generic translation mechanism.
Doing large refactorings is a normal part of kernel life.
See: Documentation/stable_api_nonsense.txt
I am also afraid that the custom logic in many drivers could only be
rewritten with the help of the respective driver's author.
If you're patching their drivers they are obliged to help out
by reviewing and testing, that is how we work. If they don't
review and test patches
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Thu, May 16, 2013 at 2:12 AM, Stephen Warren swar...@wwwdotorg.org wrote:
On 05/10/2013 02:25 AM, Christian Ruppert wrote:
(*1) TB100 GPIO ranges are defined as a phandle to the I/O function
which provides all pins of a given GPIO port. This function is not
necessarily requested from pinctrl and GPIO ports may overlap with
other functions. The pin controller knows about this and provides
whatever GPIO pin is available after mapping other requested
functions.
(*2) Here, the entire GPIOB port is explicitly requested by the GPIO
module, i.e. all pins of the port are made available as GPIOs.
So I think all you're looking for is a way in DT to represent GPIO
ranges? I don't think that should be by string name, but rather numbers:
(actually, doesn't pinctrl-simple already have this?)
Now I'm ever more confused ... we already have this :-)
It's not even pinctrl-simple-centric it is completely generic.
The code is in drivers/gpio/gpiolib-of.c.
It was written by Shiraz Hashin and Haojian Zhuang.
At the time I augmented the core code quite a bit to make
a good fit.
This is from:
Documentation/devicetree/bindings/gpio/gpio.txt
2.1) gpio-controller and pinctrl subsystem
--
gpio-controller on a SOC might be tightly coupled with the pinctrl
subsystem, in the sense that the pins can be used by other functions
together with optional gpio feature.
While the pin allocation is totally managed by the pin ctrl subsystem,
gpio (under gpiolib) is still maintained by gpio drivers. It may happen
that different pin ranges in a SoC is managed by different gpio drivers.
This makes it logical to let gpio drivers announce their pin ranges to
the pin ctrl subsystem and call 'pinctrl_request_gpio' in order to
request the corresponding pin before any gpio usage.
For this, the gpio controller can use a pinctrl phandle and pins to
announce the pinrange to the pin ctrl subsystem. For example,
qe_pio_e: gpio-controller@1460 {
#gpio-cells = 2;
compatible = fsl,qe-pario-bank-e, fsl,qe-pario-bank;
reg = 0x1460 0x18;
gpio-controller;
gpio-ranges = pinctrl1 0 20 10, pinctrl2 10 50 20;
}
where,
pinctrl1 and pinctrl2 is the phandle to the pinctrl DT node.
Next values specify the base pin and number of pins for the range
handled by 'qe_pio_e' gpio. In the given example from base pin 20 to
pin 29 under pinctrl1 with gpio offset 0 and pin 50 to pin 69 under
pinctrl2 with gpio offset 10 is handled by this gpio controller.
The pinctrl node must have #gpio-range-cells property to show number of
arguments to pass with phandle from gpio controllers node.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 05/10/2013 02:25 AM, Christian Ruppert wrote:
> On Wed, May 08, 2013 at 02:01:53PM -0600, Stephen Warren wrote:
>> On 05/08/2013 10:41 AM, Christian Ruppert wrote:
>> ...
>>> What do you think about the following modification to the pinctrl/GPIO
>>> frameworks instead (not yet a formal patch, more a request for comment
>>> to illustrate what I mean. If you agree, I will clean it up and submit a
>>> proper patch after discussion).
>>>
>>> It adds a dt_gpiorange_xlate function to the pinctrl callbacks which
>>> defaults to the conventional behaviour using kernel logical pin numbers.
>>> However, pin controllers which provide more complex mechanisms can
>>> define #gpio-range-cells and provide this callback in order to keep
>>> Linux pin numbering inside the kernel.
>>
>> Can you provide an example of the DT content, and explain exactly what
>> this patch does with it; what effect it has on the existing GPIO or
>> pinctrl code?
>
> The patch does not change the default behaviour of the kernel: In case
> no dt_gpiorange_xlate callback is defined for a given driver (e.g. for
> pre-existing drivers), the default function simply interprets the first
> argument as Linux pin number and the second as pin count, same as now.
> New drivers can use the callback to translate device specific pin
> references to Linux pin numbers (in the idea of of_xlate in the GPIO
> framework or xlate in the irqchip framework).
>
> In the case of TB10x, I was thinking of something in the lines of
>
> iomux: iomux@FF10601c {
> #gpio-range-cells = <1>; /* one cell used for gpiorange phandle */
> compatible = "abilis,tb10x-iomux";
> reg = <0xFF10601c 0x4>;
> pctl_gpio_a: pctl-gpio-a { /* define phandle to GPIOA I/O function */
> pingrp = "gpioa_pins";
> };
These aren't phandles at all; they're just plain old nodes and properties.
> pctl_gpio_b: pctl-gpio-b { /* define phandle to GPIOB I/O function */
> pingrp = "gpiob_pins";
> };
> pctl_uart0: pctl-uart0 { /* define phandle to UART0 I/O function */
> pingrp = "uart0_pins";
> };
> };
> uart@FF10 {
> compatible = "snps,dw-apb-uart";
> reg = <0xFF10 0x100>;
> clock-frequency = <1>;
> interrupts = <25 1>;
> reg-shift = <2>;
> reg-io-width = <4>;
> pinctrl-names = "default"; /* For non-GPIO modules, request I/O */
> pinctrl-0 = <_uart0>; /* functions normally */
> };
> gpioa: gpio@FF14 {
> compatible = "abilis,tb10x-gpio";
> reg = <0xFF14 0x1000>;
> gpio-controller;
> #gpio-cells = <2>;
> gpio-ranges = < _gpio_a>; /* (*1) */
I guess you had to make the pctl_gpio_a a DT node, because to reference
it by phandle, it has to be a node; a phandle can't point at a property.
> };
>
> gpioa: gpio@FF14 {
> compatible = "abilis,tb10x-gpio";
> reg = <0xFF14 0x1000>;
> gpio-controller;
> #gpio-cells = <2>;
> pinctrl-names = "default" /* here the GPIO controller requests */
> pinctrl-0 = <_gpio_b>; /* the entire GPIOB port explicitly */
> gpio-ranges = < _gpio_b>; /* (*2) */
> };
>
> (*1) TB100 GPIO ranges are defined as a phandle to the I/O function
> which provides all pins of a given GPIO port. This function is not
> necessarily requested from pinctrl and GPIO ports may overlap with
> other functions. The pin controller knows about this and provides
> whatever GPIO pin is available after mapping other requested
> functions.
> (*2) Here, the entire GPIOB port is explicitly requested by the GPIO
> module, i.e. all pins of the port are made available as GPIOs.
So I think all you're looking for is a way in DT to represent GPIO
ranges? I don't think that should be by string name, but rather numbers:
(actually, doesn't pinctrl-simple already have this?)
The GPIO driver exposes N GPIOs, probably numbered 0..n-1, but I guess
it can number them in whatever sparse way it wants.
The pinctrl driver exposes N pins, again probably numbered 0..n-1, but I
guess it can number them in whatever sparse way it wants.
You want a way to represent the mapping between the various GPIO HW
modules' internal GPIO IDs and the various pinctrl HW modules' pin IDs.
That mapping should be just by integer GPIO/pin ID, I think. What about
the following:
iomux: iomux@FF10601c {
...
};
gpio@FF14 {
gpio-ranges = ;
};
where:
GPIO_BASE: GPIO ID (within gpioa, not global) for the start of the range
covered by this entry.
SIZE: Number of GPIO IDs (and pinctrl pin IDs) covered by this entry.
: The pin controller related to this range.
IOMUX_PIN_BASE: The pinctrl pin ID (within the iomux pin controller, not
global) for the start of the range covered by this entry.
You could easily have more complex setups, with multiple disjoint ranges:
iomuxa: iomux@1000 {
...
};
iomuxb: iomux@2000 {
...
};
gpio@8000 {
gpio-ranges =
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Tue, May 14, 2013 at 02:29:46PM +0200, Linus Walleij wrote:
> On Fri, May 10, 2013 at 10:25 AM, Christian Ruppert
> wrote:
>
> >> > What do you think about the following modification to the pinctrl/GPIO
> >> > frameworks instead (not yet a formal patch, more a request for comment
> >> > to illustrate what I mean. If you agree, I will clean it up and submit a
> >> > proper patch after discussion).
> >> >
> >> > It adds a dt_gpiorange_xlate function to the pinctrl callbacks which
> >> > defaults to the conventional behaviour using kernel logical pin numbers.
> >> > However, pin controllers which provide more complex mechanisms can
> >> > define #gpio-range-cells and provide this callback in order to keep
> >> > Linux pin numbering inside the kernel.
> (...)
> > The patch does not change the default behaviour of the kernel: In case
> > no dt_gpiorange_xlate callback is defined for a given driver (e.g. for
> > pre-existing drivers), the default function simply interprets the first
> > argument as Linux pin number and the second as pin count, same as now.
> > New drivers can use the callback to translate device specific pin
> > references to Linux pin numbers (in the idea of of_xlate in the GPIO
> > framework or xlate in the irqchip framework).
>
> I like the concept.
>
> However I am totally opposed to the idea of making this something
> device tree-exclusive.
>
> Look at for example
> drivers/pinctrl/pinctrl-abx500.c:
>
> static u8 abx500_get_mode(struct pinctrl_dev *pctldev, struct gpio_chip *chip,
> unsigned gpio)
> {
> (...)
> /* on ABx5xx, there is no GPIO0, so adjust the offset */
> unsigned offset = gpio - 1;
>
> As you can see, this driver, which does not use device tree,
> is working around the same problem. Here the problem is that
> the pins are numbered starting at 1 instead of 0, a very trivial
> numberspace shuffleing.
Let me see if I understand this right:
In ABx5xx, the pin numbering is offset by 1 wrt. GPIO numbering?
> I'd be open to this approach if you:
>
> - Make it generic for all pinctrl drivers, i.e. add the translation
> to the core so it does not just apply to devices using device tree.
Do you mean what would be required here is a generic way to translate
pin numbers to GPIO numbers in the pin controller?
This translation is currently achieved by the gpio_range mechanism.
Internals of gpio_range leak into the pinctrl drivers and are bypassed
in many cases. E.g. pinctrl-abx500.c uses an internal reimplementation
of gpio ranges and the offset by one instead of the information in the
pinctrl_gpio_range structure.
Mapping pin numbers to GPIO numbers in the pin controller would have the
following advantages:
- It makes GPIO numbering well defined which is clearly an advantage
for the /sys/class/gpio interface: GPIO numbering is now controlled
by the pinctrl driver author and no longer needs to be kernel
internal.
- The device tree/acpi issue is solved since a GPIO controller could
now define its range in GPIO number space (which becomes public)
rather than kernel-internal pin number space. At least for our
products, GPIO numbering generally doesn't change between different
variants of the same chip and the /sys/class/gpio customer
documentation would apply to device also.
- The custom logic inside many pinctrl drivers would be confined in one
translation function the driver provides instead of being spread out
all over the driver.
- "Sparse GPIO ranges" are easy to implement if required by the
platform/driver.
> - Augment the pinctrl-abx500.c driver to show how this simplifies
> that driver. (Does not need to be perfect, I'll help out finalizing it
> for sure.)
The issue is that such a change is quite fundamental, all pinctrl
drivers would have to be upgraded (not just pinctrl-abx500.c) and struct
pinctrl_gpio_range would have to be removed from the gpio_request_enable
callback and friends in favour of some generic translation mechanism.
I am also afraid that the custom logic in many drivers could only be
rewritten with the help of the respective driver's author.
> - Then add DT-specific wrapper using this core feature.
GPIO ranges definitions in DT would probably be no longer be compatible
with previous definitions since the number space would change from pin
numbers to GPIO numbers. I don't think this is acceptable.
> This way the problem will be solved for everybody, including
> ACPI when they sooner or later come back with the same issue.
Given the issues highlighted above I'm not sure if I understand your
proposal correctly. Although I see the advantages, I wonder if the
migration to such a system is feasible in practise.
Greetings,
Christian
--
Christian Ruppert ,
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
_// | bilis Systems CH-1228 Plan-les-Ouates
--
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Tue, May 14, 2013 at 02:29:46PM +0200, Linus Walleij wrote:
On Fri, May 10, 2013 at 10:25 AM, Christian Ruppert
christian.rupp...@abilis.com wrote:
What do you think about the following modification to the pinctrl/GPIO
frameworks instead (not yet a formal patch, more a request for comment
to illustrate what I mean. If you agree, I will clean it up and submit a
proper patch after discussion).
It adds a dt_gpiorange_xlate function to the pinctrl callbacks which
defaults to the conventional behaviour using kernel logical pin numbers.
However, pin controllers which provide more complex mechanisms can
define #gpio-range-cells and provide this callback in order to keep
Linux pin numbering inside the kernel.
(...)
The patch does not change the default behaviour of the kernel: In case
no dt_gpiorange_xlate callback is defined for a given driver (e.g. for
pre-existing drivers), the default function simply interprets the first
argument as Linux pin number and the second as pin count, same as now.
New drivers can use the callback to translate device specific pin
references to Linux pin numbers (in the idea of of_xlate in the GPIO
framework or xlate in the irqchip framework).
I like the concept.
However I am totally opposed to the idea of making this something
device tree-exclusive.
Look at for example
drivers/pinctrl/pinctrl-abx500.c:
static u8 abx500_get_mode(struct pinctrl_dev *pctldev, struct gpio_chip *chip,
unsigned gpio)
{
(...)
/* on ABx5xx, there is no GPIO0, so adjust the offset */
unsigned offset = gpio - 1;
As you can see, this driver, which does not use device tree,
is working around the same problem. Here the problem is that
the pins are numbered starting at 1 instead of 0, a very trivial
numberspace shuffleing.
Let me see if I understand this right:
In ABx5xx, the pin numbering is offset by 1 wrt. GPIO numbering?
I'd be open to this approach if you:
- Make it generic for all pinctrl drivers, i.e. add the translation
to the core so it does not just apply to devices using device tree.
Do you mean what would be required here is a generic way to translate
pin numbers to GPIO numbers in the pin controller?
This translation is currently achieved by the gpio_range mechanism.
Internals of gpio_range leak into the pinctrl drivers and are bypassed
in many cases. E.g. pinctrl-abx500.c uses an internal reimplementation
of gpio ranges and the offset by one instead of the information in the
pinctrl_gpio_range structure.
Mapping pin numbers to GPIO numbers in the pin controller would have the
following advantages:
- It makes GPIO numbering well defined which is clearly an advantage
for the /sys/class/gpio interface: GPIO numbering is now controlled
by the pinctrl driver author and no longer needs to be kernel
internal.
- The device tree/acpi issue is solved since a GPIO controller could
now define its range in GPIO number space (which becomes public)
rather than kernel-internal pin number space. At least for our
products, GPIO numbering generally doesn't change between different
variants of the same chip and the /sys/class/gpio customer
documentation would apply to device also.
- The custom logic inside many pinctrl drivers would be confined in one
translation function the driver provides instead of being spread out
all over the driver.
- Sparse GPIO ranges are easy to implement if required by the
platform/driver.
- Augment the pinctrl-abx500.c driver to show how this simplifies
that driver. (Does not need to be perfect, I'll help out finalizing it
for sure.)
The issue is that such a change is quite fundamental, all pinctrl
drivers would have to be upgraded (not just pinctrl-abx500.c) and struct
pinctrl_gpio_range would have to be removed from the gpio_request_enable
callback and friends in favour of some generic translation mechanism.
I am also afraid that the custom logic in many drivers could only be
rewritten with the help of the respective driver's author.
- Then add DT-specific wrapper using this core feature.
GPIO ranges definitions in DT would probably be no longer be compatible
with previous definitions since the number space would change from pin
numbers to GPIO numbers. I don't think this is acceptable.
This way the problem will be solved for everybody, including
ACPI when they sooner or later come back with the same issue.
Given the issues highlighted above I'm not sure if I understand your
proposal correctly. Although I see the advantages, I wonder if the
migration to such a system is feasible in practise.
Greetings,
Christian
--
Christian Ruppert , christian.rupp...@abilis.com
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
_// | bilis Systems CH-1228 Plan-les-Ouates
--
To unsubscribe from this list:
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 05/10/2013 02:25 AM, Christian Ruppert wrote:
On Wed, May 08, 2013 at 02:01:53PM -0600, Stephen Warren wrote:
On 05/08/2013 10:41 AM, Christian Ruppert wrote:
...
What do you think about the following modification to the pinctrl/GPIO
frameworks instead (not yet a formal patch, more a request for comment
to illustrate what I mean. If you agree, I will clean it up and submit a
proper patch after discussion).
It adds a dt_gpiorange_xlate function to the pinctrl callbacks which
defaults to the conventional behaviour using kernel logical pin numbers.
However, pin controllers which provide more complex mechanisms can
define #gpio-range-cells and provide this callback in order to keep
Linux pin numbering inside the kernel.
Can you provide an example of the DT content, and explain exactly what
this patch does with it; what effect it has on the existing GPIO or
pinctrl code?
The patch does not change the default behaviour of the kernel: In case
no dt_gpiorange_xlate callback is defined for a given driver (e.g. for
pre-existing drivers), the default function simply interprets the first
argument as Linux pin number and the second as pin count, same as now.
New drivers can use the callback to translate device specific pin
references to Linux pin numbers (in the idea of of_xlate in the GPIO
framework or xlate in the irqchip framework).
In the case of TB10x, I was thinking of something in the lines of
iomux: iomux@FF10601c {
#gpio-range-cells = 1; /* one cell used for gpiorange phandle */
compatible = abilis,tb10x-iomux;
reg = 0xFF10601c 0x4;
pctl_gpio_a: pctl-gpio-a { /* define phandle to GPIOA I/O function */
pingrp = gpioa_pins;
};
These aren't phandles at all; they're just plain old nodes and properties.
pctl_gpio_b: pctl-gpio-b { /* define phandle to GPIOB I/O function */
pingrp = gpiob_pins;
};
pctl_uart0: pctl-uart0 { /* define phandle to UART0 I/O function */
pingrp = uart0_pins;
};
};
uart@FF10 {
compatible = snps,dw-apb-uart;
reg = 0xFF10 0x100;
clock-frequency = 1;
interrupts = 25 1;
reg-shift = 2;
reg-io-width = 4;
pinctrl-names = default; /* For non-GPIO modules, request I/O */
pinctrl-0 = pctl_uart0; /* functions normally */
};
gpioa: gpio@FF14 {
compatible = abilis,tb10x-gpio;
reg = 0xFF14 0x1000;
gpio-controller;
#gpio-cells = 2;
gpio-ranges = iomux pctl_gpio_a; /* (*1) */
I guess you had to make the pctl_gpio_a a DT node, because to reference
it by phandle, it has to be a node; a phandle can't point at a property.
};
gpioa: gpio@FF14 {
compatible = abilis,tb10x-gpio;
reg = 0xFF14 0x1000;
gpio-controller;
#gpio-cells = 2;
pinctrl-names = default /* here the GPIO controller requests */
pinctrl-0 = pctl_gpio_b; /* the entire GPIOB port explicitly */
gpio-ranges = iomux pctl_gpio_b; /* (*2) */
};
(*1) TB100 GPIO ranges are defined as a phandle to the I/O function
which provides all pins of a given GPIO port. This function is not
necessarily requested from pinctrl and GPIO ports may overlap with
other functions. The pin controller knows about this and provides
whatever GPIO pin is available after mapping other requested
functions.
(*2) Here, the entire GPIOB port is explicitly requested by the GPIO
module, i.e. all pins of the port are made available as GPIOs.
So I think all you're looking for is a way in DT to represent GPIO
ranges? I don't think that should be by string name, but rather numbers:
(actually, doesn't pinctrl-simple already have this?)
The GPIO driver exposes N GPIOs, probably numbered 0..n-1, but I guess
it can number them in whatever sparse way it wants.
The pinctrl driver exposes N pins, again probably numbered 0..n-1, but I
guess it can number them in whatever sparse way it wants.
You want a way to represent the mapping between the various GPIO HW
modules' internal GPIO IDs and the various pinctrl HW modules' pin IDs.
That mapping should be just by integer GPIO/pin ID, I think. What about
the following:
iomux: iomux@FF10601c {
...
};
gpio@FF14 {
gpio-ranges = GPIO_BASE SIZE iomux IOMUX_PIN_BASE;
};
where:
GPIO_BASE: GPIO ID (within gpioa, not global) for the start of the range
covered by this entry.
SIZE: Number of GPIO IDs (and pinctrl pin IDs) covered by this entry.
iomux: The pin controller related to this range.
IOMUX_PIN_BASE: The pinctrl pin ID (within the iomux pin controller, not
global) for the start of the range covered by this entry.
You could easily have more complex setups, with multiple disjoint ranges:
iomuxa: iomux@1000 {
...
};
iomuxb: iomux@2000 {
...
};
gpio@8000 {
gpio-ranges =
0 16 iomuxa 0,
/* disjoint here */
32 16 iomuxb 0,
48 5
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Fri, May 10, 2013 at 10:25 AM, Christian Ruppert
wrote:
>> > What do you think about the following modification to the pinctrl/GPIO
>> > frameworks instead (not yet a formal patch, more a request for comment
>> > to illustrate what I mean. If you agree, I will clean it up and submit a
>> > proper patch after discussion).
>> >
>> > It adds a dt_gpiorange_xlate function to the pinctrl callbacks which
>> > defaults to the conventional behaviour using kernel logical pin numbers.
>> > However, pin controllers which provide more complex mechanisms can
>> > define #gpio-range-cells and provide this callback in order to keep
>> > Linux pin numbering inside the kernel.
(...)
> The patch does not change the default behaviour of the kernel: In case
> no dt_gpiorange_xlate callback is defined for a given driver (e.g. for
> pre-existing drivers), the default function simply interprets the first
> argument as Linux pin number and the second as pin count, same as now.
> New drivers can use the callback to translate device specific pin
> references to Linux pin numbers (in the idea of of_xlate in the GPIO
> framework or xlate in the irqchip framework).
I like the concept.
However I am totally opposed to the idea of making this something
device tree-exclusive.
Look at for example
drivers/pinctrl/pinctrl-abx500.c:
static u8 abx500_get_mode(struct pinctrl_dev *pctldev, struct gpio_chip *chip,
unsigned gpio)
{
(...)
/* on ABx5xx, there is no GPIO0, so adjust the offset */
unsigned offset = gpio - 1;
As you can see, this driver, which does not use device tree,
is working around the same problem. Here the problem is that
the pins are numbered starting at 1 instead of 0, a very trivial
numberspace shuffleing.
I'd be open to this approach if you:
- Make it generic for all pinctrl drivers, i.e. add the translation
to the core so it does not just apply to devices using device tree.
- Augment the pinctrl-abx500.c driver to show how this simplifies
that driver. (Does not need to be perfect, I'll help out finalizing it
for sure.)
- Then add DT-specific wrapper using this core feature.
This way the problem will be solved for everybody, including
ACPI when they sooner or later come back with the same issue.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Fri, May 10, 2013 at 10:25 AM, Christian Ruppert
christian.rupp...@abilis.com wrote:
What do you think about the following modification to the pinctrl/GPIO
frameworks instead (not yet a formal patch, more a request for comment
to illustrate what I mean. If you agree, I will clean it up and submit a
proper patch after discussion).
It adds a dt_gpiorange_xlate function to the pinctrl callbacks which
defaults to the conventional behaviour using kernel logical pin numbers.
However, pin controllers which provide more complex mechanisms can
define #gpio-range-cells and provide this callback in order to keep
Linux pin numbering inside the kernel.
(...)
The patch does not change the default behaviour of the kernel: In case
no dt_gpiorange_xlate callback is defined for a given driver (e.g. for
pre-existing drivers), the default function simply interprets the first
argument as Linux pin number and the second as pin count, same as now.
New drivers can use the callback to translate device specific pin
references to Linux pin numbers (in the idea of of_xlate in the GPIO
framework or xlate in the irqchip framework).
I like the concept.
However I am totally opposed to the idea of making this something
device tree-exclusive.
Look at for example
drivers/pinctrl/pinctrl-abx500.c:
static u8 abx500_get_mode(struct pinctrl_dev *pctldev, struct gpio_chip *chip,
unsigned gpio)
{
(...)
/* on ABx5xx, there is no GPIO0, so adjust the offset */
unsigned offset = gpio - 1;
As you can see, this driver, which does not use device tree,
is working around the same problem. Here the problem is that
the pins are numbered starting at 1 instead of 0, a very trivial
numberspace shuffleing.
I'd be open to this approach if you:
- Make it generic for all pinctrl drivers, i.e. add the translation
to the core so it does not just apply to devices using device tree.
- Augment the pinctrl-abx500.c driver to show how this simplifies
that driver. (Does not need to be perfect, I'll help out finalizing it
for sure.)
- Then add DT-specific wrapper using this core feature.
This way the problem will be solved for everybody, including
ACPI when they sooner or later come back with the same issue.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Wed, May 08, 2013 at 02:01:53PM -0600, Stephen Warren wrote:
> On 05/08/2013 10:41 AM, Christian Ruppert wrote:
> ...
> > What do you think about the following modification to the pinctrl/GPIO
> > frameworks instead (not yet a formal patch, more a request for comment
> > to illustrate what I mean. If you agree, I will clean it up and submit a
> > proper patch after discussion).
> >
> > It adds a dt_gpiorange_xlate function to the pinctrl callbacks which
> > defaults to the conventional behaviour using kernel logical pin numbers.
> > However, pin controllers which provide more complex mechanisms can
> > define #gpio-range-cells and provide this callback in order to keep
> > Linux pin numbering inside the kernel.
>
> Can you provide an example of the DT content, and explain exactly what
> this patch does with it; what effect it has on the existing GPIO or
> pinctrl code?
The patch does not change the default behaviour of the kernel: In case
no dt_gpiorange_xlate callback is defined for a given driver (e.g. for
pre-existing drivers), the default function simply interprets the first
argument as Linux pin number and the second as pin count, same as now.
New drivers can use the callback to translate device specific pin
references to Linux pin numbers (in the idea of of_xlate in the GPIO
framework or xlate in the irqchip framework).
In the case of TB10x, I was thinking of something in the lines of
iomux: iomux@FF10601c {
#gpio-range-cells = <1>; /* one cell used for gpiorange phandle */
compatible = "abilis,tb10x-iomux";
reg = <0xFF10601c 0x4>;
pctl_gpio_a: pctl-gpio-a { /* define phandle to GPIOA I/O function */
pingrp = "gpioa_pins";
};
pctl_gpio_b: pctl-gpio-b { /* define phandle to GPIOB I/O function */
pingrp = "gpiob_pins";
};
pctl_uart0: pctl-uart0 { /* define phandle to UART0 I/O function */
pingrp = "uart0_pins";
};
};
uart@FF10 {
compatible = "snps,dw-apb-uart";
reg = <0xFF10 0x100>;
clock-frequency = <1>;
interrupts = <25 1>;
reg-shift = <2>;
reg-io-width = <4>;
pinctrl-names = "default"; /* For non-GPIO modules, request I/O */
pinctrl-0 = <_uart0>; /* functions normally */
};
gpioa: gpio@FF14 {
compatible = "abilis,tb10x-gpio";
reg = <0xFF14 0x1000>;
gpio-controller;
#gpio-cells = <2>;
gpio-ranges = < _gpio_a>; /* (*1) */
};
gpioa: gpio@FF14 {
compatible = "abilis,tb10x-gpio";
reg = <0xFF14 0x1000>;
gpio-controller;
#gpio-cells = <2>;
pinctrl-names = "default" /* here the GPIO controller requests */
pinctrl-0 = <_gpio_b>; /* the entire GPIOB port explicitly */
gpio-ranges = < _gpio_b>; /* (*2) */
};
(*1) TB100 GPIO ranges are defined as a phandle to the I/O function
which provides all pins of a given GPIO port. This function is not
necessarily requested from pinctrl and GPIO ports may overlap with
other functions. The pin controller knows about this and provides
whatever GPIO pin is available after mapping other requested
functions.
(*2) Here, the entire GPIOB port is explicitly requested by the GPIO
module, i.e. all pins of the port are made available as GPIOs.
The xlate function in pinctrl-tb10x.c would look something like this:
static int tb10x_dt_gpiorange_xlate(struct pinctrl_dev *pctldev,
struct device_node *np,
u32 *rangespec, int rangespecsize,
u32 *pin_offset, u32 *npins)
{
const char *name;
int ret;
struct tb10x_pinfuncgrp *pfg;
struct device_node *dn;
if (rangespecsize != 1)
return -EINVAL;
dn = of_find_node_by_phandle(rangespec[0]);
if (!dn)
return -EINVAL;
of_node_put(dn);
ret = of_property_read_string(dn, "pingrp", );
if (ret)
return ret;
pfg = tb10x_get_pinfuncgrp(name);
if (IS_ERR(pfg))
return PTR_ERR(pfg);
if (!pfg->isgpio)
return -EINVAL;
*pin_offset = pfg->pins[0];
*npins = pfg->pincnt;
return 0;
}
Other drivers may use physical pin numbers, assign integer identifiers
to each GPIO bank or use some other numbering scheme documented in
Documentation/devicetree/bindings.
Greetings,
Christian
--
Christian Ruppert ,
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
_// | bilis Systems CH-1228 Plan-les-Ouates
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Wed, May 08, 2013 at 02:01:53PM -0600, Stephen Warren wrote:
On 05/08/2013 10:41 AM, Christian Ruppert wrote:
...
What do you think about the following modification to the pinctrl/GPIO
frameworks instead (not yet a formal patch, more a request for comment
to illustrate what I mean. If you agree, I will clean it up and submit a
proper patch after discussion).
It adds a dt_gpiorange_xlate function to the pinctrl callbacks which
defaults to the conventional behaviour using kernel logical pin numbers.
However, pin controllers which provide more complex mechanisms can
define #gpio-range-cells and provide this callback in order to keep
Linux pin numbering inside the kernel.
Can you provide an example of the DT content, and explain exactly what
this patch does with it; what effect it has on the existing GPIO or
pinctrl code?
The patch does not change the default behaviour of the kernel: In case
no dt_gpiorange_xlate callback is defined for a given driver (e.g. for
pre-existing drivers), the default function simply interprets the first
argument as Linux pin number and the second as pin count, same as now.
New drivers can use the callback to translate device specific pin
references to Linux pin numbers (in the idea of of_xlate in the GPIO
framework or xlate in the irqchip framework).
In the case of TB10x, I was thinking of something in the lines of
iomux: iomux@FF10601c {
#gpio-range-cells = 1; /* one cell used for gpiorange phandle */
compatible = abilis,tb10x-iomux;
reg = 0xFF10601c 0x4;
pctl_gpio_a: pctl-gpio-a { /* define phandle to GPIOA I/O function */
pingrp = gpioa_pins;
};
pctl_gpio_b: pctl-gpio-b { /* define phandle to GPIOB I/O function */
pingrp = gpiob_pins;
};
pctl_uart0: pctl-uart0 { /* define phandle to UART0 I/O function */
pingrp = uart0_pins;
};
};
uart@FF10 {
compatible = snps,dw-apb-uart;
reg = 0xFF10 0x100;
clock-frequency = 1;
interrupts = 25 1;
reg-shift = 2;
reg-io-width = 4;
pinctrl-names = default; /* For non-GPIO modules, request I/O */
pinctrl-0 = pctl_uart0; /* functions normally */
};
gpioa: gpio@FF14 {
compatible = abilis,tb10x-gpio;
reg = 0xFF14 0x1000;
gpio-controller;
#gpio-cells = 2;
gpio-ranges = iomux pctl_gpio_a; /* (*1) */
};
gpioa: gpio@FF14 {
compatible = abilis,tb10x-gpio;
reg = 0xFF14 0x1000;
gpio-controller;
#gpio-cells = 2;
pinctrl-names = default /* here the GPIO controller requests */
pinctrl-0 = pctl_gpio_b; /* the entire GPIOB port explicitly */
gpio-ranges = iomux pctl_gpio_b; /* (*2) */
};
(*1) TB100 GPIO ranges are defined as a phandle to the I/O function
which provides all pins of a given GPIO port. This function is not
necessarily requested from pinctrl and GPIO ports may overlap with
other functions. The pin controller knows about this and provides
whatever GPIO pin is available after mapping other requested
functions.
(*2) Here, the entire GPIOB port is explicitly requested by the GPIO
module, i.e. all pins of the port are made available as GPIOs.
The xlate function in pinctrl-tb10x.c would look something like this:
static int tb10x_dt_gpiorange_xlate(struct pinctrl_dev *pctldev,
struct device_node *np,
u32 *rangespec, int rangespecsize,
u32 *pin_offset, u32 *npins)
{
const char *name;
int ret;
struct tb10x_pinfuncgrp *pfg;
struct device_node *dn;
if (rangespecsize != 1)
return -EINVAL;
dn = of_find_node_by_phandle(rangespec[0]);
if (!dn)
return -EINVAL;
of_node_put(dn);
ret = of_property_read_string(dn, pingrp, name);
if (ret)
return ret;
pfg = tb10x_get_pinfuncgrp(name);
if (IS_ERR(pfg))
return PTR_ERR(pfg);
if (!pfg-isgpio)
return -EINVAL;
*pin_offset = pfg-pins[0];
*npins = pfg-pincnt;
return 0;
}
Other drivers may use physical pin numbers, assign integer identifiers
to each GPIO bank or use some other numbering scheme documented in
Documentation/devicetree/bindings.
Greetings,
Christian
--
Christian Ruppert , christian.rupp...@abilis.com
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
_// | bilis Systems CH-1228 Plan-les-Ouates
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 05/08/2013 10:41 AM, Christian Ruppert wrote: ... > What do you think about the following modification to the pinctrl/GPIO > frameworks instead (not yet a formal patch, more a request for comment > to illustrate what I mean. If you agree, I will clean it up and submit a > proper patch after discussion). > > It adds a dt_gpiorange_xlate function to the pinctrl callbacks which > defaults to the conventional behaviour using kernel logical pin numbers. > However, pin controllers which provide more complex mechanisms can > define #gpio-range-cells and provide this callback in order to keep > Linux pin numbering inside the kernel. Can you provide an example of the DT content, and explain exactly what this patch does with it; what effect it has on the existing GPIO or pinctrl code? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Fri, May 03, 2013 at 08:03:27PM +0200, Linus Walleij wrote:
> On Thu, May 2, 2013 at 8:49 PM, Stephen Warren wrote:
> > On 04/29/2013 10:17 AM, Christian Ruppert wrote:
> >>>
> >>> So if this is what you want to achieve, just use the same number
> >>> as in your datasheet in the pin number -> problem solved.
> >>
> >> The problem is that we must support several products which are basically
> >> different packaging options of the same chip (or simplified versions
> >> thereof). Not all of those products will have the same number of pins
> >> and as a consequence, data sheet pin numbering will also be different.
> >> The port names are going to remain the same for all products, however.
> >> Some of the ports are just not going to be physically available in some
> >> or the products. Sorry if that wasn't clear from my previous mail.
> >
> > It sounds like you can use the exact same numbering scheme for all the
> > different packaging options; it's just that some of the pin numbers
> > simply won't exist on some of the packaging options, so while defined by
> > the DT binding, it simply won't make sense to use them?
> >
> > Certainly, Tegra20 has two packaging options, and the pinctrl driver for
> > it has zero knowledge of this. Perhaps we're just lucky though. I guess
> > the Tegra TRM doesn't define any "Pin number" (just "pin name") for the
> > pins, so there's no possibility of the same "number" meaning different
> > things in the two packages, so perhaps we're just getting lucky here.
>
> I am certain it must be possible to come up with something reasonable
> here, especially since this is using the device tree.
>
> In U300 we had something like 4 different packaging types, all with
> different names on the pins, however I could avoid the entire
> issue by using pad numbers instead, i.e. the numbers of the pads/fingers
> on the silicon die inside the chip.
> (Documentation/pinctrl.txt contains hints on this.)
>
> It seems like your situation is similar.
>
> And since you work for Abilis I assume you can access such low-level
> documentation and come up with a numbering scheme based on
> something that does not vary with packaging.
Yes but unluckily the decision which numbers to expose to customers is
not an engineering decision, thus the difficulty with everything but
physical pin numbers.
> And if you can't, and since you're using device tree, come up with
> a per-packainging pin numbering, put it into a special .dtsi file that
> you layer over the core SoC .dtsi file just to get these numbers,
> and then use the apropriate packaging .dtsi in yout eventual
> machine .dts file.
What do you think about the following modification to the pinctrl/GPIO
frameworks instead (not yet a formal patch, more a request for comment
to illustrate what I mean. If you agree, I will clean it up and submit a
proper patch after discussion).
It adds a dt_gpiorange_xlate function to the pinctrl callbacks which
defaults to the conventional behaviour using kernel logical pin numbers.
However, pin controllers which provide more complex mechanisms can
define #gpio-range-cells and provide this callback in order to keep
Linux pin numbering inside the kernel.
One could either use the phandles to pin controller sub nodes as done in
the original gpio-tb10x patch or e.g. define numbered pin groups inside
the pin controller. This is the mechanism used in many other device tree
bindings (interrupt controllers, GPIOs etc) to separate kernel internal
numbering schemes from the device tree.
It would also allow removing the module cross-calling between the tb10x
GPIO and pinctrl drivers by migrating most of the code from
tb10x_prepare_gpio_range to that callback and removing
tb10x_setup_gpio_range alltogether.
Greetings,
Christian
---
drivers/gpio/gpiolib-of.c | 61 +--
drivers/pinctrl/devicetree.c| 31
include/linux/of_gpio.h |1 +
include/linux/pinctrl/pinctrl.h | 18 +++
4 files changed, 102 insertions(+), 9 deletions(-)
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 5150df6..10df33b 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -183,10 +183,57 @@ err0:
EXPORT_SYMBOL(of_mm_gpiochip_add);
#ifdef CONFIG_PINCTRL
+static inline int of_gpiochip_parse_xlate(struct device_node *np, int index,
+ u32 *pin_offset, u32 *npins,
+ struct pinctrl_dev **pctldevret)
+{
+ int ret;
+ struct of_phandle_args pinspec;
+ struct pinctrl_dev *pctldev;
+
+ ret = of_parse_phandle_with_args(np, "gpio-ranges",
+ "#gpio-range-cells", index, );
+ if (ret)
+ return ret;
+
+ pctldev = of_pinctrl_get(pinspec.np);
+ if (!pctldev)
+ return -EINVAL;
+
+ if (pctldevret)
+ *pctldevret = pctldev;
+
+ ret =
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Fri, May 03, 2013 at 08:03:27PM +0200, Linus Walleij wrote:
On Thu, May 2, 2013 at 8:49 PM, Stephen Warren swar...@wwwdotorg.org wrote:
On 04/29/2013 10:17 AM, Christian Ruppert wrote:
So if this is what you want to achieve, just use the same number
as in your datasheet in the pin number - problem solved.
The problem is that we must support several products which are basically
different packaging options of the same chip (or simplified versions
thereof). Not all of those products will have the same number of pins
and as a consequence, data sheet pin numbering will also be different.
The port names are going to remain the same for all products, however.
Some of the ports are just not going to be physically available in some
or the products. Sorry if that wasn't clear from my previous mail.
It sounds like you can use the exact same numbering scheme for all the
different packaging options; it's just that some of the pin numbers
simply won't exist on some of the packaging options, so while defined by
the DT binding, it simply won't make sense to use them?
Certainly, Tegra20 has two packaging options, and the pinctrl driver for
it has zero knowledge of this. Perhaps we're just lucky though. I guess
the Tegra TRM doesn't define any Pin number (just pin name) for the
pins, so there's no possibility of the same number meaning different
things in the two packages, so perhaps we're just getting lucky here.
I am certain it must be possible to come up with something reasonable
here, especially since this is using the device tree.
In U300 we had something like 4 different packaging types, all with
different names on the pins, however I could avoid the entire
issue by using pad numbers instead, i.e. the numbers of the pads/fingers
on the silicon die inside the chip.
(Documentation/pinctrl.txt contains hints on this.)
It seems like your situation is similar.
And since you work for Abilis I assume you can access such low-level
documentation and come up with a numbering scheme based on
something that does not vary with packaging.
Yes but unluckily the decision which numbers to expose to customers is
not an engineering decision, thus the difficulty with everything but
physical pin numbers.
And if you can't, and since you're using device tree, come up with
a per-packainging pin numbering, put it into a special .dtsi file that
you layer over the core SoC .dtsi file just to get these numbers,
and then use the apropriate packaging .dtsi in yout eventual
machine .dts file.
What do you think about the following modification to the pinctrl/GPIO
frameworks instead (not yet a formal patch, more a request for comment
to illustrate what I mean. If you agree, I will clean it up and submit a
proper patch after discussion).
It adds a dt_gpiorange_xlate function to the pinctrl callbacks which
defaults to the conventional behaviour using kernel logical pin numbers.
However, pin controllers which provide more complex mechanisms can
define #gpio-range-cells and provide this callback in order to keep
Linux pin numbering inside the kernel.
One could either use the phandles to pin controller sub nodes as done in
the original gpio-tb10x patch or e.g. define numbered pin groups inside
the pin controller. This is the mechanism used in many other device tree
bindings (interrupt controllers, GPIOs etc) to separate kernel internal
numbering schemes from the device tree.
It would also allow removing the module cross-calling between the tb10x
GPIO and pinctrl drivers by migrating most of the code from
tb10x_prepare_gpio_range to that callback and removing
tb10x_setup_gpio_range alltogether.
Greetings,
Christian
---
drivers/gpio/gpiolib-of.c | 61 +--
drivers/pinctrl/devicetree.c| 31
include/linux/of_gpio.h |1 +
include/linux/pinctrl/pinctrl.h | 18 +++
4 files changed, 102 insertions(+), 9 deletions(-)
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 5150df6..10df33b 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -183,10 +183,57 @@ err0:
EXPORT_SYMBOL(of_mm_gpiochip_add);
#ifdef CONFIG_PINCTRL
+static inline int of_gpiochip_parse_xlate(struct device_node *np, int index,
+ u32 *pin_offset, u32 *npins,
+ struct pinctrl_dev **pctldevret)
+{
+ int ret;
+ struct of_phandle_args pinspec;
+ struct pinctrl_dev *pctldev;
+
+ ret = of_parse_phandle_with_args(np, gpio-ranges,
+ #gpio-range-cells, index, pinspec);
+ if (ret)
+ return ret;
+
+ pctldev = of_pinctrl_get(pinspec.np);
+ if (!pctldev)
+ return -EINVAL;
+
+ if (pctldevret)
+ *pctldevret = pctldev;
+
+ ret = of_pinctrl_gpiorange_xlate(pctldev, pinspec.np,
+
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 05/08/2013 10:41 AM, Christian Ruppert wrote: ... What do you think about the following modification to the pinctrl/GPIO frameworks instead (not yet a formal patch, more a request for comment to illustrate what I mean. If you agree, I will clean it up and submit a proper patch after discussion). It adds a dt_gpiorange_xlate function to the pinctrl callbacks which defaults to the conventional behaviour using kernel logical pin numbers. However, pin controllers which provide more complex mechanisms can define #gpio-range-cells and provide this callback in order to keep Linux pin numbering inside the kernel. Can you provide an example of the DT content, and explain exactly what this patch does with it; what effect it has on the existing GPIO or pinctrl code? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Thu, May 2, 2013 at 8:49 PM, Stephen Warren wrote: > On 04/29/2013 10:17 AM, Christian Ruppert wrote: >>> >>> So if this is what you want to achieve, just use the same number >>> as in your datasheet in the pin number -> problem solved. >> >> The problem is that we must support several products which are basically >> different packaging options of the same chip (or simplified versions >> thereof). Not all of those products will have the same number of pins >> and as a consequence, data sheet pin numbering will also be different. >> The port names are going to remain the same for all products, however. >> Some of the ports are just not going to be physically available in some >> or the products. Sorry if that wasn't clear from my previous mail. > > It sounds like you can use the exact same numbering scheme for all the > different packaging options; it's just that some of the pin numbers > simply won't exist on some of the packaging options, so while defined by > the DT binding, it simply won't make sense to use them? > > Certainly, Tegra20 has two packaging options, and the pinctrl driver for > it has zero knowledge of this. Perhaps we're just lucky though. I guess > the Tegra TRM doesn't define any "Pin number" (just "pin name") for the > pins, so there's no possibility of the same "number" meaning different > things in the two packages, so perhaps we're just getting lucky here. I am certain it must be possible to come up with something reasonable here, especially since this is using the device tree. In U300 we had something like 4 different packaging types, all with different names on the pins, however I could avoid the entire issue by using pad numbers instead, i.e. the numbers of the pads/fingers on the silicon die inside the chip. (Documentation/pinctrl.txt contains hints on this.) It seems like your situation is similar. And since you work for Abilis I assume you can access such low-level documentation and come up with a numbering scheme based on something that does not vary with packaging. And if you can't, and since you're using device tree, come up with a per-packainging pin numbering, put it into a special .dtsi file that you layer over the core SoC .dtsi file just to get these numbers, and then use the apropriate packaging .dtsi in yout eventual machine .dts file. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Thu, May 2, 2013 at 8:49 PM, Stephen Warren swar...@wwwdotorg.org wrote: On 04/29/2013 10:17 AM, Christian Ruppert wrote: So if this is what you want to achieve, just use the same number as in your datasheet in the pin number - problem solved. The problem is that we must support several products which are basically different packaging options of the same chip (or simplified versions thereof). Not all of those products will have the same number of pins and as a consequence, data sheet pin numbering will also be different. The port names are going to remain the same for all products, however. Some of the ports are just not going to be physically available in some or the products. Sorry if that wasn't clear from my previous mail. It sounds like you can use the exact same numbering scheme for all the different packaging options; it's just that some of the pin numbers simply won't exist on some of the packaging options, so while defined by the DT binding, it simply won't make sense to use them? Certainly, Tegra20 has two packaging options, and the pinctrl driver for it has zero knowledge of this. Perhaps we're just lucky though. I guess the Tegra TRM doesn't define any Pin number (just pin name) for the pins, so there's no possibility of the same number meaning different things in the two packages, so perhaps we're just getting lucky here. I am certain it must be possible to come up with something reasonable here, especially since this is using the device tree. In U300 we had something like 4 different packaging types, all with different names on the pins, however I could avoid the entire issue by using pad numbers instead, i.e. the numbers of the pads/fingers on the silicon die inside the chip. (Documentation/pinctrl.txt contains hints on this.) It seems like your situation is similar. And since you work for Abilis I assume you can access such low-level documentation and come up with a numbering scheme based on something that does not vary with packaging. And if you can't, and since you're using device tree, come up with a per-packainging pin numbering, put it into a special .dtsi file that you layer over the core SoC .dtsi file just to get these numbers, and then use the apropriate packaging .dtsi in yout eventual machine .dts file. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 04/18/2013 03:03 AM, Christian Ruppert wrote: > Dear Stephen and Linus, > > I am responding to this message because it touches the core issue I was > wondering about when integrating pinctrl with GPIO. I think all > unrelated comments in your other messages are valid and will be > addressed in the next iteration of both drivers. > > On Wed, Apr 17, 2013 at 12:32:00PM -0600, Stephen Warren wrote: >> On 04/10/2013 09:45 AM, Christian Ruppert wrote: >>> The pinmux driver of the Abilis Systems TB10x platform based on ARC700 CPUs. >>> Used to control the pinmux and is a prerequisite for the GPIO driver. >> >> Linus already did a review of this, but I have a few extra comments: >> >>> diff --git >>> a/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt >>> b/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt ... >> * A list of pins/groups to apply the settings to. > > This is what the pingrp string refers to. We really don't want to expose > Linux internal pin numbers! I wasn't suggesting a particular naming/numbering scheme for the pins, but simply that the document has to state what that naming/numbering scheme is, so that people know how to fill in the DT. If the naming/numbering scheme exactly matches some HW manual, then it should be enough to say e.g. "see section X in Foo manual", to avoid duplicating the information. If not, then the valid values should be listed in the binding document. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 04/29/2013 10:17 AM, Christian Ruppert wrote:
> Hello Linus,
>
> On Fri, Apr 26, 2013 at 09:47:16AM +0200, Linus Walleij wrote:
>> On Thu, Apr 18, 2013 at 11:03 AM, Christian Ruppert
>> wrote:
>>
>>> We would like to avoid the use of Linux pin numbers in the device tree.
>>> Customers are used to physical pin numbers and exposing the logical
>>> Linux-internal numbering scheme through the device tree would generate
>>> quite some confusion. There are two reasons why we cannot align the two:
>>> - Not all products supported by these drivers have the same pin outs.
>>> - GPIO ranges must be consecutive in the Linux pin numbering space
>>> which is generally not the case in physical pin numbering.
>>
>> If you are talking about the pin numbers really, i.e. the number we
>> put into
>>
>> struct pinctrl_pin_desc {
>> unsigned number; <- this
>> const char *name;
>> };
>>
>> Then are you aware that this is a sparse number space?
>>
>> I.e. you can choose whatever number you want. You do not have
>> to offset numbers from zero or anything like that. You just
>> need to make sure the numbers do not overlap (no two pins
>> have the same number).
>>
>> So if this is what you want to achieve, just use the same number
>> as in your datasheet in the pin number -> problem solved.
>
> The problem is that we must support several products which are basically
> different packaging options of the same chip (or simplified versions
> thereof). Not all of those products will have the same number of pins
> and as a consequence, data sheet pin numbering will also be different.
> The port names are going to remain the same for all products, however.
> Some of the ports are just not going to be physically available in some
> or the products. Sorry if that wasn't clear from my previous mail.
It sounds like you can use the exact same numbering scheme for all the
different packaging options; it's just that some of the pin numbers
simply won't exist on some of the packaging options, so while defined by
the DT binding, it simply won't make sense to use them?
Certainly, Tegra20 has two packaging options, and the pinctrl driver for
it has zero knowledge of this. Perhaps we're just lucky though. I guess
the Tegra TRM doesn't define any "Pin number" (just "pin name") for the
pins, so there's no possibility of the same "number" meaning different
things in the two packages, so perhaps we're just getting lucky here.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 04/29/2013 10:17 AM, Christian Ruppert wrote:
Hello Linus,
On Fri, Apr 26, 2013 at 09:47:16AM +0200, Linus Walleij wrote:
On Thu, Apr 18, 2013 at 11:03 AM, Christian Ruppert
christian.rupp...@abilis.com wrote:
We would like to avoid the use of Linux pin numbers in the device tree.
Customers are used to physical pin numbers and exposing the logical
Linux-internal numbering scheme through the device tree would generate
quite some confusion. There are two reasons why we cannot align the two:
- Not all products supported by these drivers have the same pin outs.
- GPIO ranges must be consecutive in the Linux pin numbering space
which is generally not the case in physical pin numbering.
If you are talking about the pin numbers really, i.e. the number we
put into
struct pinctrl_pin_desc {
unsigned number; - this
const char *name;
};
Then are you aware that this is a sparse number space?
I.e. you can choose whatever number you want. You do not have
to offset numbers from zero or anything like that. You just
need to make sure the numbers do not overlap (no two pins
have the same number).
So if this is what you want to achieve, just use the same number
as in your datasheet in the pin number - problem solved.
The problem is that we must support several products which are basically
different packaging options of the same chip (or simplified versions
thereof). Not all of those products will have the same number of pins
and as a consequence, data sheet pin numbering will also be different.
The port names are going to remain the same for all products, however.
Some of the ports are just not going to be physically available in some
or the products. Sorry if that wasn't clear from my previous mail.
It sounds like you can use the exact same numbering scheme for all the
different packaging options; it's just that some of the pin numbers
simply won't exist on some of the packaging options, so while defined by
the DT binding, it simply won't make sense to use them?
Certainly, Tegra20 has two packaging options, and the pinctrl driver for
it has zero knowledge of this. Perhaps we're just lucky though. I guess
the Tegra TRM doesn't define any Pin number (just pin name) for the
pins, so there's no possibility of the same number meaning different
things in the two packages, so perhaps we're just getting lucky here.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 04/18/2013 03:03 AM, Christian Ruppert wrote: Dear Stephen and Linus, I am responding to this message because it touches the core issue I was wondering about when integrating pinctrl with GPIO. I think all unrelated comments in your other messages are valid and will be addressed in the next iteration of both drivers. On Wed, Apr 17, 2013 at 12:32:00PM -0600, Stephen Warren wrote: On 04/10/2013 09:45 AM, Christian Ruppert wrote: The pinmux driver of the Abilis Systems TB10x platform based on ARC700 CPUs. Used to control the pinmux and is a prerequisite for the GPIO driver. Linus already did a review of this, but I have a few extra comments: diff --git a/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt b/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt ... * A list of pins/groups to apply the settings to. This is what the pingrp string refers to. We really don't want to expose Linux internal pin numbers! I wasn't suggesting a particular naming/numbering scheme for the pins, but simply that the document has to state what that naming/numbering scheme is, so that people know how to fill in the DT. If the naming/numbering scheme exactly matches some HW manual, then it should be enough to say e.g. see section X in Foo manual, to avoid duplicating the information. If not, then the valid values should be listed in the binding document. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
Hello Linus,
On Fri, Apr 26, 2013 at 09:47:16AM +0200, Linus Walleij wrote:
> On Thu, Apr 18, 2013 at 11:03 AM, Christian Ruppert
> wrote:
>
> > We would like to avoid the use of Linux pin numbers in the device tree.
> > Customers are used to physical pin numbers and exposing the logical
> > Linux-internal numbering scheme through the device tree would generate
> > quite some confusion. There are two reasons why we cannot align the two:
> > - Not all products supported by these drivers have the same pin outs.
> > - GPIO ranges must be consecutive in the Linux pin numbering space
> > which is generally not the case in physical pin numbering.
>
> If you are talking about the pin numbers really, i.e. the number we
> put into
>
> struct pinctrl_pin_desc {
> unsigned number; <- this
> const char *name;
> };
>
> Then are you aware that this is a sparse number space?
>
> I.e. you can choose whatever number you want. You do not have
> to offset numbers from zero or anything like that. You just
> need to make sure the numbers do not overlap (no two pins
> have the same number).
>
> So if this is what you want to achieve, just use the same number
> as in your datasheet in the pin number -> problem solved.
The problem is that we must support several products which are basically
different packaging options of the same chip (or simplified versions
thereof). Not all of those products will have the same number of pins
and as a consequence, data sheet pin numbering will also be different.
The port names are going to remain the same for all products, however.
Some of the ports are just not going to be physically available in some
or the products. Sorry if that wasn't clear from my previous mail.
> This may lead to some offsetting in your driver etc, and some
> struggle do to that. Inspect drivers/pinctrl/pinctrl-abx500.c
> for example. Here the datasheet offsets the pins from 1 rather
> than from zero, so Patrice had to struggle when cross-mapping
> these numbers, but it can surely be done.
This driver seems to avoid many of our problems by integrating both GPIO
and pinctrl driver in the same module. It would not be very difficult to
integrate our separate TB10x GPiO and pinctrl drivers in the same way:
The cross-calling would disappear and the GPIO-base specification could
also easily be removed from device tree. We would still use the named
pingroups scheme in device tree instead of Linux internal pin numbering,
however.
Is this the way to go? I was a bit reluctant from doing this since I'm
not a big fan of huge mega-drivers. Maybe in this particular case such a
driver may be justified?
>[...]
> > I am well aware of the problem but I haven't found any documentation,
> > core functions or examples which solve this. The most elegant way would
> > be some core functionality allowing to define GPIO ranges by directly
> > querying the pin data base (or to preregister GPIO ranges in the pin
> > controller to which GPIO drivers can then "attach"). Is there something
> > like this I have missed?
>
> The current preferred pattern is to have the GPIO portions register
> the ranges referring to the pin controller pin numbers.
>
> This is because it enables us to only use controller-local
> number spaces in both cases.
Agree. The only place in the TB10x drivers where non-local number space
is required is when preparing the GPIO range. I am wondering if some
generic interface in the framework could help us avoid the driver
cross-calls.
> > As a side note, the same argument does not apply to gpio-base, btw, for
> > two reasons:
> > - Our customer documentation does not talk about a global GPIO
> > numbering scheme. In our products, GPIOs are organised in banks and
> > there is no risk of confusion.
>
> This is the local numbering used when registering ranges from
> the GPIOlib side referred to above.
>
> > - The Linux-internal GPIO numbering is already exposed to customers
> > through the /sys/class/gpio interface.
>
> Yeah this sucks but we have to live with it forever.
>
> > That said, if we solve the cross-calling issue I can still move it to
> > the pin data base in the next patch set since you are right that there
> > is no real reason to make it "user-configurable" through DT.
>
> I don't understand this.
Actually, I am thinking of removing the need of specifying gpio-base in
the device tree and assigning a constant gpio-base to each functional
GPIO pin group in pinctrl-tb10x.c. This would also enforce some
coherence in the GPIO numbering between different product variants which
is probably a very good thing. Again, the issue here is that the pin
data base is in the pinctrl driver and the GPIO range is registered in
the GPIO driver but if I integrate the drivers into the same module with
a shared pin data base this issue will disappear.
> >> Perhaps this "abilis,simple-default" thing is intended to represent some
> >> specific default configuration? If so, I don't
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
Hello Linus,
On Fri, Apr 26, 2013 at 09:47:16AM +0200, Linus Walleij wrote:
On Thu, Apr 18, 2013 at 11:03 AM, Christian Ruppert
christian.rupp...@abilis.com wrote:
We would like to avoid the use of Linux pin numbers in the device tree.
Customers are used to physical pin numbers and exposing the logical
Linux-internal numbering scheme through the device tree would generate
quite some confusion. There are two reasons why we cannot align the two:
- Not all products supported by these drivers have the same pin outs.
- GPIO ranges must be consecutive in the Linux pin numbering space
which is generally not the case in physical pin numbering.
If you are talking about the pin numbers really, i.e. the number we
put into
struct pinctrl_pin_desc {
unsigned number; - this
const char *name;
};
Then are you aware that this is a sparse number space?
I.e. you can choose whatever number you want. You do not have
to offset numbers from zero or anything like that. You just
need to make sure the numbers do not overlap (no two pins
have the same number).
So if this is what you want to achieve, just use the same number
as in your datasheet in the pin number - problem solved.
The problem is that we must support several products which are basically
different packaging options of the same chip (or simplified versions
thereof). Not all of those products will have the same number of pins
and as a consequence, data sheet pin numbering will also be different.
The port names are going to remain the same for all products, however.
Some of the ports are just not going to be physically available in some
or the products. Sorry if that wasn't clear from my previous mail.
This may lead to some offsetting in your driver etc, and some
struggle do to that. Inspect drivers/pinctrl/pinctrl-abx500.c
for example. Here the datasheet offsets the pins from 1 rather
than from zero, so Patrice had to struggle when cross-mapping
these numbers, but it can surely be done.
This driver seems to avoid many of our problems by integrating both GPIO
and pinctrl driver in the same module. It would not be very difficult to
integrate our separate TB10x GPiO and pinctrl drivers in the same way:
The cross-calling would disappear and the GPIO-base specification could
also easily be removed from device tree. We would still use the named
pingroups scheme in device tree instead of Linux internal pin numbering,
however.
Is this the way to go? I was a bit reluctant from doing this since I'm
not a big fan of huge mega-drivers. Maybe in this particular case such a
driver may be justified?
[...]
I am well aware of the problem but I haven't found any documentation,
core functions or examples which solve this. The most elegant way would
be some core functionality allowing to define GPIO ranges by directly
querying the pin data base (or to preregister GPIO ranges in the pin
controller to which GPIO drivers can then attach). Is there something
like this I have missed?
The current preferred pattern is to have the GPIO portions register
the ranges referring to the pin controller pin numbers.
This is because it enables us to only use controller-local
number spaces in both cases.
Agree. The only place in the TB10x drivers where non-local number space
is required is when preparing the GPIO range. I am wondering if some
generic interface in the framework could help us avoid the driver
cross-calls.
As a side note, the same argument does not apply to gpio-base, btw, for
two reasons:
- Our customer documentation does not talk about a global GPIO
numbering scheme. In our products, GPIOs are organised in banks and
there is no risk of confusion.
This is the local numbering used when registering ranges from
the GPIOlib side referred to above.
- The Linux-internal GPIO numbering is already exposed to customers
through the /sys/class/gpio interface.
Yeah this sucks but we have to live with it forever.
That said, if we solve the cross-calling issue I can still move it to
the pin data base in the next patch set since you are right that there
is no real reason to make it user-configurable through DT.
I don't understand this.
Actually, I am thinking of removing the need of specifying gpio-base in
the device tree and assigning a constant gpio-base to each functional
GPIO pin group in pinctrl-tb10x.c. This would also enforce some
coherence in the GPIO numbering between different product variants which
is probably a very good thing. Again, the issue here is that the pin
data base is in the pinctrl driver and the GPIO range is registered in
the GPIO driver but if I integrate the drivers into the same module with
a shared pin data base this issue will disappear.
Perhaps this abilis,simple-default thing is intended to represent some
specific default configuration? If so, I don't think that's the right
way to go. Also, the DT binding should be as
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Thu, Apr 18, 2013 at 11:03 AM, Christian Ruppert
wrote:
> We would like to avoid the use of Linux pin numbers in the device tree.
> Customers are used to physical pin numbers and exposing the logical
> Linux-internal numbering scheme through the device tree would generate
> quite some confusion. There are two reasons why we cannot align the two:
> - Not all products supported by these drivers have the same pin outs.
> - GPIO ranges must be consecutive in the Linux pin numbering space
> which is generally not the case in physical pin numbering.
If you are talking about the pin numbers really, i.e. the number we
put into
struct pinctrl_pin_desc {
unsigned number; <- this
const char *name;
};
Then are you aware that this is a sparse number space?
I.e. you can choose whatever number you want. You do not have
to offset numbers from zero or anything like that. You just
need to make sure the numbers do not overlap (no two pins
have the same number).
So if this is what you want to achieve, just use the same number
as in your datasheet in the pin number -> problem solved.
This may lead to some offsetting in your driver etc, and some
struggle do to that. Inspect drivers/pinctrl/pinctrl-abx500.c
for example. Here the datasheet offsets the pins from 1 rather
than from zero, so Patrice had to struggle when cross-mapping
these numbers, but it can surely be done.
> If I understand Documentation/devicetree/bindings/gpio/gpio.txt
> correctly, the "standard" gpio-ranges definition does use Linux logical
> pin numbers, however.
There is no requirement that pin numbers be different from the
datasheet "logical". It is OK to have a 1-1 mapping.
> This disqualifies gpio-ranges from being used in
> our device tree. I haven't found a clean way around this dilemma, thus
> the current implementation duplicating parts of the core system and
> cross-calling pinctrl from GPIO.
I think I have described a solution to this.
> I am well aware of the problem but I haven't found any documentation,
> core functions or examples which solve this. The most elegant way would
> be some core functionality allowing to define GPIO ranges by directly
> querying the pin data base (or to preregister GPIO ranges in the pin
> controller to which GPIO drivers can then "attach"). Is there something
> like this I have missed?
The current preferred pattern is to have the GPIO portions register
the ranges referring to the pin controller pin numbers.
This is because it enables us to only use controller-local
number spaces in both cases.
> As a side note, the same argument does not apply to gpio-base, btw, for
> two reasons:
> - Our customer documentation does not talk about a global GPIO
> numbering scheme. In our products, GPIOs are organised in banks and
> there is no risk of confusion.
This is the local numbering used when registering ranges from
the GPIOlib side referred to above.
> - The Linux-internal GPIO numbering is already exposed to customers
> through the /sys/class/gpio interface.
Yeah this sucks but we have to live with it forever.
> That said, if we solve the cross-calling issue I can still move it to
> the pin data base in the next patch set since you are right that there
> is no real reason to make it "user-configurable" through DT.
I don't understand this.
(Questions answered I think ...)
>> Perhaps this "abilis,simple-default" thing is intended to represent some
>> specific default configuration? If so, I don't think that's the right
>> way to go. Also, the DT binding should be as complete as possible from
>> the start, rather than planning to define/implement part of it now and
>> then keep adding to it later. This all implies that some extra
>> properties really should be defined here.
>
> The abilis,simple-default thing is simply there because I missed commit
> ab78029ecc347debbd737f06688d788bd9d60c1d and followed
> Documentation/pinctrl.txt a bit too closely. ("So if possible, handle
> the pin control in platform code or some other place where you have
> access to all the affected struct device * pointers.") Currently, our
> platform code iterates through nodes compatible with
> abilis,simple-pinctrl and sets up the pin controller accordingly. I'll
> happily remove this mechanism.
Thanks!
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Thu, Apr 18, 2013 at 11:03 AM, Christian Ruppert
christian.rupp...@abilis.com wrote:
We would like to avoid the use of Linux pin numbers in the device tree.
Customers are used to physical pin numbers and exposing the logical
Linux-internal numbering scheme through the device tree would generate
quite some confusion. There are two reasons why we cannot align the two:
- Not all products supported by these drivers have the same pin outs.
- GPIO ranges must be consecutive in the Linux pin numbering space
which is generally not the case in physical pin numbering.
If you are talking about the pin numbers really, i.e. the number we
put into
struct pinctrl_pin_desc {
unsigned number; - this
const char *name;
};
Then are you aware that this is a sparse number space?
I.e. you can choose whatever number you want. You do not have
to offset numbers from zero or anything like that. You just
need to make sure the numbers do not overlap (no two pins
have the same number).
So if this is what you want to achieve, just use the same number
as in your datasheet in the pin number - problem solved.
This may lead to some offsetting in your driver etc, and some
struggle do to that. Inspect drivers/pinctrl/pinctrl-abx500.c
for example. Here the datasheet offsets the pins from 1 rather
than from zero, so Patrice had to struggle when cross-mapping
these numbers, but it can surely be done.
If I understand Documentation/devicetree/bindings/gpio/gpio.txt
correctly, the standard gpio-ranges definition does use Linux logical
pin numbers, however.
There is no requirement that pin numbers be different from the
datasheet logical. It is OK to have a 1-1 mapping.
This disqualifies gpio-ranges from being used in
our device tree. I haven't found a clean way around this dilemma, thus
the current implementation duplicating parts of the core system and
cross-calling pinctrl from GPIO.
I think I have described a solution to this.
I am well aware of the problem but I haven't found any documentation,
core functions or examples which solve this. The most elegant way would
be some core functionality allowing to define GPIO ranges by directly
querying the pin data base (or to preregister GPIO ranges in the pin
controller to which GPIO drivers can then attach). Is there something
like this I have missed?
The current preferred pattern is to have the GPIO portions register
the ranges referring to the pin controller pin numbers.
This is because it enables us to only use controller-local
number spaces in both cases.
As a side note, the same argument does not apply to gpio-base, btw, for
two reasons:
- Our customer documentation does not talk about a global GPIO
numbering scheme. In our products, GPIOs are organised in banks and
there is no risk of confusion.
This is the local numbering used when registering ranges from
the GPIOlib side referred to above.
- The Linux-internal GPIO numbering is already exposed to customers
through the /sys/class/gpio interface.
Yeah this sucks but we have to live with it forever.
That said, if we solve the cross-calling issue I can still move it to
the pin data base in the next patch set since you are right that there
is no real reason to make it user-configurable through DT.
I don't understand this.
(Questions answered I think ...)
Perhaps this abilis,simple-default thing is intended to represent some
specific default configuration? If so, I don't think that's the right
way to go. Also, the DT binding should be as complete as possible from
the start, rather than planning to define/implement part of it now and
then keep adding to it later. This all implies that some extra
properties really should be defined here.
The abilis,simple-default thing is simply there because I missed commit
ab78029ecc347debbd737f06688d788bd9d60c1d and followed
Documentation/pinctrl.txt a bit too closely. (So if possible, handle
the pin control in platform code or some other place where you have
access to all the affected struct device * pointers.) Currently, our
platform code iterates through nodes compatible with
abilis,simple-pinctrl and sets up the pin controller accordingly. I'll
happily remove this mechanism.
Thanks!
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
Dear Stephen and Linus, I am responding to this message because it touches the core issue I was wondering about when integrating pinctrl with GPIO. I think all unrelated comments in your other messages are valid and will be addressed in the next iteration of both drivers. On Wed, Apr 17, 2013 at 12:32:00PM -0600, Stephen Warren wrote: > On 04/10/2013 09:45 AM, Christian Ruppert wrote: > > The pinmux driver of the Abilis Systems TB10x platform based on ARC700 CPUs. > > Used to control the pinmux and is a prerequisite for the GPIO driver. > > Linus already did a review of this, but I have a few extra comments: > > > diff --git > > a/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt > > b/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt > > > +Abilis Systems TB10x pin controller > > +=== > > + > > +Required properties > > +--- > > + > > +- #address-cells: should be <1>. > > +- #size-cells: should be <1>; > > What are those two used for? They would normally only be required if the > child nodes of the pinctrl node contained "reg" properties. But, they > don't in the examples later in this file. Agree, this seems to be a mistake. The properties will be removed. > > +Ports are defined (and referenced) by sub-nodes of the pin controller. > > Every > > +sub-node defines exactly one port (i.e. a set of pins). Ports are defined > > by > > +their names in the pin controller driver. > > I'm not really sure what the implication here is. Does this mean that > the driver isn't expected to contain tables which define which > pins/groups/functions exist, but rather gets those lists/tables from the > DT? I prefer not to do that, although it is acceptable to write the > binding/driver this way. We have the entire pin data base implemented in the pinctrl driver. This includes if a pin can or cannot be used as GPIO and which range it belongs to. Making this information redundant in both GPIO and pinctrl drivers is a bad idea IMHO. Thus, the pin controller's sub nodes are merely defined to have phandles so that we can use pinctrl core functionality. The reasoning is the following: We would like to avoid the use of Linux pin numbers in the device tree. Customers are used to physical pin numbers and exposing the logical Linux-internal numbering scheme through the device tree would generate quite some confusion. There are two reasons why we cannot align the two: - Not all products supported by these drivers have the same pin outs. - GPIO ranges must be consecutive in the Linux pin numbering space which is generally not the case in physical pin numbering. If I understand Documentation/devicetree/bindings/gpio/gpio.txt correctly, the "standard" gpio-ranges definition does use Linux logical pin numbers, however. This disqualifies gpio-ranges from being used in our device tree. I haven't found a clean way around this dilemma, thus the current implementation duplicating parts of the core system and cross-calling pinctrl from GPIO. I am well aware of the problem but I haven't found any documentation, core functions or examples which solve this. The most elegant way would be some core functionality allowing to define GPIO ranges by directly querying the pin data base (or to preregister GPIO ranges in the pin controller to which GPIO drivers can then "attach"). Is there something like this I have missed? As a side note, the same argument does not apply to gpio-base, btw, for two reasons: - Our customer documentation does not talk about a global GPIO numbering scheme. In our products, GPIOs are organised in banks and there is no risk of confusion. - The Linux-internal GPIO numbering is already exposed to customers through the /sys/class/gpio interface. That said, if we solve the cross-calling issue I can still move it to the pin data base in the next patch set since you are right that there is no real reason to make it "user-configurable" through DT. > So there seems to be something huge missing here; the only property > defined here for the child nodes is "pingrp". In the examples, there's > nothing else used. I don't see how this binding allows the actual > desired mux functions to be specified. All other pinctrl DT bindings > have the child nodes contain something along the lines of: > > * A list of pins/groups to apply the settings to. This is what the pingrp string refers to. We really don't want to expose Linux internal pin numbers! > * A property defining the mux function to select on those pins/groups. The muxing possibilities between the individual pin groups are part of the pin database in the driver. > * Other properties defining configuration for those pins/groups, such as > pull-up/down, drive-type, drive-strength, etc. These are not configurable in the TB10x series of chips. > All that seems missing here. Surely it's required? > > Perhaps this "abilis,simple-default" thing is intended to represent
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
Dear Stephen and Linus, I am responding to this message because it touches the core issue I was wondering about when integrating pinctrl with GPIO. I think all unrelated comments in your other messages are valid and will be addressed in the next iteration of both drivers. On Wed, Apr 17, 2013 at 12:32:00PM -0600, Stephen Warren wrote: On 04/10/2013 09:45 AM, Christian Ruppert wrote: The pinmux driver of the Abilis Systems TB10x platform based on ARC700 CPUs. Used to control the pinmux and is a prerequisite for the GPIO driver. Linus already did a review of this, but I have a few extra comments: diff --git a/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt b/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt +Abilis Systems TB10x pin controller +=== + +Required properties +--- + +- #address-cells: should be 1. +- #size-cells: should be 1; What are those two used for? They would normally only be required if the child nodes of the pinctrl node contained reg properties. But, they don't in the examples later in this file. Agree, this seems to be a mistake. The properties will be removed. +Ports are defined (and referenced) by sub-nodes of the pin controller. Every +sub-node defines exactly one port (i.e. a set of pins). Ports are defined by +their names in the pin controller driver. I'm not really sure what the implication here is. Does this mean that the driver isn't expected to contain tables which define which pins/groups/functions exist, but rather gets those lists/tables from the DT? I prefer not to do that, although it is acceptable to write the binding/driver this way. We have the entire pin data base implemented in the pinctrl driver. This includes if a pin can or cannot be used as GPIO and which range it belongs to. Making this information redundant in both GPIO and pinctrl drivers is a bad idea IMHO. Thus, the pin controller's sub nodes are merely defined to have phandles so that we can use pinctrl core functionality. The reasoning is the following: We would like to avoid the use of Linux pin numbers in the device tree. Customers are used to physical pin numbers and exposing the logical Linux-internal numbering scheme through the device tree would generate quite some confusion. There are two reasons why we cannot align the two: - Not all products supported by these drivers have the same pin outs. - GPIO ranges must be consecutive in the Linux pin numbering space which is generally not the case in physical pin numbering. If I understand Documentation/devicetree/bindings/gpio/gpio.txt correctly, the standard gpio-ranges definition does use Linux logical pin numbers, however. This disqualifies gpio-ranges from being used in our device tree. I haven't found a clean way around this dilemma, thus the current implementation duplicating parts of the core system and cross-calling pinctrl from GPIO. I am well aware of the problem but I haven't found any documentation, core functions or examples which solve this. The most elegant way would be some core functionality allowing to define GPIO ranges by directly querying the pin data base (or to preregister GPIO ranges in the pin controller to which GPIO drivers can then attach). Is there something like this I have missed? As a side note, the same argument does not apply to gpio-base, btw, for two reasons: - Our customer documentation does not talk about a global GPIO numbering scheme. In our products, GPIOs are organised in banks and there is no risk of confusion. - The Linux-internal GPIO numbering is already exposed to customers through the /sys/class/gpio interface. That said, if we solve the cross-calling issue I can still move it to the pin data base in the next patch set since you are right that there is no real reason to make it user-configurable through DT. So there seems to be something huge missing here; the only property defined here for the child nodes is pingrp. In the examples, there's nothing else used. I don't see how this binding allows the actual desired mux functions to be specified. All other pinctrl DT bindings have the child nodes contain something along the lines of: * A list of pins/groups to apply the settings to. This is what the pingrp string refers to. We really don't want to expose Linux internal pin numbers! * A property defining the mux function to select on those pins/groups. The muxing possibilities between the individual pin groups are part of the pin database in the driver. * Other properties defining configuration for those pins/groups, such as pull-up/down, drive-type, drive-strength, etc. These are not configurable in the TB10x series of chips. All that seems missing here. Surely it's required? Perhaps this abilis,simple-default thing is intended to represent some specific default configuration? If so, I don't think that's the right
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 04/10/2013 09:45 AM, Christian Ruppert wrote: > The pinmux driver of the Abilis Systems TB10x platform based on ARC700 CPUs. > Used to control the pinmux and is a prerequisite for the GPIO driver. Linus already did a review of this, but I have a few extra comments: > diff --git a/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt > b/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt > +Abilis Systems TB10x pin controller > +=== > + > +Required properties > +--- > + > +- #address-cells: should be <1>. > +- #size-cells: should be <1>; What are those two used for? They would normally only be required if the child nodes of the pinctrl node contained "reg" properties. But, they don't in the examples later in this file. > +Ports are defined (and referenced) by sub-nodes of the pin controller. Every > +sub-node defines exactly one port (i.e. a set of pins). Ports are defined by > +their names in the pin controller driver. I'm not really sure what the implication here is. Does this mean that the driver isn't expected to contain tables which define which pins/groups/functions exist, but rather gets those lists/tables from the DT? I prefer not to do that, although it is acceptable to write the binding/driver this way. So there seems to be something huge missing here; the only property defined here for the child nodes is "pingrp". In the examples, there's nothing else used. I don't see how this binding allows the actual desired mux functions to be specified. All other pinctrl DT bindings have the child nodes contain something along the lines of: * A list of pins/groups to apply the settings to. * A property defining the mux function to select on those pins/groups. * Other properties defining configuration for those pins/groups, such as pull-up/down, drive-type, drive-strength, etc. All that seems missing here. Surely it's required? Perhaps this "abilis,simple-default" thing is intended to represent some specific default configuration? If so, I don't think that's the right way to go. Also, the DT binding should be as complete as possible from the start, rather than planning to define/implement part of it now and then keep adding to it later. This all implies that some extra properties really should be defined here. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Wed, Apr 10, 2013 at 5:45 PM, Christian Ruppert
wrote:
> The pinmux driver of the Abilis Systems TB10x platform based on ARC700 CPUs.
> Used to control the pinmux and is a prerequisite for the GPIO driver.
>
> Signed-off-by: Christian Ruppert
> Signed-off-by: Pierrick Hascoet
Please include Stephen Warren on cc for checking DT bindings for these
things, I feel a bit uncertain about such things time to time.
(...)
> +++ b/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt
(...)
> +In case a driver requires a port to be mapped, the corresponding node should
> +be compatible with "abilis,simple-pinctrl" and define a pinctrl state named
> +"abilis,simple-default" pointing to the port's respective node inside the pin
> +controller.
Oh um I don't understand this.
> + Drivers managing pin controller states internally can be
> +configured normally as described in the pinctrl-bindings.txt.
I don't understand this either.
Do you mean this is opposed to using hogs or something?
Have your familiarized yourself with commit
ab78029ecc347debbd737f06688d788bd9d60c1d
"drivers/pinctrl: grab default handles from device core"
?
> +The pinmux driver is connected to the TB10x GPIO driver in a way that a GPIO
> +node managing a GPIO port can point to the respective pinmux subnode using
> +the gpio-pins property
NAK - this sounds like you're reinventing the GPIO ranges.
See:
Documentation/devicetree/bindings/gpio/gpio.txt, section
2.1) gpio-controller and pinctrl subsystem
> +Example
> +---
> +
> +iomux: iomux@FF10601c {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "abilis,tb10x-iomux";
> + reg = <0xFF10601c 0x4>;
> + pctl_gpio_a: pctl-gpio-a {
> + pingrp = "gpioa_pins";
> + };
> + pctl_uart0: pctl-uart0 {
> + pingrp = "uart0_pins";
> + };
> +};
> +uart@FF10 {
> + compatible = "snps,dw-apb-uart",
> + "abilis,simple-pinctrl";
Why do you need this compatible property,
abilis,simple-pinctrl? And what is simple about it?
> + reg = <0xFF10 0x100>;
> + clock-frequency = <1>;
> + interrupts = <25 1>;
> + reg-shift = <2>;
> + reg-io-width = <4>;
> + pinctrl-names = "abilis,simple-default"; /* */
> + pinctrl-0 = <_uart0>;/* */
I suggest you just put the name of the states into pinctrl-names
pinctrl-names = "default";
> +gpioa: gpio@FF14 {
> + compatible = "abilis,tb10x-gpio";
> + reg = <0xFF14 0x1000>;
> + gpio-controller;
> + #gpio-cells = <1>;
> + gpio-count = <3>;
> + gpio-base = <0>;
> + gpio-pins = <_gpio_a>; /* */
And why doesn't this need a name?
(...)
> diff --git a/drivers/pinctrl/pinctrl-tb10x.c b/drivers/pinctrl/pinctrl-tb10x.c
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
No, put that into
> +static const struct pinctrl_pin_desc tb10x_pins[] = {
> + /* Port 1 */
> + PINCTRL_PIN(0 + 0, "MICLK_S0"),
> + PINCTRL_PIN(0 + 1, "MISTRT_S0"),
> + PINCTRL_PIN(0 + 2, "MIVAL_S0"),
> + PINCTRL_PIN(0 + 3, "MDI_S0"),
> + PINCTRL_PIN(0 + 4, "GPIOA0"),
> + PINCTRL_PIN(0 + 5, "GPIOA1"),
> + PINCTRL_PIN(0 + 6, "GPIOA2"),
> + PINCTRL_PIN(0 + 7, "MDI_S1"),
> + PINCTRL_PIN(0 + 8, "MIVAL_S1"),
> + PINCTRL_PIN(0 + 9, "MISTRT_S1"),
> + PINCTRL_PIN(0 + 10, "MICLK_S1"),
If you're using an offset like that in every such macro,
use a #define for the offset, like
#define TB10X_PORT0 0
#define TB10X_PORT1 16
Then
PINCTRL_PIN(TB10X_PORT0 + 0, "MICLK_S0"),
start to mean something when reading it.
(...)
> +/* Port 1 */
> +static const unsigned mis0_pins[] = { 0, 1, 2, 3};
> +static const unsigned gpioa_pins[] = { 4, 5, 6};
> +static const unsigned mis1_pins[] = { 7, 8, 9, 10};
> +static const unsigned mip1_pins[] = { 0, 1, 2, 3, 4, 5,
> + 6, 7, 8, 9, 10};
Likewise you should use the same #defines here I guess.
static const unsigned mis0_pins[] = { TB10X_PORT0 + 0, TB10X_PORT0 + 1,
TB10X_PORT0 +
2, TB10X_PORT0 + 3};
An alternative is to just drop the offset everywhere above, but it needs
to be consistent. Do it one way: either offsets everywhere or just
plain numbers everywhere, not a mixture like now.
(...)
> +struct tb10x_pinctrl_state {
Usually I just call such structs "tb10x", it's quite clear
from the code that it stores the state.
> + struct pinctrl_dev *pctl;
> + void *iobase;
Just "base" is more common.
> + const struct tb10x_pinfuncgrp *pingroups;
> + unsigned int pinfuncgrpcnt;
> + struct tb10x_of_pinfunc *pinfuncs;
> + unsigned int pinfuncnt;
> + struct
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On Wed, Apr 10, 2013 at 5:45 PM, Christian Ruppert
christian.rupp...@abilis.com wrote:
The pinmux driver of the Abilis Systems TB10x platform based on ARC700 CPUs.
Used to control the pinmux and is a prerequisite for the GPIO driver.
Signed-off-by: Christian Ruppert christian.rupp...@abilis.com
Signed-off-by: Pierrick Hascoet pierrick.hasc...@abilis.com
Please include Stephen Warren on cc for checking DT bindings for these
things, I feel a bit uncertain about such things time to time.
(...)
+++ b/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt
(...)
+In case a driver requires a port to be mapped, the corresponding node should
+be compatible with abilis,simple-pinctrl and define a pinctrl state named
+abilis,simple-default pointing to the port's respective node inside the pin
+controller.
Oh um I don't understand this.
+ Drivers managing pin controller states internally can be
+configured normally as described in the pinctrl-bindings.txt.
I don't understand this either.
Do you mean this is opposed to using hogs or something?
Have your familiarized yourself with commit
ab78029ecc347debbd737f06688d788bd9d60c1d
drivers/pinctrl: grab default handles from device core
?
+The pinmux driver is connected to the TB10x GPIO driver in a way that a GPIO
+node managing a GPIO port can point to the respective pinmux subnode using
+the gpio-pins property
NAK - this sounds like you're reinventing the GPIO ranges.
See:
Documentation/devicetree/bindings/gpio/gpio.txt, section
2.1) gpio-controller and pinctrl subsystem
+Example
+---
+
+iomux: iomux@FF10601c {
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = abilis,tb10x-iomux;
+ reg = 0xFF10601c 0x4;
+ pctl_gpio_a: pctl-gpio-a {
+ pingrp = gpioa_pins;
+ };
+ pctl_uart0: pctl-uart0 {
+ pingrp = uart0_pins;
+ };
+};
+uart@FF10 {
+ compatible = snps,dw-apb-uart,
+ abilis,simple-pinctrl;
Why do you need this compatible property,
abilis,simple-pinctrl? And what is simple about it?
+ reg = 0xFF10 0x100;
+ clock-frequency = 1;
+ interrupts = 25 1;
+ reg-shift = 2;
+ reg-io-width = 4;
+ pinctrl-names = abilis,simple-default; /* */
+ pinctrl-0 = pctl_uart0;/* */
I suggest you just put the name of the states into pinctrl-names
pinctrl-names = default;
+gpioa: gpio@FF14 {
+ compatible = abilis,tb10x-gpio;
+ reg = 0xFF14 0x1000;
+ gpio-controller;
+ #gpio-cells = 1;
+ gpio-count = 3;
+ gpio-base = 0;
+ gpio-pins = pctl_gpio_a; /* */
And why doesn't this need a name?
(...)
diff --git a/drivers/pinctrl/pinctrl-tb10x.c b/drivers/pinctrl/pinctrl-tb10x.c
+#include linux/stringify.h
+#include linux/pinctrl/pinctrl.h
+#include linux/pinctrl/pinmux.h
+#include linux/pinctrl/machine.h
+#include linux/platform_device.h
+#include linux/module.h
+#include linux/mutex.h
+#include linux/err.h
+#include linux/io.h
+#include linux/of.h
+#include linux/slab.h
+#include linux/pinctrl/pinctrl-tb10x.h
No, put that into linux/platform_data/pinctrl-tb10x.h
+static const struct pinctrl_pin_desc tb10x_pins[] = {
+ /* Port 1 */
+ PINCTRL_PIN(0 + 0, MICLK_S0),
+ PINCTRL_PIN(0 + 1, MISTRT_S0),
+ PINCTRL_PIN(0 + 2, MIVAL_S0),
+ PINCTRL_PIN(0 + 3, MDI_S0),
+ PINCTRL_PIN(0 + 4, GPIOA0),
+ PINCTRL_PIN(0 + 5, GPIOA1),
+ PINCTRL_PIN(0 + 6, GPIOA2),
+ PINCTRL_PIN(0 + 7, MDI_S1),
+ PINCTRL_PIN(0 + 8, MIVAL_S1),
+ PINCTRL_PIN(0 + 9, MISTRT_S1),
+ PINCTRL_PIN(0 + 10, MICLK_S1),
If you're using an offset like that in every such macro,
use a #define for the offset, like
#define TB10X_PORT0 0
#define TB10X_PORT1 16
Then
PINCTRL_PIN(TB10X_PORT0 + 0, MICLK_S0),
start to mean something when reading it.
(...)
+/* Port 1 */
+static const unsigned mis0_pins[] = { 0, 1, 2, 3};
+static const unsigned gpioa_pins[] = { 4, 5, 6};
+static const unsigned mis1_pins[] = { 7, 8, 9, 10};
+static const unsigned mip1_pins[] = { 0, 1, 2, 3, 4, 5,
+ 6, 7, 8, 9, 10};
Likewise you should use the same #defines here I guess.
static const unsigned mis0_pins[] = { TB10X_PORT0 + 0, TB10X_PORT0 + 1,
TB10X_PORT0 +
2, TB10X_PORT0 + 3};
An alternative is to just drop the offset everywhere above, but it needs
to be consistent. Do it one way: either offsets everywhere or just
plain numbers everywhere, not a mixture like now.
(...)
+struct tb10x_pinctrl_state {
Usually I just call such structs tb10x, it's quite clear
from the code that it stores the state.
+ struct pinctrl_dev *pctl;
+ void *iobase;
Just base is more common.
+ const struct tb10x_pinfuncgrp
Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver
On 04/10/2013 09:45 AM, Christian Ruppert wrote: The pinmux driver of the Abilis Systems TB10x platform based on ARC700 CPUs. Used to control the pinmux and is a prerequisite for the GPIO driver. Linus already did a review of this, but I have a few extra comments: diff --git a/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt b/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt +Abilis Systems TB10x pin controller +=== + +Required properties +--- + +- #address-cells: should be 1. +- #size-cells: should be 1; What are those two used for? They would normally only be required if the child nodes of the pinctrl node contained reg properties. But, they don't in the examples later in this file. +Ports are defined (and referenced) by sub-nodes of the pin controller. Every +sub-node defines exactly one port (i.e. a set of pins). Ports are defined by +their names in the pin controller driver. I'm not really sure what the implication here is. Does this mean that the driver isn't expected to contain tables which define which pins/groups/functions exist, but rather gets those lists/tables from the DT? I prefer not to do that, although it is acceptable to write the binding/driver this way. So there seems to be something huge missing here; the only property defined here for the child nodes is pingrp. In the examples, there's nothing else used. I don't see how this binding allows the actual desired mux functions to be specified. All other pinctrl DT bindings have the child nodes contain something along the lines of: * A list of pins/groups to apply the settings to. * A property defining the mux function to select on those pins/groups. * Other properties defining configuration for those pins/groups, such as pull-up/down, drive-type, drive-strength, etc. All that seems missing here. Surely it's required? Perhaps this abilis,simple-default thing is intended to represent some specific default configuration? If so, I don't think that's the right way to go. Also, the DT binding should be as complete as possible from the start, rather than planning to define/implement part of it now and then keep adding to it later. This all implies that some extra properties really should be defined here. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] pinmux: Add TB10x pinmux driver
The pinmux driver of the Abilis Systems TB10x platform based on ARC700 CPUs.
Used to control the pinmux and is a prerequisite for the GPIO driver.
Signed-off-by: Christian Ruppert
Signed-off-by: Pierrick Hascoet
---
.../bindings/pinctrl/abilis,tb10x-iomux.txt| 92 +++
drivers/pinctrl/Kconfig|4 +
drivers/pinctrl/Makefile |1 +
drivers/pinctrl/pinctrl-tb10x.c| 794
include/linux/pinctrl/pinctrl-tb10x.h | 46 ++
5 files changed, 937 insertions(+), 0 deletions(-)
create mode 100644
Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt
create mode 100644 drivers/pinctrl/pinctrl-tb10x.c
create mode 100644 include/linux/pinctrl/pinctrl-tb10x.h
diff --git a/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt
b/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt
new file mode 100644
index 000..9979169
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt
@@ -0,0 +1,92 @@
+Abilis Systems TB10x pin controller
+===
+
+Required properties
+---
+
+- #address-cells: should be <1>.
+- #size-cells: should be <1>;
+- compatible: should be "abilis,tb10x-iomux";
+- reg: should contain the physical address and size of the pin controller's
+ register range.
+
+
+Port definitions
+
+
+Ports are defined (and referenced) by sub-nodes of the pin controller. Every
+sub-node defines exactly one port (i.e. a set of pins). Ports are defined by
+their names in the pin controller driver.
+
+Required port definition subnode properties:
+ - pingrp: should be set to the name of the port's pin group.
+
+The following pin groups are available:
+ - GPIO ports: gpioa_pins, gpiob_pins, gpioc_pins, gpiod_pins, gpioe_pins,
+gpiof_pins, gpiog_pins, gpioh_pins, gpioi_pins, gpioj_pins,
+gpiok_pins, gpiol_pins, gpiom_pins, gpion_pins
+ - Serial TS input ports: mis0_pins, mis1_pins, mis2_pins, mis3_pins,
+ mis4_pins, mis5_pins, mis6_pins, mis7_pins
+ - Parallel TS input ports: mip1_pins, mip3_pins, mip5_pins, mip7_pins
+ - Serial TS output ports: mos0_pins, mos1_pins, mos2_pins, mos3_pins
+ - Parallel TS output port: mop_pins
+ - CI+ port: ciplus_pins
+ - CableCard (Mcard) port: mcard_pins
+ - Smart card ports: stc0_pins, stc1_pins
+ - UART ports: uart0_pins, uart1_pins
+ - SPI ports: spi1_pins, spi3_pins
+ - JTAG: jtag_pins
+
+All other ports of the chip are not multiplexed and thus not managed by this
+driver.
+
+
+Pinmux driver control
+-
+
+In case a driver requires a port to be mapped, the corresponding node should
+be compatible with "abilis,simple-pinctrl" and define a pinctrl state named
+"abilis,simple-default" pointing to the port's respective node inside the pin
+controller. Drivers managing pin controller states internally can be
+configured normally as described in the pinctrl-bindings.txt.
+
+The pinmux driver is connected to the TB10x GPIO driver in a way that a GPIO
+node managing a GPIO port can point to the respective pinmux subnode using
+the gpio-pins property
+
+
+Example
+---
+
+iomux: iomux@FF10601c {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "abilis,tb10x-iomux";
+ reg = <0xFF10601c 0x4>;
+ pctl_gpio_a: pctl-gpio-a {
+ pingrp = "gpioa_pins";
+ };
+ pctl_uart0: pctl-uart0 {
+ pingrp = "uart0_pins";
+ };
+};
+uart@FF10 {
+ compatible = "snps,dw-apb-uart",
+ "abilis,simple-pinctrl";
+ reg = <0xFF10 0x100>;
+ clock-frequency = <1>;
+ interrupts = <25 1>;
+ reg-shift = <2>;
+ reg-io-width = <4>;
+ pinctrl-names = "abilis,simple-default"; /* */
+ pinctrl-0 = <_uart0>;/* */
+};
+gpioa: gpio@FF14 {
+ compatible = "abilis,tb10x-gpio";
+ reg = <0xFF14 0x1000>;
+ gpio-controller;
+ #gpio-cells = <1>;
+ gpio-count = <3>;
+ gpio-base = <0>;
+ gpio-pins = <_gpio_a>; /* */
+};
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 34f51d2..b5b6682 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -235,6 +235,10 @@ config PINCTRL_XWAY
depends on SOC_TYPE_XWAY
depends on PINCTRL_LANTIQ
+config PINCTRL_TB10X
+ bool
+ depends on ARC_PLAT_TB10X
+
endmenu
endif
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index f82cc5b..b68d614 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_PINCTRL_EXYNOS) += pinctrl-exynos.o
obj-$(CONFIG_PINCTRL_EXYNOS5440) += pinctrl-exynos5440.o
obj-$(CONFIG_PINCTRL_XWAY) += pinctrl-xway.o
obj-$(CONFIG_PINCTRL_LANTIQ) +=
[PATCH 1/2] pinmux: Add TB10x pinmux driver
The pinmux driver of the Abilis Systems TB10x platform based on ARC700 CPUs.
Used to control the pinmux and is a prerequisite for the GPIO driver.
Signed-off-by: Christian Ruppert christian.rupp...@abilis.com
Signed-off-by: Pierrick Hascoet pierrick.hasc...@abilis.com
---
.../bindings/pinctrl/abilis,tb10x-iomux.txt| 92 +++
drivers/pinctrl/Kconfig|4 +
drivers/pinctrl/Makefile |1 +
drivers/pinctrl/pinctrl-tb10x.c| 794
include/linux/pinctrl/pinctrl-tb10x.h | 46 ++
5 files changed, 937 insertions(+), 0 deletions(-)
create mode 100644
Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt
create mode 100644 drivers/pinctrl/pinctrl-tb10x.c
create mode 100644 include/linux/pinctrl/pinctrl-tb10x.h
diff --git a/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt
b/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt
new file mode 100644
index 000..9979169
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt
@@ -0,0 +1,92 @@
+Abilis Systems TB10x pin controller
+===
+
+Required properties
+---
+
+- #address-cells: should be 1.
+- #size-cells: should be 1;
+- compatible: should be abilis,tb10x-iomux;
+- reg: should contain the physical address and size of the pin controller's
+ register range.
+
+
+Port definitions
+
+
+Ports are defined (and referenced) by sub-nodes of the pin controller. Every
+sub-node defines exactly one port (i.e. a set of pins). Ports are defined by
+their names in the pin controller driver.
+
+Required port definition subnode properties:
+ - pingrp: should be set to the name of the port's pin group.
+
+The following pin groups are available:
+ - GPIO ports: gpioa_pins, gpiob_pins, gpioc_pins, gpiod_pins, gpioe_pins,
+gpiof_pins, gpiog_pins, gpioh_pins, gpioi_pins, gpioj_pins,
+gpiok_pins, gpiol_pins, gpiom_pins, gpion_pins
+ - Serial TS input ports: mis0_pins, mis1_pins, mis2_pins, mis3_pins,
+ mis4_pins, mis5_pins, mis6_pins, mis7_pins
+ - Parallel TS input ports: mip1_pins, mip3_pins, mip5_pins, mip7_pins
+ - Serial TS output ports: mos0_pins, mos1_pins, mos2_pins, mos3_pins
+ - Parallel TS output port: mop_pins
+ - CI+ port: ciplus_pins
+ - CableCard (Mcard) port: mcard_pins
+ - Smart card ports: stc0_pins, stc1_pins
+ - UART ports: uart0_pins, uart1_pins
+ - SPI ports: spi1_pins, spi3_pins
+ - JTAG: jtag_pins
+
+All other ports of the chip are not multiplexed and thus not managed by this
+driver.
+
+
+Pinmux driver control
+-
+
+In case a driver requires a port to be mapped, the corresponding node should
+be compatible with abilis,simple-pinctrl and define a pinctrl state named
+abilis,simple-default pointing to the port's respective node inside the pin
+controller. Drivers managing pin controller states internally can be
+configured normally as described in the pinctrl-bindings.txt.
+
+The pinmux driver is connected to the TB10x GPIO driver in a way that a GPIO
+node managing a GPIO port can point to the respective pinmux subnode using
+the gpio-pins property
+
+
+Example
+---
+
+iomux: iomux@FF10601c {
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = abilis,tb10x-iomux;
+ reg = 0xFF10601c 0x4;
+ pctl_gpio_a: pctl-gpio-a {
+ pingrp = gpioa_pins;
+ };
+ pctl_uart0: pctl-uart0 {
+ pingrp = uart0_pins;
+ };
+};
+uart@FF10 {
+ compatible = snps,dw-apb-uart,
+ abilis,simple-pinctrl;
+ reg = 0xFF10 0x100;
+ clock-frequency = 1;
+ interrupts = 25 1;
+ reg-shift = 2;
+ reg-io-width = 4;
+ pinctrl-names = abilis,simple-default; /* */
+ pinctrl-0 = pctl_uart0;/* */
+};
+gpioa: gpio@FF14 {
+ compatible = abilis,tb10x-gpio;
+ reg = 0xFF14 0x1000;
+ gpio-controller;
+ #gpio-cells = 1;
+ gpio-count = 3;
+ gpio-base = 0;
+ gpio-pins = pctl_gpio_a; /* */
+};
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 34f51d2..b5b6682 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -235,6 +235,10 @@ config PINCTRL_XWAY
depends on SOC_TYPE_XWAY
depends on PINCTRL_LANTIQ
+config PINCTRL_TB10X
+ bool
+ depends on ARC_PLAT_TB10X
+
endmenu
endif
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index f82cc5b..b68d614 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_PINCTRL_EXYNOS) += pinctrl-exynos.o
obj-$(CONFIG_PINCTRL_EXYNOS5440) += pinctrl-exynos5440.o
obj-$(CONFIG_PINCTRL_XWAY) += pinctrl-xway.o
obj-$(CONFIG_PINCTRL_LANTIQ) += pinctrl-lantiq.o
