diff mbox series

[1/3,v6] input: pwm-beeper: add feature to set volume level

Message ID Y8+9L7UincSjIaD9@mt.com (mailing list archive)
State Superseded
Headers show
Series input: pwm-beeper: add feature to set volume level | expand

Commit Message

Manuel Traut Jan. 24, 2023, 11:12 a.m. UTC 
Make the driver accept switching volume levels via sysfs.
This can be helpful if the beep/bell sound intensity needs
to be adapted to the environment of the device.

The volume adjustment is done by changing the duty cycle of
the pwm signal.

This patch adds the sysfs interface with 5 default volume
levels (0 - mute, 4 - max. volume).

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
 .../ABI/testing/sysfs-devices-pwm-beeper      | 17 ++++
 drivers/input/misc/pwm-beeper.c               | 83 ++++++++++++++++++-
 2 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-pwm-beeper

Comments

kernel test robot Jan. 24, 2023, 1:18 p.m. UTC | #1 
Hi Manuel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on dtor-input/next]
[also build test ERROR on dtor-input/for-linus linus/master v6.2-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Manuel-Traut/input-pwm-beeper-add-feature-to-set-volume-level/20230124-191549
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link:    https://lore.kernel.org/r/Y8%2B9L7UincSjIaD9%40mt.com
patch subject: [PATCH 1/3 v6] input: pwm-beeper: add feature to set volume level
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20230124/202301242124.R5AWMFJb-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/3468440a8e674e649dcf11e23f3fb3d229555e7c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Manuel-Traut/input-pwm-beeper-add-feature-to-set-volume-level/20230124-191549
        git checkout 3468440a8e674e649dcf11e23f3fb3d229555e7c
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/input/misc/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/input/misc/pwm-beeper.c:73:62: error: macro "DEVICE_ATTR_RW" passed 4 arguments, but takes just 1
      73 | static DEVICE_ATTR_RW(volume, 0644, volume_show, volume_store);
         |                                                              ^
   In file included from include/linux/input.h:19,
                    from drivers/input/misc/pwm-beeper.c:11:
   include/linux/device.h:131: note: macro "DEVICE_ATTR_RW" defined here
     131 | #define DEVICE_ATTR_RW(_name) \
         | 
>> drivers/input/misc/pwm-beeper.c:73:8: error: type defaults to 'int' in declaration of 'DEVICE_ATTR_RW' [-Werror=implicit-int]
      73 | static DEVICE_ATTR_RW(volume, 0644, volume_show, volume_store);
         |        ^~~~~~~~~~~~~~
>> drivers/input/misc/pwm-beeper.c:77:10: error: 'dev_attr_volume' undeclared here (not in a function); did you mean 'dev_attr_max_volume'?
      77 |         &dev_attr_volume.attr,
         |          ^~~~~~~~~~~~~~~
         |          dev_attr_max_volume
   drivers/input/misc/pwm-beeper.c:73:8: warning: 'DEVICE_ATTR_RW' defined but not used [-Wunused-variable]
      73 | static DEVICE_ATTR_RW(volume, 0644, volume_show, volume_store);
         |        ^~~~~~~~~~~~~~
   drivers/input/misc/pwm-beeper.c:54:16: warning: 'volume_store' defined but not used [-Wunused-function]
      54 | static ssize_t volume_store(struct device *dev,
         |                ^~~~~~~~~~~~
   drivers/input/misc/pwm-beeper.c:38:16: warning: 'volume_show' defined but not used [-Wunused-function]
      38 | static ssize_t volume_show(struct device *dev,
         |                ^~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/DEVICE_ATTR_RW +73 drivers/input/misc/pwm-beeper.c

  > 11	#include <linux/input.h>
    12	#include <linux/regulator/consumer.h>
    13	#include <linux/module.h>
    14	#include <linux/kernel.h>
    15	#include <linux/of.h>
    16	#include <linux/platform_device.h>
    17	#include <linux/property.h>
    18	#include <linux/pwm.h>
    19	#include <linux/slab.h>
    20	#include <linux/workqueue.h>
    21	
    22	struct pwm_beeper {
    23		struct input_dev *input;
    24		struct pwm_device *pwm;
    25		struct regulator *amplifier;
    26		struct work_struct work;
    27		unsigned long period;
    28		unsigned int bell_frequency;
    29		bool suspended;
    30		bool amplifier_on;
    31		unsigned int volume;
    32		unsigned int *volume_levels;
    33		unsigned int max_volume;
    34	};
    35	
    36	#define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
    37	
    38	static ssize_t volume_show(struct device *dev,
    39			struct device_attribute *attr, char *buf)
    40	{
    41		struct pwm_beeper *beeper = dev_get_drvdata(dev);
    42	
    43		return sprintf(buf, "%d\n", beeper->volume);
    44	}
    45	
    46	static ssize_t max_volume_show(struct device *dev,
    47			struct device_attribute *attr, char *buf)
    48	{
    49		struct pwm_beeper *beeper = dev_get_drvdata(dev);
    50	
    51		return sprintf(buf, "%d\n", beeper->max_volume);
    52	}
    53	
    54	static ssize_t volume_store(struct device *dev,
    55			struct device_attribute *attr, const char *buf, size_t count)
    56	{
    57		int rc;
    58		struct pwm_beeper *beeper = dev_get_drvdata(dev);
    59		unsigned int volume;
    60	
    61		rc = kstrtouint(buf, 0, &volume);
    62		if (rc)
    63			return rc;
    64	
    65		if (volume > beeper->max_volume)
    66			return -EINVAL;
    67		pr_debug("set volume to %u\n", volume);
    68		beeper->volume = volume;
    69	
    70		return count;
    71	}
    72	
  > 73	static DEVICE_ATTR_RW(volume, 0644, volume_show, volume_store);
    74	static DEVICE_ATTR(max_volume, 0644, max_volume_show, NULL);
    75	
    76	static struct attribute *pwm_beeper_attributes[] = {
  > 77		&dev_attr_volume.attr,
    78		&dev_attr_max_volume.attr,
    79		NULL,
    80	};
    81
kernel test robot Jan. 24, 2023, 2:40 p.m. UTC | #2 
Hi Manuel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on dtor-input/next]
[also build test WARNING on dtor-input/for-linus linus/master v6.2-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Manuel-Traut/input-pwm-beeper-add-feature-to-set-volume-level/20230124-191549
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link:    https://lore.kernel.org/r/Y8%2B9L7UincSjIaD9%40mt.com
patch subject: [PATCH 1/3 v6] input: pwm-beeper: add feature to set volume level
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230124/202301242226.E9z7kZKT-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/3468440a8e674e649dcf11e23f3fb3d229555e7c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Manuel-Traut/input-pwm-beeper-add-feature-to-set-volume-level/20230124-191549
        git checkout 3468440a8e674e649dcf11e23f3fb3d229555e7c
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/input/misc/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/input/misc/pwm-beeper.c:73:62: error: macro "DEVICE_ATTR_RW" passed 4 arguments, but takes just 1
      73 | static DEVICE_ATTR_RW(volume, 0644, volume_show, volume_store);
         |                                                              ^
   In file included from include/linux/input.h:19,
                    from drivers/input/misc/pwm-beeper.c:11:
   include/linux/device.h:131: note: macro "DEVICE_ATTR_RW" defined here
     131 | #define DEVICE_ATTR_RW(_name) \
         | 
   drivers/input/misc/pwm-beeper.c:73:8: error: type defaults to 'int' in declaration of 'DEVICE_ATTR_RW' [-Werror=implicit-int]
      73 | static DEVICE_ATTR_RW(volume, 0644, volume_show, volume_store);
         |        ^~~~~~~~~~~~~~
   drivers/input/misc/pwm-beeper.c:77:10: error: 'dev_attr_volume' undeclared here (not in a function); did you mean 'dev_attr_max_volume'?
      77 |         &dev_attr_volume.attr,
         |          ^~~~~~~~~~~~~~~
         |          dev_attr_max_volume
>> drivers/input/misc/pwm-beeper.c:73:8: warning: 'DEVICE_ATTR_RW' defined but not used [-Wunused-variable]
      73 | static DEVICE_ATTR_RW(volume, 0644, volume_show, volume_store);
         |        ^~~~~~~~~~~~~~
   drivers/input/misc/pwm-beeper.c:54:16: warning: 'volume_store' defined but not used [-Wunused-function]
      54 | static ssize_t volume_store(struct device *dev,
         |                ^~~~~~~~~~~~
   drivers/input/misc/pwm-beeper.c:38:16: warning: 'volume_show' defined but not used [-Wunused-function]
      38 | static ssize_t volume_show(struct device *dev,
         |                ^~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/DEVICE_ATTR_RW +73 drivers/input/misc/pwm-beeper.c

    72	
  > 73	static DEVICE_ATTR_RW(volume, 0644, volume_show, volume_store);
    74	static DEVICE_ATTR(max_volume, 0644, max_volume_show, NULL);
    75
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-devices-pwm-beeper b/Documentation/ABI/testing/sysfs-devices-pwm-beeper
new file mode 100644
index 000000000000..d068c5814f48
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-pwm-beeper
@@ -0,0 +1,17 @@ 
+What:		/sys/devices/.../pwm-beeper/volume
+Date:		February 2017
+KernelVersion:
+Contact:	Frieder Schrempf <frieder.schrempf@exceet.de>
+Description:
+		Control the volume of this pwm-beeper. Values
+		are between 0 and max_volume. This file will also
+		show the current volume level stored in the driver.
+
+What:		/sys/devices/.../pwm-beeper/max_volume
+Date:		February 2017
+KernelVersion:
+Contact:	Frieder Schrempf <frieder.schrempf@exceet.de>
+Description:
+		This file shows the maximum volume level that can be
+		assigned to volume.
+
diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index d6b12477748a..fadc73e1c89a 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -1,6 +1,10 @@ 
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
+ *
+ *  Copyright (C) 2016, Frieder Schrempf <frieder.schrempf@exceet.de>
+ *  (volume support)
+ *
  *  PWM beeper driver
  */
 
@@ -24,10 +28,61 @@  struct pwm_beeper {
 	unsigned int bell_frequency;
 	bool suspended;
 	bool amplifier_on;
+	unsigned int volume;
+	unsigned int *volume_levels;
+	unsigned int max_volume;
 };
 
 #define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
 
+static ssize_t volume_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct pwm_beeper *beeper = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", beeper->volume);
+}
+
+static ssize_t max_volume_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct pwm_beeper *beeper = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", beeper->max_volume);
+}
+
+static ssize_t volume_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	int rc;
+	struct pwm_beeper *beeper = dev_get_drvdata(dev);
+	unsigned int volume;
+
+	rc = kstrtouint(buf, 0, &volume);
+	if (rc)
+		return rc;
+
+	if (volume > beeper->max_volume)
+		return -EINVAL;
+	pr_debug("set volume to %u\n", volume);
+	beeper->volume = volume;
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(volume, 0644, volume_show, volume_store);
+static DEVICE_ATTR(max_volume, 0644, max_volume_show, NULL);
+
+static struct attribute *pwm_beeper_attributes[] = {
+	&dev_attr_volume.attr,
+	&dev_attr_max_volume.attr,
+	NULL,
+};
+
+static struct attribute_group pwm_beeper_attribute_group = {
+	.attrs = pwm_beeper_attributes,
+};
+
 static int pwm_beeper_on(struct pwm_beeper *beeper, unsigned long period)
 {
 	struct pwm_state state;
@@ -37,7 +92,8 @@  static int pwm_beeper_on(struct pwm_beeper *beeper, unsigned long period)
 
 	state.enabled = true;
 	state.period = period;
-	pwm_set_relative_duty_cycle(&state, 50, 100);
+
+	pwm_set_relative_duty_cycle(&state, beeper->volume_levels[beeper->volume], 1000);
 
 	error = pwm_apply_state(beeper->pwm, &state);
 	if (error)
@@ -126,6 +182,7 @@  static int pwm_beeper_probe(struct platform_device *pdev)
 	struct pwm_state state;
 	u32 bell_frequency;
 	int error;
+	size_t size;
 
 	beeper = devm_kzalloc(dev, sizeof(*beeper), GFP_KERNEL);
 	if (!beeper)
@@ -171,6 +228,24 @@  static int pwm_beeper_probe(struct platform_device *pdev)
 
 	beeper->bell_frequency = bell_frequency;
 
+	beeper->max_volume = 4;
+
+	size = sizeof(*beeper->volume_levels) *
+		(beeper->max_volume + 1);
+
+	beeper->volume_levels = devm_kzalloc(&(pdev->dev), size,
+		GFP_KERNEL);
+	if (!beeper->volume_levels)
+		return -ENOMEM;
+
+	beeper->volume_levels[0] = 0;
+	beeper->volume_levels[1] = 8;
+	beeper->volume_levels[2] = 20;
+	beeper->volume_levels[3] = 40;
+	beeper->volume_levels[4] = 500;
+
+	beeper->volume = beeper->max_volume;
+
 	beeper->input = devm_input_allocate_device(dev);
 	if (!beeper->input) {
 		dev_err(dev, "Failed to allocate input device\n");
@@ -192,6 +267,12 @@  static int pwm_beeper_probe(struct platform_device *pdev)
 
 	input_set_drvdata(beeper->input, beeper);
 
+	error = sysfs_create_group(&pdev->dev.kobj, &pwm_beeper_attribute_group);
+	if (error) {
+		dev_err(&pdev->dev, "Failed to create sysfs group: %d\n", error);
+		return error;
+	}
+
 	error = input_register_device(beeper->input);
 	if (error) {
 		dev_err(dev, "Failed to register input device: %d\n", error);