[U-Boot] [Ac100] [PATCH 3/3] ARM: tegra: paz00: enable nveckeyboardsupport

[Marc Dietrich]

Am Samstag, 20. Juli 2013, 21:20:52 schrieb Stephen Warren:
> On 07/20/2013 03:12 AM, Marc Dietrich wrote:
> > On Friday 19 July 2013 13:14:13 Stephen Warren wrote:
> ...
> 
> > Let's skip how this may actually look like in software. Given the
> > discussions we had in the past, I propose the following binding:
> > 
> > i2c-slave at 7000c500 {
> > 
> > 	compatible = "nvidia,tegra20-i2c-slave";
> > 	reg = <0x7000c500 0x100>;
> > 	interrupts = <0 92 0x04>;
> > 	#address-cells = <1>;
> > 	#size-cells = <0>;
> > 	clock-frequency = <80000>;
> > 	slave-addr = <138>;
> 
> Hex would be more common, but that's a minor issue.

ok.

> > 	clocks = <&tegra_car 67>, <&tegra_car 124>;
> > 	clock-names = "div-clk", "fast-clk";
> > 	
> > 	nvec {
> 
> Above, it says #address-cells=<1>, which means this node needs a reg
> property. Perhaps slave-addr should be part of the child nodes (and the
> Tegra I2C controller binding would limit itself to supporting only a
> single node), so that the same binding style could be applicable to I2C
> slave devices that support multiple slave addresses.

you mean 

nvec at 87 {
		reg = <0x87>;
		...
} ?

I think that's ok. Didn't know that Tegra can support multiple slave 
addresses. To make the binding as general as possible, we could support multi-
slave for the binding, but only support single slave in the code for now.

I guess that also warrants a "simple-bus" compatibility in the i2c-slave node.

> 
> > 		compatible = "nvidia,nvec", "simple-bus";
> > 		protocol = "smbus-request-gpio";
> 
> What is that property for; doesn't compatible="nvidia,nvec" already
> imply this, or does the NVEC spec define multiple different protocols?

The GPIO is optional, but SMBUS is required. Maybe to support master initiated 
communications only. To get rid of the ugly protocol property, we could just 
check if a valid gpio is given. I think that's what the downstream kernel also 
does. The nvec still needs to tell the slave driver which protocol to use, but 
that can be hard coded.

> > 		request-gpios = <&gpio 170 0>; /* gpio PV2 */
> 
> We should use the C pre-processor to provide named constants there,
> although I guess U-Boot isn't set up for that yet. The kernel is once
> this is ported there, and once the 2013.07 release is out, U-Boot should
> be able to support this very soon too.
> 
> > 		keyboard {
> 
> Simple-bus might require a reg property; I forget. Does the NVEC
> protocol include any form of "virtual device address" that it would make
> sense to put into a reg property?
> 
> > 			compatible = "nvidia,nvec-keyboard";

well, we could misuse the nvec function select byte for this (e.g. 5 for 
keyboard, 6 for mouse, ...), but we may run into trouble with the "system 
command = 1" which is used by several drivers.

> > 
> > Does this looks better?
> 
> Yes, overall much better.
> 
> New DT bindings should be sent to devicetree at vger.kernel.org for review.
> Note that's a branch new list (it moved from a different server), so it
> might be best to wait a few days for people to subscribe before sending
> mail to it.

Ok, I'll send out a new version and cc dtml. Maybe this some more people 
respond ;-)

Thanks for review!

Marc