Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752407Ab3FZLvT (ORCPT ); Wed, 26 Jun 2013 07:51:19 -0400 Received: from mail.abilis.ch ([195.70.19.74]:17880 "EHLO mail.abilis.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752189Ab3FZLvQ convert rfc822-to-8bit (ORCPT ); Wed, 26 Jun 2013 07:51:16 -0400 Date: Wed, 26 Jun 2013 13:50:52 +0200 From: Christian Ruppert To: Stephen Warren 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 Message-ID: <20130626115051.GC7095@ab42.lan> References: <20130618092516.GC18663@ab42.lan> <1371547751-13873-2-git-send-email-christian.ruppert@abilis.com> <51C23222.2060906@wwwdotorg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8BIT In-Reply-To: <51C23222.2060906@wwwdotorg.org> User-Agent: Mutt/1.5.20 (2009-12-10) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3537 Lines: 84 On Wed, Jun 19, 2013 at 04:35:14PM -0600, Stephen Warren wrote: > On 06/18/2013 03:29 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. > > > diff --git a/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt b/Documentation/devicetree/bindings/pinctrl/abilis,tb10x-iomux.txt > > > +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 predefined > > +as named pin groups inside the pin controller driver and these names are used > > +to associate pin group predefinitions to pin controller sub-nodes. > > + > > +Required port definition subnode properties: > > + - pingrp: should be set to the name of the port's pin group. > > This seems odd.... More on that where I comment on the example. > > > +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 > ... > > + - JTAG: jtag_pins > > I'd suggest removing "_pins" from all those names, since it's the same > in all names and hence isn't necessary. > > > +GPIO ranges definition > > +---------------------- > > + > > +The named pin groups of GPIO ports can be used to define GPIO ranges as > > +explained in Documentation/devicetree/bindings/gpio/gpio.txt. > > I wouldn't mention that here; the GPIO node contains the gpio-ranges > property, not the pin controller node. Hence, the binding for the GPIO > DT node should describe the property, not the binding for this node. > > > +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? -- 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 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/