Hi Olof, On Tue, Jan 3, 2012 at 7:43 AM, Olof Johansson <olof@xxxxxxxxx> wrote: > Hi, > > On Mon, Jan 02, 2012 at 10:39:04AM -0800, Simon Glass wrote: >> > diff --git a/Documentation/devicetree/bindings/input/matrix-keymap.txt b/Documentation/devicetree/bindings/input/matrix-keymap.txt >> > new file mode 100644 >> > index 0000000..1db8e12 >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/input/matrix-keymap.txt >> > @@ -0,0 +1,27 @@ >> > +A simple common binding for matrix-connected key boards. Currently targeted at >> > +defining the keys in the scope of linux key codes since that is a stable and >> > +standardized interface at this time. >> > + >> > +Required properties: >> > +- compatible: "matrix-keyboard-controller" >> > +- linux,keymap: an array of packed 1-cell entries containing the equivalent >> > + of row, column and linux key-code. The 32-bit big endian cell is packed >> > + as: >> > + row << 24 | column << 16 | key-code >> >> This looks much better than the Samsung binding and the original 3-cell one. >> >> U-Boot Tegra (keyboard patch series) currently uses a 16x8 matrix, and >> uses a single byte (the ASCII character) for a total of 128 bytes per >> keymap. Since U-Boot does not have (or apparently want) the concept of >> key codes or the added code and intermediate data this requires, the >> binding presented here will not suit U-Boot so far. >> >> These rows and columns embedded in the cell make it more of a pain to >> write the fdt description. If you just set up the cells in (column, >> row) order and set the matrix size (rows, columns) then you end up >> with 128 entries on Tegra. If you use uint16 then this could be 256 >> bytes instead of 512. The binding you present for Tegra would be 109 * >> 4 = 436 bytes. However I take your point that Fn maps are much more >> sparse, so overall this is likely to be similar (512 bytes for either >> method once Fn mappings are taken into account). > > Looking at the keymap you posted, you define 62 keys and 16 function > keys. So if that is all you care about on your particular keyboard, > that means that the layout with the 1-cell format is 248 bytes for the > base keymap, and 64 bytes for the fn-keymap. The benefit of using the > row/column/keycode format is that the table is only as large as the > number of keys you care about. Yes - my point is that the number of entries in the 1-byte binding (with no row, column embedded) is unlikely to be more than the next power of 2. So by avoiding including 2 or 3 unnecessary bytes we can actually get a size reduction in most cases. It is also easier to enter IMO. Can the Linux key codes fit in 8 bits? > >> But going back to U-Boot, it does not have the intermediate table that >> you malloc and decant into, it does not understand key codes so >> doesn't know what to do when Shift is pressed, doesn't understand >> languages, etc. In order to use these Linux bindings in U-Boot we >> would need to add new input layer, extra decode code and intermediate >> tables. I can almost hear the NAKs already (bear in mind fdt only went >> into U-Boot in the December release and people are more sensitive to >> code size / performance there than in Linux). I did ask about this on >> this list a few weeks ago but no response yet. >> >> We could perhaps add an alternative direct ASCII binding like this >> example (which would have to be in a separate node): > > You keep saying "direct ASCII", but your map contains non-ASCII > characters. ASCII provides no way at all to specify things such as shift, > or arrow keys, or other common new keys on keyboards. Instead you have had to > make up an ad-hoc custom map that contains your own special codes for these > keys. > > For example, for arrow keys you seem to overload the FS/GS/RS/US. I'm not > saying we expect to need to use those ascii codes, but it does seem arbitrary > to just grab them for arrow keys. So in essence you have come up with your own > encoding of non-ASCII keys instead of reusing what is already available through > the linux keycodes. > > Also, for example with return, does it encode as CR, LF or CR+LF? Etc. > > Doing a binding in pseudo-ASCII is just asking for extra complications, in my > opinion. Over time that encoding is likely to swell to similar sizes anyway, > but still be incompatible. Yes it would be better to use one binding, just so long as it is efficient and doesn't introduce a lot of new complexity in U-Boot which is useful only with fdt. > >> /* Use a packed binding which doesn't include >> row,column numbers in each cell */ >> packed-binding; >> matrix-columns = <8>; >> matrix-rows = <16>; >> ascii-binding; /* ASCII characters instead of keycodes */ >> u-boot,keymap = /size/ 8 <00 00 'w' 's' 'a' 'z' 00 DE >> 00 00 00 00 00 00 00 00 >> 00 00 00 00 00 00 00 00 >> '5' '4' 'r' 'e' 'f' 'd' 'x' 00 >> '7' '6' 't' 'h' 'g' 'v' 'c' ' ' >> '9' '8' 'u' 'y' 'j' 'n' 'b' 5C >> '-' '0' 'o' 'i' 'l' 'k' ',' 'm' >> 00 '=' ']' 0D 00 00 00 00 >> 00 00 00 00 DF DF 00 00 >> 00 00 00 00 00 DC 00 DD >> 00 00 00 00 00 00 00 00 >> '[' 'p' 27 ';' '/' '.' 00 00 >> 00 00 08 '3' '2' 1E 00 00 >> 00 7F 00 00 00 1D 1F 1C >> 00 00 00 'q' 00 00 '1' 00 >> 1B '`' 00 09 00 00 00 00>; >> u-boot,keymap-DF = /size/ 8 <00 00 'W' 'S' 'A' 'Z' 00 00 >> 00 00 00 00 00 00 00 00 >> 00 00 00 00 00 00 00 00 >> ... >> >> The DC-DF codes codes denote Shift, Fn and Ctrl keys which would need >> a separate keymap - although we could probably calculate the Ctrl one. > > I had included the row/column property in my binding for KEY_FN, but in > hindsight it's probably just enough to decode that through the regular key > layout, since there will be an entry for it there. OK - just need some signal as to what key codes are modifiers. > >> From a point of view of efficiency, drivers can simply keep a pointer >> to the appropriate property and read out the codes based on >> (row,column) position. >> >> If we have something like this, then in order for the keyboard to work >> in U-Boot, vendors would need to create a completely separate ASCII >> mapping. Yes I agree this is a bit of a pain, but the above map is >> fairly easy to type in, and quite compact. > > It doesn't make sense to come up with a fresh, brand new binding that > requires anyone adding a new device to it to do extra work to do two > redunant descriptions of the same device. We should be able to find common > ground. Yes I hope so. > > The KEY_* codes that are interesting all seem to be in the range of > 0-83. In the u-boot case, that means the base KEY*-to-ascii table would > be 83 bytes. > > That means that the memory consumption for your case above would be > 4*62+4*16=312 bytes for the base+fn table, and then 3*83 = 249 bytes > for the KEY*-to-ASCII tables for regular/shift/ctrl, for a total of > 561 bytes. I'm sure there will be a few special cases in the code just > like you have now for arrow keys, etc, but they don't have to be in the > 83-byte table. > > Sure, that is somewhat more than your 4*128 byte tables, but it is also > a much more flexible binding that is less likely to hit limitations down > the road. Thanks for looking at this. We still have the need of the intermediate table but since that is in BSS I don't think it is a problem. > >> Given the push-back on the U-Boot list from Linux people about my >> bindings, I would like to work out the U-Boot side in this thread if >> possible, since it seems to be dependent on what Linux does. But I >> hope what is created is efficient enough not to bloat U-Boot or >> require an new input layer to be added just to use fdt. > > I think it's a great idea to use a common binding, since there will just be > wasted effort for vendors to have to create two maps and keep them in sync. I will have a look at this approach and see what the impact is, and reply on this thread. Regards, Simon > > > -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
