Message-ID: <50369FEE.8000003@wwwdotorg.org>
Date:	Thu, 23 Aug 2012 15:26:06 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
CC:	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Grant Likely <grant.likely@...retlab.ca>,
	Rob Herring <rob.herring@...xeda.com>,
	Rob Landley <rob@...dley.net>,
	Russell King <linux@....linux.org.uk>,
	Lior Amsalem <alior@...vell.com>, Andrew Lunn <andrew@...n.ch>,
	Gregory CLEMENT <gregory.clement@...e-electrons.com>,
	Ben Dooks <ben.dooks@...ethink.co.uk>,
	Linus Walleij <linus.walleij@...aro.org>,
	devicetree-discuss@...ts.ozlabs.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 1/9] pinctrl: mvebu: pinctrl driver core

On 08/23/2012 02:31 PM, Sebastian Hesselbarth wrote:
> On 08/23/2012 07:54 PM, Stephen Warren wrote:
>>> dt_parse() and dt_parse_function() build up structs that get used
>>> later on
>>> in mvebu_pinmux_ops that require indexed functions. I can join them with
>>> dt_node_to_map() but that would require incremental kzalloc for the
>>> corresponding array. Or I could (functionally) leave dt_parse() to
>>> allocate
>>> the array and only join dt_parse_function() with dt_node_to_map().
>>
>> So everything you said makes sense, in that the core driver is
>> parameterizable and receives data from a SoC-variant-specific driver
>> indicating which pins/groups/functions are available. I'm still not sure
>> though why the translation of the pin/group/function structures passed
>> to probe into other data structures requires accessing the DT at all;
>> the set of available pins/groups/functions isn't configured through DT,
>> and doesn't need to be limited to only those options actually used in
>> DT, so can't you just process all the data that's passed to probe
>> without interaction with the DT?
> 
> Hmm, maybe I still don't quite understand the terminology of pinctrl/pinmux
> core completely. What exactly should mvebu_pinmux_get_funcs_count return
> if not the number of DT node children?

In HW, you have:

* A number of pins

* In some cases, multiple pins are controlled by a single register
field, so there are groups of pins, whose pins all get mux'd at once.

* In some cases, SW chooses to control multiple pins at once even if the
HW allows otherwise, and these are also represented as groups.

* A (mux) function represents what options (signals, modules, ...) are
available to be mux'd onto any given pin or group of pins.

If all of your pins/groups can support 4 different mux choices 0..3
(even though the interpretation of values 0..3 may differ per pin/group)
you may simply have functions 0, 1, 2, 3 exposed by your driver.

Alternatively, you may define a function for each logical HW module
(e.g. UART1, UART2, SPI1, SPI2, MMC1...) that exists in your SoC, or
even per each separate subset of signals (e.g. UART1 RX/TX, UART 1
RTS/CTS, UART 2 RX/TX/RTS/CTS, ...).

In a driver where the set of pins, groups, and functions known to the
driver is defined as static data in the driver itself (which I believe
covers your driver; IIRC I saw all those tables in the patch), then
you'd simply count up all the defined function names/IDs available, and
return that from pinmux_ops.get_functions_count(). In this case, the
number of functions is entirely static, being defined by the SoC HW
design, and hence DT content has absolutely no impact on it. For
reference, this is how the Tegra pinctrl drivers work.

In a driver where the set of pins, groups, and functions known to the
driver is not hard-coded into the driver itself, but instead derived
from DT content (which is useful for a driver that works for say 10
different similar SoCs, each with slightly different sets of actual
pins/groups/functions, but where you don't want to hard-code all the
tables into the driver), then you may need to parse DT to extract the
definitions of those pins/groups/functions. Then, there's the question
of how to represent the set of known pins/groups/function in DT; do you
have explicit tables for this, or reverse engineer the tables based on
whichever pins/groups/function just happen to be used by the board
you're running on, in which cases the reverse-engineered lists will
probably not include all options the HW could support, given one
particular DT file. Anyway, as I said, I don't think this approach
applies to your driver anyway since IIRC I did see all the data tables
in the patch.

As you may have guessed, I strongly prefer the hard-coded static table
based approach, or at least separating the definition of known
pins/groups/functions from the configuration nodes that define which
particular mux settings to actually use.

Puting this all a little more simply, the pinctrl core wants to generate
a list of all known functions. It is a single global/unified list. It
first calls pinmux_ops.get_functions_count() to find out how many
functions there are in the list, then pinmux_ops.get_function_name() for
each one to find its name, then pinmux_ops.get_function_groups() to find
out which groups allow that function to be mux'd onto them.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists