| Message ID | 1308042491-20203-2-git-send-email-david@protonic.nl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 14, 2011 at 11:08:09AM +0200, David Jander wrote: > This patch factors out the use of struct platform_device *pdev in most > places. > > Signed-off-by: David Jander <david@protonic.nl> Okay by me. I assume this one isn't needed unless patch 2 is also merged. Acked-by: Grant Likely <grant.likely@secretlab.ca> g. > --- > drivers/input/keyboard/gpio_keys.c | 46 ++++++++++++++++------------------- > 1 files changed, 21 insertions(+), 25 deletions(-) > > diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c > index 6e6145b..987498e 100644 > --- a/drivers/input/keyboard/gpio_keys.c > +++ b/drivers/input/keyboard/gpio_keys.c > @@ -251,8 +251,7 @@ static ssize_t gpio_keys_show_##name(struct device *dev, \ > struct device_attribute *attr, \ > char *buf) \ > { \ > - struct platform_device *pdev = to_platform_device(dev); \ > - struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev); \ > + struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev); \ > \ > return gpio_keys_attr_show_helper(ddata, buf, \ > type, only_disabled); \ > @@ -278,8 +277,7 @@ static ssize_t gpio_keys_store_##name(struct device *dev, \ > const char *buf, \ > size_t count) \ > { \ > - struct platform_device *pdev = to_platform_device(dev); \ > - struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev); \ > + struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev); \ > ssize_t error; \ > \ > error = gpio_keys_attr_store_helper(ddata, buf, type); \ > @@ -364,12 +362,11 @@ static irqreturn_t gpio_keys_isr(int irq, void *dev_id) > return IRQ_HANDLED; > } > > -static int __devinit gpio_keys_setup_key(struct platform_device *pdev, > +static int __devinit gpio_keys_setup_key(struct device *dev, > struct gpio_button_data *bdata, > struct gpio_keys_button *button) > { > const char *desc = button->desc ? button->desc : "gpio_keys"; > - struct device *dev = &pdev->dev; > unsigned long irqflags; > int irq, error; > > @@ -447,9 +444,9 @@ static void gpio_keys_close(struct input_dev *input) > > static int __devinit gpio_keys_probe(struct platform_device *pdev) > { > - struct gpio_keys_platform_data *pdata = pdev->dev.platform_data; > struct gpio_keys_drvdata *ddata; > struct device *dev = &pdev->dev; > + struct gpio_keys_platform_data *pdata = dev->platform_data; > struct input_dev *input; > int i, error; > int wakeup = 0; > @@ -470,12 +467,12 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev) > ddata->disable = pdata->disable; > mutex_init(&ddata->disable_lock); > > - platform_set_drvdata(pdev, ddata); > + dev_set_drvdata(dev, ddata); > input_set_drvdata(input, ddata); > > input->name = pdata->name ? : pdev->name; > input->phys = "gpio-keys/input0"; > - input->dev.parent = &pdev->dev; > + input->dev.parent = dev; > input->open = gpio_keys_open; > input->close = gpio_keys_close; > > @@ -496,7 +493,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev) > bdata->input = input; > bdata->button = button; > > - error = gpio_keys_setup_key(pdev, bdata, button); > + error = gpio_keys_setup_key(dev, bdata, button); > if (error) > goto fail2; > > @@ -506,7 +503,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev) > input_set_capability(input, type, button->code); > } > > - error = sysfs_create_group(&pdev->dev.kobj, &gpio_keys_attr_group); > + error = sysfs_create_group(&dev->kobj, &gpio_keys_attr_group); > if (error) { > dev_err(dev, "Unable to export keys/switches, error: %d\n", > error); > @@ -525,12 +522,12 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev) > gpio_keys_report_event(&ddata->data[i]); > input_sync(input); > > - device_init_wakeup(&pdev->dev, wakeup); > + device_init_wakeup(dev, wakeup); > > return 0; > > fail3: > - sysfs_remove_group(&pdev->dev.kobj, &gpio_keys_attr_group); > + sysfs_remove_group(&dev->kobj, &gpio_keys_attr_group); > fail2: > while (--i >= 0) { > free_irq(gpio_to_irq(pdata->buttons[i].gpio), &ddata->data[i]); > @@ -540,7 +537,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev) > gpio_free(pdata->buttons[i].gpio); > } > > - platform_set_drvdata(pdev, NULL); > + dev_set_drvdata(dev, NULL); > fail1: > input_free_device(input); > kfree(ddata); > @@ -550,14 +547,15 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev) > > static int __devexit gpio_keys_remove(struct platform_device *pdev) > { > - struct gpio_keys_platform_data *pdata = pdev->dev.platform_data; > - struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev); > + struct device *dev = &pdev->dev; > + struct gpio_keys_platform_data *pdata = dev->platform_data; > + struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev); > struct input_dev *input = ddata->input; > int i; > > - sysfs_remove_group(&pdev->dev.kobj, &gpio_keys_attr_group); > + sysfs_remove_group(&dev->kobj, &gpio_keys_attr_group); > > - device_init_wakeup(&pdev->dev, 0); > + device_init_wakeup(dev, 0); > > for (i = 0; i < pdata->nbuttons; i++) { > int irq = gpio_to_irq(pdata->buttons[i].gpio); > @@ -577,11 +575,10 @@ static int __devexit gpio_keys_remove(struct platform_device *pdev) > #ifdef CONFIG_PM > static int gpio_keys_suspend(struct device *dev) > { > - struct platform_device *pdev = to_platform_device(dev); > - struct gpio_keys_platform_data *pdata = pdev->dev.platform_data; > + struct gpio_keys_platform_data *pdata = dev->platform_data; > int i; > > - if (device_may_wakeup(&pdev->dev)) { > + if (device_may_wakeup(dev)) { > for (i = 0; i < pdata->nbuttons; i++) { > struct gpio_keys_button *button = &pdata->buttons[i]; > if (button->wakeup) { > @@ -596,15 +593,14 @@ static int gpio_keys_suspend(struct device *dev) > > static int gpio_keys_resume(struct device *dev) > { > - struct platform_device *pdev = to_platform_device(dev); > - struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev); > - struct gpio_keys_platform_data *pdata = pdev->dev.platform_data; > + struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev); > + struct gpio_keys_platform_data *pdata = dev->platform_data; > int i; > > for (i = 0; i < pdata->nbuttons; i++) { > > struct gpio_keys_button *button = &pdata->buttons[i]; > - if (button->wakeup && device_may_wakeup(&pdev->dev)) { > + if (button->wakeup && device_may_wakeup(dev)) { > int irq = gpio_to_irq(button->gpio); > disable_irq_wake(irq); > } > -- > 1.7.4.1 > -- 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, Jun 14, 2011 at 11:08:09AM +0200, David Jander wrote: > This patch factors out the use of struct platform_device *pdev in most > places. > Why? We are dealing with a platform device so why would we switch to generic device? I also think that we should not be mixing dev_get/set_drvdata() and <bus>_get/set_drvdata() calls but rather use appropriate bus-specific version to access data on given layer. Thanks.
Hi Dmitry, On Sat, 18 Jun 2011 03:19:25 -0700 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Tue, Jun 14, 2011 at 11:08:09AM +0200, David Jander wrote: > > This patch factors out the use of struct platform_device *pdev in most > > places. > > > > Why? We are dealing with a platform device so why would we switch to > generic device? Actually, when I wrote this patch there still was a difference between the platform bus and the of_platform bus, and this change was necessary. There also were ifdefs around platform_driver_register and of_platform_driver_register. Now it seems this has been merged, and I am not sure it is necessary anymore, but I still think it simplifies the code quite a bit. Also, why should the driver be bus-dependent, when it doesn't even need a real "bus" (it talks to an abstract device through another driver, potentially connected to any bus), besides due to how linux views devices and drivers. > I also think that we should not be mixing dev_get/set_drvdata() and > <bus>_get/set_drvdata() calls AFAICS, we are not mixing.... it is dev_*_drvdata() only. > but rather use appropriate bus-specific version to access data on given > layer. Doesn't that make the driver much too complex? And why would that be necessary? The driver isn't bus-specific anymore... except for the binding and probing part. Best regards,
Hi David, On Mon, Jun 20, 2011 at 08:52:13AM +0200, David Jander wrote: > > Hi Dmitry, > > On Sat, 18 Jun 2011 03:19:25 -0700 > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > > > On Tue, Jun 14, 2011 at 11:08:09AM +0200, David Jander wrote: > > > This patch factors out the use of struct platform_device *pdev in most > > > places. > > > > > > > Why? We are dealing with a platform device so why would we switch to > > generic device? > > Actually, when I wrote this patch there still was a difference between > the platform bus and the of_platform bus, and this change was necessary. There > also were ifdefs around platform_driver_register and > of_platform_driver_register. Now it seems this has been merged, and I am > not sure it is necessary anymore, but I still think it simplifies the code > quite a bit. Also, why should the driver be bus-dependent, when it doesn't > even need a real "bus" (it talks to an abstract device through another driver, > potentially connected to any bus), besides due to how linux views devices and > drivers. While there isn't real hardware bus the driver is fitted into platform device framework and so we should use platform device API unless there is compelling reason for using another API. > > > I also think that we should not be mixing dev_get/set_drvdata() and > > <bus>_get/set_drvdata() calls > > AFAICS, we are not mixing.... it is dev_*_drvdata() only. "Mixing" was probably not the best word. "Using API from a different layer" would probably be better. > > > but rather use appropriate bus-specific version to access data on given > > layer. > > Doesn't that make the driver much too complex? And why would that be > necessary? The driver isn't bus-specific anymore... except for the binding and > probing part. Why would it make the driver more complex? Aside from a couple of PM methods coming from the driver core and therefore operating on "struct device *" the rest is using platform device directly. Thanks.
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index 6e6145b..987498e 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -251,8 +251,7 @@ static ssize_t gpio_keys_show_##name(struct device *dev, \ struct device_attribute *attr, \ char *buf) \ { \ - struct platform_device *pdev = to_platform_device(dev); \ - struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev); \ + struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev); \ \ return gpio_keys_attr_show_helper(ddata, buf, \ type, only_disabled); \ @@ -278,8 +277,7 @@ static ssize_t gpio_keys_store_##name(struct device *dev, \ const char *buf, \ size_t count) \ { \ - struct platform_device *pdev = to_platform_device(dev); \ - struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev); \ + struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev); \ ssize_t error; \ \ error = gpio_keys_attr_store_helper(ddata, buf, type); \ @@ -364,12 +362,11 @@ static irqreturn_t gpio_keys_isr(int irq, void *dev_id) return IRQ_HANDLED; } -static int __devinit gpio_keys_setup_key(struct platform_device *pdev, +static int __devinit gpio_keys_setup_key(struct device *dev, struct gpio_button_data *bdata, struct gpio_keys_button *button) { const char *desc = button->desc ? button->desc : "gpio_keys"; - struct device *dev = &pdev->dev; unsigned long irqflags; int irq, error; @@ -447,9 +444,9 @@ static void gpio_keys_close(struct input_dev *input) static int __devinit gpio_keys_probe(struct platform_device *pdev) { - struct gpio_keys_platform_data *pdata = pdev->dev.platform_data; struct gpio_keys_drvdata *ddata; struct device *dev = &pdev->dev; + struct gpio_keys_platform_data *pdata = dev->platform_data; struct input_dev *input; int i, error; int wakeup = 0; @@ -470,12 +467,12 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev) ddata->disable = pdata->disable; mutex_init(&ddata->disable_lock); - platform_set_drvdata(pdev, ddata); + dev_set_drvdata(dev, ddata); input_set_drvdata(input, ddata); input->name = pdata->name ? : pdev->name; input->phys = "gpio-keys/input0"; - input->dev.parent = &pdev->dev; + input->dev.parent = dev; input->open = gpio_keys_open; input->close = gpio_keys_close; @@ -496,7 +493,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev) bdata->input = input; bdata->button = button; - error = gpio_keys_setup_key(pdev, bdata, button); + error = gpio_keys_setup_key(dev, bdata, button); if (error) goto fail2; @@ -506,7 +503,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev) input_set_capability(input, type, button->code); } - error = sysfs_create_group(&pdev->dev.kobj, &gpio_keys_attr_group); + error = sysfs_create_group(&dev->kobj, &gpio_keys_attr_group); if (error) { dev_err(dev, "Unable to export keys/switches, error: %d\n", error); @@ -525,12 +522,12 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev) gpio_keys_report_event(&ddata->data[i]); input_sync(input); - device_init_wakeup(&pdev->dev, wakeup); + device_init_wakeup(dev, wakeup); return 0; fail3: - sysfs_remove_group(&pdev->dev.kobj, &gpio_keys_attr_group); + sysfs_remove_group(&dev->kobj, &gpio_keys_attr_group); fail2: while (--i >= 0) { free_irq(gpio_to_irq(pdata->buttons[i].gpio), &ddata->data[i]); @@ -540,7 +537,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev) gpio_free(pdata->buttons[i].gpio); } - platform_set_drvdata(pdev, NULL); + dev_set_drvdata(dev, NULL); fail1: input_free_device(input); kfree(ddata); @@ -550,14 +547,15 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev) static int __devexit gpio_keys_remove(struct platform_device *pdev) { - struct gpio_keys_platform_data *pdata = pdev->dev.platform_data; - struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev); + struct device *dev = &pdev->dev; + struct gpio_keys_platform_data *pdata = dev->platform_data; + struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev); struct input_dev *input = ddata->input; int i; - sysfs_remove_group(&pdev->dev.kobj, &gpio_keys_attr_group); + sysfs_remove_group(&dev->kobj, &gpio_keys_attr_group); - device_init_wakeup(&pdev->dev, 0); + device_init_wakeup(dev, 0); for (i = 0; i < pdata->nbuttons; i++) { int irq = gpio_to_irq(pdata->buttons[i].gpio); @@ -577,11 +575,10 @@ static int __devexit gpio_keys_remove(struct platform_device *pdev) #ifdef CONFIG_PM static int gpio_keys_suspend(struct device *dev) { - struct platform_device *pdev = to_platform_device(dev); - struct gpio_keys_platform_data *pdata = pdev->dev.platform_data; + struct gpio_keys_platform_data *pdata = dev->platform_data; int i; - if (device_may_wakeup(&pdev->dev)) { + if (device_may_wakeup(dev)) { for (i = 0; i < pdata->nbuttons; i++) { struct gpio_keys_button *button = &pdata->buttons[i]; if (button->wakeup) { @@ -596,15 +593,14 @@ static int gpio_keys_suspend(struct device *dev) static int gpio_keys_resume(struct device *dev) { - struct platform_device *pdev = to_platform_device(dev); - struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev); - struct gpio_keys_platform_data *pdata = pdev->dev.platform_data; + struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev); + struct gpio_keys_platform_data *pdata = dev->platform_data; int i; for (i = 0; i < pdata->nbuttons; i++) { struct gpio_keys_button *button = &pdata->buttons[i]; - if (button->wakeup && device_may_wakeup(&pdev->dev)) { + if (button->wakeup && device_may_wakeup(dev)) { int irq = gpio_to_irq(button->gpio); disable_irq_wake(irq); }
This patch factors out the use of struct platform_device *pdev in most places. Signed-off-by: David Jander <david@protonic.nl> --- drivers/input/keyboard/gpio_keys.c | 46 ++++++++++++++++------------------- 1 files changed, 21 insertions(+), 25 deletions(-)