Hi Stephen, On 4 February 2012 21:31, Stephen Warren <[email protected]> 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 > _______________________________________________ devicetree-discuss mailing list [email protected] https://lists.ozlabs.org/listinfo/devicetree-discuss
