linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3 v6] input: pwm-beeper: add feature to set volume level
@ 2023-01-24 11:11 Manuel Traut
  2023-01-24 11:12 ` [PATCH 1/3 " Manuel Traut
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Manuel Traut @ 2023-01-24 11:11 UTC (permalink / raw)
  To: linux-kernel; +Cc: manuel.traut, Dmitry Torokhov, Frieder Schrempf, linux-input

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/cover.1487323753.git.frieder.schrempf@exceet.de/

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3 v6] input: pwm-beeper: add feature to set volume level
  2023-01-24 11:11 [PATCH 0/3 v6] input: pwm-beeper: add feature to set volume level Manuel Traut
@ 2023-01-24 11:12 ` Manuel Traut
  2023-01-24 13:18   ` kernel test robot
  2023-01-24 14:40   ` kernel test robot
  2023-01-24 11:13 ` [PATCH 2/3 " Manuel Traut
  2023-01-24 11:14 ` [PATCH 3/3 " Manuel Traut
  2 siblings, 2 replies; 9+ messages in thread
From: Manuel Traut @ 2023-01-24 11:12 UTC (permalink / raw)
  To: linux-kernel; +Cc: Dmitry Torokhov, Frieder Schrempf, linux-input

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

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);
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3 v6] input: pwm-beeper: add feature to set volume level
  2023-01-24 11:11 [PATCH 0/3 v6] input: pwm-beeper: add feature to set volume level Manuel Traut
  2023-01-24 11:12 ` [PATCH 1/3 " Manuel Traut
@ 2023-01-24 11:13 ` Manuel Traut
  2023-01-24 11:29   ` Krzysztof Kozlowski
  2023-01-24 11:29   ` Krzysztof Kozlowski
  2023-01-24 11:14 ` [PATCH 3/3 " Manuel Traut
  2 siblings, 2 replies; 9+ messages in thread
From: Manuel Traut @ 2023-01-24 11:13 UTC (permalink / raw)
  To: linux-kernel; +Cc: Dmitry Torokhov, Frieder Schrempf, linux-input

This patch adds the documentation for the devicetree bindings to set
the volume levels.

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3 v6] input: pwm-beeper: add feature to set volume level
  2023-01-24 11:11 [PATCH 0/3 v6] input: pwm-beeper: add feature to set volume level Manuel Traut
  2023-01-24 11:12 ` [PATCH 1/3 " Manuel Traut
  2023-01-24 11:13 ` [PATCH 2/3 " Manuel Traut
@ 2023-01-24 11:14 ` Manuel Traut
  2 siblings, 0 replies; 9+ messages in thread
From: Manuel Traut @ 2023-01-24 11:14 UTC (permalink / raw)
  To: linux-kernel; +Cc: Dmitry Torokhov, Frieder Schrempf, linux-input

This patch adds the devicetree bindings to set the volume levels
and the default volume level.

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3 v6] input: pwm-beeper: add feature to set volume level
  2023-01-24 11:13 ` [PATCH 2/3 " Manuel Traut
@ 2023-01-24 11:29   ` Krzysztof Kozlowski
  2023-01-24 11:29   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-24 11:29 UTC (permalink / raw)
  To: Manuel Traut, linux-kernel; +Cc: Dmitry Torokhov, Frieder Schrempf, linux-input

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 <frieder.schrempf@exceet.de>

I think it is kontron.de now.

> Acked-by: Rob Herring <robh@kernel.org>

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3 v6] input: pwm-beeper: add feature to set volume level
  2023-01-24 11:13 ` [PATCH 2/3 " Manuel Traut
  2023-01-24 11:29   ` Krzysztof Kozlowski
@ 2023-01-24 11:29   ` Krzysztof Kozlowski
  2023-01-24 16:11     ` EXTERNAL - " Manuel Traut
  1 sibling, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-24 11:29 UTC (permalink / raw)
  To: Manuel Traut, linux-kernel; +Cc: Dmitry Torokhov, Frieder Schrempf, linux-input

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 <frieder.schrempf@exceet.de>
> Acked-by: Rob Herring <robh@kernel.org>
> ---

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3 v6] input: pwm-beeper: add feature to set volume level
  2023-01-24 11:12 ` [PATCH 1/3 " Manuel Traut
@ 2023-01-24 13:18   ` kernel test robot
  2023-01-24 14:40   ` kernel test robot
  1 sibling, 0 replies; 9+ messages in thread
From: kernel test robot @ 2023-01-24 13:18 UTC (permalink / raw)
  To: Manuel Traut, linux-kernel
  Cc: oe-kbuild-all, Dmitry Torokhov, Frieder Schrempf, linux-input

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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3 v6] input: pwm-beeper: add feature to set volume level
  2023-01-24 11:12 ` [PATCH 1/3 " Manuel Traut
  2023-01-24 13:18   ` kernel test robot
@ 2023-01-24 14:40   ` kernel test robot
  1 sibling, 0 replies; 9+ messages in thread
From: kernel test robot @ 2023-01-24 14:40 UTC (permalink / raw)
  To: Manuel Traut, linux-kernel
  Cc: oe-kbuild-all, Dmitry Torokhov, Frieder Schrempf, linux-input

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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: EXTERNAL - [PATCH 2/3 v6] input: pwm-beeper: add feature to set volume level
  2023-01-24 11:29   ` Krzysztof Kozlowski
@ 2023-01-24 16:11     ` Manuel Traut
  0 siblings, 0 replies; 9+ messages in thread
From: Manuel Traut @ 2023-01-24 16:11 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-kernel, Dmitry Torokhov, Frieder Schrempf, linux-input

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 <frieder.schrempf@exceet.de>
> > Acked-by: Rob Herring <robh@kernel.org>
> > ---
> 
> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-01-24 16:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-24 11:11 [PATCH 0/3 v6] input: pwm-beeper: add feature to set volume level Manuel Traut
2023-01-24 11:12 ` [PATCH 1/3 " Manuel Traut
2023-01-24 13:18   ` kernel test robot
2023-01-24 14:40   ` kernel test robot
2023-01-24 11:13 ` [PATCH 2/3 " Manuel Traut
2023-01-24 11:29   ` Krzysztof Kozlowski
2023-01-24 11:29   ` Krzysztof Kozlowski
2023-01-24 16:11     ` EXTERNAL - " Manuel Traut
2023-01-24 11:14 ` [PATCH 3/3 " Manuel Traut

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).