From patchwork Fri Nov 30 06:28:15 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Torokhov X-Patchwork-Id: 1823771 Return-Path: X-Original-To: patchwork-linux-input@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id B68F53FC23 for ; Fri, 30 Nov 2012 06:28:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755797Ab2K3G2X (ORCPT ); Fri, 30 Nov 2012 01:28:23 -0500 Received: from mail-pa0-f46.google.com ([209.85.220.46]:58604 "EHLO mail-pa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755693Ab2K3G2V (ORCPT ); Fri, 30 Nov 2012 01:28:21 -0500 Received: by mail-pa0-f46.google.com with SMTP id bh2so124219pad.19 for ; Thu, 29 Nov 2012 22:28:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=51UqDL7iYGBtPBCElXJq/UKN0HoyAbM58t2YyJPP5VU=; b=q0yf/Q5Q/DXfflKtPaw2HZCFxpZQ+/JEntwhEq96Z3zuhJpnxXw/VE6CbzbkrRC8vG jKM/NhCbVFYARpY32nL+zqUga4vXmtea3Cr92WMtG8flS82wgP2CCarE7S6Zy/VKNL33 nSPHz/8//IVrg5/En95ATD5gL1FKLxaw/54mLJRWztKFktsuxue5HgxqOQO3XLkaQ5qV JPIdoZgqRx5ablOveY728J5erD9uCrRD5CC4hhgZzJLzOgBoZPbTe7BL+74HtiN1PvS4 mGad/5NXthhVzFHCVkpiSeAiTiqGWAZpA6SOP/B/EhCegQmmgVALWaw1UMhItoggs/on HVvg== Received: by 10.66.88.197 with SMTP id bi5mr128297pab.36.1354256899242; Thu, 29 Nov 2012 22:28:19 -0800 (PST) Received: from mailhub.coreip.homeip.net (c-67-188-112-76.hsd1.ca.comcast.net. [67.188.112.76]) by mx.google.com with ESMTPS id uq5sm2515618pbc.56.2012.11.29.22.28.16 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 29 Nov 2012 22:28:17 -0800 (PST) Date: Thu, 29 Nov 2012 22:28:15 -0800 From: Dmitry Torokhov To: Alexander Shiyan Cc: linux-input@vger.kernel.org, Arnd Bergmann Subject: Re: [PATCH RFC v2] input: Add new driver for GPIO beeper Message-ID: <20121130062815.GA27232@core.coreip.homeip.net> References: <1354210139-25374-1-git-send-email-shc_work@mail.ru> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1354210139-25374-1-git-send-email-shc_work@mail.ru> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org Hi Alexander, On Thu, Nov 29, 2012 at 09:28:59PM +0400, 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, > for example drivers/input/misc/ixp4xx-beeper. > Since this patch is RFC only, inline comments are welcome. Your signed-off-by is missing... > + > +static int gpio_beeper_event(struct input_dev *dev, unsigned int type, > + unsigned int code, int value) > +{ > + struct gpio_beeper_priv *s = input_get_drvdata(dev); > + > + if ((type != EV_SND) || (code != SND_BELL)) > + return -ENOTSUPP; > + > + if (value < 0) > + return -EINVAL; > + > + if (!value) > + value = 1000; > + > + /* Turning beeper ON */ > + gpio_beeper_change(s, 1); You are running under spinlock, you can't touch GPIO here using "cansleep" operations. > + /* Schedule work for turning OFF */ > + mod_delayed_work(system_wq, &s->work, msecs_to_jiffies(value)); > + This is incorrect. The "value" here is pitch, not the duration of sound. The callers are responsible to shut off the sound. The difference between SND_BELL and SND_TONE is that latter allows controlling pitch while the former uses driver- and platform-specific pitch. I think the driver should look like in the patch below. Can you please tell me if it works for you? Thanks! diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig index 6d2bf0c..0d9b0eb 100644 --- a/drivers/input/misc/Kconfig +++ b/drivers/input/misc/Kconfig @@ -219,6 +219,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 GPIOLIB + 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 GENERIC_GPIO diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile index 1f874af..662b39a 100644 --- a/drivers/input/misc/Makefile +++ b/drivers/input/misc/Makefile @@ -26,6 +26,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_IXP4XX_BEEPER) += ixp4xx-beeper.o diff --git a/drivers/input/misc/gpio-beeper.c b/drivers/input/misc/gpio-beeper.c new file mode 100644 index 0000000..3408d43 --- /dev/null +++ b/drivers/input/misc/gpio-beeper.c @@ -0,0 +1,154 @@ +/* + * Generic GPIO beeper driver + * + * Copyright (C) 2012 Alexander Shiyan + * + * 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 +#include +#include +#include +#include +#include + +#include + +struct gpio_beeper { + struct input_dev *input; + struct work_struct work; + char phys[32]; + int gpio_nr; + bool active_low; + bool beeping; +}; + +static void gpio_beeper_toggle(struct gpio_beeper *beeper, bool on) +{ + gpio_set_value_cansleep(beeper->gpio_nr, on ^ beeper->active_low); +} + +static void gpio_beeper_work(struct work_struct *work) +{ + struct gpio_beeper *beeper = + container_of(work, struct gpio_beeper, work); + + gpio_beeper_toggle(beeper, beeper->beeping); +} + +static int gpio_beeper_event(struct input_dev *dev, unsigned int type, + unsigned int code, int value) +{ + struct gpio_beeper *beeper = input_get_drvdata(dev); + + if (type != EV_SND || code != SND_BELL) + return -ENOTSUPP; + + if (value < 0) + return -EINVAL; + + beeper->beeping = value; + /* Schedule work to actually turn the beeper on or off */ + schedule_work(&beeper->work); + + return 0; +} + +static void gpio_beeper_close(struct input_dev *input) +{ + struct gpio_beeper *beeper = input_get_drvdata(input); + + cancel_work_sync(&beeper->work); + gpio_beeper_toggle(beeper, false); +} + +static int gpio_beeper_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + const struct gpio_beeper_pdata *pdata = dev_get_platdata(dev); + struct gpio_beeper *beeper; + struct input_dev *input; + int error; + + if (!pdata) { + dev_err(dev, "Missing platform data\n"); + return -EINVAL; + } + + if (!gpio_is_valid(pdata->gpio_nr)) { + dev_err(dev, "Invalid gpio %i\n", pdata->gpio_nr); + return -EINVAL; + } + + beeper = devm_kzalloc(&pdev->dev, sizeof(struct gpio_beeper), + GFP_KERNEL); + if (!beeper) { + dev_err(dev, "Memory allocate error\n"); + return -ENOMEM; + } + + beeper->gpio_nr = pdata->gpio_nr; + beeper->active_low = pdata->active_low; + + INIT_WORK(&beeper->work, gpio_beeper_work); + snprintf(beeper->phys, sizeof(beeper->phys), + "%s/input0", dev_name(dev)); + + input = devm_input_allocate_device(dev); + if (!input) { + dev_err(&pdev->dev, "Input device allocate error\n"); + return -ENOMEM; + } + + input->name = pdata->name ?: "GPIO Beeper"; + input->phys = beeper->phys; + 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); + + error = devm_gpio_request(dev, beeper->gpio_nr, input->name); + if (error) { + dev_err(dev, "Unable to claim gpio %i\n", beeper->gpio_nr); + return error; + } + + gpio_direction_output(beeper->gpio_nr, beeper->active_low); + + beeper->input = input; + input_set_drvdata(input, beeper); + platform_set_drvdata(pdev, beeper); + + return input_register_device(beeper->input); +} + +static void gpio_beeper_shutdown(struct platform_device *pdev) +{ + struct gpio_beeper *beeper = platform_get_drvdata(pdev); + + /* Turning OFF immediately */ + gpio_beeper_close(beeper->input); +} + +static struct platform_driver gpio_beeper_platform_driver = { + .driver = { + .name = "gpio-beeper", + .owner = THIS_MODULE, + }, + .probe = gpio_beeper_probe, + .shutdown = gpio_beeper_shutdown, +}; +module_platform_driver(gpio_beeper_platform_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Alexander Shiyan "); +MODULE_DESCRIPTION("Generic GPIO beeper driver"); diff --git a/include/linux/platform_data/input-gpio-beeper.h b/include/linux/platform_data/input-gpio-beeper.h new file mode 100644 index 0000000..9eb4362 --- /dev/null +++ b/include/linux/platform_data/input-gpio-beeper.h @@ -0,0 +1,20 @@ +/* + * Copyright (C) 2012 Alexander Shiyan + * + * 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. + */ + +#ifndef __PLATFORM_DATA_INPUT_GPIO_BEEPER_H_ +#define __PLATFORM_DATA_INPUT_GPIO_BEEPER_H_ + +/* gpio-beeper platform data structure */ +struct gpio_beeper_pdata { + const char *name; /* Name of device (Optional) */ + int gpio_nr; /* GPIO number */ + bool active_low; /* Set if active level is LOW */ +}; + +#endif