[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20110815205042.GC22355@gallagher>
Date: Mon, 15 Aug 2011 21:50:42 +0100
From: Jamie Iles <jamie@...ieiles.com>
To: Stephen Warren <swarren@...dia.com>
Cc: Jamie Iles <jamie@...ieiles.com>,
Grant Likely <grant.likely@...retlab.ca>,
Colin Cross <ccross@...roid.com>,
Erik Gilling <konkers@...roid.com>,
Olof Johansson <olof@...om.net>,
Russell King <linux@....linux.org.uk>,
"devicetree-discuss@...ts.ozlabs.org"
<devicetree-discuss@...ts.ozlabs.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"Linus Walleij (linus.walleij@...aro.org)" <linus.walleij@...aro.org>
Subject: Re: [RFC PATCH 10/12] arm/tegra: Add device tree support to pinmux
driver
On Mon, Aug 15, 2011 at 01:44:53PM -0700, Stephen Warren wrote:
> Jamie Iles wrote at Monday, August 15, 2011 2:36 PM:
> > On Mon, Aug 15, 2011 at 09:07:16PM +0100, Jamie Iles wrote:
> > > Hi Stephen,
> > >
> > > On Fri, Aug 12, 2011 at 04:54:55PM -0600, Stephen Warren wrote:
> > > > Signed-off-by: Stephen Warren <swarren@...dia.com>
> ...
> > > > diff --git a/arch/arm/mach-tegra/pinmux.c b/arch/arm/mach-tegra/pinmux.c
> ...
> > > > +static void __init tegra_pinmux_probe_dt(struct platform_device *pdev)
> > > > +{
> > > > + int pg;
> > > > +
> > > > + for (pg = 0; pg < TEGRA_MAX_PINGROUP; pg++) {
> > > > + const char *pg_name = pingroup_name(pg);
> > > > + struct tegra_pingroup_config config;
> > > > + struct device_node *pg_node;
> > > > + int ret;
> > > > + const char *s;
> > > > +
> > > > + pg_node = of_find_child_node_by_name(pdev->dev.of_node,
> > > > + pg_name);
> > > > + if (pg_node == NULL)
> > > > + continue;
> > > > +
> > > > + config.pingroup = pg;
> > > > +
> > > > + ret = of_property_read_string(pg_node, "nvidia,function", &s);
> ...
> > > > + ret = of_property_read_string(pg_node, "nvidia,pull", &s);
> ...
> > > > + if (of_find_property(pg_node, "nvidia,tristate", NULL))
> ...
> > > > + tegra_pinmux_config_pingroup(&config);
> > > > +
> > > > + of_node_put(pg_node);
> > > > + }
> > > > +}
> > >
> > > I need to implement DT muxing configuration for my platform, and I believe
> > > that what you have here would work fine for me too, and to avoid duplicating
> > > the same thing, I wonder if this could be a little more generic.
> > >
> > > So if the platform specific pinmux driver called the pinmux parser with a
> > > callback for a pingroup configuration function then this wouldn't need the
> > > nvidia specific properties. I'd envisage the setup callback to be something
> > > like:
> > >
> > > int pingroup_configure(const char *name, unsigned long flags);
> >
> > and it if this took the device_node too then the platform specific bits could
> > handle more esoteric properties if required. I'll have a go at prototyping
> > this tomorrow unless there are any obvious reasons that this is a stupid idea!
>
> I expect some of the code could be shared.
>
> The only worry I have is whether some SoCs don't configure things like
> pinmux function in the same place as pad function (pullup/down, tristate),
> and hence whether a generic binding is generally applicable. I suppose the
> code could always ignore unused properties.
Yes, well our hardware doesn't support any of these features other than
setting the function so in the picoxcell backend I'd just WARN_ON() invalid
flags settings.
> I wonder how much of this is relevant to Linus W's pinctrl API?
Hmm, not sure on that one, it's been a while since I've looked at Linus'
patches.
> Note that in the updated patch series I just posted, I reworked the binding
> a little; Tegra has two sets of pin-groups, one configuring muxing, pullup/
> down, and tri-state, and the other configuring various driver strength/
> rate properties. Hence, the tree is now e.g.:
>
> pinmux: pinmux@...00000 {
> compatible = "nvidia,tegra20-pinmux";
> reg = < 0x70000000 0xc00 >;
> nvidia,mux-groups {
> cdev1 {
> nvidia,function = "plla_out";
> };
> cdev2 {
> nvidia,function = "pllp_out4";
> nvidia,pull-down;
> nvidia,tristate;
> };
> };
> nvidia,drive-groups {
> sdio1 {
> nvidia,schmitt;
> nvidia,drive-power = <1>;
> nvidia,pull-down-strength = <31>;
> nvidia,pull-up-strength = <31>;
> nvidia,slew-rate-rising = <3>;
> nvidia,slew-rate-falling = <3>;
> };
> };
> };
>
> But it's probably still reasonably easy to make the parser for the mux-groups
> node generic. Perhaps it makes sense for all SoCs to have a "mux-settings"
> node, even if they don't have any other custom nodes?
You have a much more complex chip than I do! I don't know if *all* SoC's have
to have the same muxing binding, but it feels that this one should cover a lot
of the most common bases.
Jamie
--
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