linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v10 0/4] input: pwm-beeper: add feature to set volume level
@ 2023-02-01 15:21 Manuel Traut
  2023-02-01 15:21 ` [PATCH v10 1/4] dt-bindings: input: pwm-beeper: Convert txt bindings to yaml Manuel Traut
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Manuel Traut @ 2023-02-01 15:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Manuel Traut, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Frieder Schrempf, linux-input, devicetree

This implements volume control for the pwm-beeper via sysfs.

The first patch changes the devicetree documentation from txt to yaml.

The original author of the volume support patches is Frieder Schrempf.
I picked them from this [0] LKML thread from 2017. Since it looks like
his email address changed in the meantime I changed it in the Author
and Signed-off-by, as well as in the copyright statements.
I did some minor changes on the patches that they apply and work with
the current kernel.

checkpatch still reports warnings regarding the changes:
  * from txt to yaml of the devicetree documentation:
      WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
      WARNING: DT binding docs and includes should be a separate patch.
  * and the introduction of Documentation/ABI/testing/sysfs-devices-pwm-beeper:
      WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
  I am not sure how to fix these warnings. So any suggestion would be helpful.

Changes since v9 [1]:
 * [1/4] dt-bindings: input: pwm-beeper: Convert txt bindings to yaml
    * added Reviewed-by tag from Rob Herring
 * [3/4] dt-bindings: input: pwm-beeper: add volume
    * Added missing unit specifier in examples

Changes since v8 [2]:
 * yaml devicetree doc:
    * reordered patches to introduce dt-bindings before usage
    * drop quotes from $id and $schema references
    * amp-supply: simplify description
    * examples: remove unneeded amp device node
    * use -bp suffix for volume-levels and default-volume
    * specify default-volume as value instead of pointer into volume-array

  * fixup to work with new dt-binding specification
  * squash patches as suggested by Frieder

Changes since v7 [3]:
 * yaml devicetree doc:
    * Use shorter subject
    * Fix indent
    * Use units
    * 'make dt_binding_check' succeeds
    * 'make dtbs_check' does not report new errors

 * Reworded commit messages avoiding 'this patch' phrase
 * Fix wrong indent in [PATCH 5/5 v7] input: pwm-beeper: handle module unloading properly
 * Use current date in Documentation/ABI/testing/sysfs-devices-pwm-beeper

 * Hopefully fixed my setup that
   * mails are CC'ed correctly
   * patches are sent as replies to the cover letter

Changes since v6 [4]:
 * Convert devicetree binding documentation from txt to yaml
 * Use DEVICE_ATTR_[RO|RW] properly
 * Change Frieders Mail address
 * Added Signed-off and Tested-by statements
 * Fix module unloading


Frieder Schrempf (2):
  input: pwm-beeper: add feature to set volume via sysfs
  input: pwm-beeper: set volume levels by devicetree

Manuel Traut (2):
  dt-bindings: input: pwm-beeper: Convert txt bindings to yaml
  dt-bindings: input: pwm-beeper: add volume

 .../ABI/testing/sysfs-devices-pwm-beeper      |  17 +++
 .../devicetree/bindings/input/pwm-beeper.txt  |  24 ----
 .../devicetree/bindings/input/pwm-beeper.yaml |  56 ++++++++
 drivers/input/misc/pwm-beeper.c               | 135 +++++++++++++++++-
 4 files changed, 206 insertions(+), 26 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-pwm-beeper
 delete mode 100644 Documentation/devicetree/bindings/input/pwm-beeper.txt
 create mode 100644 Documentation/devicetree/bindings/input/pwm-beeper.yaml


[0] https://lore.kernel.org/all/cover.1487323753.git.frieder.schrempf@exceet.de/
[1] https://lore.kernel.org/lkml/20230130135650.1407156-1-manuel.traut@mt.com/
[2] https://lore.kernel.org/lkml/20230126091825.220646-1-manuel.traut@mt.com/
[3] https://lore.kernel.org/all/Y9AIq3cSNzI9T%2FdU@mt.com/
[4] https://lkml.org/lkml/2023/1/24/379

-- 
2.39.1


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

* [PATCH v10 1/4] dt-bindings: input: pwm-beeper: Convert txt bindings to yaml
  2023-02-01 15:21 [PATCH v10 0/4] input: pwm-beeper: add feature to set volume level Manuel Traut
@ 2023-02-01 15:21 ` Manuel Traut
  2023-02-01 15:21 ` [PATCH v10 2/4] input: pwm-beeper: add feature to set volume via sysfs Manuel Traut
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Manuel Traut @ 2023-02-01 15:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Manuel Traut, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Frieder Schrempf, linux-input, devicetree, Rob Herring

Converts txt binding to new YAML format.

Signed-off-by: Manuel Traut <manuel.traut@mt.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/input/pwm-beeper.txt  | 24 ------------
 .../devicetree/bindings/input/pwm-beeper.yaml | 39 +++++++++++++++++++
 2 files changed, 39 insertions(+), 24 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/pwm-beeper.txt
 create mode 100644 Documentation/devicetree/bindings/input/pwm-beeper.yaml

diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
deleted file mode 100644
index 8fc0e48c20db..000000000000
--- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
+++ /dev/null
@@ -1,24 +0,0 @@
-* PWM beeper device tree bindings
-
-Registers a PWM device as beeper.
-
-Required properties:
-- compatible: should be "pwm-beeper"
-- pwms: phandle to the physical PWM device
-
-Optional properties:
-- amp-supply: phandle to a regulator that acts as an amplifier for the beeper
-- beeper-hz:  bell frequency in Hz
-
-Example:
-
-beeper_amp: amplifier {
-	compatible = "fixed-regulator";
-	gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
-};
-
-beeper {
-	compatible = "pwm-beeper";
-	pwms = <&pwm0>;
-	amp-supply = <&beeper_amp>;
-};
diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.yaml b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
new file mode 100644
index 000000000000..1ebc3a46d934
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/pwm-beeper.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: PWM beeper
+
+maintainers:
+  - Dmitry Torokhov <dmitry.torokhov@gmail.com>
+
+description: Registers a PWM device as beeper.
+
+properties:
+  compatible:
+    const: pwm-beeper
+
+  pwms:
+    maxItems: 1
+
+  amp-supply:
+    description: >
+      regulator that acts as an amplifier for the beeper
+
+  beeper-hz:
+    description: bell frequency in Hz
+
+required:
+  - compatible
+  - pwms
+
+additionalProperties: false
+
+examples:
+  - |
+    beeper {
+      compatible = "pwm-beeper";
+      pwms = <&pwm0>;
+    };
-- 
2.39.1


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

* [PATCH v10 2/4] input: pwm-beeper: add feature to set volume via sysfs
  2023-02-01 15:21 [PATCH v10 0/4] input: pwm-beeper: add feature to set volume level Manuel Traut
  2023-02-01 15:21 ` [PATCH v10 1/4] dt-bindings: input: pwm-beeper: Convert txt bindings to yaml Manuel Traut
@ 2023-02-01 15:21 ` Manuel Traut
  2023-02-01 15:21 ` [PATCH v10 3/4] dt-bindings: input: pwm-beeper: add volume Manuel Traut
  2023-02-01 15:21 ` [PATCH v10 4/4] input: pwm-beeper: set volume levels by devicetree Manuel Traut
  3 siblings, 0 replies; 8+ messages in thread
From: Manuel Traut @ 2023-02-01 15:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Frieder Schrempf, Dmitry Torokhov, Rob Herring,
	Krzysztof Kozlowski, Manuel Traut, linux-input, devicetree

From: Frieder Schrempf <frieder.schrempf@kontron.de>

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.

Add a sysfs interface with 5 default volume levels:
  0 - mute
  ..
  4 - max. volume

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Signed-off-by: Manuel Traut <manuel.traut@mt.com>
Tested-by: Manuel Traut <manuel.traut@mt.com>
---
 .../ABI/testing/sysfs-devices-pwm-beeper      | 17 ++++
 drivers/input/misc/pwm-beeper.c               | 96 ++++++++++++++++++-
 2 files changed, 112 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..d2a22516f31d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-pwm-beeper
@@ -0,0 +1,17 @@
+What:		/sys/devices/.../pwm-beeper/volume
+Date:		January 2023
+KernelVersion:
+Contact:	Frieder Schrempf <frieder.schrempf@kontron.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 2023
+KernelVersion:
+Contact:	Frieder Schrempf <frieder.schrempf@kontron.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..214d3fa0a06d 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -1,9 +1,14 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
+ *
+ *  Copyright (C) 2016, Frieder Schrempf <frieder.schrempf@kontron.de>
+ *  (volume support)
+ *
  *  PWM beeper driver
  */
 
+#include <linux/device.h>
 #include <linux/input.h>
 #include <linux/regulator/consumer.h>
 #include <linux/module.h>
@@ -24,10 +29,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);
+static DEVICE_ATTR_RO(max_volume);
+
+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 +93,7 @@ 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] = 80;
+	beeper->volume_levels[2] = 200;
+	beeper->volume_levels[3] = 400;
+	beeper->volume_levels[4] = 5000;
+
+	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,8 +267,15 @@ 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) {
+		sysfs_remove_group(&pdev->dev.kobj, &pwm_beeper_attribute_group);
 		dev_err(dev, "Failed to register input device: %d\n", error);
 		return error;
 	}
@@ -203,6 +285,17 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int pwm_beeper_remove(struct platform_device *pdev)
+{
+	struct pwm_beeper *beeper;
+
+	beeper = platform_get_drvdata(pdev);
+	input_unregister_device(beeper->input);
+	sysfs_remove_group(&pdev->dev.kobj, &pwm_beeper_attribute_group);
+
+	return 0;
+}
+
 static int __maybe_unused pwm_beeper_suspend(struct device *dev)
 {
 	struct pwm_beeper *beeper = dev_get_drvdata(dev);
@@ -248,6 +341,7 @@ MODULE_DEVICE_TABLE(of, pwm_beeper_match);
 
 static struct platform_driver pwm_beeper_driver = {
 	.probe	= pwm_beeper_probe,
+	.remove	= pwm_beeper_remove,
 	.driver = {
 		.name	= "pwm-beeper",
 		.pm	= &pwm_beeper_pm_ops,
-- 
2.39.1


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

* [PATCH v10 3/4] dt-bindings: input: pwm-beeper: add volume
  2023-02-01 15:21 [PATCH v10 0/4] input: pwm-beeper: add feature to set volume level Manuel Traut
  2023-02-01 15:21 ` [PATCH v10 1/4] dt-bindings: input: pwm-beeper: Convert txt bindings to yaml Manuel Traut
  2023-02-01 15:21 ` [PATCH v10 2/4] input: pwm-beeper: add feature to set volume via sysfs Manuel Traut
@ 2023-02-01 15:21 ` Manuel Traut
  2023-02-01 16:15   ` Krzysztof Kozlowski
  2023-02-01 15:21 ` [PATCH v10 4/4] input: pwm-beeper: set volume levels by devicetree Manuel Traut
  3 siblings, 1 reply; 8+ messages in thread
From: Manuel Traut @ 2023-02-01 15:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Manuel Traut, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Frieder Schrempf, linux-input, devicetree

Adds an array of supported volume levels and a default volume level.

Signed-off-by: Manuel Traut <manuel.traut@mt.com>
---
 .../devicetree/bindings/input/pwm-beeper.yaml   | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.yaml b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
index 1ebc3a46d934..6599e28ba5e6 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.yaml
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
@@ -25,6 +25,21 @@ properties:
   beeper-hz:
     description: bell frequency in Hz
 
+  volume-levels-bp:
+    description: >
+      Array of PWM duty cycle values that correspond to
+      linear volume levels. These need to be in the range of
+      0 to 5000, while 0 means 0% duty cycle (mute) and 5000
+      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 10 to 300 (0.1-3%
+      duty cycle).
+
+  default-volume-level-bp:
+    description: >
+      The default volume level.
+
 required:
   - compatible
   - pwms
@@ -36,4 +51,6 @@ examples:
     beeper {
       compatible = "pwm-beeper";
       pwms = <&pwm0>;
+      volume-levels-bp = <0 80 200 400 5000>;
+      default-volume-level-bp = <80>;
     };
-- 
2.39.1


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

* [PATCH v10 4/4] input: pwm-beeper: set volume levels by devicetree
  2023-02-01 15:21 [PATCH v10 0/4] input: pwm-beeper: add feature to set volume level Manuel Traut
                   ` (2 preceding siblings ...)
  2023-02-01 15:21 ` [PATCH v10 3/4] dt-bindings: input: pwm-beeper: add volume Manuel Traut
@ 2023-02-01 15:21 ` Manuel Traut
  3 siblings, 0 replies; 8+ messages in thread
From: Manuel Traut @ 2023-02-01 15:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Frieder Schrempf, Dmitry Torokhov, Rob Herring,
	Krzysztof Kozlowski, Manuel Traut, linux-input, devicetree

From: Frieder Schrempf <frieder.schrempf@kontron.de>

Add devicetree bindings to define supported volume levels and the
default volume level.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Signed-off-by: Manuel Traut <manuel.traut@mt.com>
Tested-by: Manuel Traut <manuel.traut@mt.com>
---
 drivers/input/misc/pwm-beeper.c | 73 +++++++++++++++++++++++++--------
 1 file changed, 55 insertions(+), 18 deletions(-)

diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index 214d3fa0a06d..fc367a249490 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -93,7 +93,7 @@ static int pwm_beeper_on(struct pwm_beeper *beeper, unsigned long period)
 
 	state.enabled = true;
 	state.period = period;
-	pwm_set_relative_duty_cycle(&state, beeper->volume_levels[beeper->volume], 1000);
+	pwm_set_relative_duty_cycle(&state, beeper->volume_levels[beeper->volume], 10000);
 
 	error = pwm_apply_state(beeper->pwm, &state);
 	if (error)
@@ -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, i, length;
 	size_t size;
+	u32 value;
 
 	beeper = devm_kzalloc(dev, sizeof(*beeper), GFP_KERNEL);
 	if (!beeper)
@@ -228,23 +229,59 @@ 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] = 80;
-	beeper->volume_levels[2] = 200;
-	beeper->volume_levels[3] = 400;
-	beeper->volume_levels[4] = 5000;
+	/* determine the number of volume levels */
+	length = device_property_read_u32_array(&pdev->dev, "volume-levels-bp", 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-bp",
+						beeper->volume_levels,
+						beeper->max_volume);
+
+			if (error < 0)
+				return error;
+
+			error = device_property_read_u32(&pdev->dev, "default-volume-level-bp",
+						   &value);
+
+			if (error < 0) {
+				dev_dbg(&pdev->dev, "no default volume specified, using max volume\n");
+				value = beeper->max_volume - 1;
+			} else {
+				for (i = 0; i < length; i++) {
+					if (beeper->volume_levels[i] == value) {
+						value = i;
+						break;
+					}
+				}
+				if (value != i) {
+					dev_dbg(&pdev->dev,
+						"default-volume-level-bp %d invalid, using %d\n",
+						value, beeper->max_volume - 1);
+					value = beeper->max_volume - 1;
+				}
+			}
+		} else {
+			beeper->volume_levels[0] = 5000;
+			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.1


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

* Re: [PATCH v10 3/4] dt-bindings: input: pwm-beeper: add volume
  2023-02-01 15:21 ` [PATCH v10 3/4] dt-bindings: input: pwm-beeper: add volume Manuel Traut
@ 2023-02-01 16:15   ` Krzysztof Kozlowski
  2023-02-01 17:10     ` [PATCH] " Manuel Traut
  0 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2023-02-01 16:15 UTC (permalink / raw)
  To: Manuel Traut, linux-kernel
  Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Frieder Schrempf, linux-input, devicetree

On 01/02/2023 16:21, Manuel Traut wrote:
> Adds an array of supported volume levels and a default volume level.

Adds -> Add


> 
> Signed-off-by: Manuel Traut <manuel.traut@mt.com>
> ---
>  .../devicetree/bindings/input/pwm-beeper.yaml   | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 

Thank you for your patch. There is something to discuss/improve.

> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.yaml b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> index 1ebc3a46d934..6599e28ba5e6 100644
> --- a/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> @@ -25,6 +25,21 @@ properties:
>    beeper-hz:
>      description: bell frequency in Hz
>  
> +  volume-levels-bp:
> +    description: >
> +      Array of PWM duty cycle values that correspond to
> +      linear volume levels. These need to be in the range of
> +      0 to 5000, while 0 means 0% duty cycle (mute) and 5000
> +      means 50% duty cycle (max volume).

Is range 0-5000 limitation of the device? If so, this should not be
free-form text, but "maximum: 5000" (minimum is 0 anyway).

> +      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 10 to 300 (0.1-3%
> +      duty cycle).


Best regards,
Krzysztof


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

* [PATCH] dt-bindings: input: pwm-beeper: add volume
  2023-02-01 16:15   ` Krzysztof Kozlowski
@ 2023-02-01 17:10     ` Manuel Traut
  2023-02-03 21:21       ` Rob Herring
  0 siblings, 1 reply; 8+ messages in thread
From: Manuel Traut @ 2023-02-01 17:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Manuel Traut, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	linux-input, devicetree

Add an array of supported volume levels and a default volume level.

Signed-off-by: Manuel Traut <manuel.traut@mt.com>
---
 .../devicetree/bindings/input/pwm-beeper.yaml      | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.yaml b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
index 1ebc3a46d934..7ea2954cdd6a 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.yaml
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
@@ -25,6 +25,18 @@ properties:
   beeper-hz:
     description: bell frequency in Hz
 
+  volume-levels-bp:
+    description: >
+      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 10 to 300 (0.1-3%
+      duty cycle).
+    maximum: 5000
+
+  default-volume-level-bp:
+    description: >
+      The default volume level.
+
 required:
   - compatible
   - pwms
@@ -36,4 +48,6 @@ examples:
     beeper {
       compatible = "pwm-beeper";
       pwms = <&pwm0>;
+      volume-levels-bp = <0 80 200 400 5000>;
+      default-volume-level-bp = <80>;
     };
-- 
2.39.1


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

* Re: [PATCH] dt-bindings: input: pwm-beeper: add volume
  2023-02-01 17:10     ` [PATCH] " Manuel Traut
@ 2023-02-03 21:21       ` Rob Herring
  0 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2023-02-03 21:21 UTC (permalink / raw)
  To: Manuel Traut
  Cc: linux-kernel, Dmitry Torokhov, Krzysztof Kozlowski, linux-input,
	devicetree

On Wed, Feb 01, 2023 at 06:10:25PM +0100, Manuel Traut wrote:
> Add an array of supported volume levels and a default volume level.

Don't send a single updated patch in a series. It breaks tools and 
workflows.

> 
> Signed-off-by: Manuel Traut <manuel.traut@mt.com>
> ---
>  .../devicetree/bindings/input/pwm-beeper.yaml      | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.yaml b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> index 1ebc3a46d934..7ea2954cdd6a 100644
> --- a/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> @@ -25,6 +25,18 @@ properties:
>    beeper-hz:
>      description: bell frequency in Hz
>  
> +  volume-levels-bp: 
> +    description: >
> +      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 10 to 300 (0.1-3%
> +      duty cycle).

This tells me nothing in terms of how to use this property. The index of 
the array is the volume level and the value is the PWM duty cycle?

> +    maximum: 5000

As this is an array, 'maximum' for the array doesn't make sense. It's a 
max for each entry:

items:
  maximum: 5000


> +
> +  default-volume-level-bp:
> +    description: >
> +      The default volume level.
> +
>  required:
>    - compatible
>    - pwms
> @@ -36,4 +48,6 @@ examples:
>      beeper {
>        compatible = "pwm-beeper";
>        pwms = <&pwm0>;
> +      volume-levels-bp = <0 80 200 400 5000>;
> +      default-volume-level-bp = <80>;
>      };
> -- 
> 2.39.1
> 

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

end of thread, other threads:[~2023-02-03 21:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-01 15:21 [PATCH v10 0/4] input: pwm-beeper: add feature to set volume level Manuel Traut
2023-02-01 15:21 ` [PATCH v10 1/4] dt-bindings: input: pwm-beeper: Convert txt bindings to yaml Manuel Traut
2023-02-01 15:21 ` [PATCH v10 2/4] input: pwm-beeper: add feature to set volume via sysfs Manuel Traut
2023-02-01 15:21 ` [PATCH v10 3/4] dt-bindings: input: pwm-beeper: add volume Manuel Traut
2023-02-01 16:15   ` Krzysztof Kozlowski
2023-02-01 17:10     ` [PATCH] " Manuel Traut
2023-02-03 21:21       ` Rob Herring
2023-02-01 15:21 ` [PATCH v10 4/4] input: pwm-beeper: set volume levels by devicetree 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).