* [PATCH v2] Input: pwm-beeper: support customized freq for SND_BELL
@ 2017-02-20 8:37 Heiko Schocher
2017-02-27 18:11 ` David Lechner
2017-02-27 19:10 ` [PATCH v2] " Rob Herring
0 siblings, 2 replies; 12+ messages in thread
From: Heiko Schocher @ 2017-02-20 8:37 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>
---
Changes in v2:
- add comment from Rob Herring:
rename property name "bell-frequency" to "beeper-hz"
- add comment from Dmitry Torokhov:
use device_property_read_u32() instead of of_property_read_u32()
- rebased against c470abd4fde40ea6a0846a2beab642a578c0b8cd
Linux 4.10
.../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..4e4e128 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:
+- beeper-hz: bell frequency in Hz
diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index 5f9655d..5ea6fda 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 err;
+
+ if (IS_ENABLED(CONFIG_OF)) {
+ node = dev->of_node;
+ err = device_property_read_u32(dev, "beeper-hz",
+ &bell_frequency);
+ if (err < 0)
+ dev_dbg(dev, "Failed to read beeper-hz, 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] 12+ messages in thread
* Re: [v2] Input: pwm-beeper: support customized freq for SND_BELL
@ 2017-02-27 18:11 ` David Lechner
0 siblings, 0 replies; 12+ messages in thread
From: David Lechner @ 2017-02-27 18:11 UTC (permalink / raw)
To: Heiko Schocher, linux-input
Cc: Guan Ben, Mark Jonas, devicetree, Thierry Reding, linux-kernel,
Boris Brezillon, Rob Herring, Dmitry Torokhov, Manfred Schlaegl,
Mark Rutland
On 02/20/2017 02:37 AM, 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>
> ---
>
> Changes in v2:
> - add comment from Rob Herring:
> rename property name "bell-frequency" to "beeper-hz"
Is there a separate patch for the devicetree bindings documentation?
> - add comment from Dmitry Torokhov:
> use device_property_read_u32() instead of of_property_read_u32()
> - rebased against c470abd4fde40ea6a0846a2beab642a578c0b8cd
> Linux 4.10
>
> .../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..4e4e128 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:
> +- beeper-hz: bell frequency in Hz
> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
> index 5f9655d..5ea6fda 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;
This would be much simpler if you just changed the single line above:
value = value ? beeper->bell_frequency : 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 err;
> +
> + if (IS_ENABLED(CONFIG_OF)) {
I don't think the check for CONFIG_OF is needed when using
device_property_read_u32().
> + node = dev->of_node;
node variable is never used
> + err = device_property_read_u32(dev, "beeper-hz",
> + &bell_frequency);
Does the device_property_read_u32() function guarantee that
bell_frequency is not modified when err < 0 ?
> + if (err < 0)
> + dev_dbg(dev, "Failed to read beeper-hz, using default: %u Hz\n",
"Failed" sounds like an error, but this is a perfectly normal thing to
happen. Maybe better to say "'beeper-hz' not specified, 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);
This function is so simple, it could just be done inline here.
>
> beeper->input = input_allocate_device();
> if (!beeper->input) {
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [v2] Input: pwm-beeper: support customized freq for SND_BELL
@ 2017-02-27 18:11 ` David Lechner
0 siblings, 0 replies; 12+ messages in thread
From: David Lechner @ 2017-02-27 18:11 UTC (permalink / raw)
To: Heiko Schocher, linux-input-u79uwXL29TY76Z2rM5mHXA
Cc: Guan Ben, Mark Jonas, devicetree-u79uwXL29TY76Z2rM5mHXA,
Thierry Reding, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
Boris Brezillon, Rob Herring, Dmitry Torokhov, Manfred Schlaegl,
Mark Rutland
On 02/20/2017 02:37 AM, Heiko Schocher wrote:
> From: Guan Ben <ben.guan-dUNdQdTMQaZWk0Htik3J/w@public.gmane.org>
>
> extend the pwm-beeper driver to support customized frequency
> for SND_BELL from device tree.
>
> Signed-off-by: Guan Ben <ben.guan-dUNdQdTMQaZWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Mark Jonas <mark.jonas-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
> [hs-ynQEQJNshbs@public.gmane.org: adapted to 4.10-rc7]
> Signed-off-by: Heiko Schocher <hs-ynQEQJNshbs@public.gmane.org>
> ---
>
> Changes in v2:
> - add comment from Rob Herring:
> rename property name "bell-frequency" to "beeper-hz"
Is there a separate patch for the devicetree bindings documentation?
> - add comment from Dmitry Torokhov:
> use device_property_read_u32() instead of of_property_read_u32()
> - rebased against c470abd4fde40ea6a0846a2beab642a578c0b8cd
> Linux 4.10
>
> .../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..4e4e128 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:
> +- beeper-hz: bell frequency in Hz
> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
> index 5f9655d..5ea6fda 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;
This would be much simpler if you just changed the single line above:
value = value ? beeper->bell_frequency : 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 err;
> +
> + if (IS_ENABLED(CONFIG_OF)) {
I don't think the check for CONFIG_OF is needed when using
device_property_read_u32().
> + node = dev->of_node;
node variable is never used
> + err = device_property_read_u32(dev, "beeper-hz",
> + &bell_frequency);
Does the device_property_read_u32() function guarantee that
bell_frequency is not modified when err < 0 ?
> + if (err < 0)
> + dev_dbg(dev, "Failed to read beeper-hz, using default: %u Hz\n",
"Failed" sounds like an error, but this is a perfectly normal thing to
happen. Maybe better to say "'beeper-hz' not specified, 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);
This function is so simple, it could just be done inline here.
>
> beeper->input = input_allocate_device();
> if (!beeper->input) {
>
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] Input: pwm-beeper: support customized freq for SND_BELL
2017-02-20 8:37 [PATCH v2] Input: pwm-beeper: support customized freq for SND_BELL Heiko Schocher
2017-02-27 18:11 ` David Lechner
@ 2017-02-27 19:10 ` Rob Herring
2017-02-28 5:20 ` Heiko Schocher
1 sibling, 1 reply; 12+ messages in thread
From: Rob Herring @ 2017-02-27 19:10 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 Mon, Feb 20, 2017 at 09:37:43AM +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>
>
> ---
>
> Changes in v2:
> - add comment from Rob Herring:
> rename property name "bell-frequency" to "beeper-hz"
> - add comment from Dmitry Torokhov:
> use device_property_read_u32() instead of of_property_read_u32()
> - rebased against c470abd4fde40ea6a0846a2beab642a578c0b8cd
> Linux 4.10
>
> .../devicetree/bindings/input/pwm-beeper.txt | 3 ++
Acked-by: Rob Herring <robh@kernel.org>
A few comments on the driver though.
> 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..4e4e128 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:
> +- beeper-hz: bell frequency in Hz
> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
> index 5f9655d..5ea6fda 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 err;
> +
> + if (IS_ENABLED(CONFIG_OF)) {
You don't really need this. There should be an empty version of the
function.
> + node = dev->of_node;
You don't need this line.
> + err = device_property_read_u32(dev, "beeper-hz",
> + &bell_frequency);
> + if (err < 0)
> + dev_dbg(dev, "Failed to read beeper-hz, 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 [flat|nested] 12+ messages in thread
* Re: [v2] Input: pwm-beeper: support customized freq for SND_BELL
2017-02-27 18:11 ` David Lechner
(?)
@ 2017-02-28 5:19 ` Heiko Schocher
2017-02-28 5:30 ` David Lechner
-1 siblings, 1 reply; 12+ messages in thread
From: Heiko Schocher @ 2017-02-28 5:19 UTC (permalink / raw)
To: David Lechner
Cc: linux-input, Guan Ben, Mark Jonas, devicetree, Thierry Reding,
linux-kernel, Boris Brezillon, Rob Herring, Dmitry Torokhov,
Manfred Schlaegl, Mark Rutland
Hello David,
Am 27.02.2017 um 19:11 schrieb David Lechner:
> On 02/20/2017 02:37 AM, 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>
>> ---
>>
>> Changes in v2:
>> - add comment from Rob Herring:
>> rename property name "bell-frequency" to "beeper-hz"
>
> Is there a separate patch for the devicetree bindings documentation?
No, it is in this patch ... In the meantime I got an
Acked-by from Rob Herring ...
>> - add comment from Dmitry Torokhov:
>> use device_property_read_u32() instead of of_property_read_u32()
>> - rebased against c470abd4fde40ea6a0846a2beab642a578c0b8cd
>> Linux 4.10
>>
>> .../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..4e4e128 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:
>> +- beeper-hz: bell frequency in Hz
>> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
>> index 5f9655d..5ea6fda 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;
>
> This would be much simpler if you just changed the single line above:
>
> value = value ? beeper->bell_frequency : 0;
Ok, I readded the switch statement, and changed this line.
>> - 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 err;
>> +
>> + if (IS_ENABLED(CONFIG_OF)) {
>
> I don't think the check for CONFIG_OF is needed when using device_property_read_u32().
Yes, removed.
>
>> + node = dev->of_node;
>
> node variable is never used
Removed.
>
>> + err = device_property_read_u32(dev, "beeper-hz",
>> + &bell_frequency);
>
> Does the device_property_read_u32() function guarantee that bell_frequency is not modified when err
> < 0 ?
Yes. This function "ends" in of_property_read_variable_u32_array()
which first searches the property (If not found returns) and if all
checks are fine, fills the "*out_values" with the values from the
property...
>> + if (err < 0)
>> + dev_dbg(dev, "Failed to read beeper-hz, using default: %u Hz\n",
>
> "Failed" sounds like an error, but this is a perfectly normal thing to happen. Maybe better to say
> "'beeper-hz' not specified, using default: %u Hz\n".
Changed.
>
>> + 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);
>
> This function is so simple, it could just be done inline here.
Indeed ... changed.
Thanks!
bye,
Heiko
>
>>
>> beeper->input = input_allocate_device();
>> if (!beeper->input) {
>>
>
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] Input: pwm-beeper: support customized freq for SND_BELL
2017-02-27 19:10 ` [PATCH v2] " Rob Herring
@ 2017-02-28 5:20 ` Heiko Schocher
0 siblings, 0 replies; 12+ messages in thread
From: Heiko Schocher @ 2017-02-28 5:20 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 27.02.2017 um 20:10 schrieb Rob Herring:
> On Mon, Feb 20, 2017 at 09:37:43AM +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>
>>
>> ---
>>
>> Changes in v2:
>> - add comment from Rob Herring:
>> rename property name "bell-frequency" to "beeper-hz"
>> - add comment from Dmitry Torokhov:
>> use device_property_read_u32() instead of of_property_read_u32()
>> - rebased against c470abd4fde40ea6a0846a2beab642a578c0b8cd
>> Linux 4.10
>>
>> .../devicetree/bindings/input/pwm-beeper.txt | 3 ++
>
> Acked-by: Rob Herring <robh@kernel.org>
Thanks!
> A few comments on the driver though.
>
>> 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..4e4e128 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:
>> +- beeper-hz: bell frequency in Hz
>> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
>> index 5f9655d..5ea6fda 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 err;
>> +
>> + if (IS_ENABLED(CONFIG_OF)) {
>
> You don't really need this. There should be an empty version of the
> function.
removed, as also David mentioned this.
>
>> + node = dev->of_node;
>
> You don't need this line.
Yep, removed.
>
>> + err = device_property_read_u32(dev, "beeper-hz",
>> + &bell_frequency);
>> + if (err < 0)
>> + dev_dbg(dev, "Failed to read beeper-hz, 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
>>
>
Thanks!
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] 12+ messages in thread
* Re: [v2] Input: pwm-beeper: support customized freq for SND_BELL
2017-02-28 5:19 ` Heiko Schocher
@ 2017-02-28 5:30 ` David Lechner
2017-02-28 6:13 ` Heiko Schocher
0 siblings, 1 reply; 12+ messages in thread
From: David Lechner @ 2017-02-28 5:30 UTC (permalink / raw)
To: hs
Cc: linux-input, Guan Ben, Mark Jonas, devicetree, Thierry Reding,
linux-kernel, Boris Brezillon, Rob Herring, Dmitry Torokhov,
Manfred Schlaegl, Mark Rutland
On 02/27/2017 11:19 PM, Heiko Schocher wrote:
> Hello David,
>
> Am 27.02.2017 um 19:11 schrieb David Lechner:
>> On 02/20/2017 02:37 AM, 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>
>>> ---
>>>
>>> Changes in v2:
>>> - add comment from Rob Herring:
>>> rename property name "bell-frequency" to "beeper-hz"
>>
>> Is there a separate patch for the devicetree bindings documentation?
>
> No, it is in this patch ... In the meantime I got an
> Acked-by from Rob Herring ...
>
What I should have said is that, in general, I think it is preferred to
have the device tree bindings as a separate patch from the code changes.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [v2] Input: pwm-beeper: support customized freq for SND_BELL
2017-02-28 5:30 ` David Lechner
@ 2017-02-28 6:13 ` Heiko Schocher
2017-02-28 6:20 ` Dmitry Torokhov
0 siblings, 1 reply; 12+ messages in thread
From: Heiko Schocher @ 2017-02-28 6:13 UTC (permalink / raw)
To: David Lechner
Cc: linux-input, Guan Ben, Mark Jonas, devicetree, Thierry Reding,
linux-kernel, Boris Brezillon, Rob Herring, Dmitry Torokhov,
Manfred Schlaegl, Mark Rutland
Hello David,
Am 28.02.2017 um 06:30 schrieb David Lechner:
> On 02/27/2017 11:19 PM, Heiko Schocher wrote:
>> Hello David,
>>
>> Am 27.02.2017 um 19:11 schrieb David Lechner:
>>> On 02/20/2017 02:37 AM, 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>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - add comment from Rob Herring:
>>>> rename property name "bell-frequency" to "beeper-hz"
>>>
>>> Is there a separate patch for the devicetree bindings documentation?
>>
>> No, it is in this patch ... In the meantime I got an
>> Acked-by from Rob Herring ...
>>
>
> What I should have said is that, in general, I think it is preferred to have the device tree
> bindings as a separate patch from the code changes.
I can split this into 2 patches ...
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] 12+ messages in thread
* Re: [v2] Input: pwm-beeper: support customized freq for SND_BELL
@ 2017-02-28 6:20 ` Dmitry Torokhov
0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Torokhov @ 2017-02-28 6:20 UTC (permalink / raw)
To: hs, David Lechner
Cc: linux-input, Guan Ben, Mark Jonas, devicetree, Thierry Reding,
linux-kernel, Boris Brezillon, Rob Herring, Manfred Schlaegl,
Mark Rutland
On February 27, 2017 10:13:43 PM PST, Heiko Schocher <hs@denx.de> wrote:
>Hello David,
>
>Am 28.02.2017 um 06:30 schrieb David Lechner:
>> On 02/27/2017 11:19 PM, Heiko Schocher wrote:
>>> Hello David,
>>>
>>> Am 27.02.2017 um 19:11 schrieb David Lechner:
>>>> On 02/20/2017 02:37 AM, 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>
>>>>> ---
>>>>>
>>>>> Changes in v2:
>>>>> - add comment from Rob Herring:
>>>>> rename property name "bell-frequency" to "beeper-hz"
>>>>
>>>> Is there a separate patch for the devicetree bindings
>documentation?
>>>
>>> No, it is in this patch ... In the meantime I got an
>>> Acked-by from Rob Herring ...
>>>
>>
>> What I should have said is that, in general, I think it is preferred
>to have the device tree
>> bindings as a separate patch from the code changes.
>
>I can split this into 2 patches ...
Don't - since Rob already asked DT change you don't need to post to device tree list again and I'll merge them back together since they do not make sense separately.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [v2] Input: pwm-beeper: support customized freq for SND_BELL
@ 2017-02-28 6:20 ` Dmitry Torokhov
0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Torokhov @ 2017-02-28 6:20 UTC (permalink / raw)
To: hs-ynQEQJNshbs, David Lechner
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA, Guan Ben, Mark Jonas,
devicetree-u79uwXL29TY76Z2rM5mHXA, Thierry Reding,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Boris Brezillon,
Rob Herring, Manfred Schlaegl, Mark Rutland
On February 27, 2017 10:13:43 PM PST, Heiko Schocher <hs@denx.de> wrote:
>Hello David,
>
>Am 28.02.2017 um 06:30 schrieb David Lechner:
>> On 02/27/2017 11:19 PM, Heiko Schocher wrote:
>>> Hello David,
>>>
>>> Am 27.02.2017 um 19:11 schrieb David Lechner:
>>>> On 02/20/2017 02:37 AM, 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>
>>>>> ---
>>>>>
>>>>> Changes in v2:
>>>>> - add comment from Rob Herring:
>>>>> rename property name "bell-frequency" to "beeper-hz"
>>>>
>>>> Is there a separate patch for the devicetree bindings
>documentation?
>>>
>>> No, it is in this patch ... In the meantime I got an
>>> Acked-by from Rob Herring ...
>>>
>>
>> What I should have said is that, in general, I think it is preferred
>to have the device tree
>> bindings as a separate patch from the code changes.
>
>I can split this into 2 patches ...
Don't - since Rob already asked DT change you don't need to post to device tree list again and I'll merge them back together since they do not make sense separately.
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [v2] Input: pwm-beeper: support customized freq for SND_BELL
@ 2017-03-01 5:05 ` Heiko Schocher
0 siblings, 0 replies; 12+ messages in thread
From: Heiko Schocher @ 2017-03-01 5:05 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: David Lechner, linux-input, Guan Ben, Mark Jonas, devicetree,
Thierry Reding, linux-kernel, Boris Brezillon, Rob Herring,
Manfred Schlaegl, Mark Rutland
Hello Dimitry,
Am 28.02.2017 um 07:20 schrieb Dmitry Torokhov:
> On February 27, 2017 10:13:43 PM PST, Heiko Schocher <hs@denx.de> wrote:
>> Hello David,
>>
>> Am 28.02.2017 um 06:30 schrieb David Lechner:
>>> On 02/27/2017 11:19 PM, Heiko Schocher wrote:
>>>> Hello David,
>>>>
>>>> Am 27.02.2017 um 19:11 schrieb David Lechner:
>>>>> On 02/20/2017 02:37 AM, 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>
>>>>>> ---
>>>>>>
>>>>>> Changes in v2:
>>>>>> - add comment from Rob Herring:
>>>>>> rename property name "bell-frequency" to "beeper-hz"
>>>>>
>>>>> Is there a separate patch for the devicetree bindings
>> documentation?
>>>>
>>>> No, it is in this patch ... In the meantime I got an
>>>> Acked-by from Rob Herring ...
>>>>
>>>
>>> What I should have said is that, in general, I think it is preferred
>> to have the device tree
>>> bindings as a separate patch from the code changes.
>>
>> I can split this into 2 patches ...
>
> Don't - since Rob already asked DT change you don't need to post to device tree list again and I'll merge them back together since they do not make sense separately.
Ok, I post a v3 with the changes suggested from David and Rob soon,
without splitting.
Thanks!
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] 12+ messages in thread
* Re: [v2] Input: pwm-beeper: support customized freq for SND_BELL
@ 2017-03-01 5:05 ` Heiko Schocher
0 siblings, 0 replies; 12+ messages in thread
From: Heiko Schocher @ 2017-03-01 5:05 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: David Lechner, linux-input-u79uwXL29TY76Z2rM5mHXA, Guan Ben,
Mark Jonas, devicetree-u79uwXL29TY76Z2rM5mHXA, Thierry Reding,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Boris Brezillon,
Rob Herring, Manfred Schlaegl, Mark Rutland
Hello Dimitry,
Am 28.02.2017 um 07:20 schrieb Dmitry Torokhov:
> On February 27, 2017 10:13:43 PM PST, Heiko Schocher <hs-ynQEQJNshbs@public.gmane.org> wrote:
>> Hello David,
>>
>> Am 28.02.2017 um 06:30 schrieb David Lechner:
>>> On 02/27/2017 11:19 PM, Heiko Schocher wrote:
>>>> Hello David,
>>>>
>>>> Am 27.02.2017 um 19:11 schrieb David Lechner:
>>>>> On 02/20/2017 02:37 AM, Heiko Schocher wrote:
>>>>>> From: Guan Ben <ben.guan-dUNdQdTMQaZWk0Htik3J/w@public.gmane.org>
>>>>>>
>>>>>> extend the pwm-beeper driver to support customized frequency
>>>>>> for SND_BELL from device tree.
>>>>>>
>>>>>> Signed-off-by: Guan Ben <ben.guan-dUNdQdTMQaZWk0Htik3J/w@public.gmane.org>
>>>>>> Signed-off-by: Mark Jonas <mark.jonas-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>>>>>> [hs-ynQEQJNshbs@public.gmane.org: adapted to 4.10-rc7]
>>>>>> Signed-off-by: Heiko Schocher <hs-ynQEQJNshbs@public.gmane.org>
>>>>>> ---
>>>>>>
>>>>>> Changes in v2:
>>>>>> - add comment from Rob Herring:
>>>>>> rename property name "bell-frequency" to "beeper-hz"
>>>>>
>>>>> Is there a separate patch for the devicetree bindings
>> documentation?
>>>>
>>>> No, it is in this patch ... In the meantime I got an
>>>> Acked-by from Rob Herring ...
>>>>
>>>
>>> What I should have said is that, in general, I think it is preferred
>> to have the device tree
>>> bindings as a separate patch from the code changes.
>>
>> I can split this into 2 patches ...
>
> Don't - since Rob already asked DT change you don't need to post to device tree list again and I'll merge them back together since they do not make sense separately.
Ok, I post a v3 with the changes suggested from David and Rob soon,
without splitting.
Thanks!
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-03-01 5:23 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-20 8:37 [PATCH v2] Input: pwm-beeper: support customized freq for SND_BELL Heiko Schocher
2017-02-27 18:11 ` [v2] " David Lechner
2017-02-27 18:11 ` David Lechner
2017-02-28 5:19 ` Heiko Schocher
2017-02-28 5:30 ` David Lechner
2017-02-28 6:13 ` Heiko Schocher
2017-02-28 6:20 ` Dmitry Torokhov
2017-02-28 6:20 ` Dmitry Torokhov
2017-03-01 5:05 ` Heiko Schocher
2017-03-01 5:05 ` Heiko Schocher
2017-02-27 19:10 ` [PATCH v2] " Rob Herring
2017-02-28 5:20 ` Heiko Schocher
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.