From: Schrempf Frieder <frieder.schrempf@exceet.de> To: Rob Herring <robh@kernel.org> Cc: "dmitry.torokhov@gmail.com" <dmitry.torokhov@gmail.com>, "pawel.moll@arm.com" <pawel.moll@arm.com>, "ijc+devicetree@hellion.org.uk" <ijc+devicetree@hellion.org.uk>, "galak@codeaurora.org" <galak@codeaurora.org>, "luis@debethencourt.com" <luis@debethencourt.com>, "linux-input@vger.kernel.org" <linux-input@vger.kernel.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 1/1] input: pwm-beeper: add feature to set volume via sysfs Date: Fri, 7 Oct 2016 09:00:53 +0000 [thread overview] Message-ID: <fe6349a5-7935-5be7-6dc1-7e80fa9dceeb@exceet.de> (raw) In-Reply-To: <20160411152154.GA9736@rob-hp-laptop> Thank you Rob for looking at my first kernel contribution. I come back to it now, as I had some time to process your comments. On 11.04.2016 17:21, Rob Herring wrote: > On Thu, Apr 07, 2016 at 03:58:40PM +0000, Schrempf Frieder wrote: >> Make the driver accept different volume levels via sysfs. >> This can be helpful if the beep/bell sound intensity needs >> to be adapted to the environment of the device. > > It is preferred that bindings be a separate patch. Also, this is 2 > independent features: the sysfs interface and DT properties. So that is > probably 3 patches. > > Also, sysfs interfaces are supposed to be documented. I have splitted this into 3 patches and will send them soon as v2. I also added documentation for the sysfs interface. > >> >> The number of volume levels available and their values can >> be specified via device tree (similar to pwm-backlight). >> >> This patch was tested with linux-imx 3.10.17 and was >> applied to current mainline without any changes. > > You need to test with something not 3 years old... I tested the v2 patches with Linux 4.7.4 now. > >> >> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de> >> --- >> .../devicetree/bindings/input/pwm-beeper.txt | 20 ++++ >> drivers/input/misc/pwm-beeper.c | 109 ++++++++++++++++++- >> 2 files changed, 126 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt >> index be332ae..153cd3f 100644 >> --- a/Documentation/devicetree/bindings/input/pwm-beeper.txt >> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt >> @@ -5,3 +5,23 @@ Registers a PWM device as beeper. >> Required properties: >> - compatible: should be "pwm-beeper" >> - pwms: phandle to the physical PWM device >> +- volume-levels: Array of distinct volume levels. These need to be in the >> + range of 0 to 500, while 0 means 0% duty cycle (mute) and 500 means >> + 50% duty cycle (max volume). > > Where does the scale of 0-500 come from? It is tenth of a percent? I used tenth of a percent because this is needed to define low volume levels with pwm duty cycles in the range of <1%. This is necessary because of the highly non-linear relation between duty cycle and actual perceived volume. The upper limit of 500 therefore results from 50% being the duty cycle with maximum volume. > > How many array entries? The array size and therefore the number of volume levels is dynamic and only defined by the number of entries in the volume-levels property. > >> + Please note that the actual volume of most beepers is highly >> + non-linear, which means that low volume levels are probably somewhere >> + in the range of 1 to 30 (0.1-3% duty cycle). >> +- default-volume-level: the default volume level (index into the >> + array defined by the "volume-levels" property) >> + >> +The volume level can be set via sysfs under /sys/class/input/inputX/volume. >> +The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level. >> + >> +Example: >> + >> + pwm-beeper { >> + compatible = "pwm-beeper"; >> + pwms = <&pwm4 0 5000>; >> + volume-levels = <0 8 20 40 500>; >> + default-volume-level = <4>; >> + };
WARNING: multiple messages have this Message-ID (diff)
From: Schrempf Frieder <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org> To: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Cc: "dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "pawel.moll-5wv7dgnIgG8@public.gmane.org" <pawel.moll-5wv7dgnIgG8@public.gmane.org>, "ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org" <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>, "galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org" <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>, "luis-HiykPkW1eAzzDCI4PIEvbQC/G2K4zDHf@public.gmane.org" <luis-HiykPkW1eAzzDCI4PIEvbQC/G2K4zDHf@public.gmane.org>, "linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> Subject: Re: [PATCH 1/1] input: pwm-beeper: add feature to set volume via sysfs Date: Fri, 7 Oct 2016 09:00:53 +0000 [thread overview] Message-ID: <fe6349a5-7935-5be7-6dc1-7e80fa9dceeb@exceet.de> (raw) In-Reply-To: <20160411152154.GA9736@rob-hp-laptop> Thank you Rob for looking at my first kernel contribution. I come back to it now, as I had some time to process your comments. On 11.04.2016 17:21, Rob Herring wrote: > On Thu, Apr 07, 2016 at 03:58:40PM +0000, Schrempf Frieder wrote: >> Make the driver accept different volume levels via sysfs. >> This can be helpful if the beep/bell sound intensity needs >> to be adapted to the environment of the device. > > It is preferred that bindings be a separate patch. Also, this is 2 > independent features: the sysfs interface and DT properties. So that is > probably 3 patches. > > Also, sysfs interfaces are supposed to be documented. I have splitted this into 3 patches and will send them soon as v2. I also added documentation for the sysfs interface. > >> >> The number of volume levels available and their values can >> be specified via device tree (similar to pwm-backlight). >> >> This patch was tested with linux-imx 3.10.17 and was >> applied to current mainline without any changes. > > You need to test with something not 3 years old... I tested the v2 patches with Linux 4.7.4 now. > >> >> Signed-off-by: Frieder Schrempf <frieder.schrempf-wPoT/lNZgHizQB+pC5nmwQ@public.gmane.org> >> --- >> .../devicetree/bindings/input/pwm-beeper.txt | 20 ++++ >> drivers/input/misc/pwm-beeper.c | 109 ++++++++++++++++++- >> 2 files changed, 126 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/input/pwm-beeper.txt b/Documentation/devicetree/bindings/input/pwm-beeper.txt >> index be332ae..153cd3f 100644 >> --- a/Documentation/devicetree/bindings/input/pwm-beeper.txt >> +++ b/Documentation/devicetree/bindings/input/pwm-beeper.txt >> @@ -5,3 +5,23 @@ Registers a PWM device as beeper. >> Required properties: >> - compatible: should be "pwm-beeper" >> - pwms: phandle to the physical PWM device >> +- volume-levels: Array of distinct volume levels. These need to be in the >> + range of 0 to 500, while 0 means 0% duty cycle (mute) and 500 means >> + 50% duty cycle (max volume). > > Where does the scale of 0-500 come from? It is tenth of a percent? I used tenth of a percent because this is needed to define low volume levels with pwm duty cycles in the range of <1%. This is necessary because of the highly non-linear relation between duty cycle and actual perceived volume. The upper limit of 500 therefore results from 50% being the duty cycle with maximum volume. > > How many array entries? The array size and therefore the number of volume levels is dynamic and only defined by the number of entries in the volume-levels property. > >> + Please note that the actual volume of most beepers is highly >> + non-linear, which means that low volume levels are probably somewhere >> + in the range of 1 to 30 (0.1-3% duty cycle). >> +- default-volume-level: the default volume level (index into the >> + array defined by the "volume-levels" property) >> + >> +The volume level can be set via sysfs under /sys/class/input/inputX/volume. >> +The maximum volume level index can be read from /sys/class/input/inputX/max_volume_level. >> + >> +Example: >> + >> + pwm-beeper { >> + compatible = "pwm-beeper"; >> + pwms = <&pwm4 0 5000>; >> + volume-levels = <0 8 20 40 500>; >> + default-volume-level = <4>; >> + };-- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2016-10-07 9:07 UTC|newest] Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-04-07 15:58 [PATCH 1/1] input: pwm-beeper: add feature to set volume via sysfs Schrempf Frieder 2016-04-07 15:58 ` Schrempf Frieder 2016-04-11 15:21 ` Rob Herring 2016-04-11 15:21 ` Rob Herring 2016-10-07 9:00 ` Schrempf Frieder [this message] 2016-10-07 9:00 ` Schrempf Frieder 2016-10-07 9:08 ` [PATCH v2 0/3] input: pwm-beeper: add feature to set volume level Schrempf Frieder 2016-10-07 9:08 ` [PATCH v2 1/3] input: pwm-beeper: add feature to set volume via sysfs Schrempf Frieder 2016-10-07 9:08 ` [PATCH v2 2/3] input: pwm-beeper: add documentation for volume devicetree bindings Schrempf Frieder 2016-10-07 9:08 ` Schrempf Frieder 2016-10-10 15:20 ` Rob Herring 2016-10-10 15:20 ` Rob Herring 2016-10-11 8:17 ` Schrempf Frieder 2016-10-11 8:17 ` Schrempf Frieder 2016-10-11 13:39 ` Rob Herring 2017-01-19 14:40 ` Frieder Schrempf 2017-01-19 15:24 ` [PATCH v3 0/3] input: pwm-beeper: add feature to set volume level Frieder Schrempf 2017-01-19 15:24 ` Frieder Schrempf 2017-01-19 15:24 ` [PATCH v3 1/3] input: pwm-beeper: add feature to set volume via sysfs Frieder Schrempf 2017-01-19 15:24 ` Frieder Schrempf 2017-01-19 21:29 ` Dmitry Torokhov 2017-01-19 21:29 ` Dmitry Torokhov 2017-02-16 20:37 ` Frieder Schrempf 2017-02-16 20:37 ` Frieder Schrempf 2017-01-19 15:24 ` [PATCH v3 2/3] input: pwm-beeper: add documentation for volume devicetree bindings Frieder Schrempf 2017-01-19 15:24 ` Frieder Schrempf 2017-01-23 14:40 ` Rob Herring 2017-01-23 14:40 ` Rob Herring 2017-01-19 15:24 ` [PATCH v3 3/3] input: pwm-beeper: add devicetree bindings to set volume levels Frieder Schrempf 2017-01-19 15:24 ` Frieder Schrempf 2017-01-19 21:30 ` Dmitry Torokhov 2017-02-16 20:40 ` Frieder Schrempf 2017-02-16 20:40 ` Frieder Schrempf 2017-01-19 21:37 ` [PATCH v3 0/3] input: pwm-beeper: add feature to set volume level Dmitry Torokhov 2017-01-20 19:11 ` David Lechner 2017-02-16 21:15 ` Frieder Schrempf 2017-02-16 21:15 ` Frieder Schrempf 2017-02-16 21:44 ` David Lechner 2017-02-16 21:44 ` David Lechner 2017-02-17 10:01 ` Frieder Schrempf 2017-02-17 10:01 ` Frieder Schrempf 2017-02-16 21:08 ` [PATCH v4 " Frieder Schrempf 2017-02-16 21:08 ` Frieder Schrempf 2017-02-16 21:08 ` [PATCH v4 1/3] input: pwm-beeper: add feature to set volume via sysfs Frieder Schrempf 2017-02-16 21:08 ` Frieder Schrempf 2017-02-16 21:08 ` [PATCH v4 2/3] input: pwm-beeper: add documentation for volume devicetree bindings Frieder Schrempf 2017-02-16 21:08 ` Frieder Schrempf 2017-02-16 21:08 ` [PATCH v4 3/3] input: pwm-beeper: add devicetree bindings to set volume levels Frieder Schrempf 2017-02-16 21:08 ` Frieder Schrempf 2017-02-16 23:07 ` kbuild test robot 2017-02-16 23:07 ` kbuild test robot 2017-02-17 0:14 ` kbuild test robot 2017-02-17 0:14 ` kbuild test robot 2017-02-17 9:54 ` [PATCH v5 0/3] input: pwm-beeper: add feature to set volume level Frieder Schrempf 2017-02-17 9:54 ` Frieder Schrempf 2017-02-17 9:54 ` [PATCH v5 1/3] input: pwm-beeper: add feature to set volume via sysfs Frieder Schrempf 2017-02-17 9:54 ` Frieder Schrempf 2017-02-17 9:54 ` [PATCH v5 2/3] input: pwm-beeper: add documentation for volume devicetree bindings Frieder Schrempf 2017-02-17 9:54 ` Frieder Schrempf 2017-02-17 9:54 ` [PATCH v5 3/3] input: pwm-beeper: add devicetree bindings to set volume levels Frieder Schrempf 2017-02-17 9:54 ` Frieder Schrempf 2016-10-07 9:08 ` [PATCH v2 " Schrempf Frieder
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=fe6349a5-7935-5be7-6dc1-7e80fa9dceeb@exceet.de \ --to=frieder.schrempf@exceet.de \ --cc=devicetree@vger.kernel.org \ --cc=dmitry.torokhov@gmail.com \ --cc=galak@codeaurora.org \ --cc=ijc+devicetree@hellion.org.uk \ --cc=linux-input@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=luis@debethencourt.com \ --cc=pawel.moll@arm.com \ --cc=robh@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.