Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753554Ab2BFDHy (ORCPT ); Sun, 5 Feb 2012 22:07:54 -0500 Received: from mail-vx0-f174.google.com ([209.85.220.174]:53985 "EHLO mail-vx0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753084Ab2BFDHw convert rfc822-to-8bit (ORCPT ); Sun, 5 Feb 2012 22:07:52 -0500 MIME-Version: 1.0 In-Reply-To: <74CDBE0F657A3D45AFBB94109FB122FF178E5D3160@HQMAIL01.nvidia.com> References: <74CDBE0F657A3D45AFBB94109FB122FF178E5D3160@HQMAIL01.nvidia.com> Date: Sun, 5 Feb 2012 19:07:51 -0800 Message-ID: Subject: Re: An extremely simplified pinctrl bindings proposal From: Thomas Abraham To: Stephen Warren Cc: Dong Aisheng , Shawn Guo , Dong Aisheng-B29396 , "Linus Walleij (linus.walleij@linaro.org)" , "Sascha Hauer (s.hauer@pengutronix.de)" , "rob.herring@calxeda.com" , "kernel@pengutronix.de" , "cjb@laptop.org" , "Simon Glass (sjg@chromium.org)" , "Grant Likely (grant.likely@secretlab.ca)" , "ext Tony Lindgren (tony@atomide.com)" , "devicetree-discuss@lists.ozlabs.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8489 Lines: 208 Hi Stephen, On 4 February 2012 21:31, Stephen Warren wrote: > Sorry, I haven't had a chance to read any of the pincrl emails from > Friday onwards. However, I thought a bit more about this, and decided > to propose someting much simpler: > > Thoughts: > > * Defining all the pins, groups, functions, ... takes a lot of space, > ?whether it's in static data in pinctrl drivers or in the device tree. > ?The lists must also be stored in RAM at runtime. > > * It's been very difficult to come up with a generic description of all > ?pin controller's capabilities. This is true even irrespective of device > ?tree; think pin config where we've agonized over whether we can create > ?a standardized list of pin config properties, or need to allow each > ?pinctrl driver to define its own set of properties, etc. > > * The only real use of the lists is for debugfs. Drivers shouldn't expect > ?to directly request specific pinctrl settings, since that would encode > ?knowledge of an individual SoC's pin controller. This should be > ?abstracted from drivers. > > * The data in debugfs could easily be replaced by a raw register dump > ?coupled with a SoC-specific script to print out what each register > ?means. > > My proposal below is to radically simplify the pinctrl subsystem, and > make it little more than a system to execute a list of arbitrary register > writes. Thanks for your work on pinctrl bindings proposal. With this new approach, how much of the pinctrl susbystem is used in device tree mode. Last time I had proposed something similar to this (http://marc.info/?l=linux-kernel&m=132697880016289&w=2), you were not happy about that approach since the pinctrl subsystem is largely under-utilized (http://marc.info/?l=linux-kernel&m=132735884622374&w=2), expect for providing a interface which individual pinctrl/pinmux/pinconfig drivers will use to implement a function that programs the hardware. There need not be pinmux/pinctrl/pinconfig bindings that are designed for the linux pinctrl subsystem. DT allows specifying the pinconfig/pinmux properties in a simple way, which was not possible in non-dt case and hence the pinctrl subsystem. Other OS'es which might not have a pinctrl subsystem, similar to what linux has, should also find the pinctrl bindings useful. Regards, Thomas. > > Advantages: > > * No need to define any kind of data model for pinctrl. > > * No need to store any list of pins/groups/function/parameters/... either > ?in static data, in device tree, or at run-time. > > * No need to store the the selected board-specific settings anywhere either. > > * Completely generic; can handle anything that exists now or in the future. > > * Handles pin mux and pin config identically. > > * Extremely simple to implement and understand; probably just a few K > ?of code and no data. I assume this will be very appealing to those > ?interested in using the binding within U-Boot too. > > * The lists of register writes can just as easily be provided by board > ?files as device tree, so operation should be identical. > > Disadvantages: > > * Creating the list of register values/masks may be a little painful. If > ?we move forward with this, we'd want to strongly push dtc towards > ?providing named constants, expressesion, macros, etc. Those dtc features > ?would be very useful anyway. > > * Lack of high-level semantic meaning to the data; but I assert there's > ?little benefit to having that anyway. > > ?To solve this, write a script to parse each pinctrl driver's regcache > ?file and print out all the register meanings. > > * This scheme wouldn't represent which device "owns" which pins/groups, > ?nor be able to validate that different devices' pinmux settings don't > ?conflict. > > ?I don't think this is a huge issue though, since that's mainly error- > ?checking. Any problems should be just as obvious during testing whether > ?they cause the HW to fail to operate correctly, or whether they cause > ?pinctrl to throw an error. Equally, there are many error conditions > ?that pinctrl core wasn't checking for anyway. > > Precedent: > > * There was previous discussion on using this exact technique for pinmux > ?at a previous Linaro Connect: > > ?http://www.spinics.net/lists/linux-tegra/msg01943.html > > * There is an existing binding that works very similarly, for the Mavell > ?PHY: > > ?http://www.gossamer-threads.com/lists/linux/kernel/1305624 > ?drivers/net/phy/marvell.c > > ?Explicit mention was made here that a simple list of register writes > ?avoiding the complexity of representing a huge number of combinations > ?in the device tree. > > tegra20.dtsi: > > pinmux: pinmux@70000000 { > ? ? ? ?compatible = "nvidia,tegra20-pinmux"; > ? ? ? ?reg = <0x70000014 0x10> ?/* Tri-state registers */ > ? ? ? ? ? ? ?<0x70000080 0x20> ?/* Mux registers */ > ? ? ? ? ? ? ?<0x700000a0 0x14> ?/* Pull-up/down registers */ > ? ? ? ? ? ? ?<0x70000868 0xa8>; /* Pad control registers */ > > ? ? ? ?pinmux_i2c1_spdio_active: pinmux-i2c1-spdio-active { > ? ? ? ? ? ? ? ?reg-init = > ? ? ? ? ? ? ? ? ? ?/* Set pingroup SPDO to function I2C1 */ > ? ? ? ? ? ? ? ? ? ?< > ? ? ? ? ? ? ? ? ? ? ? ?1 ? ? ? ? ?/* Register "bank" in controller */ > ? ? ? ? ? ? ? ? ? ? ? ?0x8c ? ? ? /* Register offset */ > ? ? ? ? ? ? ? ? ? ? ? ?0x000000c0 /* Write bitmask, 0 for whole register */ > ? ? ? ? ? ? ? ? ? ? ? ?0x00000080 /* Write value */ > ? ? ? ? ? ? ? ? ? ?> > ? ? ? ? ? ? ? ? ? ?/* Set pingroup SPDI to function I2C1 */ > ? ? ? ? ? ? ? ? ? ?< > ? ? ? ? ? ? ? ? ? ? ? ?1 ? ? ? ? ?/* Register "bank" in controller */ > ? ? ? ? ? ? ? ? ? ? ? ?0x8c ? ? ? /* Register offset */ > ? ? ? ? ? ? ? ? ? ? ? ?0x00000300 /* Write bitmask, 0 for whole register */ > ? ? ? ? ? ? ? ? ? ? ? ?0x00000200 /* Write value */ > ? ? ? ? ? ? ? ? ? ?>; > ? ? ? ?}; > ? ? ? ?pinmux_i2c1_spdio_suspend: pinmux-i2c1-spdio-suspend { > ? ? ? ? ? ? ? ?... > ? ? ? ?}; > }; > > tegra-seaboard.dts: > > i2c@7000c000 { > ? ? ? ?/* > ? ? ? ? * These are phandles to nodes that containthe reg-init list. Can > ? ? ? ? * we have phandles to properties instead? Then we wouldn't need > ? ? ? ? * all those nodes which each just contains 1 property. > ? ? ? ? */ > ? ? ? ?pinctrl = <&pinmux_i2c1_spdio_active &pinmux_i2c1_spdio_suspend>; > ? ? ? ?pinctrl-names = "active", "suspend"; > }; > > In order to support programming more than one pin controller at once, we > could include the phandle of the pin controller in the "reg-init" property: > > somewhere-other-than-under-a-pin-controller { > ? ? ? ?pinmux-foo { > ? ? ? ? ? ? ? ?reg-init = > ? ? ? ? ? ? ? ? ? ?< > ? ? ? ? ? ? ? ? ? ? ? ?&tegra_pmx /* Pin controller */ > ? ? ? ? ? ? ? ? ? ? ? ?2 ? ? ? ? ?/* Number of entries for this controller */ > ? ? ? ? ? ? ? ? ? ?> > ? ? ? ? ? ? ? ? ? ?< > ? ? ? ? ? ? ? ? ? ? ? ?1 ? ? ? ? ?/* Register "bank" in controller */ > ? ? ? ? ? ? ? ? ? ? ? ?0x8c ? ? ? /* Register offset */ > ? ? ? ? ? ? ? ? ? ? ? ?0x000000c0 /* Write bitmask, 0 for whole register */ > ? ? ? ? ? ? ? ? ? ? ? ?0x00000080 /* Write value */ > ? ? ? ? ? ? ? ? ? ?> > ? ? ? ? ? ? ? ? ? ?< > ? ? ? ? ? ? ? ? ? ? ? ?1 ? ? ? ? ?/* Register "bank" in controller */ > ? ? ? ? ? ? ? ? ? ? ? ?0x8c ? ? ? /* Register offset */ > ? ? ? ? ? ? ? ? ? ? ? ?0x00000300 /* Write bitmask, 0 for whole register */ > ? ? ? ? ? ? ? ? ? ? ? ?0x00000200 /* Write value */ > ? ? ? ? ? ? ? ? ? ?> > ? ? ? ? ? ? ? ? ? ?< > ? ? ? ? ? ? ? ? ? ? ? ?&other_pmx /* Pin controller */ > ? ? ? ? ? ? ? ? ? ? ? ?1 ? ? ? ? ?/* Number of entries for this controller */ > ? ? ? ? ? ? ? ? ? ?> > ? ? ? ? ? ? ? ? ? ?< > ? ? ? ? ? ? ? ? ? ? ? ?0 ? ? ? ? ?/* Register "bank" in controller */ > ? ? ? ? ? ? ? ? ? ? ? ?0x04 ? ? ? /* Register offset */ > ? ? ? ? ? ? ? ? ? ? ? ?0x000000ff /* Write bitmask, 0 for whole register */ > ? ? ? ? ? ? ? ? ? ? ? ?0x0000005a /* Write value */ > ? ? ? ? ? ? ? ? ? ?>; > ? ? ? ?} > }; > > When parsing a reg-init property, we know if it contains phandles by: > * If node containing property is a child of a pin controller, the property > ?doesn't containt pin controller phandles. > * Otherwise, it does. > > What are people's thoughts. Thinking about all the issues involved, this > is certainly the approach I like most right now. > > -- > nvpublic > -- 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/