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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/1/23 08:11, Takashi Iwai wrote:
On Tue, 01 Aug 2023 04:56:09 +0200,
Jeff LaBundy wrote:

Hi all,

On Mon, Jul 31, 2023 at 07:49:50PM +0200, Marek Vasut wrote:
On 7/31/23 18:24, Dmitry Torokhov wrote:
On Mon, Jul 31, 2023 at 04:36:01PM +0200, Marek Vasut wrote:
On 7/31/23 16:20, Takashi Iwai wrote:

[...]

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.

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.

Ah :)

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.

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

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).

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.
OTOH, for other specific use-cases, it doesn't matter much in which
interface it's implemented, and sysfs could be an easy choice.

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
     -> 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.

- 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
        in one go, and it also only extends the IOCTL interface, so older
        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).

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.

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

(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.

, 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.

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 .

I'm hesitant to overcomplicate something which can currently be controlled
via single ioctl by pulling in sound subsystem into the picture.

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?

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).

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.

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.

Nah, please forget pcsp driver.  As mentioned earlier, it's a driver
that is present just for fun.

I believe what we need is a simple sound card instance providing a
mixer control for the beep volume, something like a patch like below
(totally untested!)

Do we really want to add dependency on the entire sound subsystem (which is currently not needed on the device I care about) only to configure one single tunable of the PWM beeper ? It seems to add too much bloat to me.



[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux