* [PATCH v8 1/5] dt-bindings: input: pwm-beeper: Convert txt bindings to yaml
2023-01-26 9:18 [PATCH v8 0/5] input: pwm-beeper: add feature to set volume level Manuel Traut
@ 2023-01-26 9:18 ` Manuel Traut
2023-01-26 12:32 ` Krzysztof Kozlowski
2023-01-26 13:21 ` Rob Herring
2023-01-26 9:18 ` [PATCH v8 2/5] input: pwm-beeper: add feature to set volume via sysfs Manuel Traut
` (3 subsequent siblings)
4 siblings, 2 replies; 13+ messages in thread
From: Manuel Traut @ 2023-01-26 9:18 UTC (permalink / raw)
To: linux-kernel
Cc: Manuel Traut, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Frieder Schrempf, linux-input, devicetree
Converts txt binding to new YAML format.
Signed-off-by: Manuel Traut <manuel.traut@mt.com>
---
.../devicetree/bindings/input/pwm-beeper.txt | 24 ----------
.../devicetree/bindings/input/pwm-beeper.yaml | 48 +++++++++++++++++++
2 files changed, 48 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..351df83d5cbe
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
@@ -0,0 +1,48 @@
+# 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: >
+ phandle to a regulator that acts as an amplifier for
+ the beeper
+
+ beeper-hz:
+ description: bell frequency in Hz
+
+required:
+ - compatible
+ - pwms
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ beeper_amp: amplifier {
+ compatible = "fixed-regulator";
+ gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
+ };
+
+ beeper {
+ compatible = "pwm-beeper";
+ pwms = <&pwm0>;
+ amp-supply = <&beeper_amp>;
+ };
--
2.39.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v8 1/5] dt-bindings: input: pwm-beeper: Convert txt bindings to yaml
2023-01-26 9:18 ` [PATCH v8 1/5] dt-bindings: input: pwm-beeper: Convert txt bindings to yaml Manuel Traut
@ 2023-01-26 12:32 ` Krzysztof Kozlowski
2023-01-26 13:21 ` Rob Herring
1 sibling, 0 replies; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-26 12:32 UTC (permalink / raw)
To: Manuel Traut, linux-kernel
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Frieder Schrempf, linux-input, devicetree
On 26/01/2023 10:18, Manuel Traut wrote:
> Converts txt binding to new YAML format.
>
> Signed-off-by: Manuel Traut <manuel.traut@mt.com>
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
> new file mode 100644
> index 000000000000..351df83d5cbe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> @@ -0,0 +1,48 @@
> +# 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#"
Drop quotes from both. Apologies for not noticing it earlier.
> +
> +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: >
> + phandle to a regulator that acts as an amplifier for
> + the beeper
Drop "phandle to a"
> +
> + beeper-hz:
> + description: bell frequency in Hz
> +
> +required:
> + - compatible
> + - pwms
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + beeper_amp: amplifier {
> + compatible = "fixed-regulator";
> + gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
> + };
Drop this device node, not related.
> +
> + beeper {
> + compatible = "pwm-beeper";
> + pwms = <&pwm0>;
> + amp-supply = <&beeper_amp>;
> + };
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v8 1/5] dt-bindings: input: pwm-beeper: Convert txt bindings to yaml
2023-01-26 9:18 ` [PATCH v8 1/5] dt-bindings: input: pwm-beeper: Convert txt bindings to yaml Manuel Traut
2023-01-26 12:32 ` Krzysztof Kozlowski
@ 2023-01-26 13:21 ` Rob Herring
1 sibling, 0 replies; 13+ messages in thread
From: Rob Herring @ 2023-01-26 13:21 UTC (permalink / raw)
To: Manuel Traut
Cc: Dmitry Torokhov, devicetree, Rob Herring, Krzysztof Kozlowski,
linux-input, linux-kernel, Frieder Schrempf
On Thu, 26 Jan 2023 10:18:21 +0100, Manuel Traut wrote:
> Converts txt binding to new YAML format.
>
> Signed-off-by: Manuel Traut <manuel.traut@mt.com>
> ---
> .../devicetree/bindings/input/pwm-beeper.txt | 24 ----------
> .../devicetree/bindings/input/pwm-beeper.yaml | 48 +++++++++++++++++++
> 2 files changed, 48 insertions(+), 24 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/input/pwm-beeper.txt
> create mode 100644 Documentation/devicetree/bindings/input/pwm-beeper.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/input/pwm-beeper.example.dtb: /example-0/amplifier: failed to match any schema with compatible: ['fixed-regulator']
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230126091825.220646-2-manuel.traut@mt.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v8 2/5] input: pwm-beeper: add feature to set volume via sysfs
2023-01-26 9:18 [PATCH v8 0/5] input: pwm-beeper: add feature to set volume level Manuel Traut
2023-01-26 9:18 ` [PATCH v8 1/5] dt-bindings: input: pwm-beeper: Convert txt bindings to yaml Manuel Traut
@ 2023-01-26 9:18 ` Manuel Traut
2023-01-26 9:18 ` [PATCH v8 3/5] input: pwm-beeper: set volume levels by devicetree Manuel Traut
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Manuel Traut @ 2023-01-26 9:18 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 | 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..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..865e1ec5c39d 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] = 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] 13+ messages in thread* [PATCH v8 3/5] input: pwm-beeper: set volume levels by devicetree
2023-01-26 9:18 [PATCH v8 0/5] input: pwm-beeper: add feature to set volume level Manuel Traut
2023-01-26 9:18 ` [PATCH v8 1/5] dt-bindings: input: pwm-beeper: Convert txt bindings to yaml Manuel Traut
2023-01-26 9:18 ` [PATCH v8 2/5] input: pwm-beeper: add feature to set volume via sysfs Manuel Traut
@ 2023-01-26 9:18 ` Manuel Traut
2023-01-26 9:18 ` [PATCH v8 4/5] dt-bindings: input: pwm-beeper: add volume Manuel Traut
2023-01-26 9:18 ` [PATCH v8 5/5] input: pwm-beeper: handle module unloading properly Manuel Traut
4 siblings, 0 replies; 13+ messages in thread
From: Manuel Traut @ 2023-01-26 9:18 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 | 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 865e1ec5c39d..82b05f7f4c70 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] 13+ messages in thread* [PATCH v8 4/5] dt-bindings: input: pwm-beeper: add volume
2023-01-26 9:18 [PATCH v8 0/5] input: pwm-beeper: add feature to set volume level Manuel Traut
` (2 preceding siblings ...)
2023-01-26 9:18 ` [PATCH v8 3/5] input: pwm-beeper: set volume levels by devicetree Manuel Traut
@ 2023-01-26 9:18 ` Manuel Traut
2023-01-26 12:36 ` Krzysztof Kozlowski
2023-01-26 9:18 ` [PATCH v8 5/5] input: pwm-beeper: handle module unloading properly Manuel Traut
4 siblings, 1 reply; 13+ messages in thread
From: Manuel Traut @ 2023-01-26 9:18 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 | 20 +++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.yaml b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
index 351df83d5cbe..f1f9283ca855 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.yaml
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
@@ -26,6 +26,24 @@ properties:
beeper-hz:
description: bell frequency in Hz
+ volume-levels:
+ description: >
+ 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).
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
+ default-volume-level:
+ description: >
+ The default volume level (index into the array defined
+ by the "volume-levels" property).
+ $ref: /schemas/types.yaml#/definitions/uint32
+
required:
- compatible
- pwms
@@ -45,4 +63,6 @@ examples:
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] 13+ messages in thread* Re: [PATCH v8 4/5] dt-bindings: input: pwm-beeper: add volume
2023-01-26 9:18 ` [PATCH v8 4/5] dt-bindings: input: pwm-beeper: add volume Manuel Traut
@ 2023-01-26 12:36 ` Krzysztof Kozlowski
2023-01-26 15:11 ` Manuel Traut
0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-26 12:36 UTC (permalink / raw)
To: Manuel Traut, linux-kernel
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Frieder Schrempf, linux-input, devicetree
On 26/01/2023 10:18, Manuel Traut wrote:
> Adds an array of supported volume levels and a default volume level.
>
> Signed-off-by: Manuel Traut <manuel.traut@mt.com>
This is the second patch. Bindings must be introduced before you start
using them.
> ---
> .../devicetree/bindings/input/pwm-beeper.yaml | 20 +++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.yaml b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> index 351df83d5cbe..f1f9283ca855 100644
> --- a/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> @@ -26,6 +26,24 @@ properties:
> beeper-hz:
> description: bell frequency in Hz
>
> + volume-levels:
use -bp suffix:
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml#L44
which will mean the unit is 1/100 of %, not 1/10. Then you can also drop
the $ref.
> + description: >
> + 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).
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> + default-volume-level:
I propose to use just the value, not the index, so the name should
finish with '-bp' and the $ref can be dropped.
> + description: >
> + The default volume level (index into the array defined
> + by the "volume-levels" property).
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> required:
> - compatible
> - pwms
> @@ -45,4 +63,6 @@ examples:
> compatible = "pwm-beeper";
> pwms = <&pwm0>;
> amp-supply = <&beeper_amp>;
> + volume-levels = <0 8 20 40 500>;
> + default-volume-level = <4>;
> };
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v8 4/5] dt-bindings: input: pwm-beeper: add volume
2023-01-26 12:36 ` Krzysztof Kozlowski
@ 2023-01-26 15:11 ` Manuel Traut
2023-01-26 15:20 ` Krzysztof Kozlowski
0 siblings, 1 reply; 13+ messages in thread
From: Manuel Traut @ 2023-01-26 15:11 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-kernel, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Frieder Schrempf, linux-input, devicetree
> This is the second patch. Bindings must be introduced before you start
> using them.
OK, will be done in v9.
> > ---
> > .../devicetree/bindings/input/pwm-beeper.yaml | 20 +++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.yaml b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> > index 351df83d5cbe..f1f9283ca855 100644
> > --- a/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> > +++ b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
> > @@ -26,6 +26,24 @@ properties:
> > beeper-hz:
> > description: bell frequency in Hz
> >
> > + volume-levels:
>
> use -bp suffix:
> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml#L44
>
> which will mean the unit is 1/100 of %, not 1/10. Then you can also drop
> the $ref.
OK, this is also fine for me. Chaned the description and implementation
for v9.
> > + description: >
> > + 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).
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > +
> > + default-volume-level:
>
> I propose to use just the value, not the index, so the name should
> finish with '-bp' and the $ref can be dropped.
I am not so happy with this suggestion. What shall be displayed in
sysfs as volume if a user specifies a value that is not defined in
the array.. So I tend to keep this as is.
Thanks for your feedback
Manuel
> > + description: >
> > + The default volume level (index into the array defined
> > + by the "volume-levels" property).
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > required:
> > - compatible
> > - pwms
> > @@ -45,4 +63,6 @@ examples:
> > compatible = "pwm-beeper";
> > pwms = <&pwm0>;
> > amp-supply = <&beeper_amp>;
> > + volume-levels = <0 8 20 40 500>;
> > + default-volume-level = <4>;
> > };
>
> Best regards,
> Krzysztof
>
--
Manuel Traut
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v8 4/5] dt-bindings: input: pwm-beeper: add volume
2023-01-26 15:11 ` Manuel Traut
@ 2023-01-26 15:20 ` Krzysztof Kozlowski
2023-01-26 16:08 ` Manuel Traut
0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-26 15:20 UTC (permalink / raw)
To: Manuel Traut
Cc: linux-kernel, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Frieder Schrempf, linux-input, devicetree
On 26/01/2023 16:11, Manuel Traut wrote:
>> This is the second patch. Bindings must be introduced before you start
>> using them.
>
> OK, will be done in v9.
>
>>> ---
>>> .../devicetree/bindings/input/pwm-beeper.yaml | 20 +++++++++++++++++++
>>> 1 file changed, 20 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.yaml b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
>>> index 351df83d5cbe..f1f9283ca855 100644
>>> --- a/Documentation/devicetree/bindings/input/pwm-beeper.yaml
>>> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.yaml
>>> @@ -26,6 +26,24 @@ properties:
>>> beeper-hz:
>>> description: bell frequency in Hz
>>>
>>> + volume-levels:
>>
>> use -bp suffix:
>> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml#L44
>>
>> which will mean the unit is 1/100 of %, not 1/10. Then you can also drop
>> the $ref.
>
> OK, this is also fine for me. Chaned the description and implementation
> for v9.
>
>>> + description: >
>>> + 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).
>>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +
>>> + default-volume-level:
>>
>> I propose to use just the value, not the index, so the name should
>> finish with '-bp' and the $ref can be dropped.
>
> I am not so happy with this suggestion. What shall be displayed in
> sysfs as volume if a user specifies a value that is not defined in
> the array.. So I tend to keep this as is.
sysfs is not related to bindings. To rephrase your question: what shall
be shown in anywhere if the index is not correct? The same problem, the
same solution.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v8 4/5] dt-bindings: input: pwm-beeper: add volume
2023-01-26 15:20 ` Krzysztof Kozlowski
@ 2023-01-26 16:08 ` Manuel Traut
0 siblings, 0 replies; 13+ messages in thread
From: Manuel Traut @ 2023-01-26 16:08 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-kernel, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Frieder Schrempf, linux-input, devicetree
> >>> + default-volume-level:
> >>
> >> I propose to use just the value, not the index, so the name should
> >> finish with '-bp' and the $ref can be dropped.
> >
> > I am not so happy with this suggestion. What shall be displayed in
> > sysfs as volume if a user specifies a value that is not defined in
> > the array.. So I tend to keep this as is.
>
> sysfs is not related to bindings. To rephrase your question: what shall
> be shown in anywhere if the index is not correct? The same problem, the
> same solution.
got it, thanks for the hint. I will implement it in v9.
Regards
Manuel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v8 5/5] input: pwm-beeper: handle module unloading properly
2023-01-26 9:18 [PATCH v8 0/5] input: pwm-beeper: add feature to set volume level Manuel Traut
` (3 preceding siblings ...)
2023-01-26 9:18 ` [PATCH v8 4/5] dt-bindings: input: pwm-beeper: add volume Manuel Traut
@ 2023-01-26 9:18 ` Manuel Traut
2023-01-30 9:07 ` Frieder Schrempf
4 siblings, 1 reply; 13+ messages in thread
From: Manuel Traut @ 2023-01-26 9:18 UTC (permalink / raw)
To: linux-kernel
Cc: Manuel Traut, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Frieder Schrempf, linux-input, devicetree
'input: pwm-beeper: add feature to set volume via sysfs' adds device
attributes without removing them on error or if the module is unloaded.
If the module will be unloaded and loaded again it fails:
[ 1007.918180] sysfs: cannot create duplicate filename '/devices/platform/buzzer/volume'
Therefore remove device attributes on module unloading and in case
registration at the input subsystem fails.
Signed-off-by: Manuel Traut <manuel.traut@mt.com>
---
drivers/input/misc/pwm-beeper.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index 82b05f7f4c70..736b89bd1b42 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -299,6 +299,7 @@ static int pwm_beeper_probe(struct platform_device *pdev)
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;
}
@@ -308,6 +309,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);
@@ -353,6 +365,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.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v8 5/5] input: pwm-beeper: handle module unloading properly
2023-01-26 9:18 ` [PATCH v8 5/5] input: pwm-beeper: handle module unloading properly Manuel Traut
@ 2023-01-30 9:07 ` Frieder Schrempf
0 siblings, 0 replies; 13+ messages in thread
From: Frieder Schrempf @ 2023-01-30 9:07 UTC (permalink / raw)
To: Manuel Traut, linux-kernel
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, linux-input,
devicetree
Hi Manuel,
On 26.01.23 10:18, Manuel Traut wrote:
> 'input: pwm-beeper: add feature to set volume via sysfs' adds device
> attributes without removing them on error or if the module is unloaded.
>
> If the module will be unloaded and loaded again it fails:
> [ 1007.918180] sysfs: cannot create duplicate filename '/devices/platform/buzzer/volume'
>
> Therefore remove device attributes on module unloading and in case
> registration at the input subsystem fails.
>
> Signed-off-by: Manuel Traut <manuel.traut@mt.com>
Thanks for picking up these old patches!
I think you need to merge this fixup patch 5/5 into patch 2/5 of this
series.
Thanks
Frieder
^ permalink raw reply [flat|nested] 13+ messages in thread