On Wed, May 21, 2014 at 06:51:49AM +0100, Beomho Seo wrote:
> Add device tree support for mcs touchkey driver.
> Tested on exynos 4412 board.
>
> Signed-off-by: Beomho Seo <beomho.seo@xxxxxxxxxxx>
> Cc: Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>
> ---
> drivers/input/keyboard/mcs_touchkey.c | 73 +++++++++++++++++++++++++++++++--
> 1 file changed, 69 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/keyboard/mcs_touchkey.c b/drivers/input/keyboard/mcs_touchkey.c
> index 1da8e0b..9bff47b 100644
> --- a/drivers/input/keyboard/mcs_touchkey.c
> +++ b/drivers/input/keyboard/mcs_touchkey.c
> @@ -19,6 +19,7 @@
> #include <linux/irq.h>
> #include <linux/slab.h>
> #include <linux/pm.h>
> +#include <linux/of_gpio.h>
>
> /* MCS5000 Touchkey */
> #define MCS5000_TOUCHKEY_STATUS 0x04
> @@ -96,6 +97,60 @@ static irqreturn_t mcs_touchkey_interrupt(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +#ifdef CONFIG_OF
> +static struct mcs_platform_data *mcs_touchkey_parse_dt(struct device *dev)
> +{
> + struct mcs_platform_data *pdata;
> + struct device_node *np = dev->of_node;
> + unsigned int keymap[2];
> + unsigned int len;
> + int i = 0;
> + const __be32 *prop;
Hmm. Almost every use of __be32 *prop values is indicative of something
that can be done with existing accessors. I suspect that may be true
here...
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata) {
> + dev_err(dev, "Failed to allocate platform data\n");
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + prop = of_get_property(np, "linux,code", &len);
> + if (!prop) {
> + dev_err(dev, "Failed to get code\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + if (len % sizeof(u32)) {
> + dev_err(dev, "Malformed keycode property\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + pdata->keymap_size = len / sizeof(u32);
Use of_property_count_u32_elems. It does all of this and returns a
negative error code if anything is wrong.
> +
> + if (of_property_read_u32(np, "key_maxval", &pdata->key_maxval)) {
> + dev_err(dev, "Failed to get key max value data\n");
> + return ERR_PTR(-EINVAL);
> + }
What is this? This sounds like an implementation detail. Why is it in
the DT?
Why doesn't it follow dt conventions ('-' rather than '_')?
> +
> + if (pdata->keymap_size > pdata->key_maxval) {
> + dev_err(dev, "Key map size overflow\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + for (i = 0; i < pdata->keymap_size; i++) {
> + u32 code = be32_to_cpup(prop + i);
Use the DT accessors (e.g. of_property_read_u32_index). There is
absolutely no reason to touch the raw DTB data here.
> + keymap[i] = MCS_KEY_MAP(i, code);
> + }
> + pdata->keymap = keymap;
> + return pdata;
> +}
> +#else
> +static inline struct mcs_platform_data *mcs_touchkey_parse_dt
> + (struct device *dev)
> +{
> + return NULL;
> +}
> +#endif
> +
> static int mcs_touchkey_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> @@ -107,10 +162,14 @@ static int mcs_touchkey_probe(struct i2c_client *client,
> int error;
> int i;
>
> - pdata = dev_get_platdata(&client->dev);
> - if (!pdata) {
> - dev_err(&client->dev, "no platform data defined\n");
> - return -EINVAL;
> + if (&client->dev.of_node)
> + pdata = mcs_touchkey_parse_dt(&client->dev);
> + else
> + pdata = dev_get_platdata(&client->dev);
> +
> + if (IS_ERR(pdata)) {
> + dev_err(&client->dev, "Failed to get platform data\n");
> + return PTR_ERR(pdata);
> }
>
> data = kzalloc(sizeof(struct mcs_touchkey_data) +
> @@ -262,11 +321,17 @@ static const struct i2c_device_id mcs_touchkey_id[] = {
> };
> MODULE_DEVICE_TABLE(i2c, mcs_touchkey_id);
>
> +static struct of_device_id mcs_touchkey_dt_match[] = {
> + { .compatible = "mcs5000_touchkey", },
> + { .compatible = "mcs5080_touchkey", },
NAK. These do not follow the "$VENDOR,$DEVICE" pattern.
Mark.
> +};
> +
> static struct i2c_driver mcs_touchkey_driver = {
> .driver = {
> .name = "mcs_touchkey",
> .owner = THIS_MODULE,
> .pm = &mcs_touchkey_pm_ops,
> + .of_match_table = of_match_ptr(mcs_touchkey_dt_match),
> },
> .probe = mcs_touchkey_probe,
> .remove = mcs_touchkey_remove,
> --
> 1.7.9.5
>
--
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
