On 08/06/2013 05:45 PM, Hanumant Singh wrote:

On 7/31/2013 5:17 PM, Hanumant Singh wrote:

quoted

On 7/31/2013 2:06 PM, Stephen Warren wrote:

quoted

On 07/31/2013 01:46 PM, Hanumant Singh wrote:

quoted

On 7/30/2013 8:59 PM, Stephen Warren wrote:

quoted

On 07/30/2013 06:13 PM, Hanumant Singh wrote:

quoted

On 7/30/2013 5:08 PM, Stephen Warren wrote:

quoted

On 07/30/2013 06:01 PM, Hanumant Singh wrote:

quoted

On 7/30/2013 2:22 PM, Stephen Warren wrote:

quoted

On 07/30/2013 03:10 PM, hanumant wrote:
...

quoted

We actually have the same TLMM pinmux used by several socs of a
family.
The number of pins on each soc may vary.
Also a given soc gets used in a number of boards.
The device tree for a given soc is split into the different
boards
that
its in ie the boards inherit a common soc.dtsi but have separate
dts.
The boards for the same soc may use different pin groups for
accomplishing a function, since we have multiple i2c, spi uart
etc
peripheral instances on a soc. A different instance of each of
the
above
peripherals, can be used in different boards, utilizing different
or subset of same pin groups.
Thus I would need to have multiple C files for one soc, based
on the
boards that it goes into.

The pinctrl driver should be exposing the raw capabilities of
the HW.
All the board-specific configuration should be expressed in DT.
So, the
driver shouldn't have to know anything about different boards at
compile-time.

I agree, so I wanted to keep the pin grouping information in DT, we
already have a board based differentiation of dts files in DT,
for the
same soc.

That's the opposite of what I was saying. Pin groups are a
feature of
the SoC design, not the board.

Sorry I guess I wasn't clear.
Right now I have a soc-pinctrl.dtsi containing pin groupings.
This will be "inherited" by soc-boardtype.dts.
The pinctrl client device nodes in soc-boardtype.dts will point to
pin
groupings in soc-pinctrl.dtsi that are valid for that particular
boardtype.
Is this a valid design?

OK, so you have two types of child node inside the pinctrl DT node;
some
define the pin groups the SoC has (in soc.dtsi) and some define
pinctrl
states that reference the pin group nodes and are referenced by the
client nodes.

That's probably fine. However, I'd still question putting the pin
group
nodes in DT at all; I'm not convinced it's better than just putting
those into the driver itself. You end up with the same data tables
after
parsing the DT anyway.

Any feedback for the rest of the patch?

I'm certainly waiting for this aspect of the patch to be resolved; I
think it will impact the rest of the patch so much that it's not worth
reviewing until we decide on where to represent the pin groups (some DT
parsing could would be removed if we put the pin group definitions into
the driver, hence wouldn't need to be reviewed, and likewise there's be
some new tables to review).

I am trying to look at examples of what you are suggesting.
I was looking at the exynos implementation, and just from a brief glance
it seems like there too the pin grouping is being specified in the
device tree, using what looks like labels of the pins.
The labels are matched to group structures in soc specific files?

By having the pin groupings in DT I am able to reuse the driver without
any SOC based code bloat.
As I mentioned earlier, we have entire families of SOCs using the same
TLMM hardware.
Its not a guarantee that for a given TLMM version,
the pin groupings on that hardware are the same for every SOC that its
in. Its infact most likely that I wont be able to use the pin groupings
from one SOC to the next even if they both use the same TLMM.
It will very quickly lead to a bloat of
pinctrl-<msm_soc>.c (containing the pin groupings replicated for each
soc)
which use TLMM version specific register programming implementation
pinctrl-tlmm-<version>.c
and the DT parsing and interface to framework (which remains unchanged).
pinctrl-msm.c.

Thanks
Hanumant

Any comments on this?

No. As I said, I personally want to see all the pingroups defined in the
pinctrl driver. But, if someone else acks/... the patches without it, I
probably won't nack it.