Pulls and drive strengths in the pinctrl world
Tomasz Figa
tomasz.figa at gmail.com
Sun May 19 00:57:15 EST 2013
On Saturday 18 of May 2013 10:18:47 Jean-Christophe PLAGNIOL-VILLARD
wrote:
> On 14:17 Fri 17 May , Tomasz Figa wrote:
> > Hi Jean-Christophe,
> >
> > On Friday 17 of May 2013 14:26:25 Jean-Christophe PLAGNIOL-VILLARD
wrote:
> > > On 18:22 Wed 15 May , Stephen Warren wrote:
> > > > On 05/15/2013 06:13 PM, Tomasz Figa wrote:
> > > > > On Wednesday 15 of May 2013 16:55:37 Doug Anderson wrote:
> > > > >> Tomasz / Linus,
> > > > >>
> > > > >> On Wed, May 15, 2013 at 3:06 PM, Tomasz Figa
> > > > >> <tomasz.figa at gmail.com>
> > > > >
> > > > > wrote:
> > > > >>> Yes. I don't like the current way too much either, duplication
> > > > >>> being
> > > > >>> one of the reasons.
> > > > >>
> > > > >> Do you have any other ideas? It sounds like Linus didn't like
> > > > >> my
> > > > >> suggestion and makes some good points...
> > > > >
> > > > > I don't have anything interesting at the moment. It's a bit late
> > > > > now
> > > > > here (2 AM), so I'm going to get some sleep first.
> > > > >
> > > > > Also after reading Stephen's reply, I'm wondering if hogging
> > > > > wouldn't
> > > > > solve the problem indeed. (It might have to be fixed on
> > > > > pinctrl-samsung
> > > > > first, as last time I tried to use it, it caused some errors
> > > > > from
> > > > > pinctrl core, but haven't time to track them down, as it wasn't
> > > > > anything important at that time).
> > > >
> > > > One issue I noticed with the DT fragments earlier in this thread.
> > > > It
> > > > looks like hogs in the Samsung pinctrl bingings end up looking
> > > > like:
> > > >
> > > > pinctrl {
> > > >
> > > > pina {
> > > >
> > > > samsung,pins = <PIN_A PIN_B PIN_C>;
> > > > samsung,pin-function = <0xf>;
> > > > samsung,pin-pud = <0>;
> > > > ...
> > >
> > > I have a huge issue here that we had on at91 too
> > >
> > > we are going to have a huge numbet of node
> > >
> > > and on at91 we handle the pin the same way as samsung
> > > and ST have also a similiar IP
> > >
> > > so I'll prefer to reuse the AT91 DT bindings
> > >
> > > as said by Linus I just push a cleanup of the magic by using Macro
> > > which make it really readable now
> > >
> > > some extract of the sama5 pinctrl
> > >
> > > mmc0 {
> > >
> > > pinctrl_mmc0_clk_cmd_dat0: mmc0_clk_cmd_dat0 {
> > >
> > > atmel,pins =
> > >
> > > <AT91_PIOD 9 AT91_PERIPH_A
> >
> > AT91_PINCTRL_NONE /* PD9 periph A MCI0_CK
> >
> > > */ AT91_PIOD 0 AT91_PERIPH_A AT91_PINCTRL_PULL_UP /* PD0 periph A
> > > MCI0_CDA with pullup */ AT91_PIOD 1 AT91_PERIPH_A
> > > AT91_PINCTRL_PULL_UP>; /* PD1 periph A MCI0_DA0 with pullup */ };
> > >
> > > pinctrl_mmc0_dat1_3: mmc0_dat1_3 {
> > >
> > > atmel,pins =
> > >
> > > <AT91_PIOD 2 AT91_PERIPH_A
> >
> > AT91_PINCTRL_PULL_UP /* PD2 periph A
> >
> > > MCI0_DA1 with pullup */ AT91_PIOD 3 AT91_PERIPH_A
> > > AT91_PINCTRL_PULL_UP /* PD3 periph A MCI0_DA2 with pullup */
> > > AT91_PIOD
> > > 4 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>; /* PD4 periph A MCI0_DA3
> >
> > with
> >
> > > pullup */ };
> > >
> > > pinctrl_mmc0_dat4_7: mmc0_dat4_7 {
> > >
> > > atmel,pins =
> > >
> > > <AT91_PIOD 5 AT91_PERIPH_A
> >
> > AT91_PINCTRL_PULL_UP /* PD5 periph A
> >
> > > MCI0_DA4 with pullup, conflicts with TIOA0, PWMH2 */ AT91_PIOD 6
> > > AT91_PERIPH_A AT91_PINCTRL_PULL_UP /* PD6 periph A MCI0_DA5
with
> > > pullup, conflicts with TIOB0, PWML2 */ AT91_PIOD 7 AT91_PERIPH_A
> > > AT91_PINCTRL_PULL_UP /* PD7 periph A MCI0_DA6 with pullup,
conlicts
> > > with TCLK0, PWMH3 */ AT91_PIOD 8 AT91_PERIPH_A
> > > AT91_PINCTRL_PULL_UP>; /* PD8 periph A MCI0_DA7 with pullup,
> >
> > conflicts
> >
> > > with PWML3 */ };
> > >
> > > };
> > >
> > > of sam9g45
> > >
> > > i2c_gpio2 {
> > >
> > > pinctrl_i2c_gpio2: i2c_gpio2-0 {
> > >
> > > atmel,pins =
> > >
> > > <AT91_PIOB 4 AT91_PERIPH_GPIO
> >
> > AT91_PINCTRL_MULTI_DRIVE /* PB4 gpio
> >
> > > multidrive I2C2 data */ AT91_PIOB 5 AT91_PERIPH_GPIO
> > > AT91_PINCTRL_MULTI_DRIVE>; /* PB5 gpio multidrive I2C2 clock
*/ };
> > >
> > > };
> > >
> > > so we could share the c code too
> >
> > I'd have a question with regard to AT91 bindings.
> >
> > Using Samsung bindings we don't need to specify all configuration
> > options for a pin, only those that are relevant for the platform. Do
> > your bindings allow this?
>
> on at91 we have this too we just put NONE, and I'm planning to allow to
> drop the last option too (not yet implement)
>
> > Apparently AT91 has less configurable things and those available are
> > usually always configured together so it's not a problem. But on our
> > SoCs we have a bit more of them:
> > - function (input, output, special functions)
> > - pull-down/-up
> > - driver strength
> > - power down mode function (input, output low, output high, retention)
> > - power down mode pull-down/-up
> > - one could argue that default output value could be set this way as
> > well, by adding samsung,pin-value property.
>
> on Atmel you have
>
> first a pin need to be muxed as a gpio or a function name periph
> depending on the SoC we can have up to 4 function mode + gpio
>
> then each pin have feature that are independent of the mux function
>
> Bits used for CONFIG: (4th parameter)
> PULL_UP (1 << 0): indicate this pin need a pull up.
> MULTIDRIVE (1 << 1): indicate this pin need to be configured as
> multidrive. DEGLITCH (1 << 2): indicate this pin need deglitch.
> PULL_DOWN (1 << 3): indicate this pin need a pull down.
> DIS_SCHMIT (1 << 4): indicate this pin need to disable schmit
> trigger. DEBOUNCE (1 << 16): indicate this pin need debounce.
> DEBOUNCE_VAL (0x3fff << 17): debounce val.
>
> today I was planning to update the binding to allow to this
>
> instead of writing this
>
> dbgu {
> pinctrl_dbgu: dbgu-0 {
> atmel,pins =
> <AT91_PIOB 30 AT91_PERIPH_A AT91_PINCTRL_NONE
> AT91_PIOB 31 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>;
> };
> };
>
> we will write this
>
> dbgu {
> pinctrl_dbgu: dbgu-0 {
> atmel,pins =
> <AT91_PIOB 30 AT91_PERIPH_A>,
> AT91_PIOB 31 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>;
> };
> };
>
> so a pin can have 3 or more parameter
>
> so as a generic binding only the 3 first will be namdatory (bank pinnp
> muxid) the rest will driver specific
>
> for power down I plan to define an other node
> dbgu {
> pinctrl_dbgu_sleep: dbgu_sleep-0 {
> atmel,pins =
> <AT91_PIOB 30 AT91_PERIPH_GPIO>,
> AT91_PIOB 31 AT91_PERIPH_A
AT91_PINCTRL_PULL_DOWN>;
> };
> };
I'm afraid this won't work for Samsung SoCs. In our case normal and power
down settings are completely unrelated, i.e. stored in separate registers
and one doesn't affect another (a system controller automatically switches
between normal and power down settings when entering or leaving low power
modes, like SoC-level suspend).
Personally I'd prefer a solution with separate property for each
parameter, because it's much more flexible and allows shorter lines,
making device tree sources more readable.
Best regards,
Tomasz
>
> Best Regards,
> J.
>
> > Best regards,
> > Tomasz
> >
> > > Best Regards,
> > > J,
> > >
> > > > };
> > > > pinp {
> > > >
> > > > samsung,pins = <PIN_P PIN_Q>;
> > > > samsung,pin-function = <0xe>;
> > > > samsung,pin-pud = <1>;
> > > > ...
> > > >
> > > > };
> > > > pinx {
> > > >
> > > > samsung,pins = <PIN_X PIN_Y PIN_Z>;
> > > > samsung,pin-function = <0xd>;
> > > > samsung,pin-pud = <2>;
> > > > ...
> > > >
> > > > };
> > > >
> > > > pinctrl-names = "default";
> > > > pinctrl-0 = <&pina &pinp &pinx>;
> > > >
> > > > };
> > > >
> > > > That pinctrl-0 property could get rather large (hard to
> > > > write/maintain,
> > > > unwieldy) if it needs to set up lots of different configurations.
> > > > That's why I made the equivalent Tegra bindings be:
> > > >
> > > > pinctrl {
> > > >
> > > > pins_default {
> > > >
> > > > pina {
> > > >
> > > > samsung,pins = <PIN_A PIN_B PIN_C>;
> > > > samsung,pin-function = <0xf>;
> > > > samsung,pin-pud = <0>;
> > > > ...
> > > >
> > > > };
> > > > pinp {
> > > >
> > > > samsung,pins = <PIN_P PIN_Q>;
> > > > samsung,pin-function = <0xe>;
> > > > samsung,pin-pud = <1>;
> > > > ...
> > > >
> > > > };
> > > > pinx {
> > > >
> > > > samsung,pins = <PIN_X PIN_Y PIN_Z>;
> > > > samsung,pin-function = <0xd>;
> > > > samsung,pin-pud = <2>;
> > > > ...
> > > >
> > > > };
> > > >
> > > > };
> > > >
> > > > pinctrl-names = "default";
> > > > pinctrl-0 = <&pins_default>;
> > > >
> > > > };
> > > >
> > > > The extra level within the "pinctrl configuration node"
> > > > ("pins_default"
> > > > here) makes the pinctrl-0 property a lot easier to write, and the
> > > > advantage happens at every use-site that needs to configure
> > > > different
> > > > subsets of the relevant pins in different ways.
> > > >
> > > > If you're changing all the bindings anyway, introducing this extra
> > > > level might be something to think about.
> > > >
> > > > I did try to explain my philosophy here when we all got together
> > > > to
> > > > design the pinctrl bindings, but I obviously didn't explain it
> > > > well
> > > > enough, or people didn't like it anyway.
> > > > _______________________________________________
> > > > devicetree-discuss mailing list
> > > > devicetree-discuss at lists.ozlabs.org
> > > > https://lists.ozlabs.org/listinfo/devicetree-discuss
More information about the devicetree-discuss
mailing list