RE: [PATCH] Input: pwm-beeper - Support volume setting via sysfs | linux-input

RE: [PATCH] Input: pwm-beeper - Support volume setting via sysfs

From: Traut Manuel LCPF-CH <hidden>
Date: 2023-08-11 10:47:40
Also in: alsa-devel, linux-pwm

Hi

On Fri, 11 Aug 2023 06:19:50 +0200,
Jeff LaBundy wrote:

quoted

Hi Marek, Dmitry and Takashi,

On Tue, Aug 01, 2023 at 01:51:50PM +0200, Marek Vasut wrote:

quoted

On 8/1/23 09:28, Dmitry Torokhov wrote:

quoted

On Mon, Jul 31, 2023 at 09:56:09PM -0500, Jeff LaBundy wrote:

quoted

Hi all,

On Mon, Jul 31, 2023 at 07:49:50PM +0200, Marek Vasut wrote:

quoted

On 7/31/23 18:24, Dmitry Torokhov wrote:

quoted

On Mon, Jul 31, 2023 at 04:36:01PM +0200, Marek Vasut wrote:

quoted

On 7/31/23 16:20, Takashi Iwai wrote:

[...]

quoted

Uh, I don't need a full sound device to emit
beeps, that's not even possible with this hardware.

Heh, I also don't recommend that route, either :)
(Though, it must be possible to create a sound
device with that beep control in theory)

I mean, I can imagine one could possibly use PCM DMA
to cook samples to feed some of the PWM devices so
they could possibly be used to generate low quality
audio, as a weird limited DAC, but ... that's not really generic,

and not what I want.

quoted

Oh I see how the misunderstanding came; I didn't mean
the PCM implementation like pcsp driver.  The pcsp
driver is a real hack and it's there just for fun, not for any real

practical use.

quoted

Ah :)

quoted

What I meant was rather that you can create a sound
device containing a mixer volume control that serves
exactly like the sysfs or whatever other interface, without any

PCM stream or other interface.

quoted

Ahhh, hum, I still feel like this might be a bit overkill here.

quoted

I only need to control loudness of the beeper that
is controlled by PWM output. That's why I am
trying to extend the pwm-beeper driver, which
seems the best fit for such a device, it is only missing this

one feature (loudness control).

quoted

So the question is what's expected from user-space
POV.  If a more generic control of beep volume is
required, e.g. for desktop-like usages, an implementation

of sound driver wouldn't be too bad.

quoted

quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
OTOH, for other specific use-cases, it doesn't
matter much in which interface it's implemented, and sysfs

could be an easy choice.

quoted

The whole discussion above has been exactly about
this. Basically the thing is, we can either have:
- SND_TONE (via some /dev/input/eventX) + sysfs volume

control

quoted

     -> This is simple, but sounds racy between input
and sysfs accesses

Hmm, how can it be racy if you do proper locking?

I can imagine two applications can each grab one of the
controls and that makes the interface a bit not nice. That
would require extra synchronization in userspace and so on.

quoted

- SND_TONE + SND_TONE_SET_VOLUME
     -> User needs to do two ioctls, hum
- some new SND_TONE_WITH_VOLUME
     -> Probably the best option, user sets both tone frequency

and volume

quoted

quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
        in one go, and it also only extends the IOCTL interface, so

older

quoted

        userspace won't have issues

Those are "extensions" I have mentioned, and I'm not a
big fan for that, honestly speaking.

The fact that the beep *output* stuff is provided by the
*input* device is already confusing

I agree, this confused me as well.

This comes from the times when keyboards themselves were
capable of emitting bells (SUN, DEC, etc). In hindsight it
was not the best way of structuring things, same with the
keyboard LEDs (that are now plugged into the LED subsystem, but

still allow be driven through input).

quoted

And in the same vein I wonder if we should bite the bullet
and pay with a bit of complexity but move sound-related things to

sound subsystem.

quoted

I am not sure that's the right approach here, since the device
cannot do PCM playback, just bleeps.

quoted

(it was so just because of historical reason), and yet
you start implementing more full-featured mixer control.
I'd rather keep fingers away.

Again, if user-space requires the compatible behavior
like the existing desktop usages

It does not. These pwm-beeper devices keep showing up in
various embedded devices these days.

quoted

, it can be implemented in a similar way like the
existing ones; i.e. provide a mixer control with a
proper sound device.  The sound device doesn't need to
provide a PCM interface but just with a mixer interface.

Or, if the purpose of your target device is a special
usage, you don't need to consider too much about the
existing interface, and try to keep the change as
minimal as possible without too intrusive API changes.

My use case is almost perfectly matched by the current
input pwm-beeper driver, the only missing bit is the
ability to control the loudness at runtime. I think adding
the SND_TONE_WITH_VOLUME parameter would cover it, with

least intrusive API changes.

quoted

The SND_TONE already supports configuring tone frequency
in Hz as its parameter. Since anything above 64 kHz is
certainly not hearable by humans, I would say the
SND_TONE_WITH_VOLUME could use 16 LSbits for frequency (so

up to 65535 Hz , 0 is OFF), and 16 MSbits for volume .

quoted

I'm hesitant to overcomplicate something which can
currently be controlled via single ioctl by pulling in sound

subsystem into the picture.

quoted

Can you tell a bit more about your use case? What needs to
control the volume of beeps? Is this the only source of sounds on

the system?

quoted

Custom user space application. The entire userspace is custom
built in this case.

In this case, it is a single-use device (think e.g. the kind
of thermometer you stick in your ear when you're ill, to find out

how warm you are).

quoted

The beeper there is used to do just that, bleep (with
different frequencies to indicate different stuff), and that
works. What I need in addition to that is control the volume
of the bleeps from the application, so it isn't too noisy. And
that needs to be user-controllable at runtime, so not something that

goes in DT.

quoted

Right now there is just the bleeper , yes.

It sounds like we essentially need an option within pcsp to
drive PWM instead of PCM, but input already has pwm-beeper; it
seems harmless to gently extend the latter for this use-case as
opposed to reworking the former.

I agree that we should not invest too heavily in a legacy ABI,
however something like SND_BELL_VOL seems like a low-cost
addition that doesn't work against extending pcsp in the future.
In fact, input already has precedent for this exact same thing
by way of FF rumble effects, which are often PWM-based themselves.

If SND_BELL_VOL or similar is not acceptable, then the original
sysfs approach seems like the next-best compromise. My only
issue with it was that I felt the range was not abstracted enough.

If we want to extend the API we will need to define exactly how it
will all work. I.e. what happens if userspace mixes the old
SND_TONE and SND_BELL with the new SND_BELL_VOL or whatever.

Does

quoted

it play with previously set volume? The default one?

Default one, to preserve current behavior, yes.

This was my idea as well, but I appreciate that the devil is in the
details and each driver may have to duplicate some overhead.

quoted

How to set the default one?

We do not, we can call pwm_get_duty_cycle() to get the current duty
cycle of the PWM to figure out the default.

quoted

How
to figure out what the current volume is if we decide to make
volume "sticky"?

The patch stores the current volume configured via sysfs into
beeper->duty_cycle .

quoted

As far as userspace I expect it is more common to have one program
(or component of a program) to set volume and then something else
requests sound, so having one-shot API is of dubious value to me.

Currently the use case I have for this is a single user facing
application which configures both.

quoted

I hope we can go with Takashi's proposal downthread, but if not I
wonder if the sysfs approach is not the simplest one. Do we expect
more beepers that can control volume besides pwm-beeper?

It seems to me pulling in dependency on the entire sound subsystem
only to set beeper volume is overkill. I currently don't even have
sound subsystem compiled in.

I like Takashi's patch; it seems like a more scalable solution.
However, I can appreciate the reluctance to bring in the entire sound
subsytem for what is probably a tiny piezoelectric buzzer.

It seems like the sysfs solution is the best compromise in the
meantime. If more and more users need to shoe-horn these kind of
features in the future, we can make more informed decisions as to how to

extend the API (if at all).

That's my impression, too.  The original sysfs usage would be the right fit at
this moment.

I am fine with both using the Sound API and sysfs. I would additionally like to
specify the pwm values in device-tree like done in pwm-backlight. It really depends
on the hardware which values actually make a difference in volume.

Regards
Manuel

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help