On Mon, Aug 15, 2011 at 01:44:53PM -0700, Stephen Warren wrote:

Jamie Iles wrote at Monday, August 15, 2011 2:36 PM:

quoted

On Mon, Aug 15, 2011 at 09:07:16PM +0100, Jamie Iles wrote:

quoted

Hi Stephen,

On Fri, Aug 12, 2011 at 04:54:55PM -0600, Stephen Warren wrote:

quoted

Signed-off-by: Stephen Warren <redacted>

...

quoted

quoted

quoted

diff --git a/arch/arm/mach-tegra/pinmux.c b/arch/arm/mach-tegra/pinmux.c

...

quoted

quoted

quoted

+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);

...

quoted

quoted

quoted

+		ret = of_property_read_string(pg_node, "nvidia,pull", &s);

...

quoted

quoted

quoted

+		if (of_find_property(pg_node, "nvidia,tristate", NULL))

...

quoted

quoted

quoted

+		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 at 70000000 {
		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