Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965057Ab3GESlA (ORCPT ); Fri, 5 Jul 2013 14:41:00 -0400 Received: from avon.wwwdotorg.org ([70.85.31.133]:53345 "EHLO avon.wwwdotorg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757523Ab3GESk7 (ORCPT ); Fri, 5 Jul 2013 14:40:59 -0400 Message-ID: <51D71337.6030409@wwwdotorg.org> Date: Fri, 05 Jul 2013 12:40:55 -0600 From: Stephen Warren User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 MIME-Version: 1.0 To: Christian Ruppert CC: Linus Walleij , Patrice CHOTARD , "linux-kernel@vger.kernel.org" , Grant Likely , Rob Herring , Rob Landley , Sascha Leuenberger , Pierrick Hascoet , devicetree-discuss@lists.ozlabs.org, linux-doc@vger.kernel.org, Alexandre Courbot Subject: Re: [PATCH 2/4] pinmux: Add TB10x pinmux driver References: <20130618092516.GC18663@ab42.lan> <1371547751-13873-2-git-send-email-christian.ruppert@abilis.com> <51C23222.2060906@wwwdotorg.org> <20130626115051.GC7095@ab42.lan> <51CB279A.1090404@wwwdotorg.org> <20130705094859.GA27196@ab42.lan> In-Reply-To: <20130705094859.GA27196@ab42.lan> X-Enigmail-Version: 1.4.6 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4331 Lines: 112 On 07/05/2013 03:49 AM, Christian Ruppert wrote: > On Wed, Jun 26, 2013 at 11:40:42AM -0600, Stephen Warren wrote: >> On 06/26/2013 05:50 AM, Christian Ruppert wrote: >>> On Wed, Jun 19, 2013 at 04:35:14PM -0600, Stephen Warren wrote: >>>> On 06/18/2013 03:29 AM, Christian Ruppert wrote: >> [...] >>>>> +Example >>>>> +------- >>>>> + >>>>> +iomux: iomux@FF10601c { >>>>> + compatible = "abilis,tb10x-iomux"; >>>>> + reg = <0xFF10601c 0x4>; >>>>> + pctl_gpio_a: pctl-gpio-a { >>>>> + pingrp = "gpioa_pins"; >>>>> + }; >>>>> + pctl_uart0: pctl-uart0 { >>>>> + pingrp = "uart0_pins"; >>>>> + }; >>>>> +}; >>>> >>>> The two nodes pctl-gpio-a and pctl-uart0 seem to be missing data. The >>>> idea here is that you define nodes that says: >>>> >>>> * This node applies to these pin(s)/group(s). >>>> * Select mux function X on those pins/groups and/or apply these pin >>>> configuration options to those pins/groups. >>>> >>>> The examples above don't include any mux/config options, nor does the >>>> binding say how to do specify them. >>>> >>>> The set of pin groups defined by this binding should correspond directly >>>> to the set of pin groups that actually exist in HW. So, if you have 3 >>>> pin groups (A, B, C) in HW each of which has two mux functions (X, Y), >>>> your DT binding should define just 3 pin groups (A, B, C), not 6 (A_X, >>>> A_Y, B_X, B_Y, C_X, C_Y). In other words, the pin group name shouldn't >>>> imply the mux function. >>> >>> Can we consider it as agreed now that this implementation is acceptable >>> for the TB10x pin controller? >> >> There are two issues here: >> >> 1) What is a pin group: >> >> 1a) Must it solely represent a group of pins that actually exists in HW >> (e.g. it's an RTL port, or a set of pins all controlled at once by a >> single bit/field in a register) >> >> 1b) A SW-defined group of pins, simply because it's convenient to talk >> about that set of pins at once, even though HW doesn't impose that those >> pins are in a group in any way. >> >> Defining groups for either of those reasons is fine, although this is >> the area where my preference and LinusW's differ. >> >> 2) Can groups represent just a set of pins, or can it also imply that a >> particular mux function is selected on that group? >> >> I believe that both LinusW and I are in agreement that a group is simply >> a list/set/group of pins. You select mux functions onto groups. A >> groups's definition can't imply that a particular mux function is >> selected onto it. >> >> If we don't follow this rule, then you end up with a combinatorial >> explosion of groups; the cross-product of all possible groups of pins >> vs. the mux function to select on them, rather than simply having a list >> of groups of pins, which is a much smaller set/list. >> >> So, in the DT example above, I still believe that you need an extra >> property that defines which mux function to select onto the specified >> group. The group name can't imply this, so there needs to be some way of >> specifying it. > > In your opinion, would something in the lines of > > pctl_spi1: pctl-spi1 { > abilis,pingrp = "spi1"; So that defines a list of pins. > abilis,ioport = <4>; /* spi1 is routed to port4 inside the > pin controller */ I assume that defines the mux function value; the value that's programmed into the HW register to select which HW module's signals are routed out to the pins specified by abilis,pingrp. > abilis,ioconf = <1>; /* spi1 is available in configuration 1 > of that port. */ But I don't understand what that is. ... ... > In future, this could even be extended to allow several alternative > configurations for a given function, e.g. > > pctl_spi3: pctl-spi3 { > abilis,pingrp = "spi3"; > abilis,ioport = <6>; > abilis,ioconf = <0 3>; /* spi3 is available in both > configurations 0 and 3. Depending on > what other functions are requested, the > pinctrl driver can choose either of the > two. */ ... especially if you're talking about "spi3 being available in multiple configurations". What's a configuration? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/