Hi Heiko, ? 2017/5/23 0:29, Heiko Stuebner ??: > Hi David, > > [I've dropped Linus Walleij, so we don't spam him with our discussions :-) . > I've kept the list though, so people can read up on it if necessary ] > > Am Montag, 22. Mai 2017, 21:58:00 CEST schrieb David.Wu: >> ? 2017/5/19 17:13, Heiko Stuebner ??: >>>> If a pin-routing contains a few pins as gmac, need all the pins (pin, >>>> pinfunc) to be configured correctly from DT, then the corresponding GRF >>>> setting is configured. >>>> If configured grf-setting per pin, assuming that one of the pin's >>>> pinfunc is wrong, it may cause pin-routing not to be effective or wrong. >>> >>> I still think it should be enough to watch for one pin of a specifc group >>> to be set to its special pinmux function. If the pinmux setting is wrong >>> for that pin the ip block won't work correctly anyway (independent of >>> its inner-soc routing). >>> >>> Our pinctrl groups also are defined on a global per-soc level >>> (rk3399.dtsi etc) and thus if they are really wrong, a lot of people will >>> complain anyway ;-) . >>> >>> So for example just take >>> uart2dbga_sin, uart2dbgb_sin, uart2dbgc_sin on rk3399 >>> as pins to watch, some thing like >>> >>> struct rk3399_pinrouting[] = { >>> { >>> /* uart2dbga */ >>> .bank = 4, >>> .pin = 8, >>> .func = 2, >>> .route_offset = 0x0e21c, >>> .route_val = (3 << (10+16)), >>> }, { >>> /* uart2dbgb */ >>> .bank = 4, >>> .pin = 16, >>> .func = 2, >>> .route_offset = 0x0e21c, >>> .route_val = (3 << (10+16) | 1 << 10), >>> }, >>> ... >>> }; >>> >>> That way you can just hook it into the rockchip_set_mux function similar >>> to the recalc stuff, without to much trouble. >>> >> The gmac_m1_sel is different, it need to configure both bit[10] and >> bit[2] with value:1, used after optimization. The gmac_m1 after >> optimization need to configure all pins iomux of gmac_m1 and tx pins >> iomux of gmac_m0. The tx pins iomux of gmac_m0 is also required,so it is >> conflict of gmac_m1 after optimization. Besides, we don't need to use >> m1, because m1 's signal is bad, don't think about it, but >> gmac_m1 after optimization is obligatory. > > I'm not sure I understand the paragraph above. Especially what is the > "optimization" thing. > > But if I understand you correctly, that still leaves us with only 2 settings. > gmac-m0 and gmac-m1-optimized (or whatever we want to call it). > > For gmac-m0 just hook it onto mac_rxd0, which is only used on m0; > just use any of the gmac-m1 pins for that gmac-m1-optimized state; > and as yo wrote just ignore the gmac-m1 state, as it is bad anyway. Yes, Just use the gmac-m0 and gmac-m1-optimized two states. Another aspect is I think per pin needs to traverse, and array element too much, want to improve route match rate. How do you feel about the two points below: 1. add IOMUX_ROUTE type,that only per 8 pins need to traverse. static struct rockchip_pin_bank rk3228_pin_banks[] = { PIN_BANK_IOMUX_FLAGS(0, 32, "gpio0", IOMUX_ROUTE, IOMUX_ROUTE, 0, IOMUX_ROUTE), PIN_BANK_IOMUX_FLAGS(1, 32, "gpio1", IOMUX_ROUTE, IOMUX_ROUTE, IOMUX_ROUTE, 0), PIN_BANK_IOMUX_FLAGS(2, 32, "gpio2", IOMUX_ROUTE, 0, 0, 0), PIN_BANK_IOMUX_FLAGS(3, 32, "gpio3", IOMUX_ROUTE, IOMUX_ROUTE, IOMUX_ROUTE, IOMUX_ROUTE), }; 2. add route mark for per bank, that only if current BIT(pin) & route_mask is ture, need to traverse. static struct rockchip_pin_bank rk3228_pin_banks[] = { PIN_BANK_ROUTE_MASK(0, 32, "gpio0", 0x5c002002), ...... ...... ...... }; static const struct rockchip_mux_route_data rk3228_mux_route_data[] = { { /* pwm0-0 */ .bank = 0, .pin = 26, .func = 1, .route_offset = 0x50, .route_val = BIT(16), }, { /* pwm0-1 */ .bank = 3, .pin = 21, .func = 1, .route_offset = 0x50, .route_val = BIT(16) | BIT(0), },{ /* pwm1-0 */ .bank = 0, .pin = 27, .func = 1, .route_offset = 0x50, .route_val = BIT(16 + 1), }, { /* pwm1-1 */ .bank = 0, .pin = 30, .func = 2, .route_offset = 0x50, .route_val = BIT(16 + 1) | BIT(1), },{ /* pwm2-0 */ .bank = 0, .pin = 28, .func = 1, .route_offset = 0x50, .route_val = BIT(16 + 2), }, { /* pwm2-1 */ .bank = 1, .pin = 12, .func = 2, .route_offset = 0x50, .route_val = BIT(16 + 2) | BIT(2), },{ /* pwm3-0 */ .bank = 3, .pin = 26, .func = 1, .route_offset = 0x50, .route_val = BIT(16 + 3), }, { /* pwm3-1 */ .bank = 1, .pin = 13, .func = 2, .route_offset = 0x50, .route_val = BIT(16 + 3) | BIT(3), },{ /* sdio-0_d0 */ .bank = 1, .pin = 1, .func = 1, .route_offset = 0x50, .route_val = BIT(16 + 4), }, { /* sdio-1_d0 */ .bank = 3, .pin = 2, .func = 1, .route_offset = 0x50, .route_val = BIT(16 + 4) | BIT(4), },{ /* spi-0_rx */ .bank = 0, .pin = 13, .func = 2, .route_offset = 0x50, .route_val = BIT(16 + 5), }, { /* spi-1_rx */ .bank = 0, .pin = 1, .func = 2, .route_offset = 0x50, .route_val = BIT(16 + 5) | BIT(5), },{ /* emmc-0_cmd */ .bank = 1, .pin = 22, .func = 2, .route_offset = 0x50, .route_val = BIT(16 + 7), }, { /* emmc-1_cmd */ .bank = 2, .pin = 4, .func = 2, .route_offset = 0x50, .route_val = BIT(16 + 7) | BIT(7), },{ /* uart2-0_rx */ .bank = 1, .pin = 19, .func = 2, .route_offset = 0x50, .route_val = BIT(16 + 8), }, { /* uart2-1_rx */ .bank = 1, .pin = 9, .func = 2, .route_offset = 0x50, .route_val = BIT(16 + 8) | BIT(8), },{ /* uart1-0_rx */ .bank = 1, .pin = 9, .func = 1, .route_offset = 0x50, .route_val = BIT(16 + 11), }, { /* uart1-1_rx */ .bank = 3, .pin = 13, .func = 1, .route_offset = 0x50, .route_val = BIT(16 + 11) | BIT(11), }, }; > > > Heiko > > >>>> So i think the smallest cell is a group, which contains the pins for the >>>> pin-routing. >>> >>> Though that creates more ABI between kernel and devicetree (pingroup >>> names), which will most likely bite us some time in the future. Also I'd >>> like to keep these dependencies as minimal as possible, as depending on >>> how much more funny inventions we accumulate, we might want to restructure >>> (or split) the driver at some later point - without breaking devicetrees. >>> >>> >>> Heiko >>> >>> >>>> ? 2017/5/18 16:48, Heiko St?bner ??: >>>>> Hi Kever, David, >>>>> >>>>> Am Donnerstag, 18. Mai 2017, 16:21:09 CEST schrieb Kever Yang: >>>>>> Hi Heiko, Linus, >>>>>> Is there any news to handle this kind of IOMUX? >>>>> >>>>> I was talking with David Wu about that a short while ago as well :-) . >>>>> >>>>> From what I've heared, in the future socs may be able to select that >>>>> routing themselfs based on the iomux and having had time to ponder >>>>> the whole issue for some time now, I do guess some sort of list would be >>>>> the least intrusive solution. >>>>> >>>>> It seems we're talking about only a handful of such routings per soc, >>>>> so I guess having the iomux setting check a list >>>>> >>>>> (pin, pinfunc) -> grf-setting for the pin-routing >>>>> >>>>> really is the least intrusive thing to do - as iomux settings really >>>>> aren't changed that often on a running device and adding more >>>>> stuff on top would just complicate everything. >>>>> >>>>> >>>>> Heiko >>>>> >>>>> >>>>>> Thanks, >>>>>> - Kever >>>>>> >>>>>> On 08/05/2016 07:36 AM, Heiko St?bner wrote: >>>>>>> Hi Linus, >>>>>>> >>>>>>> >>>>>>> on the rk3399 we found an interesting new feature and would like to get >>>>>>> some input from the pinctrl expert :-) , as Doug and me currently are of >>>>>>> differing opinions on where specific control elements belong. >>>>>>> >>>>>>> >>>>>>> In a nutshell on the rk3399 some things like one specific uart can use >>>>>>> multiple pins to output data, but control of that seems to be split. The >>>>>>> actual pin config is identical for all pins - each needs to be configured >>>>>>> to function 2, pulls set etc. Then somewhere between the pin io-cells and >>>>>>> the uart it seems to have some sort of switch to decide to which pin to >>>>>>> actually route the data. >>>>>>> >>>>>>> >>>>>>> +-------+ +--------+ /- GPIO4_B1 (pinmux 2) >>>>>>> >>>>>>> | uart2 | -- | switch | --- GPIO4_C1 (pinmux 2) >>>>>>> >>>>>>> +-------+ +--------+ \- GPIO4_C4 (pinmux 2) >>>>>>> (switch selects one of the 3 pins to actually output data to) >>>>>>> >>>>>>> >>>>>>> So the question now is, should the pinctrl driver also flip that switch to >>>>>>> the correct position itself when pin-function 2 of say gpio4_bq gets >>>>>>> selected or is that routing outside of pinctrl's scope? >>>>>>> >>>>>>> ----- >>>>>>> >>>>>>> I hope to have presented the core issue above somewhat neutrally, below >>>>>>> are >>>>>>> my personal worries about doing that in pinctrl :-) . >>>>>>> >>>>>>> >>>>>>> Apart from it feeling "bolted-on" to me, I have two main worries with that >>>>>>> approach: >>>>>>> (1) Right now the unused pins are really unused on the same iomux, so when >>>>>>> flipping the switch it essentially does >>>>>>> >>>>>>> uart-sout unused >>>>>>> >>>>>>> |(iomux2) |(iomux2) >>>>>>> >>>>>>> +----------+ +----------+ >>>>>>> >>>>>>> | gpio4_b0 | | gpio4_c0 | >>>>>>> >>>>>>> +----------+ +----------+ >>>>>>> >>>>>>> going to >>>>>>> >>>>>>> unused uart-sout >>>>>>> >>>>>>> |(iomux2) |(iomux2) >>>>>>> >>>>>>> +----------+ +----------+ >>>>>>> >>>>>>> | gpio4_b0 | | gpio4_c0 | >>>>>>> >>>>>>> +----------+ +----------+ >>>>>>> >>>>>>> but nothing keeps designers from doing >>>>>>> >>>>>>> uart-sout special1 >>>>>>> >>>>>>> |(iomux2) |(iomux2) >>>>>>> >>>>>>> +----------+ +----------+ >>>>>>> >>>>>>> | gpio4_b0 | | gpio4_c0 | >>>>>>> >>>>>>> +----------+ +----------+ >>>>>>> >>>>>>> going to >>>>>>> >>>>>>> special2 uart-sout >>>>>>> >>>>>>> |(iomux2) |(iomux2) >>>>>>> >>>>>>> +----------+ +----------+ >>>>>>> >>>>>>> | gpio4_b0 | | gpio4_c0 | >>>>>>> >>>>>>> +----------+ +----------+ >>>>>>> >>>>>>> somewhere down the road, so relying on following the selected iomux feels >>>>>>> not future proof. >>>>>>> >>>>>>> (2) Looking at [0] we already have a similar case, where we configure the >>>>>>> pins for rgmii but still tell the gmac controller that it is supposed to >>>>>>> do >>>>>>> rgmii instead of rmii. >>>>>>> >>>>>>> Here the pinmux is the same for all pins, rmii just uses less pins when >>>>>>> compared to rgmii, so binding that to the pinmux isn't even possible. >>>>>>> >>>>>>> And doing it one way here and another way for the switch feels very >>>>>>> strange. >>>>>>> >>>>>>> >>>>>>> I hope this overly long mail was not to confusing and hope for some words >>>>>>> of wisdom ;-) >>>>>>> >>>>>>> >>>>>>> Big thanks >>>>>>> Heiko >>>>>>> >>>>>>> >>>>>>> [0] >>>>>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/ >>>>>>> arm/boot/dts/rk3288-miqi.dts#n139 >>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> Linux-rockchip mailing list >>>>>>> Linux-rockchip at lists.infradead.org >>>>>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip >>>>> >>>>> >>>>> >>>>> >>>>> >>>> >>> >>> >>> >>> >>> >> >> >> > > > > >