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

[Linus Walleij]

On Thu, Oct 31, 2013 at 5:58 PM, Mark Rutland <mark.rutland@xxxxxxx> wrote:

>> +     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
--
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/