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@xxxxxxxxx> > > 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 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@xxxxxxxxxxxxxxxx > https://lists.ozlabs.org/listinfo/devicetree-discuss -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html