[PATCH 1/1] input: pwm-beeper: add feature to set volume via sysfs

[PATCH 1/1] input: pwm-beeper: add feature to set volume via sysfs

[Schrempf Frieder]

Make the driver accept different 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 number of volume levels available and their values can
be specified via device tree (similar to pwm-backlight).

This patch was tested with linux-imx 3.10.17 and was
applied to current mainline without any changes.

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
 .../devicetree/bindings/input/pwm-beeper.txt    |  20 ++++
 drivers/input/misc/pwm-beeper.c                 | 109 ++++++++++++++++++-
 2 files changed, 126 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
index be332ae..153cd3f 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
@@ -5,3 +5,23 @@ Registers a PWM device as beeper.
 Required properties:
 - compatible: should be "pwm-beeper"
 - pwms: phandle to the physical PWM device
+- volume-levels: Array of distinct 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 volume level can be set via sysfs under /sys/class/input/inputX/volume.
+The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level.
+
+Example:
+
+	pwm-beeper {
+		compatible = "pwm-beeper";
+		pwms = <&pwm4 0 5000>;
+		volume-levels = <0 8 20 40 500>;
+		default-volume-level = <4>;
+	};
diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index f2261ab..f3eb8f8 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -1,5 +1,9 @@
 /*
  *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
+ *
+ *  Copyright (C) 2016, Frieder Schrempf <frieder.schrempf@exceet.de>
+ *  (volume support)
+ *
  *  PWM beeper driver
  *
  *  This program is free software; you can redistribute it and/or modify it
@@ -25,10 +29,69 @@ struct pwm_beeper {
 	struct input_dev *input;
 	struct pwm_device *pwm;
 	unsigned long period;
+	unsigned int volume;
+	unsigned int *volume_levels;
+	unsigned int max_volume_level;
 };
 
 #define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
 
+static ssize_t beeper_show_volume(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 beeper_show_max_volume_level(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_level);
+}
+
+static ssize_t beeper_store_volume(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;
+
+	rc = -ENXIO;
+	if (volume > beeper->max_volume_level)
+		volume = beeper->max_volume_level;
+	pr_debug("set volume to %u\n", volume);
+	if (beeper->volume != volume)
+		beeper->volume = volume;
+	rc = count;
+
+	return rc;
+}
+
+static DEVICE_ATTR(volume, 0644, beeper_show_volume, beeper_store_volume);
+static DEVICE_ATTR(max_volume_level, 0644, beeper_show_max_volume_level, NULL);
+
+static struct attribute *bp_device_attributes[] = {
+	&dev_attr_volume.attr,
+	&dev_attr_max_volume_level.attr,
+	NULL,
+};
+
+static struct attribute_group bp_device_attr_group = {
+	.attrs = bp_device_attributes,
+};
+
+static const struct attribute_group *bp_device_attr_groups[] = {
+	&bp_device_attr_group,
+	NULL,
+};
+
 static int pwm_beeper_event(struct input_dev *input,
 			    unsigned int type, unsigned int code, int value)
 {
@@ -53,7 +116,9 @@ static int pwm_beeper_event(struct input_dev *input,
 		pwm_disable(beeper->pwm);
 	} else {
 		period = HZ_TO_NANOSECONDS(value);
-		ret = pwm_config(beeper->pwm, period / 2, period);
+		ret = pwm_config(beeper->pwm,
+			period / 1000 * beeper->volume_levels[beeper->volume],
+			period);
 		if (ret)
 			return ret;
 		ret = pwm_enable(beeper->pwm);
@@ -68,8 +133,11 @@ static int pwm_beeper_event(struct input_dev *input,
 static int pwm_beeper_probe(struct platform_device *pdev)
 {
 	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
 	struct pwm_beeper *beeper;
-	int error;
+	struct property *prop;
+	int error, length;
+	u32 value;
 
 	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
 	if (!beeper)
@@ -87,6 +155,36 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 		goto err_free;
 	}
 
+	/* determine the number of volume levels */
+	prop = of_find_property(np, "volume-levels", &length);
+	if (!prop)
+		return -EINVAL;
+
+	beeper->max_volume_level = length / sizeof(u32);
+
+	/* read volume levels from DT property */
+	if (beeper->max_volume_level > 0) {
+		size_t size = sizeof(*beeper->volume_levels) *
+			beeper->max_volume_level;
+
+		beeper->volume_levels = devm_kzalloc(&(pdev->dev), size,
+			GFP_KERNEL);
+		if (!beeper->volume_levels)
+			return -ENOMEM;
+
+		error = of_property_read_u32_array(np, "volume-levels",
+						 beeper->volume_levels,
+						 beeper->max_volume_level);
+
+		error = of_property_read_u32(np, "default-volume-level",
+					   &value);
+		if (error < 0)
+			return error;
+
+		beeper->volume = value;
+		beeper->max_volume_level--;
+	}
+
 	beeper->input = input_allocate_device();
 	if (!beeper->input) {
 		dev_err(&pdev->dev, "Failed to allocate input device\n");
@@ -109,6 +207,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	input_set_drvdata(beeper->input, beeper);
 
+	beeper->input->dev.groups = bp_device_attr_groups;
+
 	error = input_register_device(beeper->input);
 	if (error) {
 		dev_err(&pdev->dev, "Failed to register input device: %d\n", error);
@@ -158,7 +258,10 @@ static int __maybe_unused pwm_beeper_resume(struct device *dev)
 	struct pwm_beeper *beeper = dev_get_drvdata(dev);
 
 	if (beeper->period) {
-		pwm_config(beeper->pwm, beeper->period / 2, beeper->period);
+		pwm_config(beeper->pwm,
+			beeper->period / 1000 *
+			beeper->volume_levels[beeper->volume],
+			beeper->period);
 		pwm_enable(beeper->pwm);
 	}
 
-- 
1.9.1

[PATCH 1/1] input: pwm-beeper: add feature to set volume via sysfs

[Schrempf Frieder]

Make the driver accept different 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 number of volume levels available and their values can
be specified via device tree (similar to pwm-backlight).

This patch was tested with linux-imx 3.10.17 and was
applied to current mainline without any changes.

Signed-off-by: Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
---
 .../devicetree/bindings/input/pwm-beeper.txt    |  20 ++++
 drivers/input/misc/pwm-beeper.c                 | 109 ++++++++++++++++++-
 2 files changed, 126 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
index be332ae..153cd3f 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
@@ -5,3 +5,23 @@ Registers a PWM device as beeper.
 Required properties:
 - compatible: should be "pwm-beeper"
 - pwms: phandle to the physical PWM device
+- volume-levels: Array of distinct 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 volume level can be set via sysfs under /sys/class/input/inputX/volume.
+The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level.
+
+Example:
+
+	pwm-beeper {
+		compatible = "pwm-beeper";
+		pwms = <&pwm4 0 5000>;
+		volume-levels = <0 8 20 40 500>;
+		default-volume-level = <4>;
+	};
diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index f2261ab..f3eb8f8 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -1,5 +1,9 @@
 /*
  *  Copyright (C) 2010, Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
+ *
+ *  Copyright (C) 2016, Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
+ *  (volume support)
+ *
  *  PWM beeper driver
  *
  *  This program is free software; you can redistribute it and/or modify it
@@ -25,10 +29,69 @@ struct pwm_beeper {
 	struct input_dev *input;
 	struct pwm_device *pwm;
 	unsigned long period;
+	unsigned int volume;
+	unsigned int *volume_levels;
+	unsigned int max_volume_level;
 };
 
 #define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
 
+static ssize_t beeper_show_volume(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 beeper_show_max_volume_level(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_level);
+}
+
+static ssize_t beeper_store_volume(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;
+
+	rc = -ENXIO;
+	if (volume > beeper->max_volume_level)
+		volume = beeper->max_volume_level;
+	pr_debug("set volume to %u\n", volume);
+	if (beeper->volume != volume)
+		beeper->volume = volume;
+	rc = count;
+
+	return rc;
+}
+
+static DEVICE_ATTR(volume, 0644, beeper_show_volume, beeper_store_volume);
+static DEVICE_ATTR(max_volume_level, 0644, beeper_show_max_volume_level, NULL);
+
+static struct attribute *bp_device_attributes[] = {
+	&dev_attr_volume.attr,
+	&dev_attr_max_volume_level.attr,
+	NULL,
+};
+
+static struct attribute_group bp_device_attr_group = {
+	.attrs = bp_device_attributes,
+};
+
+static const struct attribute_group *bp_device_attr_groups[] = {
+	&bp_device_attr_group,
+	NULL,
+};
+
 static int pwm_beeper_event(struct input_dev *input,
 			    unsigned int type, unsigned int code, int value)
 {
@@ -53,7 +116,9 @@ static int pwm_beeper_event(struct input_dev *input,
 		pwm_disable(beeper->pwm);
 	} else {
 		period = HZ_TO_NANOSECONDS(value);
-		ret = pwm_config(beeper->pwm, period / 2, period);
+		ret = pwm_config(beeper->pwm,
+			period / 1000 * beeper->volume_levels[beeper->volume],
+			period);
 		if (ret)
 			return ret;
 		ret = pwm_enable(beeper->pwm);
@@ -68,8 +133,11 @@ static int pwm_beeper_event(struct input_dev *input,
 static int pwm_beeper_probe(struct platform_device *pdev)
 {
 	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
 	struct pwm_beeper *beeper;
-	int error;
+	struct property *prop;
+	int error, length;
+	u32 value;
 
 	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
 	if (!beeper)
@@ -87,6 +155,36 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 		goto err_free;
 	}
 
+	/* determine the number of volume levels */
+	prop = of_find_property(np, "volume-levels", &length);
+	if (!prop)
+		return -EINVAL;
+
+	beeper->max_volume_level = length / sizeof(u32);
+
+	/* read volume levels from DT property */
+	if (beeper->max_volume_level > 0) {
+		size_t size = sizeof(*beeper->volume_levels) *
+			beeper->max_volume_level;
+
+		beeper->volume_levels = devm_kzalloc(&(pdev->dev), size,
+			GFP_KERNEL);
+		if (!beeper->volume_levels)
+			return -ENOMEM;
+
+		error = of_property_read_u32_array(np, "volume-levels",
+						 beeper->volume_levels,
+						 beeper->max_volume_level);
+
+		error = of_property_read_u32(np, "default-volume-level",
+					   &value);
+		if (error < 0)
+			return error;
+
+		beeper->volume = value;
+		beeper->max_volume_level--;
+	}
+
 	beeper->input = input_allocate_device();
 	if (!beeper->input) {
 		dev_err(&pdev->dev, "Failed to allocate input device\n");
@@ -109,6 +207,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	input_set_drvdata(beeper->input, beeper);
 
+	beeper->input->dev.groups = bp_device_attr_groups;
+
 	error = input_register_device(beeper->input);
 	if (error) {
 		dev_err(&pdev->dev, "Failed to register input device: %d\n", error);
@@ -158,7 +258,10 @@ static int __maybe_unused pwm_beeper_resume(struct device *dev)
 	struct pwm_beeper *beeper = dev_get_drvdata(dev);
 
 	if (beeper->period) {
-		pwm_config(beeper->pwm, beeper->period / 2, beeper->period);
+		pwm_config(beeper->pwm,
+			beeper->period / 1000 *
+			beeper->volume_levels[beeper->volume],
+			beeper->period);
 		pwm_enable(beeper->pwm);
 	}
 
-- 
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] input: pwm-beeper: add feature to set volume via sysfs

[Rob Herring]

On Thu, Apr 07, 2016 at 03:58:40PM +0000, Schrempf Frieder wrote:
> Make the driver accept different volume levels via sysfs.
> This can be helpful if the beep/bell sound intensity needs
> to be adapted to the environment of the device.

It is preferred that bindings be a separate patch. Also, this is 2 
independent features: the sysfs interface and DT properties. So that is 
probably 3 patches.

Also, sysfs interfaces are supposed to be documented.

> 
> The number of volume levels available and their values can
> be specified via device tree (similar to pwm-backlight).
> 
> This patch was tested with linux-imx 3.10.17 and was
> applied to current mainline without any changes.

You need to test with something not 3 years old...

> 
> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
> ---
>  .../devicetree/bindings/input/pwm-beeper.txt    |  20 ++++
>  drivers/input/misc/pwm-beeper.c                 | 109 ++++++++++++++++++-
>  2 files changed, 126 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> index be332ae..153cd3f 100644
> --- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> @@ -5,3 +5,23 @@ Registers a PWM device as beeper.
>  Required properties:
>  - compatible: should be "pwm-beeper"
>  - pwms: phandle to the physical PWM device
> +- volume-levels: Array of distinct 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).

Where does the scale of 0-500 come from? It is tenth of a percent?

How many array entries?

> +      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 volume level can be set via sysfs under /sys/class/input/inputX/volume.
> +The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level.
> +
> +Example:
> +
> +	pwm-beeper {
> +		compatible = "pwm-beeper";
> +		pwms = <&pwm4 0 5000>;
> +		volume-levels = <0 8 20 40 500>;
> +		default-volume-level = <4>;
> +	};

Re: [PATCH 1/1] input: pwm-beeper: add feature to set volume via sysfs

[Rob Herring]

On Thu, Apr 07, 2016 at 03:58:40PM +0000, Schrempf Frieder wrote:
> Make the driver accept different volume levels via sysfs.
> This can be helpful if the beep/bell sound intensity needs
> to be adapted to the environment of the device.

It is preferred that bindings be a separate patch. Also, this is 2 
independent features: the sysfs interface and DT properties. So that is 
probably 3 patches.

Also, sysfs interfaces are supposed to be documented.

> 
> The number of volume levels available and their values can
> be specified via device tree (similar to pwm-backlight).
> 
> This patch was tested with linux-imx 3.10.17 and was
> applied to current mainline without any changes.

You need to test with something not 3 years old...

> 
> Signed-off-by: Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
> ---
>  .../devicetree/bindings/input/pwm-beeper.txt    |  20 ++++
>  drivers/input/misc/pwm-beeper.c                 | 109 ++++++++++++++++++-
>  2 files changed, 126 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> index be332ae..153cd3f 100644
> --- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> @@ -5,3 +5,23 @@ Registers a PWM device as beeper.
>  Required properties:
>  - compatible: should be "pwm-beeper"
>  - pwms: phandle to the physical PWM device
> +- volume-levels: Array of distinct 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).

Where does the scale of 0-500 come from? It is tenth of a percent?

How many array entries?

> +      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 volume level can be set via sysfs under /sys/class/input/inputX/volume.
> +The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level.
> +
> +Example:
> +
> +	pwm-beeper {
> +		compatible = "pwm-beeper";
> +		pwms = <&pwm4 0 5000>;
> +		volume-levels = <0 8 20 40 500>;
> +		default-volume-level = <4>;
> +	};
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] input: pwm-beeper: add feature to set volume via sysfs

[Schrempf Frieder]

Thank you Rob for looking at my first kernel contribution.
I come back to it now, as I had some time to process your comments.

On 11.04.2016 17:21, Rob Herring wrote:
> On Thu, Apr 07, 2016 at 03:58:40PM +0000, Schrempf Frieder wrote:
>> Make the driver accept different volume levels via sysfs.
>> This can be helpful if the beep/bell sound intensity needs
>> to be adapted to the environment of the device.
>
> It is preferred that bindings be a separate patch. Also, this is 2
> independent features: the sysfs interface and DT properties. So that is
> probably 3 patches.
>
> Also, sysfs interfaces are supposed to be documented.

I have splitted this into 3 patches and will send them soon as v2.
I also added documentation for the sysfs interface.

>
>>
>> The number of volume levels available and their values can
>> be specified via device tree (similar to pwm-backlight).
>>
>> This patch was tested with linux-imx 3.10.17 and was
>> applied to current mainline without any changes.
>
> You need to test with something not 3 years old...

I tested the v2 patches with Linux 4.7.4 now.

>
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
>> ---
>>  .../devicetree/bindings/input/pwm-beeper.txt    |  20 ++++
>>  drivers/input/misc/pwm-beeper.c                 | 109 ++++++++++++++++++-
>>  2 files changed, 126 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>> index be332ae..153cd3f 100644
>> --- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
>> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>> @@ -5,3 +5,23 @@ Registers a PWM device as beeper.
>>  Required properties:
>>  - compatible: should be "pwm-beeper"
>>  - pwms: phandle to the physical PWM device
>> +- volume-levels: Array of distinct 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).
>
> Where does the scale of 0-500 come from? It is tenth of a percent?

I used tenth of a percent because this is needed to define low volume 
levels with pwm duty cycles in the range of <1%.
This is necessary because of the highly non-linear relation between duty 
cycle and actual perceived volume.

The upper limit of 500 therefore results from 50% being the duty cycle 
with maximum volume.

>
> How many array entries?

The array size and therefore the number of volume levels is dynamic and 
only defined by the number of entries in the volume-levels property.

>
>> +      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 volume level can be set via sysfs under /sys/class/input/inputX/volume.
>> +The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level.
>> +
>> +Example:
>> +
>> +	pwm-beeper {
>> +		compatible = "pwm-beeper";
>> +		pwms = <&pwm4 0 5000>;
>> +		volume-levels = <0 8 20 40 500>;
>> +		default-volume-level = <4>;
>> +	};

Re: [PATCH 1/1] input: pwm-beeper: add feature to set volume via sysfs

[Schrempf Frieder]

Thank you Rob for looking at my first kernel contribution.
I come back to it now, as I had some time to process your comments.

On 11.04.2016 17:21, Rob Herring wrote:
> On Thu, Apr 07, 2016 at 03:58:40PM +0000, Schrempf Frieder wrote:
>> Make the driver accept different volume levels via sysfs.
>> This can be helpful if the beep/bell sound intensity needs
>> to be adapted to the environment of the device.
>
> It is preferred that bindings be a separate patch. Also, this is 2
> independent features: the sysfs interface and DT properties. So that is
> probably 3 patches.
>
> Also, sysfs interfaces are supposed to be documented.

I have splitted this into 3 patches and will send them soon as v2.
I also added documentation for the sysfs interface.

>
>>
>> The number of volume levels available and their values can
>> be specified via device tree (similar to pwm-backlight).
>>
>> This patch was tested with linux-imx 3.10.17 and was
>> applied to current mainline without any changes.
>
> You need to test with something not 3 years old...

I tested the v2 patches with Linux 4.7.4 now.

>
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
>> ---
>>  .../devicetree/bindings/input/pwm-beeper.txt    |  20 ++++
>>  drivers/input/misc/pwm-beeper.c                 | 109 ++++++++++++++++++-
>>  2 files changed, 126 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>> index be332ae..153cd3f 100644
>> --- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
>> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>> @@ -5,3 +5,23 @@ Registers a PWM device as beeper.
>>  Required properties:
>>  - compatible: should be "pwm-beeper"
>>  - pwms: phandle to the physical PWM device
>> +- volume-levels: Array of distinct 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).
>
> Where does the scale of 0-500 come from? It is tenth of a percent?

I used tenth of a percent because this is needed to define low volume 
levels with pwm duty cycles in the range of <1%.
This is necessary because of the highly non-linear relation between duty 
cycle and actual perceived volume.

The upper limit of 500 therefore results from 50% being the duty cycle 
with maximum volume.

>
> How many array entries?

The array size and therefore the number of volume levels is dynamic and 
only defined by the number of entries in the volume-levels property.

>
>> +      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 volume level can be set via sysfs under /sys/class/input/inputX/volume.
>> +The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level.
>> +
>> +Example:
>> +
>> +	pwm-beeper {
>> +		compatible = "pwm-beeper";
>> +		pwms = <&pwm4 0 5000>;
>> +		volume-levels = <0 8 20 40 500>;
>> +		default-volume-level = <4>;
>> +	};--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/3] input: pwm-beeper: add feature to set volume level

[Schrempf Frieder]

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 number of volume levels available and their values can
be specified via device tree (similar to pwm-backlight).

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

Changes in v2:
 - split into 3 separate patches
 - make volume devicetree properties optional
 - tested with Linux 4.7.4

Frieder Schrempf (3):
  input: pwm-beeper: add feature to set volume via sysfs
  input: pwm-beeper: add documentation for volume devicetree bindings
  input: pwm-beeper: add devicetree bindings to set volume levels

 .../ABI/testing/sysfs-class-input-pwm-beeper       |  17 +++
 .../devicetree/bindings/input/pwm-beeper.txt       |  22 ++++
 drivers/input/misc/pwm-beeper.c                    | 118 ++++++++++++++++++++-
 3 files changed, 155 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-input-pwm-beeper

-- 
1.9.1

[PATCH v2 1/3] input: pwm-beeper: add feature to set volume via sysfs

[Schrempf Frieder]

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>
---
Changes in v2:
 - split into 3 separate patches

 .../ABI/testing/sysfs-class-input-pwm-beeper       | 17 ++++++
 drivers/input/misc/pwm-beeper.c                    | 71 +++++++++++++++++++++-
 2 files changed, 87 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-input-pwm-beeper

diff --git a/Documentation/ABI/testing/sysfs-class-input-pwm-beeper b/Documentation/ABI/testing/sysfs-class-input-pwm-beeper
new file mode 100644
index 0000000..bcf8d8b
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-input-pwm-beeper
@@ -0,0 +1,17 @@
+What:		/sys/class/input/input(x)/volume
+Date:		April 2016
+KernelVersion:
+Contact:	Frieder Schrempf <frieder.schrempf@exceet.de>
+Description:
+		Control the volume of this pwm-beeper. Values
+		are between 0 and max_volume_level. This file will also
+		show the current volume level stored in the driver.
+
+What:		/sys/class/input/input(x)/max_volume_level
+Date:		April 2016
+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 5f9655d..3ed21da 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -1,5 +1,9 @@
 /*
  *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
+ *
+ *  Copyright (C) 2016, Frieder Schrempf <frieder.schrempf@exceet.de>
+ *  (volume support)
+ *
  *  PWM beeper driver
  *
  *  This program is free software; you can redistribute it and/or modify it
@@ -27,16 +31,77 @@ struct pwm_beeper {
 	struct pwm_device *pwm;
 	struct work_struct work;
 	unsigned long period;
+	unsigned int volume;
+	unsigned int volume_levels[] = {0, 8, 20, 40, 500};
+	unsigned int max_volume_level;
 };
 
 #define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
 
+static ssize_t beeper_show_volume(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 beeper_show_max_volume_level(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_level);
+}
+
+static ssize_t beeper_store_volume(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;
+
+	rc = -ENXIO;
+	if (volume > beeper->max_volume_level)
+		volume = beeper->max_volume_level;
+	pr_debug("set volume to %u\n", volume);
+	if (beeper->volume != volume)
+		beeper->volume = volume;
+	rc = count;
+
+	return rc;
+}
+
+static DEVICE_ATTR(volume, 0644, beeper_show_volume, beeper_store_volume);
+static DEVICE_ATTR(max_volume_level, 0644, beeper_show_max_volume_level, NULL);
+
+static struct attribute *bp_device_attributes[] = {
+	&dev_attr_volume.attr,
+	&dev_attr_max_volume_level.attr,
+	NULL,
+};
+
+static struct attribute_group bp_device_attr_group = {
+	.attrs = bp_device_attributes,
+};
+
+static const struct attribute_group *bp_device_attr_groups[] = {
+	&bp_device_attr_group,
+	NULL,
+};
+
 static void __pwm_beeper_set(struct pwm_beeper *beeper)
 {
 	unsigned long period = beeper->period;
 
 	if (period) {
-		pwm_config(beeper->pwm, period / 2, period);
+		pwm_config(beeper->pwm,
+			period / 1000 * beeper->volume_levels[beeper->volume],
+			period);
 		pwm_enable(beeper->pwm);
 	} else
 		pwm_disable(beeper->pwm);
@@ -123,6 +188,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	INIT_WORK(&beeper->work, pwm_beeper_work);
 
+	beeper->max_volume_level = ARRAY_SIZE(beeper->volume_levels) - 1;
+
 	beeper->input = input_allocate_device();
 	if (!beeper->input) {
 		dev_err(&pdev->dev, "Failed to allocate input device\n");
@@ -146,6 +213,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	input_set_drvdata(beeper->input, beeper);
 
+	beeper->input->dev.groups = bp_device_attr_groups;
+
 	error = input_register_device(beeper->input);
 	if (error) {
 		dev_err(&pdev->dev, "Failed to register input device: %d\n", error);
-- 
1.9.1

[PATCH v2 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Schrempf Frieder]

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

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
Changes in v2:
 - split into 3 separate patches
 - make volume properties optional

 .../devicetree/bindings/input/pwm-beeper.txt       | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
index be332ae..6d8ba4e 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
@@ -5,3 +5,25 @@ Registers a PWM device as beeper.
 Required properties:
 - compatible: should be "pwm-beeper"
 - pwms: phandle to the physical PWM device
+
+Optional properties:
+- volume-levels: Array of distinct 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 volume level can be set via sysfs under /sys/class/input/inputX/volume.
+The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level.
+
+Example:
+
+	pwm-beeper {
+		compatible = "pwm-beeper";
+		pwms = <&pwm4 0 5000>;
+		volume-levels = <0 8 20 40 500>;
+		default-volume-level = <4>;
+	};
-- 
1.9.1

[PATCH v2 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Schrempf Frieder]

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

Signed-off-by: Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
---
Changes in v2:
 - split into 3 separate patches
 - make volume properties optional

 .../devicetree/bindings/input/pwm-beeper.txt       | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
index be332ae..6d8ba4e 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
@@ -5,3 +5,25 @@ Registers a PWM device as beeper.
 Required properties:
 - compatible: should be "pwm-beeper"
 - pwms: phandle to the physical PWM device
+
+Optional properties:
+- volume-levels: Array of distinct 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 volume level can be set via sysfs under /sys/class/input/inputX/volume.
+The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level.
+
+Example:
+
+	pwm-beeper {
+		compatible = "pwm-beeper";
+		pwms = <&pwm4 0 5000>;
+		volume-levels = <0 8 20 40 500>;
+		default-volume-level = <4>;
+	};
-- 
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 3/3] input: pwm-beeper: add devicetree bindings to set volume levels

[Schrempf Frieder]

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>
---
Changes in v2:
 - split into 3 separate patches
 - make volume properties optional

 drivers/input/misc/pwm-beeper.c | 49 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index 3ed21da..a7f9d70 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -32,7 +32,7 @@ struct pwm_beeper {
 	struct work_struct work;
 	unsigned long period;
 	unsigned int volume;
-	unsigned int volume_levels[] = {0, 8, 20, 40, 500};
+	unsigned int *volume_levels;
 	unsigned int max_volume_level;
 };
 
@@ -161,8 +161,11 @@ static void pwm_beeper_close(struct input_dev *input)
 static int pwm_beeper_probe(struct platform_device *pdev)
 {
 	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
 	struct pwm_beeper *beeper;
-	int error;
+	struct property *prop;
+	int error, length;
+	u32 value;
 
 	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
 	if (!beeper)
@@ -188,7 +191,47 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	INIT_WORK(&beeper->work, pwm_beeper_work);
 
-	beeper->max_volume_level = ARRAY_SIZE(beeper->volume_levels) - 1;
+	/* determine the number of volume levels */
+	prop = of_find_property(np, "volume-levels", &length);
+	if (!prop) {
+		dev_dbg(&pdev->dev, "no volume levels specified, using max volume\n");
+		beeper->max_volume_level = 1;
+	} else
+		beeper->max_volume_level = length / sizeof(u32);
+
+	/* read volume levels from DT property */
+	if (beeper->max_volume_level > 0) {
+		size_t size = sizeof(*beeper->volume_levels) *
+			beeper->max_volume_level;
+
+		beeper->volume_levels = devm_kzalloc(&(pdev->dev), size,
+			GFP_KERNEL);
+		if (!beeper->volume_levels)
+			return -ENOMEM;
+
+		if (prop) {
+			error = of_property_read_u32_array(np, "volume-levels",
+						beeper->volume_levels,
+						beeper->max_volume_level);
+
+			if (error < 0)
+				return error;
+
+			error = of_property_read_u32(np, "default-volume-level",
+						   &value);
+
+			if (error < 0) {
+				dev_dbg(&pdev->dev, "no default volume specified, using max volume\n");
+				value = beeper->max_volume_level - 1;
+			}
+		} else {
+			beeper->volume_levels[0] = 500;
+			value = 0;
+		}
+
+		beeper->volume = value;
+		beeper->max_volume_level--;
+	}
 
 	beeper->input = input_allocate_device();
 	if (!beeper->input) {
-- 
1.9.1

Re: [PATCH v2 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Rob Herring]

On Fri, Oct 07, 2016 at 09:08:17AM +0000, Schrempf Frieder wrote:
> This patch adds the documentation for the devicetree bindings to set
> the volume levels.
> 
> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
> ---
> Changes in v2:
>  - split into 3 separate patches
>  - make volume properties optional
> 
>  .../devicetree/bindings/input/pwm-beeper.txt       | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> index be332ae..6d8ba4e 100644
> --- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> @@ -5,3 +5,25 @@ Registers a PWM device as beeper.
>  Required properties:
>  - compatible: should be "pwm-beeper"
>  - pwms: phandle to the physical PWM device
> +
> +Optional properties:
> +- volume-levels: Array of distinct 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).

What does the index correspond to? The linear volume?

> +- default-volume-level: the default volume level (index into the
> +      array defined by the "volume-levels" property)
> +
> +The volume level can be set via sysfs under /sys/class/input/inputX/volume.
> +The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level.
> +
> +Example:
> +
> +	pwm-beeper {
> +		compatible = "pwm-beeper";
> +		pwms = <&pwm4 0 5000>;
> +		volume-levels = <0 8 20 40 500>;
> +		default-volume-level = <4>;
> +	};
> -- 
> 1.9.1

Re: [PATCH v2 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Rob Herring]

On Fri, Oct 07, 2016 at 09:08:17AM +0000, Schrempf Frieder wrote:
> This patch adds the documentation for the devicetree bindings to set
> the volume levels.
> 
> Signed-off-by: Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
> ---
> Changes in v2:
>  - split into 3 separate patches
>  - make volume properties optional
> 
>  .../devicetree/bindings/input/pwm-beeper.txt       | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> index be332ae..6d8ba4e 100644
> --- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> @@ -5,3 +5,25 @@ Registers a PWM device as beeper.
>  Required properties:
>  - compatible: should be "pwm-beeper"
>  - pwms: phandle to the physical PWM device
> +
> +Optional properties:
> +- volume-levels: Array of distinct 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).

What does the index correspond to? The linear volume?

> +- default-volume-level: the default volume level (index into the
> +      array defined by the "volume-levels" property)
> +
> +The volume level can be set via sysfs under /sys/class/input/inputX/volume.
> +The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level.
> +
> +Example:
> +
> +	pwm-beeper {
> +		compatible = "pwm-beeper";
> +		pwms = <&pwm4 0 5000>;
> +		volume-levels = <0 8 20 40 500>;
> +		default-volume-level = <4>;
> +	};
> -- 
> 1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Schrempf Frieder]

On 10.10.2016 17:20, Rob Herring wrote:
> On Fri, Oct 07, 2016 at 09:08:17AM +0000, Schrempf Frieder wrote:
>> This patch adds the documentation for the devicetree bindings to set
>> the volume levels.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
>> ---
>> Changes in v2:
>>  - split into 3 separate patches
>>  - make volume properties optional
>>
>>  .../devicetree/bindings/input/pwm-beeper.txt       | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>> index be332ae..6d8ba4e 100644
>> --- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
>> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>> @@ -5,3 +5,25 @@ Registers a PWM device as beeper.
>>  Required properties:
>>  - compatible: should be "pwm-beeper"
>>  - pwms: phandle to the physical PWM device
>> +
>> +Optional properties:
>> +- volume-levels: Array of distinct 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).
>
> What does the index correspond to? The linear volume?

In most cases users probably need linear volume levels (e.g. 0%, 25%, 
50%, 75%, 100%) and in this case the index would indeed correspond to 
the linear perceived volume.

But also non-linear relations are possible (e.g. 0%, 20%, 100%), if the 
user needs for example "mute", "low", "high" as volume levels.

The linearization (defining the corresponding duty cycle for each index) 
depends on the beeper and the perception of the user.

For the example array definition below, I tried different duty cycles 
and found values of 0.8%, 2%, 4%, 50% to be approximately correspondent 
to perceived volume levels of 25%, 50%, 75%, 100% in my case.

>
>> +- default-volume-level: the default volume level (index into the
>> +      array defined by the "volume-levels" property)
>> +
>> +The volume level can be set via sysfs under /sys/class/input/inputX/volume.
>> +The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level.
>> +
>> +Example:
>> +
>> +	pwm-beeper {
>> +		compatible = "pwm-beeper";
>> +		pwms = <&pwm4 0 5000>;
>> +		volume-levels = <0 8 20 40 500>;
>> +		default-volume-level = <4>;
>> +	};
>> --
>> 1.9.1

Re: [PATCH v2 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Schrempf Frieder]

On 10.10.2016 17:20, Rob Herring wrote:
> On Fri, Oct 07, 2016 at 09:08:17AM +0000, Schrempf Frieder wrote:
>> This patch adds the documentation for the devicetree bindings to set
>> the volume levels.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
>> ---
>> Changes in v2:
>>  - split into 3 separate patches
>>  - make volume properties optional
>>
>>  .../devicetree/bindings/input/pwm-beeper.txt       | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>> index be332ae..6d8ba4e 100644
>> --- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
>> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>> @@ -5,3 +5,25 @@ Registers a PWM device as beeper.
>>  Required properties:
>>  - compatible: should be "pwm-beeper"
>>  - pwms: phandle to the physical PWM device
>> +
>> +Optional properties:
>> +- volume-levels: Array of distinct 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).
>
> What does the index correspond to? The linear volume?

In most cases users probably need linear volume levels (e.g. 0%, 25%, 
50%, 75%, 100%) and in this case the index would indeed correspond to 
the linear perceived volume.

But also non-linear relations are possible (e.g. 0%, 20%, 100%), if the 
user needs for example "mute", "low", "high" as volume levels.

The linearization (defining the corresponding duty cycle for each index) 
depends on the beeper and the perception of the user.

For the example array definition below, I tried different duty cycles 
and found values of 0.8%, 2%, 4%, 50% to be approximately correspondent 
to perceived volume levels of 25%, 50%, 75%, 100% in my case.

>
>> +- default-volume-level: the default volume level (index into the
>> +      array defined by the "volume-levels" property)
>> +
>> +The volume level can be set via sysfs under /sys/class/input/inputX/volume.
>> +The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level.
>> +
>> +Example:
>> +
>> +	pwm-beeper {
>> +		compatible = "pwm-beeper";
>> +		pwms = <&pwm4 0 5000>;
>> +		volume-levels = <0 8 20 40 500>;
>> +		default-volume-level = <4>;
>> +	};
>> --
>> 1.9.1--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Rob Herring]

On Tue, Oct 11, 2016 at 3:17 AM, Schrempf Frieder
<frieder.schrempf@exceet.de> wrote:
> On 10.10.2016 17:20, Rob Herring wrote:
>> On Fri, Oct 07, 2016 at 09:08:17AM +0000, Schrempf Frieder wrote:
>>> This patch adds the documentation for the devicetree bindings to set
>>> the volume levels.
>>>
>>> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
>>> ---
>>> Changes in v2:
>>>  - split into 3 separate patches
>>>  - make volume properties optional
>>>
>>>  .../devicetree/bindings/input/pwm-beeper.txt       | 22 ++++++++++++++++++++++
>>>  1 file changed, 22 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>>> index be332ae..6d8ba4e 100644
>>> --- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
>>> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>>> @@ -5,3 +5,25 @@ Registers a PWM device as beeper.
>>>  Required properties:
>>>  - compatible: should be "pwm-beeper"
>>>  - pwms: phandle to the physical PWM device
>>> +
>>> +Optional properties:
>>> +- volume-levels: Array of distinct 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).
>>
>> What does the index correspond to? The linear volume?
>
> In most cases users probably need linear volume levels (e.g. 0%, 25%,
> 50%, 75%, 100%) and in this case the index would indeed correspond to
> the linear perceived volume.
>
> But also non-linear relations are possible (e.g. 0%, 20%, 100%), if the
> user needs for example "mute", "low", "high" as volume levels.

Exclude off/mute and this is still linear. Also, the user exposed
levels could be a subset of the defined h/w levels. That should be
independent of DT.

> The linearization (defining the corresponding duty cycle for each index)
> depends on the beeper and the perception of the user.

This has to be a consistent interface across h/w to have a userspace
that can work across h/w. For that, you have to define the binding as
linear. Of course, it's all measured by perception and not completely
accurate which is fine.

> For the example array definition below, I tried different duty cycles
> and found values of 0.8%, 2%, 4%, 50% to be approximately correspondent
> to perceived volume levels of 25%, 50%, 75%, 100% in my case.
>
>>
>>> +- default-volume-level: the default volume level (index into the
>>> +      array defined by the "volume-levels" property)
>>> +
>>> +The volume level can be set via sysfs under /sys/class/input/inputX/volume.
>>> +The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level.

Also, drop this. Not relevant to the binding.

Rob

Re: [PATCH v2 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Frieder Schrempf]

On 11.10.2016 15:39, Rob Herring wrote:
> On Tue, Oct 11, 2016 at 3:17 AM, Schrempf Frieder
> <frieder.schrempf@exceet.de> wrote:
>> On 10.10.2016 17:20, Rob Herring wrote:
>>> On Fri, Oct 07, 2016 at 09:08:17AM +0000, Schrempf Frieder wrote:
>>>> This patch adds the documentation for the devicetree bindings to set
>>>> the volume levels.
>>>>
>>>> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
>>>> ---
>>>> Changes in v2:
>>>>  - split into 3 separate patches
>>>>  - make volume properties optional
>>>>
>>>>  .../devicetree/bindings/input/pwm-beeper.txt       | 22 ++++++++++++++++++++++
>>>>  1 file changed, 22 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>>>> index be332ae..6d8ba4e 100644
>>>> --- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
>>>> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>>>> @@ -5,3 +5,25 @@ Registers a PWM device as beeper.
>>>>  Required properties:
>>>>  - compatible: should be "pwm-beeper"
>>>>  - pwms: phandle to the physical PWM device
>>>> +
>>>> +Optional properties:
>>>> +- volume-levels: Array of distinct 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).
>>>
>>> What does the index correspond to? The linear volume?
>>
>> In most cases users probably need linear volume levels (e.g. 0%, 25%,
>> 50%, 75%, 100%) and in this case the index would indeed correspond to
>> the linear perceived volume.
>>
>> But also non-linear relations are possible (e.g. 0%, 20%, 100%), if the
>> user needs for example "mute", "low", "high" as volume levels.
>
> Exclude off/mute and this is still linear. Also, the user exposed
> levels could be a subset of the defined h/w levels. That should be
> independent of DT.

Ok
>
>> The linearization (defining the corresponding duty cycle for each index)
>> depends on the beeper and the perception of the user.
>
> This has to be a consistent interface across h/w to have a userspace
> that can work across h/w. For that, you have to define the binding as
> linear. Of course, it's all measured by perception and not completely
> accurate which is fine.

I see. I will resend and change the description to make clear, that the
volume-levels property is meant to specify linear volume levels.
>
>> For the example array definition below, I tried different duty cycles
>> and found values of 0.8%, 2%, 4%, 50% to be approximately correspondent
>> to perceived volume levels of 25%, 50%, 75%, 100% in my case.
>>
>>>
>>>> +- default-volume-level: the default volume level (index into the
>>>> +      array defined by the "volume-levels" property)
>>>> +
>>>> +The volume level can be set via sysfs under /sys/class/input/inputX/volume.
>>>> +The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level.
>
> Also, drop this. Not relevant to the binding.

Ok, I will drop this.
>
> Rob
>

[PATCH v3 0/3] input: pwm-beeper: add feature to set volume level

[Frieder Schrempf]

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 number of volume levels available and their values can
be specified via device tree (similar to pwm-backlight).

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

Changes in v3:
 - update date
 - change description of volume-levels to be used for linear levels

Frieder Schrempf (3):
  input: pwm-beeper: add feature to set volume via sysfs
  input: pwm-beeper: add documentation for volume devicetree bindings
  input: pwm-beeper: add devicetree bindings to set volume levels

 .../ABI/testing/sysfs-class-input-pwm-beeper       |  17 +++
 .../devicetree/bindings/input/pwm-beeper.txt       |  20 ++++
 drivers/input/misc/pwm-beeper.c                    | 116 ++++++++++++++++++++-
 3 files changed, 151 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-input-pwm-beeper

-- 
2.7.4

[PATCH v3 0/3] input: pwm-beeper: add feature to set volume level

[Frieder Schrempf]

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 number of volume levels available and their values can
be specified via device tree (similar to pwm-backlight).

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

Changes in v3:
 - update date
 - change description of volume-levels to be used for linear levels

Frieder Schrempf (3):
  input: pwm-beeper: add feature to set volume via sysfs
  input: pwm-beeper: add documentation for volume devicetree bindings
  input: pwm-beeper: add devicetree bindings to set volume levels

 .../ABI/testing/sysfs-class-input-pwm-beeper       |  17 +++
 .../devicetree/bindings/input/pwm-beeper.txt       |  20 ++++
 drivers/input/misc/pwm-beeper.c                    | 116 ++++++++++++++++++++-
 3 files changed, 151 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-input-pwm-beeper

-- 
2.7.4

[PATCH v3 1/3] input: pwm-beeper: add feature to set volume via sysfs

[Frieder Schrempf]

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>
---
Changes in v3:
 - update date
 
 .../ABI/testing/sysfs-class-input-pwm-beeper       | 17 ++++++
 drivers/input/misc/pwm-beeper.c                    | 71 +++++++++++++++++++++-
 2 files changed, 87 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-input-pwm-beeper

diff --git a/Documentation/ABI/testing/sysfs-class-input-pwm-beeper b/Documentation/ABI/testing/sysfs-class-input-pwm-beeper
new file mode 100644
index 0000000..c878a1d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-input-pwm-beeper
@@ -0,0 +1,17 @@
+What:		/sys/class/input/input(x)/volume
+Date:		January 2017
+KernelVersion:
+Contact:	Frieder Schrempf <frieder.schrempf@exceet.de>
+Description:
+		Control the volume of this pwm-beeper. Values
+		are between 0 and max_volume_level. This file will also
+		show the current volume level stored in the driver.
+
+What:		/sys/class/input/input(x)/max_volume_level
+Date:		January 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 5f9655d..3ed21da 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -1,5 +1,9 @@
 /*
  *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
+ *
+ *  Copyright (C) 2016, Frieder Schrempf <frieder.schrempf@exceet.de>
+ *  (volume support)
+ *
  *  PWM beeper driver
  *
  *  This program is free software; you can redistribute it and/or modify it
@@ -27,16 +31,77 @@ struct pwm_beeper {
 	struct pwm_device *pwm;
 	struct work_struct work;
 	unsigned long period;
+	unsigned int volume;
+	unsigned int volume_levels[] = {0, 8, 20, 40, 500};
+	unsigned int max_volume_level;
 };
 
 #define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
 
+static ssize_t beeper_show_volume(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 beeper_show_max_volume_level(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_level);
+}
+
+static ssize_t beeper_store_volume(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;
+
+	rc = -ENXIO;
+	if (volume > beeper->max_volume_level)
+		volume = beeper->max_volume_level;
+	pr_debug("set volume to %u\n", volume);
+	if (beeper->volume != volume)
+		beeper->volume = volume;
+	rc = count;
+
+	return rc;
+}
+
+static DEVICE_ATTR(volume, 0644, beeper_show_volume, beeper_store_volume);
+static DEVICE_ATTR(max_volume_level, 0644, beeper_show_max_volume_level, NULL);
+
+static struct attribute *bp_device_attributes[] = {
+	&dev_attr_volume.attr,
+	&dev_attr_max_volume_level.attr,
+	NULL,
+};
+
+static struct attribute_group bp_device_attr_group = {
+	.attrs = bp_device_attributes,
+};
+
+static const struct attribute_group *bp_device_attr_groups[] = {
+	&bp_device_attr_group,
+	NULL,
+};
+
 static void __pwm_beeper_set(struct pwm_beeper *beeper)
 {
 	unsigned long period = beeper->period;
 
 	if (period) {
-		pwm_config(beeper->pwm, period / 2, period);
+		pwm_config(beeper->pwm,
+			period / 1000 * beeper->volume_levels[beeper->volume],
+			period);
 		pwm_enable(beeper->pwm);
 	} else
 		pwm_disable(beeper->pwm);
@@ -123,6 +188,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	INIT_WORK(&beeper->work, pwm_beeper_work);
 
+	beeper->max_volume_level = ARRAY_SIZE(beeper->volume_levels) - 1;
+
 	beeper->input = input_allocate_device();
 	if (!beeper->input) {
 		dev_err(&pdev->dev, "Failed to allocate input device\n");
@@ -146,6 +213,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	input_set_drvdata(beeper->input, beeper);
 
+	beeper->input->dev.groups = bp_device_attr_groups;
+
 	error = input_register_device(beeper->input);
 	if (error) {
 		dev_err(&pdev->dev, "Failed to register input device: %d\n", error);
-- 
2.7.4

[PATCH v3 1/3] input: pwm-beeper: add feature to set volume via sysfs

[Frieder Schrempf]

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-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
---
Changes in v3:
 - update date
 
 .../ABI/testing/sysfs-class-input-pwm-beeper       | 17 ++++++
 drivers/input/misc/pwm-beeper.c                    | 71 +++++++++++++++++++++-
 2 files changed, 87 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-input-pwm-beeper

diff --git a/Documentation/ABI/testing/sysfs-class-input-pwm-beeper b/Documentation/ABI/testing/sysfs-class-input-pwm-beeper
new file mode 100644
index 0000000..c878a1d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-input-pwm-beeper
@@ -0,0 +1,17 @@
+What:		/sys/class/input/input(x)/volume
+Date:		January 2017
+KernelVersion:
+Contact:	Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
+Description:
+		Control the volume of this pwm-beeper. Values
+		are between 0 and max_volume_level. This file will also
+		show the current volume level stored in the driver.
+
+What:		/sys/class/input/input(x)/max_volume_level
+Date:		January 2017
+KernelVersion:
+Contact:	Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
+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 5f9655d..3ed21da 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -1,5 +1,9 @@
 /*
  *  Copyright (C) 2010, Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
+ *
+ *  Copyright (C) 2016, Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
+ *  (volume support)
+ *
  *  PWM beeper driver
  *
  *  This program is free software; you can redistribute it and/or modify it
@@ -27,16 +31,77 @@ struct pwm_beeper {
 	struct pwm_device *pwm;
 	struct work_struct work;
 	unsigned long period;
+	unsigned int volume;
+	unsigned int volume_levels[] = {0, 8, 20, 40, 500};
+	unsigned int max_volume_level;
 };
 
 #define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
 
+static ssize_t beeper_show_volume(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 beeper_show_max_volume_level(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_level);
+}
+
+static ssize_t beeper_store_volume(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;
+
+	rc = -ENXIO;
+	if (volume > beeper->max_volume_level)
+		volume = beeper->max_volume_level;
+	pr_debug("set volume to %u\n", volume);
+	if (beeper->volume != volume)
+		beeper->volume = volume;
+	rc = count;
+
+	return rc;
+}
+
+static DEVICE_ATTR(volume, 0644, beeper_show_volume, beeper_store_volume);
+static DEVICE_ATTR(max_volume_level, 0644, beeper_show_max_volume_level, NULL);
+
+static struct attribute *bp_device_attributes[] = {
+	&dev_attr_volume.attr,
+	&dev_attr_max_volume_level.attr,
+	NULL,
+};
+
+static struct attribute_group bp_device_attr_group = {
+	.attrs = bp_device_attributes,
+};
+
+static const struct attribute_group *bp_device_attr_groups[] = {
+	&bp_device_attr_group,
+	NULL,
+};
+
 static void __pwm_beeper_set(struct pwm_beeper *beeper)
 {
 	unsigned long period = beeper->period;
 
 	if (period) {
-		pwm_config(beeper->pwm, period / 2, period);
+		pwm_config(beeper->pwm,
+			period / 1000 * beeper->volume_levels[beeper->volume],
+			period);
 		pwm_enable(beeper->pwm);
 	} else
 		pwm_disable(beeper->pwm);
@@ -123,6 +188,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	INIT_WORK(&beeper->work, pwm_beeper_work);
 
+	beeper->max_volume_level = ARRAY_SIZE(beeper->volume_levels) - 1;
+
 	beeper->input = input_allocate_device();
 	if (!beeper->input) {
 		dev_err(&pdev->dev, "Failed to allocate input device\n");
@@ -146,6 +213,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	input_set_drvdata(beeper->input, beeper);
 
+	beeper->input->dev.groups = bp_device_attr_groups;
+
 	error = input_register_device(beeper->input);
 	if (error) {
 		dev_err(&pdev->dev, "Failed to register input device: %d\n", error);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Frieder Schrempf]

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

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
Changes in v3:
 - change description of volume-levels to be used for linear levels
 
 .../devicetree/bindings/input/pwm-beeper.txt         | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
index be332ae..496b68f 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
@@ -5,3 +5,23 @@ Registers a PWM device as beeper.
 Required properties:
 - compatible: should be "pwm-beeper"
 - pwms: phandle to the physical PWM device
+
+Optional properties:
+- 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:
+
+	pwm-beeper {
+		compatible = "pwm-beeper";
+		pwms = <&pwm4 0 5000>;
+		volume-levels = <0 8 20 40 500>;
+		default-volume-level = <4>;
+	};
-- 
2.7.4

[PATCH v3 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Frieder Schrempf]

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

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
Changes in v3:
 - change description of volume-levels to be used for linear levels
 
 .../devicetree/bindings/input/pwm-beeper.txt         | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
index be332ae..496b68f 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
@@ -5,3 +5,23 @@ Registers a PWM device as beeper.
 Required properties:
 - compatible: should be "pwm-beeper"
 - pwms: phandle to the physical PWM device
+
+Optional properties:
+- 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:
+
+	pwm-beeper {
+		compatible = "pwm-beeper";
+		pwms = <&pwm4 0 5000>;
+		volume-levels = <0 8 20 40 500>;
+		default-volume-level = <4>;
+	};
-- 
2.7.4

[PATCH v3 3/3] input: pwm-beeper: add devicetree bindings to set volume levels

[Frieder Schrempf]

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>
---
Changes in v3:
 - none
 
 drivers/input/misc/pwm-beeper.c | 49 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index 3ed21da..a7f9d70 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -32,7 +32,7 @@ struct pwm_beeper {
 	struct work_struct work;
 	unsigned long period;
 	unsigned int volume;
-	unsigned int volume_levels[] = {0, 8, 20, 40, 500};
+	unsigned int *volume_levels;
 	unsigned int max_volume_level;
 };
 
@@ -161,8 +161,11 @@ static void pwm_beeper_close(struct input_dev *input)
 static int pwm_beeper_probe(struct platform_device *pdev)
 {
 	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
 	struct pwm_beeper *beeper;
-	int error;
+	struct property *prop;
+	int error, length;
+	u32 value;
 
 	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
 	if (!beeper)
@@ -188,7 +191,47 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	INIT_WORK(&beeper->work, pwm_beeper_work);
 
-	beeper->max_volume_level = ARRAY_SIZE(beeper->volume_levels) - 1;
+	/* determine the number of volume levels */
+	prop = of_find_property(np, "volume-levels", &length);
+	if (!prop) {
+		dev_dbg(&pdev->dev, "no volume levels specified, using max volume\n");
+		beeper->max_volume_level = 1;
+	} else
+		beeper->max_volume_level = length / sizeof(u32);
+
+	/* read volume levels from DT property */
+	if (beeper->max_volume_level > 0) {
+		size_t size = sizeof(*beeper->volume_levels) *
+			beeper->max_volume_level;
+
+		beeper->volume_levels = devm_kzalloc(&(pdev->dev), size,
+			GFP_KERNEL);
+		if (!beeper->volume_levels)
+			return -ENOMEM;
+
+		if (prop) {
+			error = of_property_read_u32_array(np, "volume-levels",
+						beeper->volume_levels,
+						beeper->max_volume_level);
+
+			if (error < 0)
+				return error;
+
+			error = of_property_read_u32(np, "default-volume-level",
+						   &value);
+
+			if (error < 0) {
+				dev_dbg(&pdev->dev, "no default volume specified, using max volume\n");
+				value = beeper->max_volume_level - 1;
+			}
+		} else {
+			beeper->volume_levels[0] = 500;
+			value = 0;
+		}
+
+		beeper->volume = value;
+		beeper->max_volume_level--;
+	}
 
 	beeper->input = input_allocate_device();
 	if (!beeper->input) {
-- 
2.7.4

[PATCH v3 3/3] input: pwm-beeper: add devicetree bindings to set volume levels

[Frieder Schrempf]

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>
---
Changes in v3:
 - none
 
 drivers/input/misc/pwm-beeper.c | 49 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index 3ed21da..a7f9d70 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -32,7 +32,7 @@ struct pwm_beeper {
 	struct work_struct work;
 	unsigned long period;
 	unsigned int volume;
-	unsigned int volume_levels[] = {0, 8, 20, 40, 500};
+	unsigned int *volume_levels;
 	unsigned int max_volume_level;
 };
 
@@ -161,8 +161,11 @@ static void pwm_beeper_close(struct input_dev *input)
 static int pwm_beeper_probe(struct platform_device *pdev)
 {
 	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
 	struct pwm_beeper *beeper;
-	int error;
+	struct property *prop;
+	int error, length;
+	u32 value;
 
 	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
 	if (!beeper)
@@ -188,7 +191,47 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	INIT_WORK(&beeper->work, pwm_beeper_work);
 
-	beeper->max_volume_level = ARRAY_SIZE(beeper->volume_levels) - 1;
+	/* determine the number of volume levels */
+	prop = of_find_property(np, "volume-levels", &length);
+	if (!prop) {
+		dev_dbg(&pdev->dev, "no volume levels specified, using max volume\n");
+		beeper->max_volume_level = 1;
+	} else
+		beeper->max_volume_level = length / sizeof(u32);
+
+	/* read volume levels from DT property */
+	if (beeper->max_volume_level > 0) {
+		size_t size = sizeof(*beeper->volume_levels) *
+			beeper->max_volume_level;
+
+		beeper->volume_levels = devm_kzalloc(&(pdev->dev), size,
+			GFP_KERNEL);
+		if (!beeper->volume_levels)
+			return -ENOMEM;
+
+		if (prop) {
+			error = of_property_read_u32_array(np, "volume-levels",
+						beeper->volume_levels,
+						beeper->max_volume_level);
+
+			if (error < 0)
+				return error;
+
+			error = of_property_read_u32(np, "default-volume-level",
+						   &value);
+
+			if (error < 0) {
+				dev_dbg(&pdev->dev, "no default volume specified, using max volume\n");
+				value = beeper->max_volume_level - 1;
+			}
+		} else {
+			beeper->volume_levels[0] = 500;
+			value = 0;
+		}
+
+		beeper->volume = value;
+		beeper->max_volume_level--;
+	}
 
 	beeper->input = input_allocate_device();
 	if (!beeper->input) {
-- 
2.7.4

Re: [PATCH v3 1/3] input: pwm-beeper: add feature to set volume via sysfs

[Dmitry Torokhov]

Hi Frieder,

On Thu, Jan 19, 2017 at 04:24:08PM +0100, Frieder Schrempf wrote:
> 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>
> ---
> Changes in v3:
>  - update date
>  
>  .../ABI/testing/sysfs-class-input-pwm-beeper       | 17 ++++++
>  drivers/input/misc/pwm-beeper.c                    | 71 +++++++++++++++++++++-
>  2 files changed, 87 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-input-pwm-beeper
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-input-pwm-beeper b/Documentation/ABI/testing/sysfs-class-input-pwm-beeper
> new file mode 100644
> index 0000000..c878a1d
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-input-pwm-beeper
> @@ -0,0 +1,17 @@
> +What:		/sys/class/input/input(x)/volume

Only generic (i.e. applicable to all input devices) attributes can be
present in /sys/class/input/input(x)/, and volume certainly isn't such
attribute. Please move them to the pwm-beeper platform device itself.

> +Date:		January 2017
> +KernelVersion:
> +Contact:	Frieder Schrempf <frieder.schrempf@exceet.de>
> +Description:
> +		Control the volume of this pwm-beeper. Values
> +		are between 0 and max_volume_level. This file will also
> +		show the current volume level stored in the driver.
> +
> +What:		/sys/class/input/input(x)/max_volume_level
> +Date:		January 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 5f9655d..3ed21da 100644
> --- a/drivers/input/misc/pwm-beeper.c
> +++ b/drivers/input/misc/pwm-beeper.c
> @@ -1,5 +1,9 @@
>  /*
>   *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
> + *
> + *  Copyright (C) 2016, Frieder Schrempf <frieder.schrempf@exceet.de>
> + *  (volume support)
> + *
>   *  PWM beeper driver
>   *
>   *  This program is free software; you can redistribute it and/or modify it
> @@ -27,16 +31,77 @@ struct pwm_beeper {
>  	struct pwm_device *pwm;
>  	struct work_struct work;
>  	unsigned long period;
> +	unsigned int volume;
> +	unsigned int volume_levels[] = {0, 8, 20, 40, 500};

Does this actually work? Anyway, you are not allowing to set differentr
values in for different beepers, so take the array out of the structure.

> +	unsigned int max_volume_level;
>  };
>  
>  #define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
>  
> +static ssize_t beeper_show_volume(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 beeper_show_max_volume_level(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_level);
> +}
> +
> +static ssize_t beeper_store_volume(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;
> +
> +	rc = -ENXIO;

Why? There are no failures below.

> +	if (volume > beeper->max_volume_level)
> +		volume = beeper->max_volume_level;

Return -EINVAL maybe?

> +	pr_debug("set volume to %u\n", volume);
> +	if (beeper->volume != volume)
> +		beeper->volume = volume;

Why?

> +	rc = count;
> +
> +	return rc;
> +}
> +
> +static DEVICE_ATTR(volume, 0644, beeper_show_volume, beeper_store_volume);
> +static DEVICE_ATTR(max_volume_level, 0644, beeper_show_max_volume_level, NULL);

Drop "level", it is cleaner.

> +
> +static struct attribute *bp_device_attributes[] = {

pwm_beeper_atttributes

> +	&dev_attr_volume.attr,
> +	&dev_attr_max_volume_level.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group bp_device_attr_group = {

pwm_beeper_attribute_group

> +	.attrs = bp_device_attributes,
> +};
> +
> +static const struct attribute_group *bp_device_attr_groups[] = {
> +	&bp_device_attr_group,
> +	NULL,
> +};
> +
>  static void __pwm_beeper_set(struct pwm_beeper *beeper)
>  {
>  	unsigned long period = beeper->period;
>  
>  	if (period) {
> -		pwm_config(beeper->pwm, period / 2, period);
> +		pwm_config(beeper->pwm,
> +			period / 1000 * beeper->volume_levels[beeper->volume],
> +			period);
>  		pwm_enable(beeper->pwm);
>  	} else
>  		pwm_disable(beeper->pwm);
> @@ -123,6 +188,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
>  
>  	INIT_WORK(&beeper->work, pwm_beeper_work);
>  
> +	beeper->max_volume_level = ARRAY_SIZE(beeper->volume_levels) - 1;
> +
>  	beeper->input = input_allocate_device();
>  	if (!beeper->input) {
>  		dev_err(&pdev->dev, "Failed to allocate input device\n");
> @@ -146,6 +213,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
>  
>  	input_set_drvdata(beeper->input, beeper);
>  
> +	beeper->input->dev.groups = bp_device_attr_groups;
> +
>  	error = input_register_device(beeper->input);
>  	if (error) {
>  		dev_err(&pdev->dev, "Failed to register input device: %d\n", error);
> -- 
> 2.7.4
> 

Thanks.

-- 
Dmitry

Re: [PATCH v3 1/3] input: pwm-beeper: add feature to set volume via sysfs

[Dmitry Torokhov]

Hi Frieder,

On Thu, Jan 19, 2017 at 04:24:08PM +0100, Frieder Schrempf wrote:
> 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-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
> ---
> Changes in v3:
>  - update date
>  
>  .../ABI/testing/sysfs-class-input-pwm-beeper       | 17 ++++++
>  drivers/input/misc/pwm-beeper.c                    | 71 +++++++++++++++++++++-
>  2 files changed, 87 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-input-pwm-beeper
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-input-pwm-beeper b/Documentation/ABI/testing/sysfs-class-input-pwm-beeper
> new file mode 100644
> index 0000000..c878a1d
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-input-pwm-beeper
> @@ -0,0 +1,17 @@
> +What:		/sys/class/input/input(x)/volume

Only generic (i.e. applicable to all input devices) attributes can be
present in /sys/class/input/input(x)/, and volume certainly isn't such
attribute. Please move them to the pwm-beeper platform device itself.

> +Date:		January 2017
> +KernelVersion:
> +Contact:	Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
> +Description:
> +		Control the volume of this pwm-beeper. Values
> +		are between 0 and max_volume_level. This file will also
> +		show the current volume level stored in the driver.
> +
> +What:		/sys/class/input/input(x)/max_volume_level
> +Date:		January 2017
> +KernelVersion:
> +Contact:	Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
> +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 5f9655d..3ed21da 100644
> --- a/drivers/input/misc/pwm-beeper.c
> +++ b/drivers/input/misc/pwm-beeper.c
> @@ -1,5 +1,9 @@
>  /*
>   *  Copyright (C) 2010, Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
> + *
> + *  Copyright (C) 2016, Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
> + *  (volume support)
> + *
>   *  PWM beeper driver
>   *
>   *  This program is free software; you can redistribute it and/or modify it
> @@ -27,16 +31,77 @@ struct pwm_beeper {
>  	struct pwm_device *pwm;
>  	struct work_struct work;
>  	unsigned long period;
> +	unsigned int volume;
> +	unsigned int volume_levels[] = {0, 8, 20, 40, 500};

Does this actually work? Anyway, you are not allowing to set differentr
values in for different beepers, so take the array out of the structure.

> +	unsigned int max_volume_level;
>  };
>  
>  #define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
>  
> +static ssize_t beeper_show_volume(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 beeper_show_max_volume_level(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_level);
> +}
> +
> +static ssize_t beeper_store_volume(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;
> +
> +	rc = -ENXIO;

Why? There are no failures below.

> +	if (volume > beeper->max_volume_level)
> +		volume = beeper->max_volume_level;

Return -EINVAL maybe?

> +	pr_debug("set volume to %u\n", volume);
> +	if (beeper->volume != volume)
> +		beeper->volume = volume;

Why?

> +	rc = count;
> +
> +	return rc;
> +}
> +
> +static DEVICE_ATTR(volume, 0644, beeper_show_volume, beeper_store_volume);
> +static DEVICE_ATTR(max_volume_level, 0644, beeper_show_max_volume_level, NULL);

Drop "level", it is cleaner.

> +
> +static struct attribute *bp_device_attributes[] = {

pwm_beeper_atttributes

> +	&dev_attr_volume.attr,
> +	&dev_attr_max_volume_level.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group bp_device_attr_group = {

pwm_beeper_attribute_group

> +	.attrs = bp_device_attributes,
> +};
> +
> +static const struct attribute_group *bp_device_attr_groups[] = {
> +	&bp_device_attr_group,
> +	NULL,
> +};
> +
>  static void __pwm_beeper_set(struct pwm_beeper *beeper)
>  {
>  	unsigned long period = beeper->period;
>  
>  	if (period) {
> -		pwm_config(beeper->pwm, period / 2, period);
> +		pwm_config(beeper->pwm,
> +			period / 1000 * beeper->volume_levels[beeper->volume],
> +			period);
>  		pwm_enable(beeper->pwm);
>  	} else
>  		pwm_disable(beeper->pwm);
> @@ -123,6 +188,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
>  
>  	INIT_WORK(&beeper->work, pwm_beeper_work);
>  
> +	beeper->max_volume_level = ARRAY_SIZE(beeper->volume_levels) - 1;
> +
>  	beeper->input = input_allocate_device();
>  	if (!beeper->input) {
>  		dev_err(&pdev->dev, "Failed to allocate input device\n");
> @@ -146,6 +213,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
>  
>  	input_set_drvdata(beeper->input, beeper);
>  
> +	beeper->input->dev.groups = bp_device_attr_groups;
> +
>  	error = input_register_device(beeper->input);
>  	if (error) {
>  		dev_err(&pdev->dev, "Failed to register input device: %d\n", error);
> -- 
> 2.7.4
> 

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/3] input: pwm-beeper: add devicetree bindings to set volume levels

[Dmitry Torokhov]

On Thu, Jan 19, 2017 at 04:24:10PM +0100, Frieder Schrempf wrote:
> 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>
> ---
> Changes in v3:
>  - none
>  
>  drivers/input/misc/pwm-beeper.c | 49 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 46 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
> index 3ed21da..a7f9d70 100644
> --- a/drivers/input/misc/pwm-beeper.c
> +++ b/drivers/input/misc/pwm-beeper.c
> @@ -32,7 +32,7 @@ struct pwm_beeper {
>  	struct work_struct work;
>  	unsigned long period;
>  	unsigned int volume;
> -	unsigned int volume_levels[] = {0, 8, 20, 40, 500};
> +	unsigned int *volume_levels;
>  	unsigned int max_volume_level;
>  };
>  
> @@ -161,8 +161,11 @@ static void pwm_beeper_close(struct input_dev *input)
>  static int pwm_beeper_probe(struct platform_device *pdev)
>  {
>  	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
> +	struct device_node *np = pdev->dev.of_node;
>  	struct pwm_beeper *beeper;
> -	int error;
> +	struct property *prop;
> +	int error, length;
> +	u32 value;
>  
>  	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
>  	if (!beeper)
> @@ -188,7 +191,47 @@ static int pwm_beeper_probe(struct platform_device *pdev)
>  
>  	INIT_WORK(&beeper->work, pwm_beeper_work);
>  
> -	beeper->max_volume_level = ARRAY_SIZE(beeper->volume_levels) - 1;
> +	/* determine the number of volume levels */
> +	prop = of_find_property(np, "volume-levels", &length);

Please use generic device properties, not OF-specific ones.

> +	if (!prop) {
> +		dev_dbg(&pdev->dev, "no volume levels specified, using max volume\n");
> +		beeper->max_volume_level = 1;
> +	} else
> +		beeper->max_volume_level = length / sizeof(u32);
> +
> +	/* read volume levels from DT property */
> +	if (beeper->max_volume_level > 0) {
> +		size_t size = sizeof(*beeper->volume_levels) *
> +			beeper->max_volume_level;
> +
> +		beeper->volume_levels = devm_kzalloc(&(pdev->dev), size,
> +			GFP_KERNEL);
> +		if (!beeper->volume_levels)
> +			return -ENOMEM;
> +
> +		if (prop) {
> +			error = of_property_read_u32_array(np, "volume-levels",
> +						beeper->volume_levels,
> +						beeper->max_volume_level);
> +
> +			if (error < 0)
> +				return error;
> +
> +			error = of_property_read_u32(np, "default-volume-level",
> +						   &value);
> +
> +			if (error < 0) {
> +				dev_dbg(&pdev->dev, "no default volume specified, using max volume\n");
> +				value = beeper->max_volume_level - 1;
> +			}
> +		} else {
> +			beeper->volume_levels[0] = 500;
> +			value = 0;
> +		}
> +
> +		beeper->volume = value;
> +		beeper->max_volume_level--;
> +	}
>  
>  	beeper->input = input_allocate_device();
>  	if (!beeper->input) {
> -- 
> 2.7.4
> 

-- 
Dmitry

Re: [PATCH v3 0/3] input: pwm-beeper: add feature to set volume level

[Dmitry Torokhov]

On Thu, Jan 19, 2017 at 04:24:07PM +0100, Frieder Schrempf wrote:
> 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 number of volume levels available and their values can
> be specified via device tree (similar to pwm-backlight).
> 
> The volume adjustment is done by changing the duty cycle of
> the pwm signal.

I wonder how this all will mesh up with beepers that have dedicated
amplifiers (support is being added by David Lechner).

Thanks.

-- 
Dmitry

Re: [PATCH v3 0/3] input: pwm-beeper: add feature to set volume level

[David Lechner]

On 01/19/2017 03:37 PM, Dmitry Torokhov wrote:
> On Thu, Jan 19, 2017 at 04:24:07PM +0100, Frieder Schrempf wrote:
>> 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 number of volume levels available and their values can
>> be specified via device tree (similar to pwm-backlight).
>>
>> The volume adjustment is done by changing the duty cycle of
>> the pwm signal.
>
> I wonder how this all will mesh up with beepers that have dedicated
> amplifiers (support is being added by David Lechner).

This will work very well with it. I fact, it is a feature I would like 
to have but I was not sure about a good way to implement it. Please Cc: 
me on future revisions of this series and I will be glad to test it.

One thing that would be nice would be for a more generic way to change 
the volume from userspace. Having a sysfs attribute on the platform 
device will work, but it requires very specific knowledge from any 
userspace program that wants to control the volume. Would it be possible 
to add an alsa mixer volume control or something like that?

Re: [PATCH v3 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Rob Herring]

On Thu, Jan 19, 2017 at 04:24:09PM +0100, Frieder Schrempf wrote:
> This patch adds the documentation for the devicetree bindings to set
> the volume levels.
> 
> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
> ---
> Changes in v3:
>  - change description of volume-levels to be used for linear levels
>  
>  .../devicetree/bindings/input/pwm-beeper.txt         | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)

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

Re: [PATCH v3 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Rob Herring]

On Thu, Jan 19, 2017 at 04:24:09PM +0100, Frieder Schrempf wrote:
> This patch adds the documentation for the devicetree bindings to set
> the volume levels.
> 
> Signed-off-by: Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
> ---
> Changes in v3:
>  - change description of volume-levels to be used for linear levels
>  
>  .../devicetree/bindings/input/pwm-beeper.txt         | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] input: pwm-beeper: add feature to set volume via sysfs

[Frieder Schrempf]

Hi Dmitry,

On 19.01.2017 22:29, Dmitry Torokhov wrote:
> Hi Frieder,
>
> On Thu, Jan 19, 2017 at 04:24:08PM +0100, Frieder Schrempf wrote:
>> 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>
>> ---
>> Changes in v3:
>>  - update date
>>
>>  .../ABI/testing/sysfs-class-input-pwm-beeper       | 17 ++++++
>>  drivers/input/misc/pwm-beeper.c                    | 71 +++++++++++++++++++++-
>>  2 files changed, 87 insertions(+), 1 deletion(-)
>>  create mode 100644 Documentation/ABI/testing/sysfs-class-input-pwm-beeper
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-input-pwm-beeper b/Documentation/ABI/testing/sysfs-class-input-pwm-beeper
>> new file mode 100644
>> index 0000000..c878a1d
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-input-pwm-beeper
>> @@ -0,0 +1,17 @@
>> +What:		/sys/class/input/input(x)/volume
>
> Only generic (i.e. applicable to all input devices) attributes can be
> present in /sys/class/input/input(x)/, and volume certainly isn't such
> attribute. Please move them to the pwm-beeper platform device itself.

Ok, I will move this in v4.
>
>> +Date:		January 2017
>> +KernelVersion:
>> +Contact:	Frieder Schrempf <frieder.schrempf@exceet.de>
>> +Description:
>> +		Control the volume of this pwm-beeper. Values
>> +		are between 0 and max_volume_level. This file will also
>> +		show the current volume level stored in the driver.
>> +
>> +What:		/sys/class/input/input(x)/max_volume_level
>> +Date:		January 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 5f9655d..3ed21da 100644
>> --- a/drivers/input/misc/pwm-beeper.c
>> +++ b/drivers/input/misc/pwm-beeper.c
>> @@ -1,5 +1,9 @@
>>  /*
>>   *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
>> + *
>> + *  Copyright (C) 2016, Frieder Schrempf <frieder.schrempf@exceet.de>
>> + *  (volume support)
>> + *
>>   *  PWM beeper driver
>>   *
>>   *  This program is free software; you can redistribute it and/or modify it
>> @@ -27,16 +31,77 @@ struct pwm_beeper {
>>  	struct pwm_device *pwm;
>>  	struct work_struct work;
>>  	unsigned long period;
>> +	unsigned int volume;
>> +	unsigned int volume_levels[] = {0, 8, 20, 40, 500};
>
> Does this actually work? Anyway, you are not allowing to set differentr
> values in for different beepers, so take the array out of the structure.

You're right, this was not really clever. I moved the array allocation 
to probe in v4 as a preparation for the devicetree implementation in the 
following patch.
>
>> +	unsigned int max_volume_level;
>>  };
>>
>>  #define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
>>
>> +static ssize_t beeper_show_volume(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 beeper_show_max_volume_level(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_level);
>> +}
>> +
>> +static ssize_t beeper_store_volume(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;
>> +
>> +	rc = -ENXIO;
>
> Why? There are no failures below.

Yes, you're right.
>
>> +	if (volume > beeper->max_volume_level)
>> +		volume = beeper->max_volume_level;
>
> Return -EINVAL maybe?

Yes, probably makes sense.
>
>> +	pr_debug("set volume to %u\n", volume);
>> +	if (beeper->volume != volume)
>> +		beeper->volume = volume;
>
> Why?

I took this implementation from the pwm-backlight driver, but you're 
right, this does not seem to be necessary.
>
>> +	rc = count;
>> +
>> +	return rc;
>> +}
>> +
>> +static DEVICE_ATTR(volume, 0644, beeper_show_volume, beeper_store_volume);
>> +static DEVICE_ATTR(max_volume_level, 0644, beeper_show_max_volume_level, NULL);
>
> Drop "level", it is cleaner.

Ok.
>
>> +
>> +static struct attribute *bp_device_attributes[] = {
>
> pwm_beeper_atttributes

Ok.
>
>> +	&dev_attr_volume.attr,
>> +	&dev_attr_max_volume_level.attr,
>> +	NULL,
>> +};
>> +
>> +static struct attribute_group bp_device_attr_group = {
>
> pwm_beeper_attribute_group

Ok.
>
>> +	.attrs = bp_device_attributes,
>> +};
>> +
>> +static const struct attribute_group *bp_device_attr_groups[] = {
>> +	&bp_device_attr_group,
>> +	NULL,
>> +};
>> +
>>  static void __pwm_beeper_set(struct pwm_beeper *beeper)
>>  {
>>  	unsigned long period = beeper->period;
>>
>>  	if (period) {
>> -		pwm_config(beeper->pwm, period / 2, period);
>> +		pwm_config(beeper->pwm,
>> +			period / 1000 * beeper->volume_levels[beeper->volume],
>> +			period);
>>  		pwm_enable(beeper->pwm);
>>  	} else
>>  		pwm_disable(beeper->pwm);
>> @@ -123,6 +188,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
>>
>>  	INIT_WORK(&beeper->work, pwm_beeper_work);
>>
>> +	beeper->max_volume_level = ARRAY_SIZE(beeper->volume_levels) - 1;
>> +
>>  	beeper->input = input_allocate_device();
>>  	if (!beeper->input) {
>>  		dev_err(&pdev->dev, "Failed to allocate input device\n");
>> @@ -146,6 +213,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
>>
>>  	input_set_drvdata(beeper->input, beeper);
>>
>> +	beeper->input->dev.groups = bp_device_attr_groups;
>> +
>>  	error = input_register_device(beeper->input);
>>  	if (error) {
>>  		dev_err(&pdev->dev, "Failed to register input device: %d\n", error);
>> --
>> 2.7.4
>>
>
> Thanks.
>

Re: [PATCH v3 1/3] input: pwm-beeper: add feature to set volume via sysfs

[Frieder Schrempf]

Hi Dmitry,

On 19.01.2017 22:29, Dmitry Torokhov wrote:
> Hi Frieder,
>
> On Thu, Jan 19, 2017 at 04:24:08PM +0100, Frieder Schrempf wrote:
>> 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>
>> ---
>> Changes in v3:
>>  - update date
>>
>>  .../ABI/testing/sysfs-class-input-pwm-beeper       | 17 ++++++
>>  drivers/input/misc/pwm-beeper.c                    | 71 +++++++++++++++++++++-
>>  2 files changed, 87 insertions(+), 1 deletion(-)
>>  create mode 100644 Documentation/ABI/testing/sysfs-class-input-pwm-beeper
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-input-pwm-beeper b/Documentation/ABI/testing/sysfs-class-input-pwm-beeper
>> new file mode 100644
>> index 0000000..c878a1d
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-input-pwm-beeper
>> @@ -0,0 +1,17 @@
>> +What:		/sys/class/input/input(x)/volume
>
> Only generic (i.e. applicable to all input devices) attributes can be
> present in /sys/class/input/input(x)/, and volume certainly isn't such
> attribute. Please move them to the pwm-beeper platform device itself.

Ok, I will move this in v4.
>
>> +Date:		January 2017
>> +KernelVersion:
>> +Contact:	Frieder Schrempf <frieder.schrempf@exceet.de>
>> +Description:
>> +		Control the volume of this pwm-beeper. Values
>> +		are between 0 and max_volume_level. This file will also
>> +		show the current volume level stored in the driver.
>> +
>> +What:		/sys/class/input/input(x)/max_volume_level
>> +Date:		January 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 5f9655d..3ed21da 100644
>> --- a/drivers/input/misc/pwm-beeper.c
>> +++ b/drivers/input/misc/pwm-beeper.c
>> @@ -1,5 +1,9 @@
>>  /*
>>   *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
>> + *
>> + *  Copyright (C) 2016, Frieder Schrempf <frieder.schrempf@exceet.de>
>> + *  (volume support)
>> + *
>>   *  PWM beeper driver
>>   *
>>   *  This program is free software; you can redistribute it and/or modify it
>> @@ -27,16 +31,77 @@ struct pwm_beeper {
>>  	struct pwm_device *pwm;
>>  	struct work_struct work;
>>  	unsigned long period;
>> +	unsigned int volume;
>> +	unsigned int volume_levels[] = {0, 8, 20, 40, 500};
>
> Does this actually work? Anyway, you are not allowing to set differentr
> values in for different beepers, so take the array out of the structure.

You're right, this was not really clever. I moved the array allocation 
to probe in v4 as a preparation for the devicetree implementation in the 
following patch.
>
>> +	unsigned int max_volume_level;
>>  };
>>
>>  #define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
>>
>> +static ssize_t beeper_show_volume(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 beeper_show_max_volume_level(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_level);
>> +}
>> +
>> +static ssize_t beeper_store_volume(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;
>> +
>> +	rc = -ENXIO;
>
> Why? There are no failures below.

Yes, you're right.
>
>> +	if (volume > beeper->max_volume_level)
>> +		volume = beeper->max_volume_level;
>
> Return -EINVAL maybe?

Yes, probably makes sense.
>
>> +	pr_debug("set volume to %u\n", volume);
>> +	if (beeper->volume != volume)
>> +		beeper->volume = volume;
>
> Why?

I took this implementation from the pwm-backlight driver, but you're 
right, this does not seem to be necessary.
>
>> +	rc = count;
>> +
>> +	return rc;
>> +}
>> +
>> +static DEVICE_ATTR(volume, 0644, beeper_show_volume, beeper_store_volume);
>> +static DEVICE_ATTR(max_volume_level, 0644, beeper_show_max_volume_level, NULL);
>
> Drop "level", it is cleaner.

Ok.
>
>> +
>> +static struct attribute *bp_device_attributes[] = {
>
> pwm_beeper_atttributes

Ok.
>
>> +	&dev_attr_volume.attr,
>> +	&dev_attr_max_volume_level.attr,
>> +	NULL,
>> +};
>> +
>> +static struct attribute_group bp_device_attr_group = {
>
> pwm_beeper_attribute_group

Ok.
>
>> +	.attrs = bp_device_attributes,
>> +};
>> +
>> +static const struct attribute_group *bp_device_attr_groups[] = {
>> +	&bp_device_attr_group,
>> +	NULL,
>> +};
>> +
>>  static void __pwm_beeper_set(struct pwm_beeper *beeper)
>>  {
>>  	unsigned long period = beeper->period;
>>
>>  	if (period) {
>> -		pwm_config(beeper->pwm, period / 2, period);
>> +		pwm_config(beeper->pwm,
>> +			period / 1000 * beeper->volume_levels[beeper->volume],
>> +			period);
>>  		pwm_enable(beeper->pwm);
>>  	} else
>>  		pwm_disable(beeper->pwm);
>> @@ -123,6 +188,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
>>
>>  	INIT_WORK(&beeper->work, pwm_beeper_work);
>>
>> +	beeper->max_volume_level = ARRAY_SIZE(beeper->volume_levels) - 1;
>> +
>>  	beeper->input = input_allocate_device();
>>  	if (!beeper->input) {
>>  		dev_err(&pdev->dev, "Failed to allocate input device\n");
>> @@ -146,6 +213,8 @@ static int pwm_beeper_probe(struct platform_device *pdev)
>>
>>  	input_set_drvdata(beeper->input, beeper);
>>
>> +	beeper->input->dev.groups = bp_device_attr_groups;
>> +
>>  	error = input_register_device(beeper->input);
>>  	if (error) {
>>  		dev_err(&pdev->dev, "Failed to register input device: %d\n", error);
>> --
>> 2.7.4
>>
>
> Thanks.
>

Re: [PATCH v3 3/3] input: pwm-beeper: add devicetree bindings to set volume levels

[Frieder Schrempf]

On 19.01.2017 22:30, Dmitry Torokhov wrote:
> On Thu, Jan 19, 2017 at 04:24:10PM +0100, Frieder Schrempf wrote:
>> 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>
>> ---
>> Changes in v3:
>>  - none
>>
>>  drivers/input/misc/pwm-beeper.c | 49 ++++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 46 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
>> index 3ed21da..a7f9d70 100644
>> --- a/drivers/input/misc/pwm-beeper.c
>> +++ b/drivers/input/misc/pwm-beeper.c
>> @@ -32,7 +32,7 @@ struct pwm_beeper {
>>  	struct work_struct work;
>>  	unsigned long period;
>>  	unsigned int volume;
>> -	unsigned int volume_levels[] = {0, 8, 20, 40, 500};
>> +	unsigned int *volume_levels;
>>  	unsigned int max_volume_level;
>>  };
>>
>> @@ -161,8 +161,11 @@ static void pwm_beeper_close(struct input_dev *input)
>>  static int pwm_beeper_probe(struct platform_device *pdev)
>>  {
>>  	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
>> +	struct device_node *np = pdev->dev.of_node;
>>  	struct pwm_beeper *beeper;
>> -	int error;
>> +	struct property *prop;
>> +	int error, length;
>> +	u32 value;
>>
>>  	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
>>  	if (!beeper)
>> @@ -188,7 +191,47 @@ static int pwm_beeper_probe(struct platform_device *pdev)
>>
>>  	INIT_WORK(&beeper->work, pwm_beeper_work);
>>
>> -	beeper->max_volume_level = ARRAY_SIZE(beeper->volume_levels) - 1;
>> +	/* determine the number of volume levels */
>> +	prop = of_find_property(np, "volume-levels", &length);
>
> Please use generic device properties, not OF-specific ones.

Ok, I will change this in v4.
>
>> +	if (!prop) {
>> +		dev_dbg(&pdev->dev, "no volume levels specified, using max volume\n");
>> +		beeper->max_volume_level = 1;
>> +	} else
>> +		beeper->max_volume_level = length / sizeof(u32);
>> +
>> +	/* read volume levels from DT property */
>> +	if (beeper->max_volume_level > 0) {
>> +		size_t size = sizeof(*beeper->volume_levels) *
>> +			beeper->max_volume_level;
>> +
>> +		beeper->volume_levels = devm_kzalloc(&(pdev->dev), size,
>> +			GFP_KERNEL);
>> +		if (!beeper->volume_levels)
>> +			return -ENOMEM;
>> +
>> +		if (prop) {
>> +			error = of_property_read_u32_array(np, "volume-levels",
>> +						beeper->volume_levels,
>> +						beeper->max_volume_level);
>> +
>> +			if (error < 0)
>> +				return error;
>> +
>> +			error = of_property_read_u32(np, "default-volume-level",
>> +						   &value);
>> +
>> +			if (error < 0) {
>> +				dev_dbg(&pdev->dev, "no default volume specified, using max volume\n");
>> +				value = beeper->max_volume_level - 1;
>> +			}
>> +		} else {
>> +			beeper->volume_levels[0] = 500;
>> +			value = 0;
>> +		}
>> +
>> +		beeper->volume = value;
>> +		beeper->max_volume_level--;
>> +	}
>>
>>  	beeper->input = input_allocate_device();
>>  	if (!beeper->input) {
>> --
>> 2.7.4
>>
>

Re: [PATCH v3 3/3] input: pwm-beeper: add devicetree bindings to set volume levels

[Frieder Schrempf]

On 19.01.2017 22:30, Dmitry Torokhov wrote:
> On Thu, Jan 19, 2017 at 04:24:10PM +0100, Frieder Schrempf wrote:
>> 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>
>> ---
>> Changes in v3:
>>  - none
>>
>>  drivers/input/misc/pwm-beeper.c | 49 ++++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 46 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
>> index 3ed21da..a7f9d70 100644
>> --- a/drivers/input/misc/pwm-beeper.c
>> +++ b/drivers/input/misc/pwm-beeper.c
>> @@ -32,7 +32,7 @@ struct pwm_beeper {
>>  	struct work_struct work;
>>  	unsigned long period;
>>  	unsigned int volume;
>> -	unsigned int volume_levels[] = {0, 8, 20, 40, 500};
>> +	unsigned int *volume_levels;
>>  	unsigned int max_volume_level;
>>  };
>>
>> @@ -161,8 +161,11 @@ static void pwm_beeper_close(struct input_dev *input)
>>  static int pwm_beeper_probe(struct platform_device *pdev)
>>  {
>>  	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
>> +	struct device_node *np = pdev->dev.of_node;
>>  	struct pwm_beeper *beeper;
>> -	int error;
>> +	struct property *prop;
>> +	int error, length;
>> +	u32 value;
>>
>>  	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
>>  	if (!beeper)
>> @@ -188,7 +191,47 @@ static int pwm_beeper_probe(struct platform_device *pdev)
>>
>>  	INIT_WORK(&beeper->work, pwm_beeper_work);
>>
>> -	beeper->max_volume_level = ARRAY_SIZE(beeper->volume_levels) - 1;
>> +	/* determine the number of volume levels */
>> +	prop = of_find_property(np, "volume-levels", &length);
>
> Please use generic device properties, not OF-specific ones.

Ok, I will change this in v4.
>
>> +	if (!prop) {
>> +		dev_dbg(&pdev->dev, "no volume levels specified, using max volume\n");
>> +		beeper->max_volume_level = 1;
>> +	} else
>> +		beeper->max_volume_level = length / sizeof(u32);
>> +
>> +	/* read volume levels from DT property */
>> +	if (beeper->max_volume_level > 0) {
>> +		size_t size = sizeof(*beeper->volume_levels) *
>> +			beeper->max_volume_level;
>> +
>> +		beeper->volume_levels = devm_kzalloc(&(pdev->dev), size,
>> +			GFP_KERNEL);
>> +		if (!beeper->volume_levels)
>> +			return -ENOMEM;
>> +
>> +		if (prop) {
>> +			error = of_property_read_u32_array(np, "volume-levels",
>> +						beeper->volume_levels,
>> +						beeper->max_volume_level);
>> +
>> +			if (error < 0)
>> +				return error;
>> +
>> +			error = of_property_read_u32(np, "default-volume-level",
>> +						   &value);
>> +
>> +			if (error < 0) {
>> +				dev_dbg(&pdev->dev, "no default volume specified, using max volume\n");
>> +				value = beeper->max_volume_level - 1;
>> +			}
>> +		} else {
>> +			beeper->volume_levels[0] = 500;
>> +			value = 0;
>> +		}
>> +
>> +		beeper->volume = value;
>> +		beeper->max_volume_level--;
>> +	}
>>
>>  	beeper->input = input_allocate_device();
>>  	if (!beeper->input) {
>> --
>> 2.7.4
>>
>

[PATCH v4 0/3] input: pwm-beeper: add feature to set volume level

[Frieder Schrempf]

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 number of volume levels available and their values can
be specified via device tree (similar to pwm-backlight).

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

Changes in v4:
 - move sysfs attributes from class/input to devices/pwm-beeper
 - rename max_volume_level to max_volume
 - move array allocation to probe function
 - rename attribute structs
 - remove needless code
 - update date
 - use generic device properties instead of dt properties

Frieder Schrempf (3):
  input: pwm-beeper: add feature to set volume via sysfs
  input: pwm-beeper: add documentation for volume devicetree bindings
  input: pwm-beeper: add devicetree bindings to set volume levels

 Documentation/ABI/testing/sysfs-devices-pwm-beeper |  17 ++++
 .../devicetree/bindings/input/pwm-beeper.txt       |  20 ++++
 drivers/input/misc/pwm-beeper.c                    | 111 ++++++++++++++++++++-
 3 files changed, 146 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-pwm-beeper

-- 
2.7.4

[PATCH v4 0/3] input: pwm-beeper: add feature to set volume level

[Frieder Schrempf]

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 number of volume levels available and their values can
be specified via device tree (similar to pwm-backlight).

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

Changes in v4:
 - move sysfs attributes from class/input to devices/pwm-beeper
 - rename max_volume_level to max_volume
 - move array allocation to probe function
 - rename attribute structs
 - remove needless code
 - update date
 - use generic device properties instead of dt properties

Frieder Schrempf (3):
  input: pwm-beeper: add feature to set volume via sysfs
  input: pwm-beeper: add documentation for volume devicetree bindings
  input: pwm-beeper: add devicetree bindings to set volume levels

 Documentation/ABI/testing/sysfs-devices-pwm-beeper |  17 ++++
 .../devicetree/bindings/input/pwm-beeper.txt       |  20 ++++
 drivers/input/misc/pwm-beeper.c                    | 111 ++++++++++++++++++++-
 3 files changed, 146 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-pwm-beeper

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 1/3] input: pwm-beeper: add feature to set volume via sysfs

[Frieder Schrempf]

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>
---
Changes in v4:
 - move sysfs attributes from class/input to devices/pwm-beeper
 - rename max_volume_level to max_volume
 - move array allocation to probe function
 - rename attribute structs
 - remove needless code
 - update date

 Documentation/ABI/testing/sysfs-devices-pwm-beeper | 17 +++++
 drivers/input/misc/pwm-beeper.c                    | 84 +++++++++++++++++++++-
 2 files changed, 100 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 0000000..d068c58
--- /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 5f9655d..e29904e 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -1,5 +1,9 @@
 /*
  *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
+ *
+ *  Copyright (C) 2016, Frieder Schrempf <frieder.schrempf@exceet.de>
+ *  (volume support)
+ *
  *  PWM beeper driver
  *
  *  This program is free software; you can redistribute it and/or modify it
@@ -27,16 +31,69 @@ struct pwm_beeper {
 	struct pwm_device *pwm;
 	struct work_struct work;
 	unsigned long period;
+	unsigned int volume;
+	unsigned int *volume_levels;
+	unsigned int max_volume;
 };
 
 #define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
 
+static ssize_t beeper_show_volume(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 beeper_show_max_volume(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 beeper_store_volume(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(volume, 0644, beeper_show_volume, beeper_store_volume);
+static DEVICE_ATTR(max_volume, 0644, beeper_show_max_volume, 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 void __pwm_beeper_set(struct pwm_beeper *beeper)
 {
 	unsigned long period = beeper->period;
 
 	if (period) {
-		pwm_config(beeper->pwm, period / 2, period);
+		pwm_config(beeper->pwm,
+			period / 1000 * beeper->volume_levels[beeper->volume],
+			period);
 		pwm_enable(beeper->pwm);
 	} else
 		pwm_disable(beeper->pwm);
@@ -98,6 +155,7 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
 	struct pwm_beeper *beeper;
 	int error;
+	size_t size;
 
 	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
 	if (!beeper)
@@ -123,6 +181,24 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	INIT_WORK(&beeper->work, pwm_beeper_work);
 
+	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 = input_allocate_device();
 	if (!beeper->input) {
 		dev_err(&pdev->dev, "Failed to allocate input device\n");
@@ -146,6 +222,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);
+		goto err_pwm_free;
+	}
+
 	error = input_register_device(beeper->input);
 	if (error) {
 		dev_err(&pdev->dev, "Failed to register input device: %d\n", error);
-- 
2.7.4

[PATCH v4 1/3] input: pwm-beeper: add feature to set volume via sysfs

[Frieder Schrempf]

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-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
---
Changes in v4:
 - move sysfs attributes from class/input to devices/pwm-beeper
 - rename max_volume_level to max_volume
 - move array allocation to probe function
 - rename attribute structs
 - remove needless code
 - update date

 Documentation/ABI/testing/sysfs-devices-pwm-beeper | 17 +++++
 drivers/input/misc/pwm-beeper.c                    | 84 +++++++++++++++++++++-
 2 files changed, 100 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 0000000..d068c58
--- /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-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
+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-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
+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 5f9655d..e29904e 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -1,5 +1,9 @@
 /*
  *  Copyright (C) 2010, Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
+ *
+ *  Copyright (C) 2016, Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org>
+ *  (volume support)
+ *
  *  PWM beeper driver
  *
  *  This program is free software; you can redistribute it and/or modify it
@@ -27,16 +31,69 @@ struct pwm_beeper {
 	struct pwm_device *pwm;
 	struct work_struct work;
 	unsigned long period;
+	unsigned int volume;
+	unsigned int *volume_levels;
+	unsigned int max_volume;
 };
 
 #define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
 
+static ssize_t beeper_show_volume(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 beeper_show_max_volume(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 beeper_store_volume(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(volume, 0644, beeper_show_volume, beeper_store_volume);
+static DEVICE_ATTR(max_volume, 0644, beeper_show_max_volume, 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 void __pwm_beeper_set(struct pwm_beeper *beeper)
 {
 	unsigned long period = beeper->period;
 
 	if (period) {
-		pwm_config(beeper->pwm, period / 2, period);
+		pwm_config(beeper->pwm,
+			period / 1000 * beeper->volume_levels[beeper->volume],
+			period);
 		pwm_enable(beeper->pwm);
 	} else
 		pwm_disable(beeper->pwm);
@@ -98,6 +155,7 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
 	struct pwm_beeper *beeper;
 	int error;
+	size_t size;
 
 	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
 	if (!beeper)
@@ -123,6 +181,24 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	INIT_WORK(&beeper->work, pwm_beeper_work);
 
+	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 = input_allocate_device();
 	if (!beeper->input) {
 		dev_err(&pdev->dev, "Failed to allocate input device\n");
@@ -146,6 +222,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);
+		goto err_pwm_free;
+	}
+
 	error = input_register_device(beeper->input);
 	if (error) {
 		dev_err(&pdev->dev, "Failed to register input device: %d\n", error);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Frieder Schrempf]

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>
---
Changes in v4:
 - none

 .../devicetree/bindings/input/pwm-beeper.txt         | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
index be332ae..496b68f 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
@@ -5,3 +5,23 @@ Registers a PWM device as beeper.
 Required properties:
 - compatible: should be "pwm-beeper"
 - pwms: phandle to the physical PWM device
+
+Optional properties:
+- 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:
+
+	pwm-beeper {
+		compatible = "pwm-beeper";
+		pwms = <&pwm4 0 5000>;
+		volume-levels = <0 8 20 40 500>;
+		default-volume-level = <4>;
+	};
-- 
2.7.4

[PATCH v4 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Frieder Schrempf]

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>
---
Changes in v4:
 - none

 .../devicetree/bindings/input/pwm-beeper.txt         | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
index be332ae..496b68f 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
@@ -5,3 +5,23 @@ Registers a PWM device as beeper.
 Required properties:
 - compatible: should be "pwm-beeper"
 - pwms: phandle to the physical PWM device
+
+Optional properties:
+- 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:
+
+	pwm-beeper {
+		compatible = "pwm-beeper";
+		pwms = <&pwm4 0 5000>;
+		volume-levels = <0 8 20 40 500>;
+		default-volume-level = <4>;
+	};
-- 
2.7.4

[PATCH v4 3/3] input: pwm-beeper: add devicetree bindings to set volume levels

[Frieder Schrempf]

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>
---
Changes in v4:
 - use generic device properties instead of dt properties

 drivers/input/misc/pwm-beeper.c | 63 ++++++++++++++++++++++++++++-------------
 1 file changed, 44 insertions(+), 19 deletions(-)

diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index e29904e..6fe54de 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -154,8 +154,9 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 {
 	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
 	struct pwm_beeper *beeper;
-	int error;
-	size_t size;
+	int error, length;
+	size_t size;
+	u32 value;
 
 	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
 	if (!beeper)
@@ -181,23 +182,47 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	INIT_WORK(&beeper->work, pwm_beeper_work);
 
-	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;
+	/* 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_level = 1;
+	} else
+		beeper->max_volume_level = length;
+
+	/* read volume levels from DT property */
+	if (beeper->max_volume_level > 0) {
+		size_t size = sizeof(*beeper->volume_levels) *
+			beeper->max_volume_level;
+
+		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_level);
+
+			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_level - 1;
+			}
+		} else {
+			beeper->volume_levels[0] = 500;
+			value = 0;
+		}
+
+		beeper->volume = value;
+		beeper->max_volume_level--;
+	}
 
 	beeper->input = input_allocate_device();
 	if (!beeper->input) {
-- 
2.7.4

[PATCH v4 3/3] input: pwm-beeper: add devicetree bindings to set volume levels

[Frieder Schrempf]

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>
---
Changes in v4:
 - use generic device properties instead of dt properties

 drivers/input/misc/pwm-beeper.c | 63 ++++++++++++++++++++++++++++-------------
 1 file changed, 44 insertions(+), 19 deletions(-)

diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index e29904e..6fe54de 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -154,8 +154,9 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 {
 	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
 	struct pwm_beeper *beeper;
-	int error;
-	size_t size;
+	int error, length;
+	size_t size;
+	u32 value;
 
 	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
 	if (!beeper)
@@ -181,23 +182,47 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	INIT_WORK(&beeper->work, pwm_beeper_work);
 
-	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;
+	/* 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_level = 1;
+	} else
+		beeper->max_volume_level = length;
+
+	/* read volume levels from DT property */
+	if (beeper->max_volume_level > 0) {
+		size_t size = sizeof(*beeper->volume_levels) *
+			beeper->max_volume_level;
+
+		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_level);
+
+			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_level - 1;
+			}
+		} else {
+			beeper->volume_levels[0] = 500;
+			value = 0;
+		}
+
+		beeper->volume = value;
+		beeper->max_volume_level--;
+	}
 
 	beeper->input = input_allocate_device();
 	if (!beeper->input) {
-- 
2.7.4

Re: [PATCH v3 0/3] input: pwm-beeper: add feature to set volume level

[Frieder Schrempf]

Hello David,

On 20.01.2017 20:11, David Lechner wrote:
> On 01/19/2017 03:37 PM, Dmitry Torokhov wrote:
>> On Thu, Jan 19, 2017 at 04:24:07PM +0100, Frieder Schrempf wrote:
>>> 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 number of volume levels available and their values can
>>> be specified via device tree (similar to pwm-backlight).
>>>
>>> The volume adjustment is done by changing the duty cycle of
>>> the pwm signal.
>>
>> I wonder how this all will mesh up with beepers that have dedicated
>> amplifiers (support is being added by David Lechner).
>
> This will work very well with it. I fact, it is a feature I would like
> to have but I was not sure about a good way to implement it. Please Cc:
> me on future revisions of this series and I will be glad to test it.

I have just send v4.
>
> One thing that would be nice would be for a more generic way to change
> the volume from userspace. Having a sysfs attribute on the platform
> device will work, but it requires very specific knowledge from any
> userspace program that wants to control the volume. Would it be possible
> to add an alsa mixer volume control or something like that?

An alsa mixer control for the pwm-beeper volume sounds nice, but 
unfortunately I have no idea how one would implement this, or if this is 
even possible.
>

Re: [PATCH v3 0/3] input: pwm-beeper: add feature to set volume level

[Frieder Schrempf]

Hello David,

On 20.01.2017 20:11, David Lechner wrote:
> On 01/19/2017 03:37 PM, Dmitry Torokhov wrote:
>> On Thu, Jan 19, 2017 at 04:24:07PM +0100, Frieder Schrempf wrote:
>>> 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 number of volume levels available and their values can
>>> be specified via device tree (similar to pwm-backlight).
>>>
>>> The volume adjustment is done by changing the duty cycle of
>>> the pwm signal.
>>
>> I wonder how this all will mesh up with beepers that have dedicated
>> amplifiers (support is being added by David Lechner).
>
> This will work very well with it. I fact, it is a feature I would like
> to have but I was not sure about a good way to implement it. Please Cc:
> me on future revisions of this series and I will be glad to test it.

I have just send v4.
>
> One thing that would be nice would be for a more generic way to change
> the volume from userspace. Having a sysfs attribute on the platform
> device will work, but it requires very specific knowledge from any
> userspace program that wants to control the volume. Would it be possible
> to add an alsa mixer volume control or something like that?

An alsa mixer control for the pwm-beeper volume sounds nice, but 
unfortunately I have no idea how one would implement this, or if this is 
even possible.
>

Re: [PATCH v3 0/3] input: pwm-beeper: add feature to set volume level

[David Lechner]

On 02/16/2017 03:15 PM, Frieder Schrempf wrote:
> Hello David,
>
> On 20.01.2017 20:11, David Lechner wrote:
>> On 01/19/2017 03:37 PM, Dmitry Torokhov wrote:
>>> On Thu, Jan 19, 2017 at 04:24:07PM +0100, Frieder Schrempf wrote:
>>>> 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 number of volume levels available and their values can
>>>> be specified via device tree (similar to pwm-backlight).
>>>>
>>>> The volume adjustment is done by changing the duty cycle of
>>>> the pwm signal.
>>>
>>> I wonder how this all will mesh up with beepers that have dedicated
>>> amplifiers (support is being added by David Lechner).
>>
>> This will work very well with it. I fact, it is a feature I would like
>> to have but I was not sure about a good way to implement it. Please Cc:
>> me on future revisions of this series and I will be glad to test it.
>
> I have just send v4.
>>
>> One thing that would be nice would be for a more generic way to change
>> the volume from userspace. Having a sysfs attribute on the platform
>> device will work, but it requires very specific knowledge from any
>> userspace program that wants to control the volume. Would it be possible
>> to add an alsa mixer volume control or something like that?
>
> An alsa mixer control for the pwm-beeper volume sounds nice, but
> unfortunately I have no idea how one would implement this, or if this is
> even possible.
>>


I know it is possible because I have done it. ;-)

https://github.com/ev3dev/lego-linux-drivers/blob/master/evb/evb_sound.c

The driver in the link above does PCM playback using PWM. But, it also 
included an input device similar to pwm-beeper and it includes an ALSA 
volume control specifically for the beeper.

Re: [PATCH v3 0/3] input: pwm-beeper: add feature to set volume level

[David Lechner]

On 02/16/2017 03:15 PM, Frieder Schrempf wrote:
> Hello David,
>
> On 20.01.2017 20:11, David Lechner wrote:
>> On 01/19/2017 03:37 PM, Dmitry Torokhov wrote:
>>> On Thu, Jan 19, 2017 at 04:24:07PM +0100, Frieder Schrempf wrote:
>>>> 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 number of volume levels available and their values can
>>>> be specified via device tree (similar to pwm-backlight).
>>>>
>>>> The volume adjustment is done by changing the duty cycle of
>>>> the pwm signal.
>>>
>>> I wonder how this all will mesh up with beepers that have dedicated
>>> amplifiers (support is being added by David Lechner).
>>
>> This will work very well with it. I fact, it is a feature I would like
>> to have but I was not sure about a good way to implement it. Please Cc:
>> me on future revisions of this series and I will be glad to test it.
>
> I have just send v4.
>>
>> One thing that would be nice would be for a more generic way to change
>> the volume from userspace. Having a sysfs attribute on the platform
>> device will work, but it requires very specific knowledge from any
>> userspace program that wants to control the volume. Would it be possible
>> to add an alsa mixer volume control or something like that?
>
> An alsa mixer control for the pwm-beeper volume sounds nice, but
> unfortunately I have no idea how one would implement this, or if this is
> even possible.
>>


I know it is possible because I have done it. ;-)

https://github.com/ev3dev/lego-linux-drivers/blob/master/evb/evb_sound.c

The driver in the link above does PCM playback using PWM. But, it also 
included an input device similar to pwm-beeper and it includes an ALSA 
volume control specifically for the beeper.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/3] input: pwm-beeper: add devicetree bindings to set volume levels

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2866 bytes --]

Hi Frieder,

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.10-rc8]
[cannot apply to input/next next-20170216]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Frieder-Schrempf/input-pwm-beeper-add-feature-to-set-volume-level/20170217-064600
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: x86_64-randconfig-x009-201707 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/input/misc/pwm-beeper.c: In function 'pwm_beeper_probe':
>> drivers/input/misc/pwm-beeper.c:189:9: error: 'struct pwm_beeper' has no member named 'max_volume_level'; did you mean 'volume_levels'?
      beeper->max_volume_level = 1;
            ^~
   drivers/input/misc/pwm-beeper.c:191:9: error: 'struct pwm_beeper' has no member named 'max_volume_level'; did you mean 'volume_levels'?
      beeper->max_volume_level = length;
            ^~
   drivers/input/misc/pwm-beeper.c:194:12: error: 'struct pwm_beeper' has no member named 'max_volume_level'; did you mean 'volume_levels'?
     if (beeper->max_volume_level > 0) {
               ^~
   drivers/input/misc/pwm-beeper.c:196:10: error: 'struct pwm_beeper' has no member named 'max_volume_level'; did you mean 'volume_levels'?
       beeper->max_volume_level;
             ^~
   drivers/input/misc/pwm-beeper.c:206:13: error: 'struct pwm_beeper' has no member named 'max_volume_level'; did you mean 'volume_levels'?
          beeper->max_volume_level);
                ^~
   drivers/input/misc/pwm-beeper.c:216:19: error: 'struct pwm_beeper' has no member named 'max_volume_level'; did you mean 'volume_levels'?
        value = beeper->max_volume_level - 1;
                      ^~
   drivers/input/misc/pwm-beeper.c:224:9: error: 'struct pwm_beeper' has no member named 'max_volume_level'; did you mean 'volume_levels'?
      beeper->max_volume_level--;
            ^~
   drivers/input/misc/pwm-beeper.c:158:9: warning: unused variable 'size' [-Wunused-variable]
     size_t size;
            ^~~~

vim +189 drivers/input/misc/pwm-beeper.c

   183		INIT_WORK(&beeper->work, pwm_beeper_work);
   184	
   185		/* determine the number of volume levels */
   186		length = device_property_read_u32_array(&pdev->dev, "volume-levels", NULL, 0);
   187		if (length <= 0) {
   188			dev_dbg(&pdev->dev, "no volume levels specified, using max volume\n");
 > 189			beeper->max_volume_level = 1;
   190		} else
   191			beeper->max_volume_level = length;
   192	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27583 bytes --]

Re: [PATCH v4 3/3] input: pwm-beeper: add devicetree bindings to set volume levels

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2866 bytes --]

Hi Frieder,

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.10-rc8]
[cannot apply to input/next next-20170216]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Frieder-Schrempf/input-pwm-beeper-add-feature-to-set-volume-level/20170217-064600
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: x86_64-randconfig-x009-201707 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/input/misc/pwm-beeper.c: In function 'pwm_beeper_probe':
>> drivers/input/misc/pwm-beeper.c:189:9: error: 'struct pwm_beeper' has no member named 'max_volume_level'; did you mean 'volume_levels'?
      beeper->max_volume_level = 1;
            ^~
   drivers/input/misc/pwm-beeper.c:191:9: error: 'struct pwm_beeper' has no member named 'max_volume_level'; did you mean 'volume_levels'?
      beeper->max_volume_level = length;
            ^~
   drivers/input/misc/pwm-beeper.c:194:12: error: 'struct pwm_beeper' has no member named 'max_volume_level'; did you mean 'volume_levels'?
     if (beeper->max_volume_level > 0) {
               ^~
   drivers/input/misc/pwm-beeper.c:196:10: error: 'struct pwm_beeper' has no member named 'max_volume_level'; did you mean 'volume_levels'?
       beeper->max_volume_level;
             ^~
   drivers/input/misc/pwm-beeper.c:206:13: error: 'struct pwm_beeper' has no member named 'max_volume_level'; did you mean 'volume_levels'?
          beeper->max_volume_level);
                ^~
   drivers/input/misc/pwm-beeper.c:216:19: error: 'struct pwm_beeper' has no member named 'max_volume_level'; did you mean 'volume_levels'?
        value = beeper->max_volume_level - 1;
                      ^~
   drivers/input/misc/pwm-beeper.c:224:9: error: 'struct pwm_beeper' has no member named 'max_volume_level'; did you mean 'volume_levels'?
      beeper->max_volume_level--;
            ^~
   drivers/input/misc/pwm-beeper.c:158:9: warning: unused variable 'size' [-Wunused-variable]
     size_t size;
            ^~~~

vim +189 drivers/input/misc/pwm-beeper.c

   183		INIT_WORK(&beeper->work, pwm_beeper_work);
   184	
   185		/* determine the number of volume levels */
   186		length = device_property_read_u32_array(&pdev->dev, "volume-levels", NULL, 0);
   187		if (length <= 0) {
   188			dev_dbg(&pdev->dev, "no volume levels specified, using max volume\n");
 > 189			beeper->max_volume_level = 1;
   190		} else
   191			beeper->max_volume_level = length;
   192	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27583 bytes --]

Re: [PATCH v4 3/3] input: pwm-beeper: add devicetree bindings to set volume levels

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 9602 bytes --]

Hi Frieder,

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.10-rc8]
[cannot apply to input/next next-20170216]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Frieder-Schrempf/input-pwm-beeper-add-feature-to-set-volume-level/20170217-064600
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: x86_64-randconfig-it0-02170238 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   drivers/input/misc/pwm-beeper.c: In function 'pwm_beeper_probe':
>> drivers/input/misc/pwm-beeper.c:189:9: error: 'struct pwm_beeper' has no member named 'max_volume_level'
      beeper->max_volume_level = 1;
            ^
   drivers/input/misc/pwm-beeper.c:191:9: error: 'struct pwm_beeper' has no member named 'max_volume_level'
      beeper->max_volume_level = length;
            ^
   In file included from include/uapi/linux/stddef.h:1:0,
                    from include/linux/stddef.h:4,
                    from include/uapi/linux/posix_types.h:4,
                    from include/uapi/linux/types.h:13,
                    from include/linux/types.h:5,
                    from include/uapi/linux/sysinfo.h:4,
                    from include/uapi/linux/kernel.h:4,
                    from include/linux/cache.h:4,
                    from include/linux/time.h:4,
                    from include/linux/input.h:11,
                    from drivers/input/misc/pwm-beeper.c:20:
   drivers/input/misc/pwm-beeper.c:194:12: error: 'struct pwm_beeper' has no member named 'max_volume_level'
     if (beeper->max_volume_level > 0) {
               ^
   include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^
>> drivers/input/misc/pwm-beeper.c:194:2: note: in expansion of macro 'if'
     if (beeper->max_volume_level > 0) {
     ^
   drivers/input/misc/pwm-beeper.c:194:12: error: 'struct pwm_beeper' has no member named 'max_volume_level'
     if (beeper->max_volume_level > 0) {
               ^
   include/linux/compiler.h:149:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^
>> drivers/input/misc/pwm-beeper.c:194:2: note: in expansion of macro 'if'
     if (beeper->max_volume_level > 0) {
     ^
   drivers/input/misc/pwm-beeper.c:194:12: error: 'struct pwm_beeper' has no member named 'max_volume_level'
     if (beeper->max_volume_level > 0) {
               ^
   include/linux/compiler.h:160:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^
>> drivers/input/misc/pwm-beeper.c:194:2: note: in expansion of macro 'if'
     if (beeper->max_volume_level > 0) {
     ^
   drivers/input/misc/pwm-beeper.c:196:10: error: 'struct pwm_beeper' has no member named 'max_volume_level'
       beeper->max_volume_level;
             ^
   drivers/input/misc/pwm-beeper.c:206:13: error: 'struct pwm_beeper' has no member named 'max_volume_level'
          beeper->max_volume_level);
                ^
   drivers/input/misc/pwm-beeper.c:216:19: error: 'struct pwm_beeper' has no member named 'max_volume_level'
        value = beeper->max_volume_level - 1;
                      ^
   drivers/input/misc/pwm-beeper.c:224:9: error: 'struct pwm_beeper' has no member named 'max_volume_level'
      beeper->max_volume_level--;
            ^
   drivers/input/misc/pwm-beeper.c:158:9: warning: unused variable 'size' [-Wunused-variable]
     size_t size;
            ^

vim +189 drivers/input/misc/pwm-beeper.c

    14	 *  You should have received a copy of the GNU General Public License along
    15	 *  with this program; if not, write to the Free Software Foundation, Inc.,
    16	 *  675 Mass Ave, Cambridge, MA 02139, USA.
    17	 *
    18	 */
    19	
  > 20	#include <linux/input.h>
    21	#include <linux/module.h>
    22	#include <linux/kernel.h>
    23	#include <linux/of.h>
    24	#include <linux/platform_device.h>
    25	#include <linux/pwm.h>
    26	#include <linux/slab.h>
    27	#include <linux/workqueue.h>
    28	
    29	struct pwm_beeper {
    30		struct input_dev *input;
    31		struct pwm_device *pwm;
    32		struct work_struct work;
    33		unsigned long period;
    34		unsigned int volume;
    35		unsigned int *volume_levels;
    36		unsigned int max_volume;
    37	};
    38	
    39	#define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
    40	
    41	static ssize_t beeper_show_volume(struct device *dev,
    42			struct device_attribute *attr, char *buf)
    43	{
    44		struct pwm_beeper *beeper = dev_get_drvdata(dev);
    45	
    46		return sprintf(buf, "%d\n", beeper->volume);
    47	}
    48	
    49	static ssize_t beeper_show_max_volume(struct device *dev,
    50			struct device_attribute *attr, char *buf)
    51	{
    52		struct pwm_beeper *beeper = dev_get_drvdata(dev);
    53	
    54		return sprintf(buf, "%d\n", beeper->max_volume);
    55	}
    56	
    57	static ssize_t beeper_store_volume(struct device *dev,
    58			struct device_attribute *attr, const char *buf, size_t count)
    59	{
    60		int rc;
    61		struct pwm_beeper *beeper = dev_get_drvdata(dev);
    62		unsigned int volume;
    63	
    64		rc = kstrtouint(buf, 0, &volume);
    65		if (rc)
    66			return rc;
    67	
    68		if (volume > beeper->max_volume)
    69			return -EINVAL;
    70		pr_debug("set volume to %u\n", volume);
    71		beeper->volume = volume;
    72	
    73		return count;
    74	}
    75	
    76	static DEVICE_ATTR(volume, 0644, beeper_show_volume, beeper_store_volume);
    77	static DEVICE_ATTR(max_volume, 0644, beeper_show_max_volume, NULL);
    78	
    79	static struct attribute *pwm_beeper_attributes[] = {
    80		&dev_attr_volume.attr,
    81		&dev_attr_max_volume.attr,
    82		NULL,
    83	};
    84	
    85	static struct attribute_group pwm_beeper_attribute_group = {
    86		.attrs = pwm_beeper_attributes,
    87	};
    88	
    89	static void __pwm_beeper_set(struct pwm_beeper *beeper)
    90	{
    91		unsigned long period = beeper->period;
    92	
    93		if (period) {
    94			pwm_config(beeper->pwm,
    95				period / 1000 * beeper->volume_levels[beeper->volume],
    96				period);
    97			pwm_enable(beeper->pwm);
    98		} else
    99			pwm_disable(beeper->pwm);
   100	}
   101	
   102	static void pwm_beeper_work(struct work_struct *work)
   103	{
   104		struct pwm_beeper *beeper =
   105			container_of(work, struct pwm_beeper, work);
   106	
   107		__pwm_beeper_set(beeper);
   108	}
   109	
   110	static int pwm_beeper_event(struct input_dev *input,
   111				    unsigned int type, unsigned int code, int value)
   112	{
   113		struct pwm_beeper *beeper = input_get_drvdata(input);
   114	
   115		if (type != EV_SND || value < 0)
   116			return -EINVAL;
   117	
   118		switch (code) {
   119		case SND_BELL:
   120			value = value ? 1000 : 0;
   121			break;
   122		case SND_TONE:
   123			break;
   124		default:
   125			return -EINVAL;
   126		}
   127	
   128		if (value == 0)
   129			beeper->period = 0;
   130		else
   131			beeper->period = HZ_TO_NANOSECONDS(value);
   132	
   133		schedule_work(&beeper->work);
   134	
   135		return 0;
   136	}
   137	
   138	static void pwm_beeper_stop(struct pwm_beeper *beeper)
   139	{
   140		cancel_work_sync(&beeper->work);
   141	
   142		if (beeper->period)
   143			pwm_disable(beeper->pwm);
   144	}
   145	
   146	static void pwm_beeper_close(struct input_dev *input)
   147	{
   148		struct pwm_beeper *beeper = input_get_drvdata(input);
   149	
   150		pwm_beeper_stop(beeper);
   151	}
   152	
   153	static int pwm_beeper_probe(struct platform_device *pdev)
   154	{
   155		unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
   156		struct pwm_beeper *beeper;
   157		int error, length;
   158		size_t size;
   159		u32 value;
   160	
   161		beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
   162		if (!beeper)
   163			return -ENOMEM;
   164	
   165		beeper->pwm = pwm_get(&pdev->dev, NULL);
   166		if (IS_ERR(beeper->pwm)) {
   167			dev_dbg(&pdev->dev, "unable to request PWM, trying legacy API\n");
   168			beeper->pwm = pwm_request(pwm_id, "pwm beeper");
   169		}
   170	
   171		if (IS_ERR(beeper->pwm)) {
   172			error = PTR_ERR(beeper->pwm);
   173			dev_err(&pdev->dev, "Failed to request pwm device: %d\n", error);
   174			goto err_free;
   175		}
   176	
   177		/*
   178		 * FIXME: pwm_apply_args() should be removed when switching to
   179		 * the atomic PWM API.
   180		 */
   181		pwm_apply_args(beeper->pwm);
   182	
   183		INIT_WORK(&beeper->work, pwm_beeper_work);
   184	
   185		/* determine the number of volume levels */
   186		length = device_property_read_u32_array(&pdev->dev, "volume-levels", NULL, 0);
   187		if (length <= 0) {
   188			dev_dbg(&pdev->dev, "no volume levels specified, using max volume\n");
 > 189			beeper->max_volume_level = 1;
   190		} else
   191			beeper->max_volume_level = length;
   192	
   193		/* read volume levels from DT property */
 > 194		if (beeper->max_volume_level > 0) {
   195			size_t size = sizeof(*beeper->volume_levels) *
   196				beeper->max_volume_level;
   197	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29368 bytes --]

Re: [PATCH v4 3/3] input: pwm-beeper: add devicetree bindings to set volume levels

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 9602 bytes --]

Hi Frieder,

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.10-rc8]
[cannot apply to input/next next-20170216]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Frieder-Schrempf/input-pwm-beeper-add-feature-to-set-volume-level/20170217-064600
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: x86_64-randconfig-it0-02170238 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   drivers/input/misc/pwm-beeper.c: In function 'pwm_beeper_probe':
>> drivers/input/misc/pwm-beeper.c:189:9: error: 'struct pwm_beeper' has no member named 'max_volume_level'
      beeper->max_volume_level = 1;
            ^
   drivers/input/misc/pwm-beeper.c:191:9: error: 'struct pwm_beeper' has no member named 'max_volume_level'
      beeper->max_volume_level = length;
            ^
   In file included from include/uapi/linux/stddef.h:1:0,
                    from include/linux/stddef.h:4,
                    from include/uapi/linux/posix_types.h:4,
                    from include/uapi/linux/types.h:13,
                    from include/linux/types.h:5,
                    from include/uapi/linux/sysinfo.h:4,
                    from include/uapi/linux/kernel.h:4,
                    from include/linux/cache.h:4,
                    from include/linux/time.h:4,
                    from include/linux/input.h:11,
                    from drivers/input/misc/pwm-beeper.c:20:
   drivers/input/misc/pwm-beeper.c:194:12: error: 'struct pwm_beeper' has no member named 'max_volume_level'
     if (beeper->max_volume_level > 0) {
               ^
   include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^
>> drivers/input/misc/pwm-beeper.c:194:2: note: in expansion of macro 'if'
     if (beeper->max_volume_level > 0) {
     ^
   drivers/input/misc/pwm-beeper.c:194:12: error: 'struct pwm_beeper' has no member named 'max_volume_level'
     if (beeper->max_volume_level > 0) {
               ^
   include/linux/compiler.h:149:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^
>> drivers/input/misc/pwm-beeper.c:194:2: note: in expansion of macro 'if'
     if (beeper->max_volume_level > 0) {
     ^
   drivers/input/misc/pwm-beeper.c:194:12: error: 'struct pwm_beeper' has no member named 'max_volume_level'
     if (beeper->max_volume_level > 0) {
               ^
   include/linux/compiler.h:160:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^
>> drivers/input/misc/pwm-beeper.c:194:2: note: in expansion of macro 'if'
     if (beeper->max_volume_level > 0) {
     ^
   drivers/input/misc/pwm-beeper.c:196:10: error: 'struct pwm_beeper' has no member named 'max_volume_level'
       beeper->max_volume_level;
             ^
   drivers/input/misc/pwm-beeper.c:206:13: error: 'struct pwm_beeper' has no member named 'max_volume_level'
          beeper->max_volume_level);
                ^
   drivers/input/misc/pwm-beeper.c:216:19: error: 'struct pwm_beeper' has no member named 'max_volume_level'
        value = beeper->max_volume_level - 1;
                      ^
   drivers/input/misc/pwm-beeper.c:224:9: error: 'struct pwm_beeper' has no member named 'max_volume_level'
      beeper->max_volume_level--;
            ^
   drivers/input/misc/pwm-beeper.c:158:9: warning: unused variable 'size' [-Wunused-variable]
     size_t size;
            ^

vim +189 drivers/input/misc/pwm-beeper.c

    14	 *  You should have received a copy of the GNU General Public License along
    15	 *  with this program; if not, write to the Free Software Foundation, Inc.,
    16	 *  675 Mass Ave, Cambridge, MA 02139, USA.
    17	 *
    18	 */
    19	
  > 20	#include <linux/input.h>
    21	#include <linux/module.h>
    22	#include <linux/kernel.h>
    23	#include <linux/of.h>
    24	#include <linux/platform_device.h>
    25	#include <linux/pwm.h>
    26	#include <linux/slab.h>
    27	#include <linux/workqueue.h>
    28	
    29	struct pwm_beeper {
    30		struct input_dev *input;
    31		struct pwm_device *pwm;
    32		struct work_struct work;
    33		unsigned long period;
    34		unsigned int volume;
    35		unsigned int *volume_levels;
    36		unsigned int max_volume;
    37	};
    38	
    39	#define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
    40	
    41	static ssize_t beeper_show_volume(struct device *dev,
    42			struct device_attribute *attr, char *buf)
    43	{
    44		struct pwm_beeper *beeper = dev_get_drvdata(dev);
    45	
    46		return sprintf(buf, "%d\n", beeper->volume);
    47	}
    48	
    49	static ssize_t beeper_show_max_volume(struct device *dev,
    50			struct device_attribute *attr, char *buf)
    51	{
    52		struct pwm_beeper *beeper = dev_get_drvdata(dev);
    53	
    54		return sprintf(buf, "%d\n", beeper->max_volume);
    55	}
    56	
    57	static ssize_t beeper_store_volume(struct device *dev,
    58			struct device_attribute *attr, const char *buf, size_t count)
    59	{
    60		int rc;
    61		struct pwm_beeper *beeper = dev_get_drvdata(dev);
    62		unsigned int volume;
    63	
    64		rc = kstrtouint(buf, 0, &volume);
    65		if (rc)
    66			return rc;
    67	
    68		if (volume > beeper->max_volume)
    69			return -EINVAL;
    70		pr_debug("set volume to %u\n", volume);
    71		beeper->volume = volume;
    72	
    73		return count;
    74	}
    75	
    76	static DEVICE_ATTR(volume, 0644, beeper_show_volume, beeper_store_volume);
    77	static DEVICE_ATTR(max_volume, 0644, beeper_show_max_volume, NULL);
    78	
    79	static struct attribute *pwm_beeper_attributes[] = {
    80		&dev_attr_volume.attr,
    81		&dev_attr_max_volume.attr,
    82		NULL,
    83	};
    84	
    85	static struct attribute_group pwm_beeper_attribute_group = {
    86		.attrs = pwm_beeper_attributes,
    87	};
    88	
    89	static void __pwm_beeper_set(struct pwm_beeper *beeper)
    90	{
    91		unsigned long period = beeper->period;
    92	
    93		if (period) {
    94			pwm_config(beeper->pwm,
    95				period / 1000 * beeper->volume_levels[beeper->volume],
    96				period);
    97			pwm_enable(beeper->pwm);
    98		} else
    99			pwm_disable(beeper->pwm);
   100	}
   101	
   102	static void pwm_beeper_work(struct work_struct *work)
   103	{
   104		struct pwm_beeper *beeper =
   105			container_of(work, struct pwm_beeper, work);
   106	
   107		__pwm_beeper_set(beeper);
   108	}
   109	
   110	static int pwm_beeper_event(struct input_dev *input,
   111				    unsigned int type, unsigned int code, int value)
   112	{
   113		struct pwm_beeper *beeper = input_get_drvdata(input);
   114	
   115		if (type != EV_SND || value < 0)
   116			return -EINVAL;
   117	
   118		switch (code) {
   119		case SND_BELL:
   120			value = value ? 1000 : 0;
   121			break;
   122		case SND_TONE:
   123			break;
   124		default:
   125			return -EINVAL;
   126		}
   127	
   128		if (value == 0)
   129			beeper->period = 0;
   130		else
   131			beeper->period = HZ_TO_NANOSECONDS(value);
   132	
   133		schedule_work(&beeper->work);
   134	
   135		return 0;
   136	}
   137	
   138	static void pwm_beeper_stop(struct pwm_beeper *beeper)
   139	{
   140		cancel_work_sync(&beeper->work);
   141	
   142		if (beeper->period)
   143			pwm_disable(beeper->pwm);
   144	}
   145	
   146	static void pwm_beeper_close(struct input_dev *input)
   147	{
   148		struct pwm_beeper *beeper = input_get_drvdata(input);
   149	
   150		pwm_beeper_stop(beeper);
   151	}
   152	
   153	static int pwm_beeper_probe(struct platform_device *pdev)
   154	{
   155		unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
   156		struct pwm_beeper *beeper;
   157		int error, length;
   158		size_t size;
   159		u32 value;
   160	
   161		beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
   162		if (!beeper)
   163			return -ENOMEM;
   164	
   165		beeper->pwm = pwm_get(&pdev->dev, NULL);
   166		if (IS_ERR(beeper->pwm)) {
   167			dev_dbg(&pdev->dev, "unable to request PWM, trying legacy API\n");
   168			beeper->pwm = pwm_request(pwm_id, "pwm beeper");
   169		}
   170	
   171		if (IS_ERR(beeper->pwm)) {
   172			error = PTR_ERR(beeper->pwm);
   173			dev_err(&pdev->dev, "Failed to request pwm device: %d\n", error);
   174			goto err_free;
   175		}
   176	
   177		/*
   178		 * FIXME: pwm_apply_args() should be removed when switching to
   179		 * the atomic PWM API.
   180		 */
   181		pwm_apply_args(beeper->pwm);
   182	
   183		INIT_WORK(&beeper->work, pwm_beeper_work);
   184	
   185		/* determine the number of volume levels */
   186		length = device_property_read_u32_array(&pdev->dev, "volume-levels", NULL, 0);
   187		if (length <= 0) {
   188			dev_dbg(&pdev->dev, "no volume levels specified, using max volume\n");
 > 189			beeper->max_volume_level = 1;
   190		} else
   191			beeper->max_volume_level = length;
   192	
   193		/* read volume levels from DT property */
 > 194		if (beeper->max_volume_level > 0) {
   195			size_t size = sizeof(*beeper->volume_levels) *
   196				beeper->max_volume_level;
   197	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29368 bytes --]

[PATCH v5 0/3] input: pwm-beeper: add feature to set volume level

[Frieder Schrempf]

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 number of volume levels available and their values can
be specified via device tree (similar to pwm-backlight).

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

Changes in v5:
 - fix renaming of max_volume_level to max_volume
 - remove needless variable declaration

Frieder Schrempf (3):
  input: pwm-beeper: add feature to set volume via sysfs
  input: pwm-beeper: add documentation for volume devicetree bindings
  input: pwm-beeper: add devicetree bindings to set volume levels

 Documentation/ABI/testing/sysfs-devices-pwm-beeper |  17 ++++
 .../devicetree/bindings/input/pwm-beeper.txt       |  20 ++++
 drivers/input/misc/pwm-beeper.c                    | 110 ++++++++++++++++++++-
 3 files changed, 145 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-pwm-beeper

-- 
2.7.4

[PATCH v5 0/3] input: pwm-beeper: add feature to set volume level

[Frieder Schrempf]

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 number of volume levels available and their values can
be specified via device tree (similar to pwm-backlight).

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

Changes in v5:
 - fix renaming of max_volume_level to max_volume
 - remove needless variable declaration

Frieder Schrempf (3):
  input: pwm-beeper: add feature to set volume via sysfs
  input: pwm-beeper: add documentation for volume devicetree bindings
  input: pwm-beeper: add devicetree bindings to set volume levels

 Documentation/ABI/testing/sysfs-devices-pwm-beeper |  17 ++++
 .../devicetree/bindings/input/pwm-beeper.txt       |  20 ++++
 drivers/input/misc/pwm-beeper.c                    | 110 ++++++++++++++++++++-
 3 files changed, 145 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-pwm-beeper

-- 
2.7.4

[PATCH v5 1/3] input: pwm-beeper: add feature to set volume via sysfs

[Frieder Schrempf]

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>
---
Changes in v5:
 - none

 Documentation/ABI/testing/sysfs-devices-pwm-beeper | 17 +++++
 drivers/input/misc/pwm-beeper.c                    | 84 +++++++++++++++++++++-
 2 files changed, 100 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 0000000..d068c58
--- /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 5f9655d..5fbc198 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -1,5 +1,9 @@
 /*
  *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
+ *
+ *  Copyright (C) 2016, Frieder Schrempf <frieder.schrempf@exceet.de>
+ *  (volume support)
+ *
  *  PWM beeper driver
  *
  *  This program is free software; you can redistribute it and/or modify it
@@ -27,16 +31,69 @@ struct pwm_beeper {
 	struct pwm_device *pwm;
 	struct work_struct work;
 	unsigned long period;
+	unsigned int volume;
+	unsigned int *volume_levels;
+	unsigned int max_volume;
 };
 
 #define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
 
+static ssize_t beeper_show_volume(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 beeper_show_max_volume(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 beeper_store_volume(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(volume, 0644, beeper_show_volume, beeper_store_volume);
+static DEVICE_ATTR(max_volume, 0644, beeper_show_max_volume, 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 void __pwm_beeper_set(struct pwm_beeper *beeper)
 {
 	unsigned long period = beeper->period;
 
 	if (period) {
-		pwm_config(beeper->pwm, period / 2, period);
+		pwm_config(beeper->pwm,
+			period / 1000 * beeper->volume_levels[beeper->volume],
+			period);
 		pwm_enable(beeper->pwm);
 	} else
 		pwm_disable(beeper->pwm);
@@ -98,6 +155,7 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
 	struct pwm_beeper *beeper;
 	int error;
+	size_t size;
 
 	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
 	if (!beeper)
@@ -123,6 +181,24 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	INIT_WORK(&beeper->work, pwm_beeper_work);
 
+	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 = input_allocate_device();
 	if (!beeper->input) {
 		dev_err(&pdev->dev, "Failed to allocate input device\n");
@@ -146,6 +222,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);
+		goto err_pwm_free;
+	}
+
 	error = input_register_device(beeper->input);
 	if (error) {
 		dev_err(&pdev->dev, "Failed to register input device: %d\n", error);
-- 
2.7.4

[PATCH v5 1/3] input: pwm-beeper: add feature to set volume via sysfs

[Frieder Schrempf]

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>
---
Changes in v5:
 - none

 Documentation/ABI/testing/sysfs-devices-pwm-beeper | 17 +++++
 drivers/input/misc/pwm-beeper.c                    | 84 +++++++++++++++++++++-
 2 files changed, 100 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 0000000..d068c58
--- /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 5f9655d..5fbc198 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -1,5 +1,9 @@
 /*
  *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
+ *
+ *  Copyright (C) 2016, Frieder Schrempf <frieder.schrempf@exceet.de>
+ *  (volume support)
+ *
  *  PWM beeper driver
  *
  *  This program is free software; you can redistribute it and/or modify it
@@ -27,16 +31,69 @@ struct pwm_beeper {
 	struct pwm_device *pwm;
 	struct work_struct work;
 	unsigned long period;
+	unsigned int volume;
+	unsigned int *volume_levels;
+	unsigned int max_volume;
 };
 
 #define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
 
+static ssize_t beeper_show_volume(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 beeper_show_max_volume(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 beeper_store_volume(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(volume, 0644, beeper_show_volume, beeper_store_volume);
+static DEVICE_ATTR(max_volume, 0644, beeper_show_max_volume, 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 void __pwm_beeper_set(struct pwm_beeper *beeper)
 {
 	unsigned long period = beeper->period;
 
 	if (period) {
-		pwm_config(beeper->pwm, period / 2, period);
+		pwm_config(beeper->pwm,
+			period / 1000 * beeper->volume_levels[beeper->volume],
+			period);
 		pwm_enable(beeper->pwm);
 	} else
 		pwm_disable(beeper->pwm);
@@ -98,6 +155,7 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
 	struct pwm_beeper *beeper;
 	int error;
+	size_t size;
 
 	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
 	if (!beeper)
@@ -123,6 +181,24 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	INIT_WORK(&beeper->work, pwm_beeper_work);
 
+	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 = input_allocate_device();
 	if (!beeper->input) {
 		dev_err(&pdev->dev, "Failed to allocate input device\n");
@@ -146,6 +222,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);
+		goto err_pwm_free;
+	}
+
 	error = input_register_device(beeper->input);
 	if (error) {
 		dev_err(&pdev->dev, "Failed to register input device: %d\n", error);
-- 
2.7.4

[PATCH v5 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Frieder Schrempf]

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>
---
Changes in v5:
 - none

 .../devicetree/bindings/input/pwm-beeper.txt         | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
index be332ae..496b68f 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
@@ -5,3 +5,23 @@ Registers a PWM device as beeper.
 Required properties:
 - compatible: should be "pwm-beeper"
 - pwms: phandle to the physical PWM device
+
+Optional properties:
+- 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:
+
+	pwm-beeper {
+		compatible = "pwm-beeper";
+		pwms = <&pwm4 0 5000>;
+		volume-levels = <0 8 20 40 500>;
+		default-volume-level = <4>;
+	};
-- 
2.7.4

[PATCH v5 2/3] input: pwm-beeper: add documentation for volume devicetree bindings

[Frieder Schrempf]

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>
---
Changes in v5:
 - none

 .../devicetree/bindings/input/pwm-beeper.txt         | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
index be332ae..496b68f 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
@@ -5,3 +5,23 @@ Registers a PWM device as beeper.
 Required properties:
 - compatible: should be "pwm-beeper"
 - pwms: phandle to the physical PWM device
+
+Optional properties:
+- 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:
+
+	pwm-beeper {
+		compatible = "pwm-beeper";
+		pwms = <&pwm4 0 5000>;
+		volume-levels = <0 8 20 40 500>;
+		default-volume-level = <4>;
+	};
-- 
2.7.4

[PATCH v5 3/3] input: pwm-beeper: add devicetree bindings to set volume levels

[Frieder Schrempf]

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>
---
Changes in v5:
 - fix renaming of max_volume_level to max_volume
 - remove needless variable declaration

 drivers/input/misc/pwm-beeper.c | 60 ++++++++++++++++++++++++++++-------------
 1 file changed, 42 insertions(+), 18 deletions(-)

diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index 5fbc198..aee88d6 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -154,8 +154,9 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 {
 	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
 	struct pwm_beeper *beeper;
-	int error;
+	int error, length;
 	size_t size;
+	u32 value;
 
 	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
 	if (!beeper)
@@ -181,23 +182,46 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	INIT_WORK(&beeper->work, pwm_beeper_work);
 
-	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;
+	/* 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 = value;
+		beeper->max_volume--;
+	}
 
 	beeper->input = input_allocate_device();
 	if (!beeper->input) {
-- 
2.7.4

[PATCH v5 3/3] input: pwm-beeper: add devicetree bindings to set volume levels

[Frieder Schrempf]

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>
---
Changes in v5:
 - fix renaming of max_volume_level to max_volume
 - remove needless variable declaration

 drivers/input/misc/pwm-beeper.c | 60 ++++++++++++++++++++++++++++-------------
 1 file changed, 42 insertions(+), 18 deletions(-)

diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index 5fbc198..aee88d6 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -154,8 +154,9 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 {
 	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
 	struct pwm_beeper *beeper;
-	int error;
+	int error, length;
 	size_t size;
+	u32 value;
 
 	beeper = kzalloc(sizeof(*beeper), GFP_KERNEL);
 	if (!beeper)
@@ -181,23 +182,46 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 
 	INIT_WORK(&beeper->work, pwm_beeper_work);
 
-	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;
+	/* 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 = value;
+		beeper->max_volume--;
+	}
 
 	beeper->input = input_allocate_device();
 	if (!beeper->input) {
-- 
2.7.4

Re: [PATCH v3 0/3] input: pwm-beeper: add feature to set volume level

[Frieder Schrempf]

On 16.02.2017 22:44, David Lechner wrote:
> On 02/16/2017 03:15 PM, Frieder Schrempf wrote:
>> Hello David,
>>
>> On 20.01.2017 20:11, David Lechner wrote:
>>> On 01/19/2017 03:37 PM, Dmitry Torokhov wrote:
>>>> On Thu, Jan 19, 2017 at 04:24:07PM +0100, Frieder Schrempf wrote:
>>>>> 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 number of volume levels available and their values can
>>>>> be specified via device tree (similar to pwm-backlight).
>>>>>
>>>>> The volume adjustment is done by changing the duty cycle of
>>>>> the pwm signal.
>>>>
>>>> I wonder how this all will mesh up with beepers that have dedicated
>>>> amplifiers (support is being added by David Lechner).
>>>
>>> This will work very well with it. I fact, it is a feature I would like
>>> to have but I was not sure about a good way to implement it. Please Cc:
>>> me on future revisions of this series and I will be glad to test it.
>>
>> I have just send v4.
>>>
>>> One thing that would be nice would be for a more generic way to change
>>> the volume from userspace. Having a sysfs attribute on the platform
>>> device will work, but it requires very specific knowledge from any
>>> userspace program that wants to control the volume. Would it be possible
>>> to add an alsa mixer volume control or something like that?
>>
>> An alsa mixer control for the pwm-beeper volume sounds nice, but
>> unfortunately I have no idea how one would implement this, or if this is
>> even possible.
>>>
>
>
> I know it is possible because I have done it. ;-)
>
> https://github.com/ev3dev/lego-linux-drivers/blob/master/evb/evb_sound.c
>
> The driver in the link above does PCM playback using PWM. But, it also
> included an input device similar to pwm-beeper and it includes an ALSA
> volume control specifically for the beeper.

Aha, thanks. Doesn't look too complicated. I think I will play with this 
a bit, when I have time. So maybe the sysfs volume control can be 
replaced with a proper volume mixer control at some point.

Re: [PATCH v3 0/3] input: pwm-beeper: add feature to set volume level

[Frieder Schrempf]

On 16.02.2017 22:44, David Lechner wrote:
> On 02/16/2017 03:15 PM, Frieder Schrempf wrote:
>> Hello David,
>>
>> On 20.01.2017 20:11, David Lechner wrote:
>>> On 01/19/2017 03:37 PM, Dmitry Torokhov wrote:
>>>> On Thu, Jan 19, 2017 at 04:24:07PM +0100, Frieder Schrempf wrote:
>>>>> 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 number of volume levels available and their values can
>>>>> be specified via device tree (similar to pwm-backlight).
>>>>>
>>>>> The volume adjustment is done by changing the duty cycle of
>>>>> the pwm signal.
>>>>
>>>> I wonder how this all will mesh up with beepers that have dedicated
>>>> amplifiers (support is being added by David Lechner).
>>>
>>> This will work very well with it. I fact, it is a feature I would like
>>> to have but I was not sure about a good way to implement it. Please Cc:
>>> me on future revisions of this series and I will be glad to test it.
>>
>> I have just send v4.
>>>
>>> One thing that would be nice would be for a more generic way to change
>>> the volume from userspace. Having a sysfs attribute on the platform
>>> device will work, but it requires very specific knowledge from any
>>> userspace program that wants to control the volume. Would it be possible
>>> to add an alsa mixer volume control or something like that?
>>
>> An alsa mixer control for the pwm-beeper volume sounds nice, but
>> unfortunately I have no idea how one would implement this, or if this is
>> even possible.
>>>
>
>
> I know it is possible because I have done it. ;-)
>
> https://github.com/ev3dev/lego-linux-drivers/blob/master/evb/evb_sound.c
>
> The driver in the link above does PCM playback using PWM. But, it also
> included an input device similar to pwm-beeper and it includes an ALSA
> volume control specifically for the beeper.

Aha, thanks. Doesn't look too complicated. I think I will play with this 
a bit, when I have time. So maybe the sysfs volume control can be 
replaced with a proper volume mixer control at some point.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html