| Message ID | 1384250833-4600-1-git-send-email-shc_work@mail.ru |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tue, Nov 12, 2013 at 10:07:13AM +0000, Alexander Shiyan wrote: > This patch adds a new driver for the beeper controlled via GPIO pin. > The driver does not depend on the architecture and is positioned as > a replacement for the specific drivers that are used for this function. > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru> > --- > .../devicetree/bindings/input/gpio-beeper.txt | 15 +++ > drivers/input/misc/Kconfig | 9 ++ > drivers/input/misc/Makefile | 1 + > drivers/input/misc/gpio-beeper.c | 129 +++++++++++++++++++++ > 4 files changed, 154 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/gpio-beeper.txt > create mode 100644 drivers/input/misc/gpio-beeper.c > > diff --git a/Documentation/devicetree/bindings/input/gpio-beeper.txt b/Documentation/devicetree/bindings/input/gpio-beeper.txt > new file mode 100644 > index 0000000..8081605 > --- /dev/null > +++ b/Documentation/devicetree/bindings/input/gpio-beeper.txt > @@ -0,0 +1,15 @@ > +* GPIO beeper device tree bindings > + > +Registers a beeper connected to GPIO pin. > + > +Required properties: > +- compatible: should be "gpio-beeper". > +- gpios: From common gpio binding; gpio connection to beeper enable pin. > + > +Example: > + > +beeper: input@0 { > + compatible = "gpio-beeper"; > + reg = <0>; > + gpios = <&gpio3 23 0>; > +}; What are the reg / unit-address for? Also, a beeper doesn't strike me as an input device, regardless of how the kernel sees it internally. I'd expect the node to just be named "beeper" or something along those lines. Otherwise I believe this looks sane. Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 12, 2013 at 10:47:57AM +0000, Alexander Shiyan wrote: > Hello. > > > On Tue, Nov 12, 2013 at 10:07:13AM +0000, Alexander Shiyan wrote: > > > This patch adds a new driver for the beeper controlled via GPIO pin. > > > The driver does not depend on the architecture and is positioned as > > > a replacement for the specific drivers that are used for this function. > > > > > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru> > ... > > > diff --git a/Documentation/devicetree/bindings/input/gpio-beeper.txt b/Documentation/devicetree/bindings/input/gpio-beeper.txt > ... > > > +Example: > > > + > > > +beeper: input@0 { > > > + compatible = "gpio-beeper"; > > > + reg = <0>; > > > + gpios = <&gpio3 23 0>; > > > +}; > > > > What are the reg / unit-address for? > > Just an example from "simple-bus" container. If they have no meaning, they should go. They're unnecessary and make things more confusing. I'd expect the example to be: beeper: beeper { compatible = "gpio-beeper"; gpios - <&gpio3 23 0>; }; And if we have multiple beepers, something like: beeper0: beeper0 { ... }; beeper1: beeper1 { ... }; Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/12/2013 03:59 AM, Mark Rutland wrote: > On Tue, Nov 12, 2013 at 10:47:57AM +0000, Alexander Shiyan wrote: >> Hello. >> >>> On Tue, Nov 12, 2013 at 10:07:13AM +0000, Alexander Shiyan wrote: >>>> This patch adds a new driver for the beeper controlled via GPIO pin. >>>> The driver does not depend on the architecture and is positioned as >>>> a replacement for the specific drivers that are used for this function. >>>> >>>> Signed-off-by: Alexander Shiyan <shc_work@mail.ru> >> ... >>>> diff --git a/Documentation/devicetree/bindings/input/gpio-beeper.txt b/Documentation/devicetree/bindings/input/gpio-beeper.txt >> ... >>>> +Example: >>>> + >>>> +beeper: input@0 { >>>> + compatible = "gpio-beeper"; >>>> + reg = <0>; >>>> + gpios = <&gpio3 23 0>; >>>> +}; >>> >>> What are the reg / unit-address for? >> >> Just an example from "simple-bus" container. > > If they have no meaning, they should go. They're unnecessary and make > things more confusing. > > I'd expect the example to be: > > beeper: beeper { > compatible = "gpio-beeper"; > gpios - <&gpio3 23 0>; > }; > > And if we have multiple beepers, something like: > > beeper0: beeper0 { ... }; > beeper1: beeper1 { ... }; DT node names aren't meant to encode identity though. What we've done in the past for nodes without a reg where multiple instances were desired is to put them into simple-bus and add a reg, so: beeper0: beeper@0 { reg = <0>; ... }; beeper1: beeper@1 { reg = <1>; ... }; Of course, if there's only one of them, then it could just be "beeper" with no reg. The binding and example should probably reflect that simple case. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 12, 2013 at 12:23:46PM -0700, Stephen Warren wrote: > On 11/12/2013 03:59 AM, Mark Rutland wrote: > > On Tue, Nov 12, 2013 at 10:47:57AM +0000, Alexander Shiyan wrote: > >> Hello. > >> > >>> On Tue, Nov 12, 2013 at 10:07:13AM +0000, Alexander Shiyan wrote: > >>>> This patch adds a new driver for the beeper controlled via GPIO pin. > >>>> The driver does not depend on the architecture and is positioned as > >>>> a replacement for the specific drivers that are used for this function. > >>>> > >>>> Signed-off-by: Alexander Shiyan <shc_work@mail.ru> > >> ... > >>>> diff --git a/Documentation/devicetree/bindings/input/gpio-beeper.txt b/Documentation/devicetree/bindings/input/gpio-beeper.txt > >> ... > >>>> +Example: > >>>> + > >>>> +beeper: input@0 { > >>>> + compatible = "gpio-beeper"; > >>>> + reg = <0>; > >>>> + gpios = <&gpio3 23 0>; > >>>> +}; > >>> > >>> What are the reg / unit-address for? > >> > >> Just an example from "simple-bus" container. > > > > If they have no meaning, they should go. They're unnecessary and make > > things more confusing. > > > > I'd expect the example to be: > > > > beeper: beeper { > > compatible = "gpio-beeper"; > > gpios - <&gpio3 23 0>; > > }; > > > > And if we have multiple beepers, something like: > > > > beeper0: beeper0 { ... }; > > beeper1: beeper1 { ... }; > > DT node names aren't meant to encode identity though. What we've done in > the past for nodes without a reg where multiple instances were desired > is to put them into simple-bus and add a reg, so: > > beeper0: beeper@0 { reg = <0>; ... }; > beeper1: beeper@1 { reg = <1>; ... }; > > Of course, if there's only one of them, then it could just be "beeper" > with no reg. The binding and example should probably reflect that simple > case. So do we have an agreement on bindings? Otherwise the driver looks good to me.
On Tue, 19 Nov 2013 13:32:39 -0800 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > > >>>> This patch adds a new driver for the beeper controlled via GPIO pin. > > >>>> The driver does not depend on the architecture and is positioned as > > >>>> a replacement for the specific drivers that are used for this function. > > >>>> > > >>>> Signed-off-by: Alexander Shiyan <shc_work@mail.ru> > > >> ... > > >>>> diff --git a/Documentation/devicetree/bindings/input/gpio-beeper.txt b/Documentation/devicetree/bindings/input/gpio-beeper.txt > > >> ... > > >>>> +Example: > > >>>> + > > >>>> +beeper: input@0 { > > >>>> + compatible = "gpio-beeper"; > > >>>> + reg = <0>; > > >>>> + gpios = <&gpio3 23 0>; > > >>>> +}; > > >>> > > >>> What are the reg / unit-address for? > > >> > > >> Just an example from "simple-bus" container. > > > > > > If they have no meaning, they should go. They're unnecessary and make > > > things more confusing. > > > > > > I'd expect the example to be: > > > > > > beeper: beeper { > > > compatible = "gpio-beeper"; > > > gpios - <&gpio3 23 0>; > > > }; > > > > > > And if we have multiple beepers, something like: > > > > > > beeper0: beeper0 { ... }; > > > beeper1: beeper1 { ... }; > > > > DT node names aren't meant to encode identity though. What we've done in > > the past for nodes without a reg where multiple instances were desired > > is to put them into simple-bus and add a reg, so: > > > > beeper0: beeper@0 { reg = <0>; ... }; > > beeper1: beeper@1 { reg = <1>; ... }; > > > > Of course, if there's only one of them, then it could just be "beeper" > > with no reg. The binding and example should probably reflect that simple > > case. > > So do we have an agreement on bindings? Otherwise the driver looks good > to me. I'll send v2 of this patch. Thanks.
diff --git a/Documentation/devicetree/bindings/input/gpio-beeper.txt b/Documentation/devicetree/bindings/input/gpio-beeper.txt new file mode 100644 index 0000000..8081605 --- /dev/null +++ b/Documentation/devicetree/bindings/input/gpio-beeper.txt @@ -0,0 +1,15 @@ +* GPIO beeper device tree bindings + +Registers a beeper connected to GPIO pin. + +Required properties: +- compatible: should be "gpio-beeper". +- gpios: From common gpio binding; gpio connection to beeper enable pin. + +Example: + +beeper: input@0 { + compatible = "gpio-beeper"; + reg = <0>; + gpios = <&gpio3 23 0>; +}; diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig index 5f4967d..4ffc397 100644 --- a/drivers/input/misc/Kconfig +++ b/drivers/input/misc/Kconfig @@ -222,6 +222,15 @@ config INPUT_GP2A To compile this driver as a module, choose M here: the module will be called gp2ap002a00f. +config INPUT_GPIO_BEEPER + tristate "Generic GPIO Beeper support" + depends on OF_GPIO + help + Say Y here if you have a beeper connected to a GPIO pin. + + To compile this driver as a module, choose M here: the + module will be called gpio-beeper. + config INPUT_GPIO_TILT_POLLED tristate "Polled GPIO tilt switch" depends on GPIOLIB diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile index 0ebfb6d..cda71fc 100644 --- a/drivers/input/misc/Makefile +++ b/drivers/input/misc/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_INPUT_DA9052_ONKEY) += da9052_onkey.o obj-$(CONFIG_INPUT_DA9055_ONKEY) += da9055_onkey.o obj-$(CONFIG_INPUT_DM355EVM) += dm355evm_keys.o obj-$(CONFIG_INPUT_GP2A) += gp2ap002a00f.o +obj-$(CONFIG_INPUT_GPIO_BEEPER) += gpio-beeper.o obj-$(CONFIG_INPUT_GPIO_TILT_POLLED) += gpio_tilt_polled.o obj-$(CONFIG_HP_SDC_RTC) += hp_sdc_rtc.o obj-$(CONFIG_INPUT_IMS_PCU) += ims-pcu.o diff --git a/drivers/input/misc/gpio-beeper.c b/drivers/input/misc/gpio-beeper.c new file mode 100644 index 0000000..832c838 --- /dev/null +++ b/drivers/input/misc/gpio-beeper.c @@ -0,0 +1,129 @@ +/* + * Generic GPIO beeper driver + * + * Copyright (C) 2013 Alexander Shiyan <shc_work@mail.ru> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include <linux/input.h> +#include <linux/module.h> +#include <linux/of_gpio.h> +#include <linux/workqueue.h> +#include <linux/platform_device.h> + +#define BEEPER_MODNAME "gpio-beeper" + +struct gpio_beeper { + struct work_struct work; + int gpio; + bool active_low; + bool beeping; +}; + +static void gpio_beeper_toggle(struct gpio_beeper *beep, bool on) +{ + gpio_set_value_cansleep(beep->gpio, on ^ beep->active_low); +} + +static void gpio_beeper_work(struct work_struct *work) +{ + struct gpio_beeper *beep = container_of(work, struct gpio_beeper, work); + + gpio_beeper_toggle(beep, beep->beeping); +} + +static int gpio_beeper_event(struct input_dev *dev, unsigned int type, + unsigned int code, int value) +{ + struct gpio_beeper *beep = input_get_drvdata(dev); + + if (type != EV_SND || code != SND_BELL) + return -ENOTSUPP; + + if (value < 0) + return -EINVAL; + + beep->beeping = value; + /* Schedule work to actually turn the beeper on or off */ + schedule_work(&beep->work); + + return 0; +} + +static void gpio_beeper_close(struct input_dev *input) +{ + struct gpio_beeper *beep = input_get_drvdata(input); + + cancel_work_sync(&beep->work); + gpio_beeper_toggle(beep, false); +} + +static int gpio_beeper_probe(struct platform_device *pdev) +{ + struct gpio_beeper *beep; + enum of_gpio_flags flags; + struct input_dev *input; + unsigned long gflags; + int err; + + beep = devm_kzalloc(&pdev->dev, sizeof(*beep), GFP_KERNEL); + if (!beep) + return -ENOMEM; + + beep->gpio = of_get_named_gpio_flags(pdev->dev.of_node, "gpios", + 0, &flags); + if (!gpio_is_valid(beep->gpio)) + return -EINVAL; + + input = devm_input_allocate_device(&pdev->dev); + if (!input) + return -ENOMEM; + + INIT_WORK(&beep->work, gpio_beeper_work); + + input->name = pdev->name; + input->phys = BEEPER_MODNAME "/input0"; + input->id.bustype = BUS_HOST; + input->id.vendor = 0x0001; + input->id.product = 0x0001; + input->id.version = 0x0100; + input->close = gpio_beeper_close; + input->event = gpio_beeper_event; + + input_set_capability(input, EV_SND, SND_BELL); + + beep->active_low = flags & OF_GPIO_ACTIVE_LOW; + gflags = beep->active_low ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW; + + err = devm_gpio_request_one(&pdev->dev, beep->gpio, gflags, pdev->name); + if (err) + return err; + + input_set_drvdata(input, beep); + + return input_register_device(input); +} + +static struct of_device_id gpio_beeper_of_match[] = { + { .compatible = BEEPER_MODNAME, }, + { } +}; +MODULE_DEVICE_TABLE(of, gpio_beeper_of_match); + +static struct platform_driver gpio_beeper_platform_driver = { + .driver = { + .name = BEEPER_MODNAME, + .owner = THIS_MODULE, + .of_match_table = gpio_beeper_of_match, + }, + .probe = gpio_beeper_probe, +}; +module_platform_driver(gpio_beeper_platform_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Alexander Shiyan <shc_work@mail.ru>"); +MODULE_DESCRIPTION("Generic GPIO beeper driver");
This patch adds a new driver for the beeper controlled via GPIO pin. The driver does not depend on the architecture and is positioned as a replacement for the specific drivers that are used for this function. Signed-off-by: Alexander Shiyan <shc_work@mail.ru> --- .../devicetree/bindings/input/gpio-beeper.txt | 15 +++ drivers/input/misc/Kconfig | 9 ++ drivers/input/misc/Makefile | 1 + drivers/input/misc/gpio-beeper.c | 129 +++++++++++++++++++++ 4 files changed, 154 insertions(+) create mode 100644 Documentation/devicetree/bindings/input/gpio-beeper.txt create mode 100644 drivers/input/misc/gpio-beeper.c