Re: [PATCH 4/5] input: tc3589x-keypad: support probing from device tree

On Thu, Oct 31, 2013 at 5:58 PM, Mark Rutland [off-list ref] wrote:

quoted

+     plat = devm_kzalloc(dev, sizeof(*plat), GFP_KERNEL);
+     if (!plat)
+             return ERR_PTR(-ENOMEM);
+
+     of_property_read_u8(np, "keypad,num-columns", &plat->kcol);
+     of_property_read_u8(np, "keypad,num-rows", &plat->krow);

These look wrong to me, as almost every single use of of_property_read_u8 (or
of_property_read_u16) do. They read _packed_ values out of the dt, and do not
read (u32) cells as u8s or u16s.

Yes...

The matrix-keymap binding doesn't define these as 8-bit, and the example
binding they are u32 cells. Either the binding document or this code is wrong.

The biding is in patch titled:
"mfd: tc3589x: Add device tree bindings"
and yes, you are right, this seems wrong. (The example in that patch
is wrong too.)

I'm confused as to how this can work. Are you using /bits/ 8 in your dts?

Yes indeed I do. So it was working fine...

I'll adjust this to use u32:s, even if it seems odd supporting keypads
with 255+ columns and rows... well it's in there.

Yours,
Linus Walleij