Hi,
I found an old patchset on the ML that enables setting the volume of a
pwm-beeper via sysfs here [0].
This is an updated version of the patches that they apply to the current
version of the driver.
[0] https://lore.kernel.org/all/[email protected]/
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 <[email protected]>
---
.../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
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 <[email protected]>
+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 <[email protected]>
+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 <[email protected]>
+ *
+ * Copyright (C) 2016, Frieder Schrempf <[email protected]>
+ * (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);
--
2.39.0
This patch adds the documentation for the devicetree bindings to set
the volume levels.
Signed-off-by: Frieder Schrempf <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
.../devicetree/bindings/input/pwm-beeper.txt | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
index 8fc0e48c20db..93cab5eee9f2 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
@@ -9,6 +9,15 @@ Required properties:
Optional properties:
- amp-supply: phandle to a regulator that acts as an amplifier for the beeper
- beeper-hz: bell frequency in Hz
+- volume-levels: Array of PWM duty cycle values that correspond to
+ linear volume levels. These need to be in the range of 0 to 500,
+ while 0 means 0% duty cycle (mute) and 500 means 50% duty cycle
+ (max volume).
+ Please note that the actual volume of most beepers is highly
+ non-linear, which means that low volume levels are probably somewhere
+ in the range of 1 to 30 (0.1-3% duty cycle).
+- default-volume-level: the default volume level (index into the
+ array defined by the "volume-levels" property)
Example:
@@ -21,4 +30,6 @@ beeper {
compatible = "pwm-beeper";
pwms = <&pwm0>;
amp-supply = <&beeper_amp>;
+ volume-levels = <0 8 20 40 500>;
+ default-volume-level = <4>;
};
--
2.39.0
This patch adds the devicetree bindings to set the volume levels
and the default volume level.
Signed-off-by: Frieder Schrempf <[email protected]>
---
drivers/input/misc/pwm-beeper.c | 58 +++++++++++++++++++++++----------
1 file changed, 41 insertions(+), 17 deletions(-)
diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index fadc73e1c89a..f95d6b5899c7 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -181,8 +181,9 @@ static int pwm_beeper_probe(struct platform_device *pdev)
struct pwm_beeper *beeper;
struct pwm_state state;
u32 bell_frequency;
- int error;
+ int error, length;
size_t size;
+ u32 value;
beeper = devm_kzalloc(dev, sizeof(*beeper), GFP_KERNEL);
if (!beeper)
@@ -228,23 +229,46 @@ 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;
+ /* determine the number of volume levels */
+ length = device_property_read_u32_array(&pdev->dev, "volume-levels", NULL, 0);
+ if (length <= 0) {
+ dev_dbg(&pdev->dev, "no volume levels specified, using max volume\n");
+ beeper->max_volume = 1;
+ } else
+ beeper->max_volume = length;
+
+ /* read volume levels from DT property */
+ if (beeper->max_volume > 0) {
+ size = sizeof(*beeper->volume_levels) * beeper->max_volume;
+
+ beeper->volume_levels = devm_kzalloc(&(pdev->dev), size,
+ GFP_KERNEL);
+ if (!beeper->volume_levels)
+ return -ENOMEM;
+
+ if (length > 0) {
+ error = device_property_read_u32_array(&pdev->dev, "volume-levels",
+ beeper->volume_levels,
+ beeper->max_volume);
+
+ if (error < 0)
+ return error;
+
+ error = device_property_read_u32(&pdev->dev, "default-volume-level",
+ &value);
+
+ if (error < 0) {
+ dev_dbg(&pdev->dev, "no default volume specified, using max volume\n");
+ value = beeper->max_volume - 1;
+ }
+ } else {
+ beeper->volume_levels[0] = 500;
+ value = 0;
+ }
- beeper->volume = beeper->max_volume;
+ beeper->volume = value;
+ beeper->max_volume--;
+ }
beeper->input = devm_input_allocate_device(dev);
if (!beeper->input) {
--
2.39.0
On 24/01/2023 12:13, Manuel Traut wrote:
> This patch adds the documentation for the devicetree bindings to set
> the volume levels.
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.
>
> Signed-off-by: Frieder Schrempf <[email protected]>
I think it is kontron.de now.
> Acked-by: Rob Herring <[email protected]>
This misses your SoB.
> ---
> .../devicetree/bindings/input/pwm-beeper.txt | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> index 8fc0e48c20db..93cab5eee9f2 100644
> --- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> @@ -9,6 +9,15 @@ Required properties:
> Optional properties:
> - amp-supply: phandle to a regulator that acts as an amplifier for the beeper
> - beeper-hz: bell frequency in Hz
> +- volume-levels: Array of PWM duty cycle values that correspond to
> + linear volume levels. These need to be in the range of 0 to 500,
> + while 0 means 0% duty cycle (mute) and 500 means 50% duty cycle
> + (max volume).
> + Please note that the actual volume of most beepers is highly
> + non-linear, which means that low volume levels are probably somewhere
> + in the range of 1 to 30 (0.1-3% duty cycle).
> +- default-volume-level: the default volume level (index into the
> + array defined by the "volume-levels" property)
The bindings should be converted to DT schema first and then new
properties added.
Best regards,
Krzysztof
On 24/01/2023 12:13, Manuel Traut wrote:
> This patch adds the documentation for the devicetree bindings to set
> the volume levels.
>
> Signed-off-by: Frieder Schrempf <[email protected]>
> Acked-by: Rob Herring <[email protected]>
> ---
One more:
Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).
Best regards,
Krzysztof
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/[email protected]/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 <[email protected]>
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
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/[email protected]/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 <[email protected]>
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Hi Krzysztof,
Am 24.01.2023 um 12:29 hat Krzysztof Kozlowski geschrieben:
> On 24/01/2023 12:13, Manuel Traut wrote:
> > This patch adds the documentation for the devicetree bindings to set
> > the volume levels.
> >
> > Signed-off-by: Frieder Schrempf <[email protected]>
> > Acked-by: Rob Herring <[email protected]>
> > ---
>
> One more:
>
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching).
Thanks for your feedback. I will post a v7 soon that includes your
feedback, the findings from the kernelbot and a fix that allows
unloading and loading of the module.
Best regards
Manuel