Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220912072945.760949-1-yunjunlee@chromium.org> <87pmg1hvw7.wl-tiwai@suse.de>
In-Reply-To: <87pmg1hvw7.wl-tiwai@suse.de>
From:   YJ Lee <yunjunlee@chromium.org>
Date:   Mon, 12 Sep 2022 16:48:27 +0800
Message-ID: <CAPm_npbzeq8twNavfWfrAi-v8eky1QNivS7PKOocgxax+T4X3Q@mail.gmail.com>
Subject: Re: [PATCH v1] ALSA: dummy: Add customizable volume min/max.
To:     Takashi Iwai <tiwai@suse.de>
Cc:     Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
        alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
        yuhsuan@chromium.org, whalechang@chromium.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Sep 12, 2022 at 3:52 PM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Mon, 12 Sep 2022 09:29:45 +0200,
> YJ Lee wrote:
> >
> > Add module parameters to support customized min/max volume leveling,
> > which will be useful to test devices with different volume granularity.
> >
> > Signed-off-by: YJ Lee <yunjunlee@chromium.org>
> > ---
> >  sound/drivers/dummy.c | 34 ++++++++++++++++++++++++----------
> >  1 file changed, 24 insertions(+), 10 deletions(-)
> >
> > diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
> > index 2a7fc49c1a7c5..64fb2778f1e9a 100644
> > --- a/sound/drivers/dummy.c
> > +++ b/sound/drivers/dummy.c
> > @@ -42,6 +42,8 @@ MODULE_LICENSE("GPL");
> >  #define USE_CHANNELS_MAX     2
> >  #define USE_PERIODS_MIN      1
> >  #define USE_PERIODS_MAX      1024
> > +#define USE_MIXER_VOLUME_LEVEL_MIN   -50
> > +#define USE_MIXER_VOLUME_LEVEL_MAX   100
> >
> >  static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;   /* Index 0-MAX */
> >  static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR;    /* ID for this card */
> > @@ -50,6 +52,8 @@ static char *model[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = NULL};
> >  static int pcm_devs[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = 1};
> >  static int pcm_substreams[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = 8};
> >  //static int midi_devs[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = 2};
> > +static int mixer_volume_level_min = USE_MIXER_VOLUME_LEVEL_MIN;
> > +static int mixer_volume_level_max = USE_MIXER_VOLUME_LEVEL_MAX;
> >  #ifdef CONFIG_HIGH_RES_TIMERS
> >  static bool hrtimer = 1;
> >  #endif
> > @@ -69,6 +73,10 @@ module_param_array(pcm_substreams, int, NULL, 0444);
> >  MODULE_PARM_DESC(pcm_substreams, "PCM substreams # (1-128) for dummy driver.");
> >  //module_param_array(midi_devs, int, NULL, 0444);
> >  //MODULE_PARM_DESC(midi_devs, "MIDI devices # (0-2) for dummy driver.");
> > +module_param(mixer_volume_level_min, int, 0444);
>
> I can imagine that the permission could be 0644, so that the
> parameters can be changed dynamically via sysfs, too.  But it may skip
> the sanity check at probe, hence more code would be needed, OTOH.
>
> So I applied the patch as is now.
>
>
> thanks,
>
> Takashi

Thanks for the insight! Learned a lot from your kind explanation and
constant help.

Appreciated,
YJ