RE: [PATCH v6 2/2] ASoC: rt715:add micmute led state control supports

[Yuan, Perry]

Hi Mark:


> -----Original Message-----
> From: Mark Brown <broonie@xxxxxxxxxx>
> Sent: 2021年4月7日 23:16
> To: Yuan, Perry
> Cc: pobrn@xxxxxxxxxxxxxx; pierre-louis.bossart@xxxxxxxxxxxxxxx;
> oder_chiou@xxxxxxxxxxx; perex@xxxxxxxx; tiwai@xxxxxxxx;
> hdegoede@xxxxxxxxxx; mgross@xxxxxxxxxxxxxxx; Limonciello, Mario;
> lgirdwood@xxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; platform-driver-x86@xxxxxxxxxxxxxxx;
> mario.limonciello@xxxxxxxxxxx; Dell Client Kernel
> Subject: Re: [PATCH v6 2/2] ASoC: rt715:add micmute led state control
> supports
> 
> On Sun, Apr 04, 2021 at 04:31:59PM +0800, Perry Yuan wrote:
> 
> > +static bool micmute_led_set;
> > +static int  dmi_matched(const struct dmi_system_id *dmi) {
> > +	micmute_led_set = 1;
> > +	return 1;
> > +}
> 
> This isn't how we usually record DMI quirks, usually we'd query once on probe
> and store the data in the driver data struct - see other users for examples.
> 
> > @@ -358,6 +388,7 @@ static int rt715_sdca_put_volsw(struct snd_kcontrol
> *kcontrol,
> >  	unsigned int mask = (1 << fls(max)) - 1;
> 
> > +	dmi_check_system(micmute_led_dmi_table);
> > +	if (invert && micmute_led_set) {
> 
> This check for invert is odd and could probably use a comment.

Thanks for your review.
I add one short comments for the reason in V7 patch.
I tried not to use the invert , the LED state and mute state will be not matching
So it has to use the invert to check if the mute state changed. Then notifiy the led trigger driver to set the new led state.

Perry