[lkml]   [2012]   [Jan]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRE: Pinmux bindings proposal V2
    Tony Lindgren wrote at Tuesday, January 24, 2012 5:04 PM:
    > * Stephen Warren <> [120124 14:02]:
    > > Tony Lindgren wrote at Monday, January 23, 2012 6:21 PM:
    > > > * Stephen Warren <> [120123 14:37]:
    > > > > Tony Lindgren wrote at Monday, January 23, 2012 2:01 PM:
    > > > > > This mostly looks pretty good to me, few more comments below.
    > > ...
    > > > > > > pmx_sdhci_active: pinconfig-sdhci-active {
    > > > > > > /*
    > > > > > > * In each of these nodes, both the mux and config
    > > > > > > * properties are optional. This node represents the
    > > > > > > * additions to pmx_sdhci that are specific to an
    > > > > > > * active state. In this case, only pin configuration
    > > > > > > * settings are different.
    > > > > > > */
    > > > > > > config =
    > > > > > > };
    > > > > > > pmx_sdhci_standby: pinconfig-sdhci-standby {
    > > > > > > config =
    > > > > > > };
    > > > > >
    > > > > > After thinking about this a bit more, I'm now thinking that we should
    > > > > > probably only describe the active state in the device tree to keep things
    > > > > > simple.
    > > > > >
    > > > > > Anything PM related could be initialized later on during the boot by the
    > > > > > device driver, or even from userspace using /lib/firmware or /sys entries.
    > > > > > This would cut down the device tree bloat quite a bit.
    > > > >
    > > > > I think it makes sense to describe everything in one place. It's much
    > > > > easier to ensure everything is consistent and correct if you don't have
    > > > > to cross-check two sources of data for the same thing.
    > > >
    > > > But we end up repeating the same bits over and over again making the device
    > > > tree bloated.
    > >
    > > What exactly gets repeated; can you please show an example?
    > Well in your example, you have TEGRA_PMX_PG_DTA and TEGRA_PMX_PG_DTD
    > repeated. It seems you're trying to list slightly different register
    > values for the the pincontrol modes with pmx_sdhci_active and
    > pmx_sdhci_standby.

    Hmmm. I guess I was taking "repeated" a little too literally then...

    There's an entry for each of DTA and DTD in each of the two nodes because
    in this example, I want the value of parameter TEGRA_PMX_CONF_TRISTATE to
    be different in each of those two nodes.

    If the value wasn't different, there'd be no need to specify the value
    of TEGRA_PMX_CONF_TRISTATE in each of the two nodes; you'd typically
    specify it just once in the common node.

    So, the binding itself isn't forcing you to repeat anything, it's just
    in the example we actually want different values for this parameter in
    different status. You need /some/ way to specify that.

    If we don't do that in the common pinctrl binding, I see a couple choices:

    a) Put it in static data in the pinctrl driver.

    b) Put it in device tree in a binding that's specific to the individual
    pinctrl driver, rather than a common binding.

    Now in each case, if you want the value of some parameter on some pin to
    be different in each of two states, you need /some/ way to represent that.
    Perhaps in (a) above, the literal 32-bit value "DTA" could be implicit,
    in that it's an array index into some table, but I don't see how to avoid
    specifying the two different values that are desired...

    > IMHO already that list of mux states seems limited as you may need to
    > list other driver states too like pmx_sdhci_standby_wake_up_disabled and
    > pmx_sdhci_standby_off. So it starts to sound a bit like PM policy instead
    > of letting the driver know what hardware is on the system.

    First off, the binding isn't enforcing any specific set of named states,
    it's providing a mechanism to define a set of states with completely
    arbitrary names. As such (as I think I gave an example elsewhere in this
    thread), a dumb driver could require just one state e.g. "active", a
    simple driver could require two states e.g. "active", "standby", and a
    complex driver could require a whole bunch of different named states per
    its needs. All this is allowed by the common binding which is just
    providing a transport not policy. The pinctrl client drivers are what
    are defining the required state names, in their individual binding.

    Second, as I mentioned before, while some of the states are certainly
    PM-related, I don't think all will be, e.g. the case of running an SD
    controller at different clock rates to the SD card, and needing to
    set different pin parameters based on the clock rate. Is runtime PM
    intended cover that kind of thing? The idea here is that the common
    pinctrl binding can allow the driver to require different named states
    for those different clock rate cases.

    > Having multiple config options also means that you end up setting
    > #pinconfig-cell to something unnecessarily large for all mux nodes even
    > if you only need the alternative values for one pin on the system.

    I don't understand here.

    In the example above, which I'll repeate:

    config =

    #pinconfig-cells would be 3:
    cell 0 is pin/group ID
    cell 1 is configuration parameter
    cell 2 is value for that configuration parameter

    The parsing of the config node involves splitting the whole cell list
    into chunks of #pinconfig-cells each, and interpreting each one in turn,
    in exactly the same way as #gpio-cells can be used to parse a DT property
    containing a list of GPIOs.

    > > Given that each named state can reference n nodes defining part of that
    > > state's pinmux configuration, and some of those nodes can affect the same
    > > pin controller, you can easily split the overall configuration into
    > > "stuff that's common for all states for this device" and "stuff that's
    > > common between these n states of the device but not these m others" and
    > > "stuff that's unique to this 1 state", and hence avoid repeating any
    > > identical information.
    > >
    > > > I'm suspecting that the initial state can be used to set the PM states for
    > > > pins. Probably most pins can have the PM configuration set from the start.
    > >
    > > Can you please explain more?
    > For the cases I know the default values work for 95% - 100% of the pins in
    > the system for PM also. Only few pins on some systems need to be dynamically
    > remuxed during the runtime. The rest of the pins can be discarded after init.

    So that's fine.

    95% the same means that the nodes you use to describe the differences are
    only 5% of the total; no repetition necessary in the DT nodes.

    Now for the 95% of the programming that happens at device probe time:
    Perhaps we could throw away all the data representing that after it was

    However, that's not related to the DT binding; it's an implementation
    detail of the pinctrl subsystem that it parses the pinmux mapping table
    early in boot and keeps it around in memory (e.g. to display in sysfs).
    I don't believe there's anything in the DT binding that forces the
    implementation to do that.

    Similarly, if you don't need any (or very little) of the dynamic per-
    device stuff, you could probably just set up your base pinmux config
    as a huge array of (in pinctrl subsystem terms) "system hog" entries,
    or in terms of this DT binding, pin configuration nodes that are associated
    with the pin controller itself rather than individual devices, and just
    list the differences (if any) in the per-device nodes.

    I need a bit more time to digest the rest of your email; the parts where
    I think you're saying:

    a) For stuff like active/suspend, have the pinctrl driver know what
    that means and implement it rather than explicitly representing
    those states in device tree (but where do we represent them?)

    b) For other states, do everything through runtime PM. I need to
    familiarize myself with runtime PM in order to understand how that'd
    work I guess.


     \ /
      Last update: 2012-01-26 20:37    [W:0.032 / U:219.984 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site