linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Input: pwm-beeper: support customized freq for SND_BELL
@ 2017-02-07  5:21 Heiko Schocher
  2017-02-07 18:13 ` Dmitry Torokhov
  2017-02-10 15:48 ` Rob Herring
  0 siblings, 2 replies; 7+ messages in thread
From: Heiko Schocher @ 2017-02-07  5:21 UTC (permalink / raw)
  To: linux-input
  Cc: Guan Ben, Mark Jonas, Heiko Schocher, devicetree, Thierry Reding,
	linux-kernel, Boris Brezillon, Rob Herring, Dmitry Torokhov,
	Manfred Schlaegl, Mark Rutland

From: Guan Ben <ben.guan@cn.bosch.com>

extend the pwm-beeper driver to support customized frequency
for SND_BELL from device tree.

Signed-off-by: Guan Ben <ben.guan@cn.bosch.com>
Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
[hs@denx.de: adapted to 4.10-rc7]
Signed-off-by: Heiko Schocher <hs@denx.de>

---

 .../devicetree/bindings/input/pwm-beeper.txt       |  3 ++
 drivers/input/misc/pwm-beeper.c                    | 36 ++++++++++++++++------
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
index be332ae..438c6e0 100644
--- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
+++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
@@ -5,3 +5,6 @@ Registers a PWM device as beeper.
 Required properties:
 - compatible: should be "pwm-beeper"
 - pwms: phandle to the physical PWM device
+
+optional properties:
+- bell-frequency:  bell frequency in Hz
diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index 5f9655d..99591d5 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -27,6 +27,7 @@ struct pwm_beeper {
 	struct pwm_device *pwm;
 	struct work_struct work;
 	unsigned long period;
+	unsigned int bell_frequency;
 };
 
 #define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
@@ -58,20 +59,17 @@ static int pwm_beeper_event(struct input_dev *input,
 	if (type != EV_SND || value < 0)
 		return -EINVAL;
 
-	switch (code) {
-	case SND_BELL:
-		value = value ? 1000 : 0;
-		break;
-	case SND_TONE:
-		break;
-	default:
+	if (code != SND_BELL && code != SND_TONE)
 		return -EINVAL;
-	}
 
 	if (value == 0)
 		beeper->period = 0;
-	else
+	else {
+		if (code == SND_BELL)
+			value = beeper->bell_frequency;
+
 		beeper->period = HZ_TO_NANOSECONDS(value);
+	}
 
 	schedule_work(&beeper->work);
 
@@ -93,6 +91,25 @@ static void pwm_beeper_close(struct input_dev *input)
 	pwm_beeper_stop(beeper);
 }
 
+static void pwm_beeper_init_bell_frequency(struct device *dev,
+					   struct pwm_beeper *beeper)
+{
+	struct device_node *node;
+	unsigned int bell_frequency = 1000;
+	int error;
+
+	if (IS_ENABLED(CONFIG_OF)) {
+		node = dev->of_node;
+		error = of_property_read_u32(node, "bell-frequency",
+					     &bell_frequency);
+		if (error < 0)
+			dev_dbg(dev, "Failed to read bell-frequency, using default: %u Hz\n",
+				bell_frequency);
+	}
+
+	beeper->bell_frequency = bell_frequency;
+}
+
 static int pwm_beeper_probe(struct platform_device *pdev)
 {
 	unsigned long pwm_id = (unsigned long)dev_get_platdata(&pdev->dev);
@@ -122,6 +139,7 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 	pwm_apply_args(beeper->pwm);
 
 	INIT_WORK(&beeper->work, pwm_beeper_work);
+	pwm_beeper_init_bell_frequency(&pdev->dev, beeper);
 
 	beeper->input = input_allocate_device();
 	if (!beeper->input) {
-- 
2.7.4

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

* Re: [PATCH] Input: pwm-beeper: support customized freq for SND_BELL
  2017-02-07  5:21 [PATCH] Input: pwm-beeper: support customized freq for SND_BELL Heiko Schocher
@ 2017-02-07 18:13 ` Dmitry Torokhov
  2017-02-08 10:11   ` AW: " Jonas Mark (ST-FIR/ENG1)
  2017-02-10 15:48 ` Rob Herring
  1 sibling, 1 reply; 7+ messages in thread
From: Dmitry Torokhov @ 2017-02-07 18:13 UTC (permalink / raw)
  To: Heiko Schocher
  Cc: linux-input, Guan Ben, Mark Jonas, devicetree, Thierry Reding,
	linux-kernel, Boris Brezillon, Rob Herring, Manfred Schlaegl,
	Mark Rutland

On Tue, Feb 07, 2017 at 06:21:34AM +0100, Heiko Schocher wrote:
> From: Guan Ben <ben.guan@cn.bosch.com>
> 
> extend the pwm-beeper driver to support customized frequency
> for SND_BELL from device tree.

No, SND_BELL is literally SND_TONE @1000Hz. There should be no
customizing. If applications want to use different frequency then should
be using SND_TONE.

Thanks.

-- 
Dmitry

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

* AW: [PATCH] Input: pwm-beeper: support customized freq for SND_BELL
  2017-02-07 18:13 ` Dmitry Torokhov
@ 2017-02-08 10:11   ` Jonas Mark (ST-FIR/ENG1)
  2017-02-14  4:27     ` Dmitry Torokhov
  0 siblings, 1 reply; 7+ messages in thread
From: Jonas Mark (ST-FIR/ENG1) @ 2017-02-08 10:11 UTC (permalink / raw)
  To: Dmitry Torokhov, Heiko Schocher
  Cc: linux-input, GUAN Ben (ST-FIR/ENG1-Zhu),
	devicetree, Thierry Reding, linux-kernel, Boris Brezillon,
	Rob Herring, Manfred Schlaegl, Mark Rutland,
	Jonas Mark (ST-FIR/ENG1)

Hello Dmitry,

> > extend the pwm-beeper driver to support customized frequency
> > for SND_BELL from device tree.
> 
> No, SND_BELL is literally SND_TONE @1000Hz. There should be no
> customizing. If applications want to use different frequency then should
> be using SND_TONE.

We are not aiming for an application shortcut here. Instead, changing the bell frequency shall be a system setting. That is, every application which wants to make a bell sound shall use the alternative frequency.

The reason why we are deviating from the default 1000 Hz is that on our hardware we are using a loudspeaker which is rated for 2.7 kHz. That is, it will only sound at the specified volume and frequency if you feed it with a 2.7 kHz square wave. If you deviate from it, e.g. by using 1000 Hz, the output will be dim and squeaky. Worst case, SND_BELL would be completely silent on our system. So the only bell sound we can reliably generate on our system has 2.7 kHz.

The good news for everybody else's systems is that the patch does not change the default behavior. If not specified in the DT, the frequency will still be 1000 Hz. But other systems which are similarly challenged could now offer a reasonable bell sound, too.

Can you point me to a specification or standard which defines that SND_BELL has to be 1000 Hz without any exception?

I did some research on that topic and was not able to discover such a specification or standard. What I did find though was that the ASCII or Unicode BEL character is most likely very closely related to SND_BELL. And for BEL there is no frequency specified. Historically it once was a real bell which was rung. The intention was to notify the human on the other end of a teletype to have a look at it. And that is guaranteed with 1000 Hz as well as with 2.7 kHz as long as the sound is perceptible to the human ear.

https://en.wikipedia.org/wiki/Bell_character

Regards,
Mark

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

* Re: [PATCH] Input: pwm-beeper: support customized freq for SND_BELL
  2017-02-07  5:21 [PATCH] Input: pwm-beeper: support customized freq for SND_BELL Heiko Schocher
  2017-02-07 18:13 ` Dmitry Torokhov
@ 2017-02-10 15:48 ` Rob Herring
  2017-02-13  6:12   ` Heiko Schocher
  1 sibling, 1 reply; 7+ messages in thread
From: Rob Herring @ 2017-02-10 15:48 UTC (permalink / raw)
  To: Heiko Schocher
  Cc: linux-input, Guan Ben, Mark Jonas, devicetree, Thierry Reding,
	linux-kernel, Boris Brezillon, Dmitry Torokhov, Manfred Schlaegl,
	Mark Rutland

On Tue, Feb 07, 2017 at 06:21:34AM +0100, Heiko Schocher wrote:
> From: Guan Ben <ben.guan@cn.bosch.com>
> 
> extend the pwm-beeper driver to support customized frequency
> for SND_BELL from device tree.
> 
> Signed-off-by: Guan Ben <ben.guan@cn.bosch.com>
> Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
> [hs@denx.de: adapted to 4.10-rc7]
> Signed-off-by: Heiko Schocher <hs@denx.de>
> 
> ---
> 
>  .../devicetree/bindings/input/pwm-beeper.txt       |  3 ++
>  drivers/input/misc/pwm-beeper.c                    | 36 ++++++++++++++++------
>  2 files changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> index be332ae..438c6e0 100644
> --- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
> @@ -5,3 +5,6 @@ Registers a PWM device as beeper.
>  Required properties:
>  - compatible: should be "pwm-beeper"
>  - pwms: phandle to the physical PWM device
> +
> +optional properties:
> +- bell-frequency:  bell frequency in Hz

Needs a unit suffix:
bell-frequency-hz or just bell-hz as hz implies frequency.

Or maybe beeper-hz would be more consistant.

Rob

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

* Re: [PATCH] Input: pwm-beeper: support customized freq for SND_BELL
  2017-02-10 15:48 ` Rob Herring
@ 2017-02-13  6:12   ` Heiko Schocher
  0 siblings, 0 replies; 7+ messages in thread
From: Heiko Schocher @ 2017-02-13  6:12 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-input, Guan Ben, Mark Jonas, devicetree, Thierry Reding,
	linux-kernel, Boris Brezillon, Dmitry Torokhov, Manfred Schlaegl,
	Mark Rutland

Hello Rob,

Am 10.02.2017 um 16:48 schrieb Rob Herring:
> On Tue, Feb 07, 2017 at 06:21:34AM +0100, Heiko Schocher wrote:
>> From: Guan Ben <ben.guan@cn.bosch.com>
>>
>> extend the pwm-beeper driver to support customized frequency
>> for SND_BELL from device tree.
>>
>> Signed-off-by: Guan Ben <ben.guan@cn.bosch.com>
>> Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
>> [hs@denx.de: adapted to 4.10-rc7]
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>>
>> ---
>>
>>   .../devicetree/bindings/input/pwm-beeper.txt       |  3 ++
>>   drivers/input/misc/pwm-beeper.c                    | 36 ++++++++++++++++------
>>   2 files changed, 30 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>> index be332ae..438c6e0 100644
>> --- a/Documentation/devicetree/bindings/input/pwm-beeper.txt
>> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt
>> @@ -5,3 +5,6 @@ Registers a PWM device as beeper.
>>   Required properties:
>>   - compatible: should be "pwm-beeper"
>>   - pwms: phandle to the physical PWM device
>> +
>> +optional properties:
>> +- bell-frequency:  bell frequency in Hz
>
> Needs a unit suffix:
> bell-frequency-hz or just bell-hz as hz implies frequency.
>
> Or maybe beeper-hz would be more consistant.

Ok, I change it to "beeper-hz". Are this all issues with this patch?

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

* Re: [PATCH] Input: pwm-beeper: support customized freq for SND_BELL
  2017-02-08 10:11   ` AW: " Jonas Mark (ST-FIR/ENG1)
@ 2017-02-14  4:27     ` Dmitry Torokhov
  2017-02-14  5:02       ` Heiko Schocher
  0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Torokhov @ 2017-02-14  4:27 UTC (permalink / raw)
  To: Jonas Mark (ST-FIR/ENG1)
  Cc: Heiko Schocher, linux-input, GUAN Ben (ST-FIR/ENG1-Zhu),
	devicetree, Thierry Reding, linux-kernel, Boris Brezillon,
	Rob Herring, Manfred Schlaegl, Mark Rutland

On Wed, Feb 08, 2017 at 10:11:21AM +0000, Jonas Mark (ST-FIR/ENG1) wrote:
> Hello Dmitry,
> 
> > > extend the pwm-beeper driver to support customized frequency
> > > for SND_BELL from device tree.
> > 
> > No, SND_BELL is literally SND_TONE @1000Hz. There should be no
> > customizing. If applications want to use different frequency then should
> > be using SND_TONE.
> 
> We are not aiming for an application shortcut here. Instead, changing
> the bell frequency shall be a system setting. That is, every
> application which wants to make a bell sound shall use the alternative
> frequency.
> 
> The reason why we are deviating from the default 1000 Hz is that on
> our hardware we are using a loudspeaker which is rated for 2.7 kHz.
> That is, it will only sound at the specified volume and frequency if
> you feed it with a 2.7 kHz square wave. If you deviate from it, e.g.
> by using 1000 Hz, the output will be dim and squeaky. Worst case,
> SND_BELL would be completely silent on our system. So the only bell
> sound we can reliably generate on our system has 2.7 kHz.

OK, fair enough. Please address Rob's comments on binding and resend.

Also I am not sure why you needed to change the switch statement around,
you only need to replace 1000 with value from the attribute.

Oh, and please use device_property_*() API instead of of_*().

Thanks.

-- 
Dmitry

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

* Re: [PATCH] Input: pwm-beeper: support customized freq for SND_BELL
  2017-02-14  4:27     ` Dmitry Torokhov
@ 2017-02-14  5:02       ` Heiko Schocher
  0 siblings, 0 replies; 7+ messages in thread
From: Heiko Schocher @ 2017-02-14  5:02 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Jonas Mark (ST-FIR/ENG1),
	Heiko Schocher, linux-input, GUAN Ben (ST-FIR/ENG1-Zhu),
	devicetree, Thierry Reding, linux-kernel, Boris Brezillon,
	Rob Herring, Manfred Schlaegl, Mark Rutland

Hello Dmitry,

Am 14.02.2017 um 05:27 schrieb Dmitry Torokhov:
> On Wed, Feb 08, 2017 at 10:11:21AM +0000, Jonas Mark (ST-FIR/ENG1) wrote:
>> Hello Dmitry,
>>
>>>> extend the pwm-beeper driver to support customized frequency
>>>> for SND_BELL from device tree.
>>>
>>> No, SND_BELL is literally SND_TONE @1000Hz. There should be no
>>> customizing. If applications want to use different frequency then should
>>> be using SND_TONE.
>>
>> We are not aiming for an application shortcut here. Instead, changing
>> the bell frequency shall be a system setting. That is, every
>> application which wants to make a bell sound shall use the alternative
>> frequency.
>>
>> The reason why we are deviating from the default 1000 Hz is that on
>> our hardware we are using a loudspeaker which is rated for 2.7 kHz.
>> That is, it will only sound at the specified volume and frequency if
>> you feed it with a 2.7 kHz square wave. If you deviate from it, e.g.
>> by using 1000 Hz, the output will be dim and squeaky. Worst case,
>> SND_BELL would be completely silent on our system. So the only bell
>> sound we can reliably generate on our system has 2.7 kHz.
>
> OK, fair enough. Please address Rob's comments on binding and resend.

Done already, just waited for more comments before sending v2.

> Also I am not sure why you needed to change the switch statement around,
> you only need to replace 1000 with value from the attribute.

Hmm.. the resulting code looks cleaner to me ... First we check all
the reasons for returning an error code, after that, we can do
the functions work ... but I can revert this part ... should I ?

> Oh, and please use device_property_*() API instead of of_*().

Thanks! Good tip, changed.

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

end of thread, other threads:[~2017-02-14  5:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-07  5:21 [PATCH] Input: pwm-beeper: support customized freq for SND_BELL Heiko Schocher
2017-02-07 18:13 ` Dmitry Torokhov
2017-02-08 10:11   ` AW: " Jonas Mark (ST-FIR/ENG1)
2017-02-14  4:27     ` Dmitry Torokhov
2017-02-14  5:02       ` Heiko Schocher
2017-02-10 15:48 ` Rob Herring
2017-02-13  6:12   ` Heiko Schocher

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