From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 596E2C001DC for ; Mon, 31 Jul 2023 06:21:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229595AbjGaGVO (ORCPT ); Mon, 31 Jul 2023 02:21:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49684 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229503AbjGaGVN (ORCPT ); Mon, 31 Jul 2023 02:21:13 -0400 Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D6F94188; Sun, 30 Jul 2023 23:21:11 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 00F4B1F749; Mon, 31 Jul 2023 06:21:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1690784470; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=8uz9UreL669MhgY12mZE3fMA6oQ2OnCJzzU3AJy8+Mo=; b=P3quShTI/SP9802brh982beWhEVRti+Mlin4ocYqCpfbtLL81e+KRY9kaqVg0H4beLHSQv ZcZiYyi2Iaz7p1ChGiOruATUsnxtoj47MiEfKLC6Ib3CvqAAS+3Y9IDOrkbQgv+h7Zl+LN yF1rmZltUCMQ2tIzOYPdE+wjhvuqJiQ= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1690784470; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=8uz9UreL669MhgY12mZE3fMA6oQ2OnCJzzU3AJy8+Mo=; b=eJ9CnDvPEYmi+XuAA5musIwZy53yH95HKgw3o1VvJU5kw+sia3Ep1x7GBiGPJ/fn9Dz4o4 wnJYrnwEY7dCsYCQ== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id AC4EF133F7; Mon, 31 Jul 2023 06:21:09 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id NI35KNVSx2TaYQAAMHmgww (envelope-from ); Mon, 31 Jul 2023 06:21:09 +0000 Date: Mon, 31 Jul 2023 08:21:09 +0200 Message-ID: <873514d2ju.wl-tiwai@suse.de> From: Takashi Iwai To: Dmitry Torokhov Cc: Marek Vasut , Jeff LaBundy , linux-input@vger.kernel.org, Uwe =?ISO-8859-1?Q?Kleine-K=F6nig?= , Frieder Schrempf , Manuel Traut , Thierry Reding , linux-pwm@vger.kernel.org, alsa-devel@alsa-project.org, Jaroslav Kysela , Takashi Iwai Subject: Re: [PATCH] Input: pwm-beeper - Support volume setting via sysfs In-Reply-To: References: <20230512185551.183049-1-marex@denx.de> <0ef98ec1-6191-c72e-2362-310db7f09b84@denx.de> <06379f26-ab24-85f9-783f-0c49d4291b23@denx.de> User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0 MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-pwm@vger.kernel.org On Mon, 31 Jul 2023 07:36:38 +0200, Dmitry Torokhov wrote: > > On Sat, May 13, 2023 at 11:02:30PM +0200, Marek Vasut wrote: > > On 5/13/23 03:51, Marek Vasut wrote: > > > On 5/13/23 03:12, Jeff LaBundy wrote: > > > > Hi Marek, > > > > > > Hi, > > > > > > > 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 > > > > > --- > > > > > 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 ? > > > > > --- > > > > > > > > Thanks for the patch; this seems like a useful feature. > > > > > > > > My first thought is that 50000 seems like an oddly specific limit to > > > > impose > > > > upon user space. Ideally, user space need not even care that the > > > > beeper is > > > > implemented via PWM and why 50000 is significant. > > > > > > > > Instead, what about accepting 0..255 as the LED subsystem does for > > > > brightness, > > > > then map these values to 0..50000 internally? In fact, the leds-pwm > > > > driver > > > > does something similar. > > > > > > The pwm_set_relative_duty_cycle() function can map whatever range to > > > whatever other range of the PWM already, so that's not an issues here. > > > It seems to me the 0..127 or 0..255 range is a bit too limiting . I > > > think even for the LEDs the reason for that limit is legacy design, but > > > here I might be wrong. > > > > > > > I'm also curious as to whether this function should be a rogue sysfs > > > > control > > > > limited to this driver, or a generic operation in input. For > > > > example, input > > > > already allows user space to specify the magnitude of an FF effect; > > > > perhaps > > > > something similar is warranted here? > > > > > > See the "An alternative ..." part above, I was wondering about this too, > > > whether this can be added into the input ABI, but I am somewhat > > > reluctant to fiddle with the ABI. > > > > Thinking about this further, we could try and add some > > > > EV_SND SND_TONE_WITH_VOLUME > > > > to avoid overloading EV_SND SND_TONE , and at the same time allow the user > > to set both frequency and volume for the tone without any race condition > > between the two. > > > > The EV_SND SND_TONE_WITH_VOLUME would still take one 32bit parameter, except > > this time the parameter 16 LSbits would be the frequency and 16 MSbits would > > be the volume. > > > > But again, here I would like input from the maintainers. > > Beeper was supposed to be an extremely simple device with minimal > controls. I wonder if there is need for volume controls, etc, etc are we > not better moving it over to the sound subsystem. We already have: > > sound/drivers/pcsp/pcsp.c > > and > > sound/pci/hda/hda_beep.c > > there, can we have other "advanced" beepers there as well? Adding sound > maintainers to CC... I don't mind it put to sound/*. But, note that pcsp.c you pointed in the above is a PCM tone generator driver with a PC beep device, and it provides the normal SND_BEEP input only for compatibility. Indeed there have been already many sound drivers providing the beep capability, and they bind with the input device using SND_BEEP. And, for the beep volume, "Beep Playback Volume" mixer control is provided, too. thanks, Takashi