Re: [PATCH] pinctrl: Add generic pinctrl-simple driver thatsupports omap2+ padconf

[Tony Lindgren]

* Stephen Warren <swarren@xxxxxxxxxxxxx> [120509 13:19]:
> On 05/04/2012 03:57 PM, Tony Lindgren wrote:
> > * Stephen Warren <swarren@xxxxxxxxxxxxx> [120504 12:27]:
> >> On 05/02/2012 11:24 AM, Tony Lindgren wrote:
> >>
> >>> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-simple.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-simple.txt
> ...
> >> On the other hand, I worry about whether using "pinctrl-simple" here as
> >> the compatible value is going to cause issues:
> >>
> >> Certainly, this is a pretty simple driver, and most likely reasonably
> >> generic an applicable to many SoCs. However, it doesn't cover a bunch of
> >> cases that I'd still consider "simple". For example, what if each pin
> >> has a separate mux and pinconf register? There are probably many such
> >> small cases that would add up to something more complex. to cover those
> >> cases, will we be able to extend pinctrl-simple to cover them, and
> >> continue to be backwards compatible, or will we need to create a
> >> binding/driver for pinctrl-simple-1, pinctrl-simple-2, pinctrl-simple-3
> >> each of which covers some different, yet still simple, configuration?
> > 
> > Yes getting the binding right is the critical part here, everything else
> > can be added as needed. I was thinking about using separate properties
> > for auxilary registers, but now thinking about it a bit more, it may not
> > be sufficient.
> > 
> > How about we make some of these properties into arrays? For example:
> > 
> > #pinctrl-cells = 6;
> > pinctrl-simple,function-mask = <0x0000ffff 0x0000ffff 0xffff0000>;
> > pinctrl-simple,function-off = <0x7 0x7 0x70000>;
> > pinctrl-simple,pinconf-mask = <0xffff0000 0xffff0000 0x0000ffff>;
> 
> I'm not sure what the 3 entries in the array are meant to describe?

If you have let's say three registers per pin, those would be the
related function and pinconf masks for those registers.
 
> > Because we need to specify GPIO for some pins. There may be additional flags
> 
> What do you mean by "specify GPIO"?
> 
> Nothing in this pinctrl-simple binding seems to imply that it's also a
> GPIO controller.

It is not a GPIO controller, but eventually needs to deal with existing
GPIO controllers.
 
> If "GPIO" is one of the functions that can be mux'd onto a pin, then I'd
> expect that to be represented in exactly the same way as any other
> function that could be mux'd onto the pin.

Right. But additionally we also need to know the mux register to GPIO
mapping for things like irq_set_irq_wake()/enable_irq_wake()/disable_irq_wake()
that may be set dynamically depending on what the user wants.
 
> So, I'm not sure what GPIO-related information you want to represent.

It seems that we should be able to do pinctrl_request_gpio that uses
an external GPIO controller and also sets up the desired wake-up flags
as needed. Anyways, not needed yet.

> > too, we do have external DMA request lines for few pins available.. I'm not
> > saying pinctrl fwk should know about that, but it's a similar mapping of pins
> > to GPIO lines.
> 
> Aren't DMA request lines also just another function that can be mux'd
> onto a pin?

Yes it's a function for routing the signal. But that also needs to be
configured in the DMA controller. Right now there's no need to have
that mapping in the binding.

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/