| Message ID | 1381872071-23455-1-git-send-email-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 15, 2013 at 11:21:11PM +0200, Linus Walleij wrote: > Implement device tree probing for the tc3589x keypad driver. > This is modeled on the STMPE keypad driver and tested on the > Ux500 TVK1281618 UIB. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Looks good, should I merge it or you want to take it through another tree?? > --- > drivers/input/keyboard/tc3589x-keypad.c | 63 +++++++++++++++++++++++++++++++-- > 1 file changed, 61 insertions(+), 2 deletions(-) > > diff --git a/drivers/input/keyboard/tc3589x-keypad.c b/drivers/input/keyboard/tc3589x-keypad.c > index 208de7c..f6ec0d7 100644 > --- a/drivers/input/keyboard/tc3589x-keypad.c > +++ b/drivers/input/keyboard/tc3589x-keypad.c > @@ -297,6 +297,62 @@ static void tc3589x_keypad_close(struct input_dev *input) > tc3589x_keypad_disable(keypad); > } > > +#ifdef CONFIG_OF > +static const struct tc3589x_keypad_platform_data * > +tc3589x_keypad_of_probe(struct device *dev) > +{ > + struct device_node *np = dev->of_node; > + struct tc3589x_keypad_platform_data *plat; > + u32 debounce_ms; > + int proplen; > + > + if (!np) > + return ERR_PTR(-ENODEV); > + > + 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); > + if (!plat->krow || !plat->kcol || > + plat->krow > TC_KPD_ROWS || plat->kcol > TC_KPD_COLUMNS) { > + dev_err(dev, > + "keypad columns/rows not properly specified (%ux%u)\n", > + plat->kcol, plat->krow); > + return ERR_PTR(-EINVAL); > + } > + > + if (!of_get_property(np, "linux,keymap", &proplen)) { > + dev_err(dev, "property linux,keymap not found\n"); > + return ERR_PTR(-ENOENT); > + } > + > + plat->no_autorepeat = of_property_read_bool(np, "linux,no-autorepeat"); > + plat->enable_wakeup = of_property_read_bool(np, "linux,wakeup"); > + > + /* The custom delay format is ms/16 */ > + of_property_read_u32(np, "debounce-delay-ms", &debounce_ms); > + if (debounce_ms) > + plat->debounce_period = debounce_ms * 16; > + else > + plat->debounce_period = TC_KPD_DEBOUNCE_PERIOD; > + > + plat->settle_time = TC_KPD_SETTLE_TIME; > + /* FIXME: should be property of the IRQ resource? */ > + plat->irqtype = IRQF_TRIGGER_FALLING; > + > + return plat; > +} > +#else > +static inline const struct tc3589x_keypad_platform_data * > +tc3589x_keypad_of_probe(struct device *dev) > +{ > + return ERR_PTR(-ENODEV); > +} > +#endif > + > + > static int tc3589x_keypad_probe(struct platform_device *pdev) > { > struct tc3589x *tc3589x = dev_get_drvdata(pdev->dev.parent); > @@ -307,8 +363,11 @@ static int tc3589x_keypad_probe(struct platform_device *pdev) > > plat = tc3589x->pdata->keypad; > if (!plat) { > - dev_err(&pdev->dev, "invalid keypad platform data\n"); > - return -EINVAL; > + plat = tc3589x_keypad_of_probe(&pdev->dev); > + if (IS_ERR(plat)) { > + dev_err(&pdev->dev, "invalid keypad platform data\n"); > + return PTR_ERR(plat); > + } > } > > irq = platform_get_irq(pdev, 0); > -- > 1.8.3.1 >
On Wed, Oct 16, 2013 at 8:39 AM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Tue, Oct 15, 2013 at 11:21:11PM +0200, Linus Walleij wrote: >> Implement device tree probing for the tc3589x keypad driver. >> This is modeled on the STMPE keypad driver and tested on the >> Ux500 TVK1281618 UIB. >> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > Looks good, should I merge it or you want to take it through another > tree?? As this one is only using old device tree bindings (Documentation/devicetree/bindings/input/matrix-keymap.txt and friends) I think you can just merge it. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 15, 2013 at 10:21:11PM +0100, Linus Walleij wrote: > Implement device tree probing for the tc3589x keypad driver. > This is modeled on the STMPE keypad driver and tested on the > Ux500 TVK1281618 UIB. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/input/keyboard/tc3589x-keypad.c | 63 +++++++++++++++++++++++++++++++-- > 1 file changed, 61 insertions(+), 2 deletions(-) > > diff --git a/drivers/input/keyboard/tc3589x-keypad.c b/drivers/input/keyboard/tc3589x-keypad.c > index 208de7c..f6ec0d7 100644 > --- a/drivers/input/keyboard/tc3589x-keypad.c > +++ b/drivers/input/keyboard/tc3589x-keypad.c > @@ -297,6 +297,62 @@ static void tc3589x_keypad_close(struct input_dev *input) > tc3589x_keypad_disable(keypad); > } > > +#ifdef CONFIG_OF > +static const struct tc3589x_keypad_platform_data * > +tc3589x_keypad_of_probe(struct device *dev) > +{ > + struct device_node *np = dev->of_node; > + struct tc3589x_keypad_platform_data *plat; > + u32 debounce_ms; > + int proplen; > + > + if (!np) > + return ERR_PTR(-ENODEV); > + > + 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. 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. I'm confused as to how this can work. Are you using /bits/ 8 in your dts? Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 31, 2013 at 5:58 PM, Mark Rutland <mark.rutland@arm.com> 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-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/input/keyboard/tc3589x-keypad.c b/drivers/input/keyboard/tc3589x-keypad.c index 208de7c..f6ec0d7 100644 --- a/drivers/input/keyboard/tc3589x-keypad.c +++ b/drivers/input/keyboard/tc3589x-keypad.c @@ -297,6 +297,62 @@ static void tc3589x_keypad_close(struct input_dev *input) tc3589x_keypad_disable(keypad); } +#ifdef CONFIG_OF +static const struct tc3589x_keypad_platform_data * +tc3589x_keypad_of_probe(struct device *dev) +{ + struct device_node *np = dev->of_node; + struct tc3589x_keypad_platform_data *plat; + u32 debounce_ms; + int proplen; + + if (!np) + return ERR_PTR(-ENODEV); + + 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); + if (!plat->krow || !plat->kcol || + plat->krow > TC_KPD_ROWS || plat->kcol > TC_KPD_COLUMNS) { + dev_err(dev, + "keypad columns/rows not properly specified (%ux%u)\n", + plat->kcol, plat->krow); + return ERR_PTR(-EINVAL); + } + + if (!of_get_property(np, "linux,keymap", &proplen)) { + dev_err(dev, "property linux,keymap not found\n"); + return ERR_PTR(-ENOENT); + } + + plat->no_autorepeat = of_property_read_bool(np, "linux,no-autorepeat"); + plat->enable_wakeup = of_property_read_bool(np, "linux,wakeup"); + + /* The custom delay format is ms/16 */ + of_property_read_u32(np, "debounce-delay-ms", &debounce_ms); + if (debounce_ms) + plat->debounce_period = debounce_ms * 16; + else + plat->debounce_period = TC_KPD_DEBOUNCE_PERIOD; + + plat->settle_time = TC_KPD_SETTLE_TIME; + /* FIXME: should be property of the IRQ resource? */ + plat->irqtype = IRQF_TRIGGER_FALLING; + + return plat; +} +#else +static inline const struct tc3589x_keypad_platform_data * +tc3589x_keypad_of_probe(struct device *dev) +{ + return ERR_PTR(-ENODEV); +} +#endif + + static int tc3589x_keypad_probe(struct platform_device *pdev) { struct tc3589x *tc3589x = dev_get_drvdata(pdev->dev.parent); @@ -307,8 +363,11 @@ static int tc3589x_keypad_probe(struct platform_device *pdev) plat = tc3589x->pdata->keypad; if (!plat) { - dev_err(&pdev->dev, "invalid keypad platform data\n"); - return -EINVAL; + plat = tc3589x_keypad_of_probe(&pdev->dev); + if (IS_ERR(plat)) { + dev_err(&pdev->dev, "invalid keypad platform data\n"); + return PTR_ERR(plat); + } } irq = platform_get_irq(pdev, 0);
Implement device tree probing for the tc3589x keypad driver. This is modeled on the STMPE keypad driver and tested on the Ux500 TVK1281618 UIB. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/input/keyboard/tc3589x-keypad.c | 63 +++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 2 deletions(-)