Re: Pinmux bindings proposal V2

From: Tony Lindgren
Date: Fri Jan 27 2012 - 13:10:17 EST


* Stephen Warren <swarren@xxxxxxxxxx> [120127 09:21]:
> Tony Lindgren wrote at Friday, January 27, 2012 10:38 AM:
> ...
> > So how about let's do separate static and dynamic bindings,
> > something like this:
> >
> > /*
> > * Static init time only mux where
> > * we only specify phandle to driver
> > * and, offset of the mux, and the value.
> > * These pins are discarded after init.
> > *
> > * Format: mux_ctrl offset value
> > */
> > pinctrl-static = <&pmx_driver1 0x0020 0x1245
> > &pmx_driver2 0x0022 0x6578>;
>
> So those are direct register writes? That sounds pretty scary, and a
> royal pain to author the device tree.

Driver specific, either registers or enumeration. It could also
it could be also generic defines, something along the lines of
PMX_MUX_FUNCTION_1 | PMX_MUX_GPIO_INPUT.

> If we do go with this, I think we'd need a mask for each register write
> too, so you can leave certain bits unaffected; there's no reason to
> believe in general that each pin has a dedicated register only for that
> pin, or even that only pinmux/config data is in the register.

I'm currently using "pinmux-simple,function-mask", but yeah sounds like
it should be a generic mask name.

> This also makes it difficult to extract semantic information from the
> DT. How can the pinctrl subsystem know which pins are in use and which
> aren't here? This is relevant if some module loads later and attempts
> to claim some pins - are they already in use by another driver or not?

We could just have one spinlock for all the discarded pins instead of
having a single spinlock for each discarded pin?

> Now individual pinctrl drivers could interpret those register values
> and know that this means pin/group "x" is programmed to mux value "y",
> but does that mean pin "x" is actually /used/, or just that the init
> table had to program value "y" because the default for that pin is "z"
> which conflicted in HW with some other mux setting that the board
> needed (e.g. muxing signal "y" to some other pin).
>
> (Put another way, this binding completely bypasses the pinctrl subsystem;
> is that OK?)

Well I was thinking we should still register the pins, and have pinctrl
fwk set those values, then discard those pins but still keep them as
locked.

> > /*
> > * Dynamic mux where the mux is kept around after
> > * init and multiple states can be defined for
> > * a mux as a subnode of the pinmux controller.
> > *
> > * Format: mux_phandle initial state
> > */
> > pinctrl-dynamic = <&pmx_sdhci PMX_STATE_ENABLED
> > &pmx_ehci_xcv PMX_STATE_ENABLED>;
> >
> > This would make pinctrl-static binding follow the same
> > standard as GPIO binding and can be parsed easily with
> > of_parse_phandle_with_args.
> >
> > Then for pinctrl-dynamic we can make a custom parser,
> > and the binding can follow the more readable format as
> > Simon posted.
>
> I don't think there's any point in having 2 separate bindings; it's been
> hard enough to come up with /one/ binding! If we do go for raw register
> writes for the static stuff, we should just do the same for the dynamic
> stuff too.

But then we need to waste a register for the static/dynamic flag for
each mux.. Or make the tree deeper.

> In fact, given this would all bypass the pinctrl subsystem entirely,
> perhaps lets not even define a standard format for pinctrl-static or
> pinctrl-dynamic, and just have each pin controller driver parse tables
> inside its own node, in a format specific to that pin controller's
> binding. I already had that working for the static case back in last
> August and would love to just apply those patches and be done with this.

Hmm I don't think it would by pass it, we just need to let pinctrl
fwk deal with the init time only pins too.

For the dynamic pins, note that PMX_STATE_* defines would be one of the
standard states supported by the pinctrl fwk. Then of course where
pmx_sdhci binding is implemented, you could have either hardware specific
register values, enumeration or generic defines depending on how the
pinctrl driver is implemented.

Regards,

Tony

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/