* [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: [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: [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
* 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: [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
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.