Re: [RESEND/PATCHv5 1/2] drivers: input: keypad: Add device tree support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dmitry,

On Wed, Jul 11, 2012 at 3:15 PM, Poddar, Sourav <sourav.poddar@xxxxxx> wrote:
> Hi Dmitry,
>
> On Tue, Jul 10, 2012 at 11:43 AM, Dmitry Torokhov
> <dmitry.torokhov@xxxxxxxxx> wrote:
>> Hi Sourav,
>>
>> On Fri, Jun 08, 2012 at 04:22:59PM +0530, Sourav Poddar wrote:
>>> Update the Documentation with omap4 keypad device tree
>>> binding information.
>>> Add device tree support for omap4 keypad driver.
>>>
>>> Tested on omap4430 sdp.
>>>
>>
>> Sorry for the delay, I have a few comments:
>>
>>>
>>>       /* platform data */
>>>       pdata = pdev->dev.platform_data;
>>> -     if (!pdata) {
>>> +     if (np) {
>>> +             of_property_read_u32(np, "keypad,num-rows", &num_rows);
>>> +             of_property_read_u32(np, "keypad,num-columns", &num_cols);
>>> +             if (!num_rows || !num_cols) {
>>> +                     dev_err(&pdev->dev, "number of keypad rows/columns not specified\n");
>>> +                     return -EINVAL;
>>> +             }
>>> +     } else if (pdata) {
>>> +             num_rows = pdata->rows;
>>> +             num_cols = pdata->cols;
>>> +     } else {
>>>               dev_err(&pdev->dev, "no platform data defined\n");
>>>               return -EINVAL;
>>>       }
>>>
>>
>> I believe drivers should use platform data if it is supplied and use DT
>> data if platform data is omitted. This way one can override firmware
>> data if needed.
>>
>> Does the patch below (if applied on top of your) work for you?
>>
> Yes, the above patch works fine for me.
> Acked-by: Sourav Poddar <sourav.poddar@xxxxxx>
>
> Note, I did some mux setting changes in the bootloader for DT case.
>
To be explicit, these was done throughout for testing. Nothing specific to your
incremental patch.
> Thanks,
> Sourav
>> Thanks.
>>
>> --
>> Dmitry
>>
>> Input: omap4-keypad - misc fixes
>>
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
>> ---
>>
>>  drivers/input/keyboard/omap4-keypad.c |  152 ++++++++++++++++-----------------
>>  1 file changed, 74 insertions(+), 78 deletions(-)
>>
>>
>> diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
>> index d5a2d1a..033168e 100644
>> --- a/drivers/input/keyboard/omap4-keypad.c
>> +++ b/drivers/input/keyboard/omap4-keypad.c
>> @@ -76,7 +76,6 @@ enum {
>>
>>  struct omap4_keypad {
>>         struct input_dev *input;
>> -       struct matrix_keymap_data *keymap_data;
>>
>>         void __iomem *base;
>>         unsigned int irq;
>> @@ -88,7 +87,7 @@ struct omap4_keypad {
>>         unsigned int row_shift;
>>         bool no_autorepeat;
>>         unsigned char key_state[8];
>> -       unsigned short keymap[];
>> +       unsigned short *keymap;
>>  };
>>
>>  static int kbd_readl(struct omap4_keypad *keypad_data, u32 offset)
>> @@ -211,74 +210,52 @@ static void omap4_keypad_close(struct input_dev *input)
>>         pm_runtime_put_sync(input->dev.parent);
>>  }
>>
>> -static struct omap4_keypad *omap_keypad_parse_dt(struct device *dev,
>> -                               uint32_t rows, uint32_t cols,
>> -                               struct input_dev *input_dev)
>> +#ifdef CONFIG_OF
>> +static int __devinit omap4_keypad_parse_dt(struct device *dev,
>> +                                          struct omap4_keypad *keypad_data)
>>  {
>>         struct device_node *np = dev->of_node;
>> -       struct platform_device *pdev = to_platform_device(dev);
>> -       struct omap4_keypad *keypad_data = platform_get_drvdata(pdev);
>>         int error;
>>
>> -       error = matrix_keypad_build_keymap(NULL, "linux,keymap",
>> -                               rows, cols, keypad_data->keymap, input_dev);
>> -       if (error) {
>> -               dev_err(&pdev->dev, "failed to build keymap\n");
>> -               input_free_device(input_dev);
>> +       if (!np) {
>> +               dev_err(dev, "missing DT data");
>> +               return -EINVAL;
>> +       }
>> +
>> +       of_property_read_u32(np, "keypad,num-rows", &keypad_data->rows);
>> +       of_property_read_u32(np, "keypad,num-columns", &keypad_data->cols);
>> +       if (!keypad_data->rows || !keypad_data->cols) {
>> +               dev_err(dev, "number of keypad rows/columns not specified\n");
>> +               return -EINVAL;
>>         }
>>
>>         if (of_get_property(np, "linux,input-no-autorepeat", NULL))
>>                 keypad_data->no_autorepeat = true;
>>
>> -       return keypad_data;
>> +       return 0;
>> +}
>> +#else
>> +static inline int omap4_keypad_parse_dt(struct device *dev,
>> +                                       struct omap4_keypad *keypad_data)
>> +{
>> +       return -ENOSYS;
>>  }
>> +#endif
>>
>>  static int __devinit omap4_keypad_probe(struct platform_device *pdev)
>>  {
>> -       struct device *dev = &pdev->dev;
>> -       struct device_node *np = dev->of_node;
>> -       const struct omap4_keypad_platform_data *pdata;
>> +       const struct omap4_keypad_platform_data *pdata =
>> +                               dev_get_platdata(&pdev->dev);
>> +       const struct matrix_keymap_data *keymap_data =
>> +                               pdata ? pdata->keymap_data : NULL;
>>         struct omap4_keypad *keypad_data;
>>         struct input_dev *input_dev;
>>         struct resource *res;
>> -       resource_size_t size;
>> -       unsigned int row_shift = 0, max_keys = 0;
>> -       uint32_t num_rows = 0, num_cols = 0;
>> +       unsigned int max_keys;
>>         int rev;
>>         int irq;
>>         int error;
>>
>> -       /* platform data */
>> -       pdata = pdev->dev.platform_data;
>> -       if (np) {
>> -               of_property_read_u32(np, "keypad,num-rows", &num_rows);
>> -               of_property_read_u32(np, "keypad,num-columns", &num_cols);
>> -               if (!num_rows || !num_cols) {
>> -                       dev_err(&pdev->dev, "number of keypad rows/columns not specified\n");
>> -                       return -EINVAL;
>> -               }
>> -       } else if (pdata) {
>> -               num_rows = pdata->rows;
>> -               num_cols = pdata->cols;
>> -       } else {
>> -               dev_err(&pdev->dev, "no platform data defined\n");
>> -               return -EINVAL;
>> -       }
>> -
>> -       row_shift = get_count_order(num_cols);
>> -       max_keys = num_rows << row_shift;
>> -
>> -       keypad_data = devm_kzalloc(dev, sizeof(struct omap4_keypad) +
>> -                       max_keys * sizeof(keypad_data->keymap[0]),
>> -                               GFP_KERNEL);
>> -
>> -       if (!keypad_data) {
>> -               dev_err(&pdev->dev, "keypad_data memory allocation failed\n");
>> -               return -ENOMEM;
>> -       }
>> -
>> -       platform_set_drvdata(pdev, keypad_data);
>> -
>>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>         if (!res) {
>>                 dev_err(&pdev->dev, "no base address specified\n");
>> @@ -291,9 +268,24 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
>>                 return -EINVAL;
>>         }
>>
>> -       size = resource_size(res);
>> +       keypad_data = kzalloc(sizeof(struct omap4_keypad), GFP_KERNEL);
>> +       if (!keypad_data) {
>> +               dev_err(&pdev->dev, "keypad_data memory allocation failed\n");
>> +               return -ENOMEM;
>> +       }
>> +
>> +       keypad_data->irq = irq;
>> +
>> +       if (pdata) {
>> +               keypad_data->rows = pdata->rows;
>> +               keypad_data->cols = pdata->cols;
>> +       } else {
>> +               error = omap4_keypad_parse_dt(&pdev->dev, keypad_data);
>> +               if (error)
>> +                       return error;
>> +       }
>>
>> -       res = request_mem_region(res->start, size, pdev->name);
>> +       res = request_mem_region(res->start, resource_size(res), pdev->name);
>>         if (!res) {
>>                 dev_err(&pdev->dev, "can't request mem region\n");
>>                 error = -EBUSY;
>> @@ -307,15 +299,11 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
>>                 goto err_release_mem;
>>         }
>>
>> -       keypad_data->rows = num_rows;
>> -       keypad_data->cols = num_cols;
>> -       keypad_data->irq = irq;
>> -       keypad_data->row_shift = row_shift;
>>
>>         /*
>> -       * Enable clocks for the keypad module so that we can read
>> -       * revision register.
>> -       */
>> +        * Enable clocks for the keypad module so that we can read
>> +        * revision register.
>> +        */
>>         pm_runtime_enable(&pdev->dev);
>>         error = pm_runtime_get_sync(&pdev->dev);
>>         if (error) {
>> @@ -358,29 +346,30 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
>>         input_dev->open = omap4_keypad_open;
>>         input_dev->close = omap4_keypad_close;
>>
>> -       if (np) {
>> -               keypad_data = omap_keypad_parse_dt(&pdev->dev,
>> -                               keypad_data->rows, keypad_data->cols,
>> -                               input_dev);
>> -       } else {
>> -               keypad_data->keymap_data =
>> -                       (struct matrix_keymap_data *)pdata->keymap_data;
>> -               error = matrix_keypad_build_keymap(keypad_data->keymap_data,
>> -                               NULL, keypad_data->rows, keypad_data->cols,
>> -                               keypad_data->keymap, input_dev);
>> -               if (error) {
>> -                       dev_err(&pdev->dev, "failed to build keymap\n");
>> -                       goto err_free_input;
>> -               }
>> -       }
>> -
>> +       input_set_capability(input_dev, EV_MSC, MSC_SCAN);
>>         if (!keypad_data->no_autorepeat)
>>                 __set_bit(EV_REP, input_dev->evbit);
>>
>> -       input_set_capability(input_dev, EV_MSC, MSC_SCAN);
>> -
>>         input_set_drvdata(input_dev, keypad_data);
>>
>> +       keypad_data->row_shift = get_count_order(keypad_data->cols);
>> +       max_keys = keypad_data->rows << keypad_data->row_shift;
>> +       keypad_data->keymap = kzalloc(max_keys * sizeof(keypad_data->keymap[0]),
>> +                                     GFP_KERNEL);
>> +       if (!keypad_data->keymap) {
>> +               dev_err(&pdev->dev, "Not enough memory for keymap\n");
>> +               error = -ENOMEM;
>> +               goto err_free_input;
>> +       }
>> +
>> +       error = matrix_keypad_build_keymap(keymap_data, NULL,
>> +                                          keypad_data->rows, keypad_data->cols,
>> +                                          keypad_data->keymap, input_dev);
>> +       if (error) {
>> +               dev_err(&pdev->dev, "failed to build keymap\n");
>> +               goto err_free_keymap;
>> +       }
>> +
>>         error = request_irq(keypad_data->irq, omap4_keypad_interrupt,
>>                              IRQF_TRIGGER_RISING,
>>                              "omap4-keypad", keypad_data);
>> @@ -397,11 +386,14 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
>>                 goto err_pm_disable;
>>         }
>>
>> +       platform_set_drvdata(pdev, keypad_data);
>>         return 0;
>>
>>  err_pm_disable:
>>         pm_runtime_disable(&pdev->dev);
>>         free_irq(keypad_data->irq, keypad_data);
>> +err_free_keymap:
>> +       kfree(keypad_data->keymap);
>>  err_free_input:
>>         input_free_device(input_dev);
>>  err_pm_put_sync:
>> @@ -409,7 +401,7 @@ err_pm_put_sync:
>>  err_unmap:
>>         iounmap(keypad_data->base);
>>  err_release_mem:
>> -       release_mem_region(res->start, size);
>> +       release_mem_region(res->start, resource_size(res));
>>  err_free_keypad:
>>         kfree(keypad_data);
>>         return error;
>> @@ -431,17 +423,21 @@ static int __devexit omap4_keypad_remove(struct platform_device *pdev)
>>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>         release_mem_region(res->start, resource_size(res));
>>
>> +       kfree(keypad_data->keymap);
>>         kfree(keypad_data);
>> +
>>         platform_set_drvdata(pdev, NULL);
>>
>>         return 0;
>>  }
>>
>> +#ifdef CONFIG_OF
>>  static const struct of_device_id omap_keypad_dt_match[] = {
>>         { .compatible = "ti,omap4-keypad" },
>>         {},
>>  };
>>  MODULE_DEVICE_TABLE(of, omap_keypad_dt_match);
>> +#endif
>>
>>  static struct platform_driver omap4_keypad_driver = {
>>         .probe          = omap4_keypad_probe,
--
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


[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux