On Fri, May 12, 2023 at 08:55:51PM +0200, Marek Vasut wrote: > The PWM beeper volume can be controlled by adjusting the PWM duty cycle, > expose volume setting via sysfs, so users can make the beeper quieter. > This patch adds sysfs attribute 'volume' in range 0..50000, i.e. from 0 > to 50% in 1/1000th of percent steps, this resolution should be sufficient. > > The reason for 50000 cap on volume or PWM duty cycle is because duty cycle > above 50% again reduces the loudness, the PWM wave form is inverted wave > form of the one for duty cycle below 50% and the beeper gets quieter the > closer the setting is to 100% . Hence, 50% cap where the wave form yields > the loudest result. > > Signed-off-by: Marek Vasut <marex@xxxxxxx> > --- > An alternative option would be to extend the userspace input ABI, e.g. by > using SND_TONE top 16bits to encode the duty cycle in 0..50000 range, and > bottom 16bit to encode the existing frequency in Hz . Since frequency in > Hz is likely to be below some 25 kHz for audible bell, this fits in 16bits > just fine. Thoughts ? > --- > NOTE: This uses approach similar to [1], except it is much simpler. > [1] https://patchwork.kernel.org/project/linux-input/cover/20230201152128.614439-1-manuel.traut@xxxxxx/ > --- > Cc: "Uwe Kleine-König" <u.kleine-koenig@xxxxxxxxxxxxxx> > Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> > Cc: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx> > Cc: Manuel Traut <manuel.traut@xxxxxx> > Cc: Marek Vasut <marex@xxxxxxx> > Cc: Thierry Reding <thierry.reding@xxxxxxxxx> > Cc: linux-input@xxxxxxxxxxxxxxx > Cc: linux-pwm@xxxxxxxxxxxxxxx > --- > drivers/input/misc/pwm-beeper.c | 58 ++++++++++++++++++++++++++++++++- > 1 file changed, 57 insertions(+), 1 deletion(-) > > diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c > index 3cf1812384e6a..f63d0ebbaf573 100644 > --- a/drivers/input/misc/pwm-beeper.c > +++ b/drivers/input/misc/pwm-beeper.c > @@ -21,6 +21,7 @@ struct pwm_beeper { > struct regulator *amplifier; > struct work_struct work; > unsigned long period; > + unsigned long duty_cycle; > unsigned int bell_frequency; > bool suspended; > bool amplifier_on; > @@ -37,7 +38,7 @@ static int pwm_beeper_on(struct pwm_beeper *beeper, unsigned long period) > > state.enabled = true; > state.period = period; > - pwm_set_relative_duty_cycle(&state, 50, 100); > + pwm_set_relative_duty_cycle(&state, beeper->duty_cycle, 100000); > > error = pwm_apply_state(beeper->pwm, &state); > if (error) > @@ -119,6 +120,53 @@ static void pwm_beeper_close(struct input_dev *input) > pwm_beeper_stop(beeper); > } > > +static ssize_t volume_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct pwm_beeper *beeper = dev_get_drvdata(dev); > + > + return sysfs_emit(buf, "%ld\n", beeper->duty_cycle); > +} > + > +static ssize_t volume_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct pwm_beeper *beeper = dev_get_drvdata(dev); > + unsigned long val; > + > + if (kstrtoul(buf, 0, &val) < 0) > + return -EINVAL; > + > + /* > + * Volume is really PWM duty cycle in pcm (per cent mille, 1/1000th > + * of percent). This value therefore ranges from 0 to 50000 . Duty > + * cycle of 50% = 50000pcm is the maximum volume . > + */ > + val = clamp(val, 0UL, 50000UL); I wonder if you want to refuse values here that are not in the specified range, that is, something like: if (val != clamp(val, 0UL, 50000UL)) return -EINVAL; I think this is more in line who other sysfs properties work?! Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
